Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@stefanofornari
stefanofornari committed Jun 29, 2023
2 parents 04a3966 + 979c067 commit 45666f0
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 85 deletions.
1 change: 0 additions & 1 deletion src/main/java/software/amazon/nio/spi/s3/S3ClientProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,6 @@ private S3AsyncClient asyncClientForRegion(String endpoint, String bucket, Strin
}

if ((endpoint != null) && (endpoint.length() > 0)) {
// TODO: shall we have the protocol in the endpoint already?
asyncClientBuilder.endpointOverride(URI.create(configuration.getEndpointProtocol() + "://" + endpoint));
}

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/software/amazon/nio/spi/s3/config/S3NioSpiConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ public class S3NioSpiConfiguration {
// aws.accessKey, leaving the framework and the underlying AWS client
// the possibility to use the standard behaviour.
//
// TODO: shall we instead incorporate take the value of those properties
// too (aws.region, aws.accessKey, aws.secretAccessKey)?
// TOTO: shall we return an Optional instead returning null?
// TOTO: shall we return an Optional instead returning null?
//

Expand Down
53 changes: 15 additions & 38 deletions src/test/java/software/amazon/nio/spi/s3/S3FileSystemEndpointTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,59 +40,36 @@ public void init() {

@Test
public void clientWithProvidedEndpoint() throws Exception {
final String URI1 = "s3://endpoint1.io/bucket/resource";
final String URI2 = "s3://endpoint2.io:8080/bucket/resource";
final String URI1 = "s3://endpoint2.io:8080/bucket/resource";

//
// For non AWS S3 buckets, backet's region is not discovered runtime and it
// must be provided in the AWS profile
// For non AWS S3 buckets, backet's region is not discovered runtime and
// it must be provided in the AWS profile
//
S3FileSystem fs = new S3FileSystem(URI.create(URI1), provider, new S3NioSpiConfiguration(CONFIG));
fs.clientProvider.asyncClientBuilder = BUILDER;

S3AsyncClient client = fs.client();
assertEquals(URI.create("https://endpoint1.io"), BUILDER.endpointOverride);
assertNull(BUILDER.credentialsProvider);

fs = new S3FileSystem(URI.create(URI2), provider);
fs.clientProvider.asyncClientBuilder = BUILDER;

fs.client();
assertEquals(URI.create("https://endpoint2.io:8080"), BUILDER.endpointOverride);
assertNull(BUILDER.credentialsProvider);
}

@Test
public void clientWithProvidedEndpointAndCredentials() throws Exception {
final String URI1 = "s3://key1:[email protected]/bucket/resource";
final String URI2 = "s3://key2:[email protected]:8080/bucket/resource";

restoreSystemProperties(() -> {
System.setProperty("aws.region", "aws-east-1");

S3FileSystem fs = new S3FileSystem(URI.create(URI1), provider);
fs.clientProvider.asyncClientBuilder = BUILDER;

//
// For non AWS S3 buckets, backet's region is not discovered runtime and it
// must be provided in the AWS profile
//
S3AsyncClient client = fs.client();
final String URI1 = "s3://key2:[email protected]:8080/bucket/resource";

assertEquals(URI.create("https://endpoint1.io"), BUILDER.endpointOverride);
assertNotNull(BUILDER.credentialsProvider);
assertEquals("key1", BUILDER.credentialsProvider.resolveCredentials().accessKeyId());
assertEquals("secret1", BUILDER.credentialsProvider.resolveCredentials().secretAccessKey());

fs = new S3FileSystem(URI.create(URI2), provider);
fs.clientProvider.asyncClientBuilder = BUILDER;
//
// For non AWS S3 buckets, backet's region is not discovered runtime
// and it must be provided in the AWS profile
//
S3FileSystem fs = new S3FileSystem(URI.create(URI1), provider, new S3NioSpiConfiguration(CONFIG));
fs.clientProvider.asyncClientBuilder = BUILDER;

fs.client();
assertEquals(URI.create("https://endpoint2.io:8080"), BUILDER.endpointOverride);
assertNotNull(BUILDER.credentialsProvider);
assertEquals("key2", BUILDER.credentialsProvider.resolveCredentials().accessKeyId());
assertEquals("secret2", BUILDER.credentialsProvider.resolveCredentials().secretAccessKey());
});
fs.client();
assertEquals(URI.create("https://endpoint2.io:8080"), BUILDER.endpointOverride);
assertNotNull(BUILDER.credentialsProvider);
assertEquals("key2", BUILDER.credentialsProvider.resolveCredentials().accessKeyId());
assertEquals("secret2", BUILDER.credentialsProvider.resolveCredentials().secretAccessKey());
}

}
72 changes: 32 additions & 40 deletions src/test/java/software/amazon/nio/spi/s3/S3FileSystemProviderConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Test;
import static software.amazon.nio.spi.s3.config.S3NioSpiConfiguration.AWS_ACCESS_KEY_PROPERTY;
import static software.amazon.nio.spi.s3.config.S3NioSpiConfiguration.AWS_REGION_PROPERTY;
import static software.amazon.nio.spi.s3.config.S3NioSpiConfiguration.AWS_SECRET_ACCESS_KEY_PROPERTY;
import static software.amazon.nio.spi.s3.config.S3NioSpiConfiguration.S3_SPI_ENDPOINT_PROTOCOL_PROPERTY;

Expand All @@ -23,62 +24,53 @@ public class S3FileSystemProviderConfigurationTest {
@Test
public void setEndpointProtocolThroughEnvironment() throws Exception {
Map<String, String> env = new HashMap<>();
env.put(AWS_REGION_PROPERTY, "us-west-1");

S3FileSystemProvider p = new S3FileSystemProvider();

//
// TODO: remove in favor of env.put("aws.region", "...");
//
restoreSystemProperties(() -> {
System.setProperty("aws.region", "us-west-1");
S3FileSystem fs = p.newFileSystem(URI.create("s3://some.where.com:1010/bucket"), env);
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();

S3FileSystem fs = p.newFileSystem(URI.create("s3://some.where.com:1010/bucket"), env);
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();
assertEquals("bucket", fs.bucketName());
assertEquals("some.where.com:1010", fs.endpoint());
assertEquals("https://some.where.com:1010", BUILDER.endpointOverride.toString());

assertEquals("bucket", fs.bucketName());
assertEquals("some.where.com:1010", fs.endpoint());
assertEquals("https://some.where.com:1010", BUILDER.endpointOverride.toString());

env.put(S3_SPI_ENDPOINT_PROTOCOL_PROPERTY, "http");
env.put(S3_SPI_ENDPOINT_PROTOCOL_PROPERTY, "http");

fs = p.newFileSystem(URI.create("s3://any.where.com:2020/foo"), env);
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();
fs = p.newFileSystem(URI.create("s3://any.where.com:2020/foo"), env);
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();

assertEquals("foo", fs.bucketName());
assertEquals("any.where.com:2020", fs.endpoint());
assertEquals("http://any.where.com:2020", BUILDER.endpointOverride.toString());
});
assertEquals("foo", fs.bucketName());
assertEquals("any.where.com:2020", fs.endpoint());
assertEquals("http://any.where.com:2020", BUILDER.endpointOverride.toString());
}

@Test
public void setEndpointProtocolThroughSystemProperties() throws Exception {
Map<String, String> env = new HashMap<>();
env.put(AWS_REGION_PROPERTY, "us-west-1");

S3FileSystemProvider p = new S3FileSystemProvider();

//
// TODO: remove in favor of env.put("aws.region", "...");
//
restoreSystemProperties(() -> {
System.setProperty("aws.region", "us-west-1");
S3FileSystem fs = p.newFileSystem(URI.create("s3://some.where.com:1010/bucket"), env);
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();

S3FileSystem fs = p.newFileSystem(URI.create("s3://some.where.com:1010/bucket"));
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();
assertEquals("bucket", fs.bucketName());
assertEquals("some.where.com:1010", fs.endpoint());
assertEquals("https://some.where.com:1010", BUILDER.endpointOverride.toString());

assertEquals("bucket", fs.bucketName());
assertEquals("some.where.com:1010", fs.endpoint());
assertEquals("https://some.where.com:1010", BUILDER.endpointOverride.toString());

System.setProperty(S3_SPI_ENDPOINT_PROTOCOL_PROPERTY, "http");
System.setProperty(S3_SPI_ENDPOINT_PROTOCOL_PROPERTY, "http");

fs = p.newFileSystem(URI.create("s3://any.where.com:2020/foo"));
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();
fs = p.newFileSystem(URI.create("s3://any.where.com:2020/foo"));
fs.clientProvider.asyncClientBuilder = BUILDER;
fs.client(); fs.close();

assertEquals("foo", fs.bucketName());
assertEquals("any.where.com:2020", fs.endpoint());
assertEquals("http://any.where.com:2020", BUILDER.endpointOverride.toString());
});
assertEquals("foo", fs.bucketName());
assertEquals("any.where.com:2020", fs.endpoint());
assertEquals("http://any.where.com:2020", BUILDER.endpointOverride.toString());
}

@Test
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/software/amazon/nio/spi/s3/S3FileSystemProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,11 @@ public void getFileSystem() {
URI.create("s3://key:[email protected]:2000/foo2/baa2")
};

assertThrows(FileSystemNotFoundException.class, () -> {
provider.getFileSystem(URI.create("s3://nowhere.com:2000/foo2/baa2"));
fail("wrong file system!");
});
assertThrows(
FileSystemNotFoundException.class, () -> {
provider.getFileSystem(URI.create("s3://nowhere.com:2000/foo2/baa2"));
}
);
}

@Test
Expand Down

0 comments on commit 45666f0

Please sign in to comment.