Skip to content

Commit

Permalink
fix: S3FileSystem configuration visibility and name update (#602)
Browse files Browse the repository at this point in the history
* fix: make the S3FileSystem configuration accessible and the getter name more appropriate and Kotlin friendly, deprecates old method
  • Loading branch information
sielenk-yara authored Feb 5, 2025
1 parent 1d2c9c9 commit 4a37208
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 17 deletions.
18 changes: 17 additions & 1 deletion src/main/java/software/amazon/nio/spi/s3/S3FileSystem.java
Original file line number Diff line number Diff line change
Expand Up @@ -387,10 +387,26 @@ public WatchService newWatchService() {
*
* @return the configuration object for this file system
*/
S3NioSpiConfiguration configuration() {
public S3NioSpiConfiguration getConfiguration() {
return configuration;
}

/**
* Returns the configuration object passed in the constructor or created
* by default.
*
* @return the configuration object for this file system
*
* @deprecated This method is deprecated and may be removed in a future release.
* Use {@link #getConfiguration()} instead to retrieve the configuration object,
* as it provides the same functionality with better naming conventions that
* align with standard practices.
*/
@Deprecated
public S3NioSpiConfiguration configuration() {
return getConfiguration();
}

/**
* Returns the client provider used to build aws clients
*
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/software/amazon/nio/spi/s3/S3Path.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static S3Path getPath(S3FileSystem fsForBucket, String first, String... more) {
throw new IllegalArgumentException("first element of the path may not be null");
}

var configuration = fsForBucket.configuration();
var configuration = fsForBucket.getConfiguration();

first = first.trim();

Expand Down Expand Up @@ -660,9 +660,9 @@ public URI toUri() {
var elements = path.iterator();

var uri = new StringBuilder(fileSystem.provider().getScheme() + "://");
var endpoint = fileSystem.configuration().getEndpoint();
var endpoint = fileSystem.getConfiguration().getEndpoint();
if (!endpoint.isEmpty()) {
uri.append(fileSystem.configuration().getEndpoint()).append(PATH_SEPARATOR);
uri.append(fileSystem.getConfiguration().getEndpoint()).append(PATH_SEPARATOR);
}
uri.append(bucketName());
elements.forEachRemaining(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private S3SeekableByteChannel(S3Path s3Path, S3AsyncClient s3Client, long startA
throw new IOException("The SYNC/DSYNC options is not supported");
}

var config = s3Path.getFileSystem().configuration();
var config = s3Path.getFileSystem().getConfiguration();
if (options.contains(StandardOpenOption.WRITE)) {
LOGGER.debug("using S3WritableByteChannel as write delegate for path '{}'", s3Path.toUri());
readDelegate = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void configureDirectory() throws IOException {
S3FileSystem fs = mock();
FileSystemProvider provider = mock();
when(fs.provider()).thenReturn(provider);
when(fs.configuration()).thenReturn(new S3NioSpiConfiguration());
when(fs.getConfiguration()).thenReturn(new S3NioSpiConfiguration());
when(provider.getScheme()).thenReturn("s3");

var directory = S3Path.getPath(fs, "/somedirectory/");
Expand Down Expand Up @@ -130,7 +130,7 @@ void configureRegularFile() throws IOException {
when(fs.provider()).thenReturn(provider);
when(provider.getScheme()).thenReturn("s3");

when(fs.configuration()).thenReturn(new S3NioSpiConfiguration());
when(fs.getConfiguration()).thenReturn(new S3NioSpiConfiguration());

when(fs.client()).thenReturn(mockClient);

Expand Down Expand Up @@ -217,7 +217,7 @@ void sizeOfFileThrowsWhenTimeout(){

S3FileSystem fs = mock();
when(fs.provider()).thenReturn(provider);
when(fs.configuration()).thenReturn(new S3NioSpiConfiguration());
when(fs.getConfiguration()).thenReturn(new S3NioSpiConfiguration());

S3AsyncClient mockClient = mock();
when(fs.client()).thenReturn(mockClient);
Expand All @@ -240,4 +240,4 @@ void sizeOfFileThrowsWhenTimeout(){
.hasMessageContainingAll("operation timed out", "connectivity", "S3");
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ private S3SeekableByteChannel seekableByteChannelForRead() throws IOException {
// test that the S3SeekableByteChannel uses the buffer size from the configuration set for the FileSystem
@Test
public void testBufferSize() throws IOException {
fs.configuration().withMaxFragmentSize(10000);
fs.configuration().withMaxFragmentNumber(10);
fs.getConfiguration().withMaxFragmentSize(10000);
fs.getConfiguration().withMaxFragmentNumber(10);
try(var channel = (S3SeekableByteChannel) fs.provider().newByteChannel(path, Set.of(READ))) {
assertEquals(10000, ((S3ReadAheadByteChannel) channel.getReadDelegate()).getMaxFragmentSize());
assertEquals(10, ((S3ReadAheadByteChannel) channel.getReadDelegate()).getMaxNumberFragments());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public void nio_provider() {

var fs = path.getFileSystem();
then(fs.provider()).isInstanceOf(S3XFileSystemProvider.class);
then(fs.configuration().getEndpoint()).isEqualTo("myendpoint");
then(fs.configuration().getBucketName()).isEqualTo("mybucket");
then(fs.getConfiguration().getEndpoint()).isEqualTo("myendpoint");
then(fs.getConfiguration().getBucketName()).isEqualTo("mybucket");
then(path.getKey()).isEqualTo("myfolder");
}

Expand Down Expand Up @@ -75,12 +75,12 @@ public void setCredentialsThroughURI() throws Exception {
fs.client();
fs.close();

then(fs.configuration().getBucketName()).isEqualTo("bucket");
then(fs.configuration().getEndpoint()).isEqualTo("some.where.com:1010");
then(fs.getConfiguration().getBucketName()).isEqualTo("bucket");
then(fs.getConfiguration().getEndpoint()).isEqualTo("some.where.com:1010");

verify(BUILDER).endpointOverride(URI.create("https://some.where.com:1010"));
then(fs.configuration().getCredentials().accessKeyId()).isEqualTo("urikey");
then(fs.configuration().getCredentials().secretAccessKey()).isEqualTo("urisecret");
then(fs.getConfiguration().getCredentials().accessKeyId()).isEqualTo("urikey");
then(fs.getConfiguration().getCredentials().secretAccessKey()).isEqualTo("urisecret");

});
}
Expand Down

0 comments on commit 4a37208

Please sign in to comment.