Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
reta committed Feb 21, 2023
1 parent 0c37c17 commit 652a7da
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,12 @@
import com.amazonaws.http.IdleConnectionReaper;

import org.junit.AfterClass;
import org.opensearch.common.io.PathUtils;
import org.opensearch.common.settings.MockSecureSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.test.OpenSearchTestCase;

import java.io.Closeable;
import java.io.IOException;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
Expand All @@ -57,7 +55,7 @@
import static org.opensearch.repositories.s3.S3ClientSettings.PROTOCOL_SETTING;
import static org.opensearch.repositories.s3.S3ClientSettings.PROXY_TYPE_SETTING;

public class AwsS3ServiceImplTests extends OpenSearchTestCase {
public class AwsS3ServiceImplTests extends OpenSearchTestCase implements ConfigPathSupport {
@AfterClass
public static void shutdownIdleConnectionReaper() {
// created by default STS client
Expand Down Expand Up @@ -368,8 +366,4 @@ private void assertEndpoint(Settings repositorySettings, Settings settings, Stri
final S3ClientSettings clientSettings = S3ClientSettings.getClientSettings(settings, configName, configPath());
assertThat(clientSettings.endpoint, is(expectedEndpoint));
}

private Path configPath() {
return PathUtils.get("config");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.repositories.s3;

import org.opensearch.common.io.PathUtils;

import java.nio.file.Path;

/**
* The trait that adds the config path to the test cases
*/
interface ConfigPathSupport {
default Path configPath() {
return PathUtils.get("config");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.opensearch.common.blobstore.BlobContainer;
import org.opensearch.common.blobstore.BlobPath;
import org.opensearch.common.bytes.BytesReference;
import org.opensearch.common.io.PathUtils;
import org.opensearch.common.io.Streams;
import org.opensearch.common.lucene.store.ByteArrayIndexInput;
import org.opensearch.common.lucene.store.InputStreamIndexInput;
Expand All @@ -64,7 +63,6 @@
import java.net.InetSocketAddress;
import java.net.SocketTimeoutException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Locale;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -83,7 +81,7 @@
* This class tests how a {@link S3BlobContainer} and its underlying AWS S3 client are retrying requests when reading or writing blobs.
*/
@SuppressForbidden(reason = "use a http server")
public class S3BlobContainerRetriesTests extends AbstractBlobContainerRetriesTestCase {
public class S3BlobContainerRetriesTests extends AbstractBlobContainerRetriesTestCase implements ConfigPathSupport {

private S3Service service;

Expand Down Expand Up @@ -403,8 +401,4 @@ public void close() throws IOException {
}
}
}

private Path configPath() {
return PathUtils.get("config");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,13 @@
import com.amazonaws.Protocol;
import com.amazonaws.services.s3.AmazonS3Client;

import org.opensearch.common.io.PathUtils;
import org.opensearch.common.settings.MockSecureSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.settings.SettingsException;
import org.opensearch.test.OpenSearchTestCase;

import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.file.Path;
import java.util.Locale;
import java.util.Map;

Expand All @@ -54,7 +52,7 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

public class S3ClientSettingsTests extends OpenSearchTestCase {
public class S3ClientSettingsTests extends OpenSearchTestCase implements ConfigPathSupport {
public void testThereIsADefaultClientByDefault() {
final Map<String, S3ClientSettings> settings = S3ClientSettings.load(Settings.EMPTY, configPath());
assertThat(settings.keySet(), contains("default"));
Expand Down Expand Up @@ -375,8 +373,4 @@ public void testSocksDoesNotSupportForHttpProtocol() {
.build();
expectThrows(SettingsException.class, () -> S3ClientSettings.load(settings, configPath()));
}

private Path configPath() {
return PathUtils.get("config");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

import com.amazonaws.services.s3.AbstractAmazonS3;
import org.opensearch.cluster.metadata.RepositoryMetadata;
import org.opensearch.common.io.PathUtils;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.ByteSizeUnit;
Expand All @@ -54,7 +53,7 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;

public class S3RepositoryTests extends OpenSearchTestCase {
public class S3RepositoryTests extends OpenSearchTestCase implements ConfigPathSupport {

private static class DummyS3Client extends AbstractAmazonS3 {

Expand Down Expand Up @@ -155,8 +154,4 @@ protected void assertSnapshotOrGenericThread() {
}
};
}

private Path configPath() {
return PathUtils.get("config");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@
package org.opensearch.repositories.s3;

import org.opensearch.cluster.metadata.RepositoryMetadata;
import org.opensearch.common.io.PathUtils;
import org.opensearch.common.settings.MockSecureSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.test.OpenSearchTestCase;

import java.nio.file.Path;
import java.util.Map;

public class S3ServiceTests extends OpenSearchTestCase {
public class S3ServiceTests extends OpenSearchTestCase implements ConfigPathSupport {

public void testCachedClientsAreReleased() {
final S3Service s3Service = new S3Service(configPath());
Expand Down Expand Up @@ -86,8 +84,4 @@ public void testCachedClientsWithCredentialsAreReleased() {
final S3ClientSettings clientSettingsReloaded = s3Service.settings(metadata1);
assertNotSame(clientSettings, clientSettingsReloaded);
}

private Path configPath() {
return PathUtils.get("config");
}
}

0 comments on commit 652a7da

Please sign in to comment.