Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: S3FileSystem configuration visibility and name update #602

Merged
merged 2 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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