Improve maintainability / readability / javadoc

src/main/java/software/amazon/nio/spi/s3/S3FileSystemProvider.java

@@ -79,7 +79,7 @@ public class S3FileSystemProvider extends FileSystemProvider {
 
     private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());
 
-    private static Map<String, S3FileSystem> cache = new HashMap<>();
+    private static final Map<String, S3FileSystem> cache = new HashMap<>();
 
     /**
      * Returns the URI scheme that identifies this provider.
@@ -91,8 +91,8 @@ public String getScheme() {
         return SCHEME;
     }
 
-    /*
-     * @throws NotYetImplementedException
+    /**
+     * @throws NotYetImplementedException This method is not yet supported in v2.x. It might be implemented for bucket creation
      */
     @Override
     public FileSystem newFileSystem(URI uri, Map<String, ?> env) {
@@ -245,7 +245,7 @@ public DirectoryStream<Path> newDirectoryStream(Path dir, DirectoryStream.Filter
 
         final Iterator<Path> iterator = pathIteratorForPublisher(filter, fs, finalDirName, listObjectsV2Publisher);
 
-        return new DirectoryStream<Path>() {
+        return new DirectoryStream<>() {
             @Override
             public void close() {
             }
@@ -693,50 +693,36 @@ public void setAttribute(Path path, String attribute, Object value, LinkOption..
      */
     S3FileSystem getFileSystem(URI uri, boolean create) {
         S3FileSystemInfo info = fileSystemInfo(uri);
-        S3FileSystem fs = cache.get(info.key());
-
-        if (fs == null) {
+        return cache.computeIfAbsent(info.key(), (key) -> {
             if (!create) {
                 throw new FileSystemNotFoundException("file system not found for '" + info.key() + "'");
             }
-            fs = forUri(uri);
-        }
-
-        return fs;
+            return forUri(uri, info);
+        });
     }
 
-    S3FileSystem forUri(URI uri){
+    S3FileSystem forUri(URI uri, S3FileSystemInfo info){
         if (uri == null) {
             throw new IllegalArgumentException("uri can not be null");
         }
         if (uri.getScheme() == null) {
             throw new IllegalArgumentException(
-                    String.format("invalid uri '%s', please provide an uri as s3://bucket", uri.toString())
+                    String.format("invalid uri '%s', please provide an uri as s3://bucket", uri)
             );
         }
         if (uri.getAuthority() == null) {
             throw new IllegalArgumentException(
-                    String.format("invalid uri '%s', please provide an uri as s3://bucket", uri.toString())
+                    String.format("invalid uri '%s', please provide an uri as s3://bucket", uri)
             );
         }
 
-        S3FileSystemInfo info = fileSystemInfo(uri);
-        if (cache.containsKey(info.key())) {
-            throw new FileSystemAlreadyExistsException("a file system already exists for '" + info.key() + "', use getFileSystem() instead");
-        }
-
         S3NioSpiConfiguration config = new S3NioSpiConfiguration().withEndpoint(info.endpoint()).withBucketName(info.bucket());
 
         if (info.accessKey() != null) {
             config.withCredentials(info.accessKey(), info.accessSecret());
         }
 
-        S3FileSystem fs = null;
-        cache.put(
-                info.key(),
-                fs = new S3FileSystem(this, config)
-        );
-        return fs;
+        return new S3FileSystem(this, config);
     }
 
     void closeFileSystem(FileSystem fs) {

src/main/java/software/amazon/nio/spi/s3/S3Path.java

@@ -7,9 +7,9 @@
 
 import java.io.File;
 import java.io.IOError;
-import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.FileSystem;
 import java.nio.file.InvalidPathException;
 import java.nio.file.LinkOption;
@@ -70,35 +70,23 @@ private S3Path from(String path){
      * @return a new S3Path
      */
     static S3Path getPath(S3FileSystem fsForBucket, String first, String... more) {
-        if(fsForBucket == null)  throw new IllegalArgumentException("The S3FileSystem may not be null");
-        if(first == null ){
+        if(fsForBucket == null) throw new IllegalArgumentException("The S3FileSystem may not be null");
+        if(first == null) {
           throw new IllegalArgumentException("first element of the path may not be null");
         }
 
         S3NioSpiConfiguration configuration = fsForBucket.configuration();
 
         first = first.trim();
 
-        if((first.isEmpty()) && !(more == null || more.length == 0)) throw new IllegalArgumentException("The first element of the path may not be empty when more exists");
-        if(first.startsWith(fsForBucket.provider().getScheme()+":/")) {
-            first = first.substring(fsForBucket.provider().getScheme().length()+2);
+        if( first.isEmpty() && !(more == null || more.length == 0)) throw new IllegalArgumentException("The first element of the path may not be empty when more exists");
 
-            String part = null;
-            if (configuration.getCredentials() != null) {
-                AwsCredentials credentials = configuration.getCredentials();
-                part = credentials.accessKeyId() + ':' + credentials.secretAccessKey();
-                if (first.startsWith('/' + part)) {
-                    first = PATH_SEPARATOR + first.substring(part.length()+2);
-                }
-            }
-            part = configuration.getEndpoint();
-            if (!part.isEmpty() && first.startsWith(PATH_SEPARATOR + part)) {
-                first = first.substring(part.length()+1);
-            }
-            part = configuration.getBucketName();
-            if (first.startsWith(PATH_SEPARATOR + part)) {
-                first = first.substring(part.length()+1);
-            }
+        String scheme = fsForBucket.provider().getScheme();
+        if(first.startsWith(scheme +":/")) {
+            first = removeScheme(first, scheme);
+            first = removeCredentials(first, configuration);
+            first = removeEndpoint(first, configuration.getEndpoint());
+            first = removeBucket(first, configuration.getBucketName());
         }
 
         return new S3Path(fsForBucket, PosixLikePathRepresentation.of(first, more));
@@ -631,17 +619,10 @@ public URI toUri() {
         elements.forEachRemaining(
             (e) -> {
                 String name = e.getFileName().toString();
-                try {
-                    if (name.endsWith(PATH_SEPARATOR)) {
-                        name = name.substring(0, name.length()-1);
-                    }
-                    uri.append(PATH_SEPARATOR).append(URLEncoder.encode(name, "UTF-8"));
-                } catch (UnsupportedEncodingException x) {
-                    //
-                    // NOTE: I do not know how to reproduce this case...
-                    //
-                    throw new IllegalArgumentException("path '" + uri + "' can not be converted to URI: " + x.getMessage(), x);
+                if (name.endsWith(PATH_SEPARATOR)) {
+                    name = name.substring(0, name.length()-1);
                 }
+                uri.append(PATH_SEPARATOR).append(URLEncoder.encode(name, StandardCharsets.UTF_8));
             }
         );
         if (isDirectory()) {
@@ -868,4 +849,32 @@ public boolean hasNext() {
         }
     }
 
+    private static String removeScheme(String path, String scheme){
+        return path.substring(scheme.length()+2);
+    }
+
+    private static String removeCredentials(String first, S3NioSpiConfiguration configuration) {
+        if (configuration.getCredentials() != null) {
+            AwsCredentials credentials = configuration.getCredentials();
+            String credentialsAsString = credentials.accessKeyId() + ':' + credentials.secretAccessKey();
+            if (first.startsWith('/' + credentialsAsString)) {
+                first = PATH_SEPARATOR + first.substring(credentialsAsString.length()+2);
+            }
+        }
+        return first;
+    }
+
+    private static String removeEndpoint(String first, String endpoint) {
+        if (!endpoint.isEmpty() && first.startsWith(PATH_SEPARATOR + endpoint)) {
+            first = first.substring(endpoint.length()+1);
+        }
+        return first;
+    }
+
+    private static String removeBucket(String first, String part) {
+        if (first.startsWith(PATH_SEPARATOR + part)) {
+            first = first.substring(part.length()+1);
+        }
+        return first;
+    }
 }

src/main/java/software/amazon/nio/spi/s3/S3ReadAheadByteChannel.java

@@ -178,7 +178,7 @@ private void clearPriorFragments(int currentFragIndx) {
                 .filter(idx -> idx < currentFragIndx)
                 .collect(Collectors.toSet());
 
-        if (priorIndexes.size() > 0) {
+        if (!priorIndexes.isEmpty()) {
             logger.debug("invalidating fragment(s) '{}' from '{}'",
                     priorIndexes.stream().map(Objects::toString).collect(Collectors.joining(", ")), path.toUri());
 

src/main/java/software/amazon/nio/spi/s3/S3SeekableByteChannel.java

@@ -66,7 +66,7 @@ private S3SeekableByteChannel(S3Path s3Path, S3AsyncClient s3Client, long startA
             readDelegate = null;
             writeDelegate = new S3WritableByteChannel(s3Path, s3Client, options, timeout, timeUnit);
             position = 0L;
-        } else if (options.contains(StandardOpenOption.READ) || options.size() == 0) {
+        } else if (options.contains(StandardOpenOption.READ) || options.isEmpty()) {
             LOGGER.debug("using S3ReadAheadByteChannel as read delegate for path '{}'", s3Path.toUri());
             readDelegate = new S3ReadAheadByteChannel(s3Path, config.getMaxFragmentSize(), config.getMaxFragmentNumber(), s3Client, this, timeout, timeUnit);
             writeDelegate = null;

src/test/java/software/amazon/nio/spi/s3/S3ClientProviderTest.java

@@ -6,6 +6,8 @@
 package software.amazon.nio.spi.s3;
 
 import java.util.NoSuchElementException;
+
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.BDDAssertions.then;
 import static org.junit.jupiter.api.Assertions.*;
 import org.junit.jupiter.api.BeforeEach;
@@ -47,11 +49,12 @@ public void initialization() {
 
         assertNotNull(P.configuration);
 
-        assertTrue(P.universalClient() instanceof S3Client);
         assertNotNull(P.universalClient());
+        assertThat(P.universalClient()).isInstanceOf(S3Client.class);
 
-        assertTrue(P.universalClient(true) instanceof S3AsyncClient);
-        assertNotNull(P.universalClient());
+        S3AsyncClient t = P.universalClient(true);
+        assertNotNull(t);
+        assertThat(t).isInstanceOf(S3AsyncClient.class);
 
         S3NioSpiConfiguration config = new S3NioSpiConfiguration();
         assertSame(config, new S3ClientProvider(config).configuration);

src/test/java/software/amazon/nio/spi/s3/S3FileSystemProviderTest.java

@@ -23,12 +23,12 @@
 import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
 import software.amazon.awssdk.services.s3.model.ListObjectsV2Request;
 import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;
+import software.amazon.awssdk.services.s3.model.ObjectIdentifier;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectResponse;
 import software.amazon.awssdk.services.s3.model.S3Object;
 import software.amazon.awssdk.services.s3.paginators.ListObjectsV2Publisher;
 
-import java.io.IOException;
 import java.net.URI;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.AccessDeniedException;
@@ -52,11 +52,11 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.*;
@@ -151,9 +151,7 @@ public void getFileSystem() {
         provider.closeFileSystem(cfs);
 
         assertThrows(
-            FileSystemNotFoundException.class, () -> {
-                provider.getFileSystem(URI.create("s3://nobucket"));
-            }
+            FileSystemNotFoundException.class, () -> provider.getFileSystem(URI.create("s3://nobucket"))
         );
     }
 
@@ -171,11 +169,11 @@ public void closingFileSystemDiscardsItFromCache() {
     public void newByteChannel() throws Exception {
         final SeekableByteChannel channel = provider.newByteChannel(Paths.get(URI.create(pathUri)), Collections.singleton(StandardOpenOption.READ));
         assertNotNull(channel);
-        assertTrue(channel instanceof S3SeekableByteChannel);
+        assertThat(channel).isInstanceOf(S3SeekableByteChannel.class);
     }
 
     @Test
-    public void newDirectoryStream() throws IOException, ExecutionException, InterruptedException {
+    public void newDirectoryStream() {
 
         S3Object object1 = S3Object.builder().key(pathUri+"/key1").build();
         S3Object object2 = S3Object.builder().key(pathUri+"/key2").build();
@@ -196,7 +194,7 @@ public void newDirectoryStream() throws IOException, ExecutionException, Interru
     }
 
     @Test
-    public void pathIteratorForPublisher_withPagination() throws IOException {
+    public void pathIteratorForPublisher_withPagination() {
         final ListObjectsV2Publisher publisher = new ListObjectsV2Publisher(mockClient,
                 ListObjectsV2Request.builder()
                         .bucket(fs.bucketName())
@@ -223,7 +221,7 @@ public void pathIteratorForPublisher_withPagination() throws IOException {
     }
 
     @Test
-    public void pathIteratorForPublisher_appliesFilter() throws IOException {
+    public void pathIteratorForPublisher_appliesFilter() {
         final ListObjectsV2Publisher publisher = new ListObjectsV2Publisher(mockClient,
                 ListObjectsV2Request.builder()
                         .bucket(fs.bucketName())
@@ -283,7 +281,7 @@ public void delete() throws Exception {
         verify(mockClient, times(1)).deleteObjects(argumentCaptor.capture());
         DeleteObjectsRequest captorValue = argumentCaptor.getValue();
         assertEquals("foo", captorValue.bucket());
-        List<String> keys = captorValue.delete().objects().stream().map(objectIdentifier -> objectIdentifier.key()).collect(Collectors.toList());
+        List<String> keys = captorValue.delete().objects().stream().map(ObjectIdentifier::key).collect(Collectors.toList());
         assertEquals(2, keys.size());
         assertTrue(keys.contains("dir/key1"));
         assertTrue(keys.contains("dir/subdir/key2"));
@@ -351,7 +349,7 @@ public void move() throws Exception {
         assertEquals("dir2/subdir/key2", requestValues.get(1).destinationKey());
         ArgumentCaptor<DeleteObjectsRequest> deleteArgumentCaptor = ArgumentCaptor.forClass(DeleteObjectsRequest.class);
         verify(mockClient, times(1)).deleteObjects(deleteArgumentCaptor.capture());
-        List<String> keys = deleteArgumentCaptor.getValue().delete().objects().stream().map(objectIdentifier -> objectIdentifier.key()).collect(Collectors.toList());
+        List<String> keys = deleteArgumentCaptor.getValue().delete().objects().stream().map(ObjectIdentifier::key).collect(Collectors.toList());
         assertEquals(2, keys.size());
         assertTrue(keys.contains("dir1/key1"));
         assertTrue(keys.contains("dir1/subdir/key2"));
@@ -404,7 +402,7 @@ public void checkAccessWithoutException() throws Exception {
     }
 
     @Test
-    public void checkAccessWhenAccessDenied() throws Exception {
+    public void checkAccessWhenAccessDenied() {
         when(mockClient.headObject(any(Consumer.class))).thenReturn(CompletableFuture.supplyAsync(() ->
                 HeadObjectResponse.builder()
                         .sdkHttpResponse(SdkHttpResponse.builder().statusCode(403).build())
@@ -415,7 +413,7 @@ public void checkAccessWhenAccessDenied() throws Exception {
     }
 
     @Test
-    public void checkAccessWhenNoSuchFile() throws Exception {
+    public void checkAccessWhenNoSuchFile() {
         when(mockClient.headObject(any(Consumer.class))).thenReturn(CompletableFuture.supplyAsync(() ->
                 HeadObjectResponse.builder()
                         .sdkHttpResponse(SdkHttpResponse.builder().statusCode(404).build())
@@ -453,7 +451,7 @@ public void readAttributes() {
         Path foo = fs.getPath("/foo");
         final BasicFileAttributes basicFileAttributes = provider.readAttributes(foo, BasicFileAttributes.class);
         assertNotNull(basicFileAttributes);
-        assertTrue(basicFileAttributes instanceof S3BasicFileAttributes);
+        assertThat(basicFileAttributes).isInstanceOf(S3BasicFileAttributes.class);
     }
 
     @Test