715 WIP

src/main/java/org/carlspring/cloud/storage/s3fs/DefaultS3FileSystemRegistry.java

@@ -0,0 +1,26 @@
+package org.carlspring.cloud.storage.s3fs;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+class DefaultS3FileSystemRegistry implements S3FileSystemRegistry
+{
+
+    private final ConcurrentMap<String, S3FileSystem> fileSystems = new ConcurrentHashMap<>();
+
+    private DefaultS3FileSystemRegistry() {
+    }
+
+    private static class Holder {
+        private static final DefaultS3FileSystemRegistry INSTANCE = new DefaultS3FileSystemRegistry();
+    }
+
+    public static DefaultS3FileSystemRegistry getInstance() {
+        return Holder.INSTANCE;
+    }
+
+    public ConcurrentMap<String, S3FileSystem> getFileSystems() {
+        return fileSystems;
+    }
+
+}

src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProvider.java

@@ -36,7 +36,6 @@
 import java.nio.file.spi.FileSystemProvider;
 import java.util.*;
 import java.util.concurrent.CompletionException;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutorService;
 
@@ -83,8 +82,6 @@ public class S3FileSystemProvider
 
     public static final String S3_FACTORY_CLASS = "s3fs.amazon.s3.factory.class";
 
-    private volatile static ConcurrentMap<String, S3FileSystem> fileSystems = new ConcurrentHashMap<>();
-
     private static final List<String> PROPS_TO_OVERLOAD = Arrays.asList(ACCESS_KEY,
                                                                         SECRET_KEY,
                                                                         CACHE_ATTRIBUTES_TTL,
@@ -134,7 +131,7 @@ public FileSystem newFileSystem(URI uri,
 
         // try to get the filesystem by the key
         String key = getFileSystemKey(uri, props);
-        if (fileSystems.containsKey(key))
+        if (getFileSystems().containsKey(key))
         {
             String safeName = uri.getScheme() + "://";
             String userInfo = uri.getUserInfo();
@@ -148,7 +145,7 @@ public FileSystem newFileSystem(URI uri,
         // create the filesystem with the final properties, store and return
         S3FileSystem fileSystem = createFileSystem(uri, props);
 
-        fileSystems.put(fileSystem.getKey(), fileSystem);
+        getFileSystems().put(fileSystem.getKey(), fileSystem);
 
         return fileSystem;
     }
@@ -383,9 +380,9 @@ public FileSystem getFileSystem(URI uri,
 
         String key = this.getFileSystemKey(uri, props); // s3fs.access.key is part of the key here.
 
-        if (fileSystems.containsKey(key))
+        if (getFileSystems().containsKey(key))
         {
-            return fileSystems.get(key);
+            return getFileSystems().get(key);
         }
 
         return newFileSystem(uri, env);
@@ -398,9 +395,9 @@ public S3FileSystem getFileSystem(URI uri)
 
         String key = this.getFileSystemKey(uri);
 
-        if (fileSystems.containsKey(key))
+        if (getFileSystems().containsKey(key))
         {
-            return fileSystems.get(key);
+            return getFileSystems().get(key);
         }
         else
         {
@@ -1114,25 +1111,36 @@ boolean exists(S3Path path)
 
     public void close(S3FileSystem fileSystem)
     {
-        if (fileSystem.getKey() != null && fileSystems.containsKey(fileSystem.getKey()))
+        if (fileSystem.getKey() != null && getFileSystems().containsKey(fileSystem.getKey()))
         {
             fileSystem.getFileAttributesCache().invalidateAll();
-            fileSystems.remove(fileSystem.getKey());
+            getFileSystems().remove(fileSystem.getKey());
+        }
+    }
+
+    /**
+     * Close all opened s3 filesystems.
+     */
+    public void close() {
+        for (S3FileSystem fileSystem : getFileSystems().values()) {
+            close(fileSystem);
         }
     }
 
     public boolean isOpen(S3FileSystem s3FileSystem)
     {
-        return fileSystems.containsKey(s3FileSystem.getKey());
+        return getFileSystems().containsKey(s3FileSystem.getKey());
     }
 
-    /**
-     * only 4 testing
-     */
 
-    protected static ConcurrentMap<String, S3FileSystem> getFilesystems()
+    @SuppressWarnings("unchecked")
+    protected <T extends S3FileSystemRegistry> T getFileSystemRegistry()
     {
-        return fileSystems;
+        return (T) DefaultS3FileSystemRegistry.getInstance();
     }
 
+    public ConcurrentMap<String, S3FileSystem> getFileSystems()
+    {
+        return getFileSystemRegistry().getFileSystems();
+    }
 }

src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemRegistry.java

@@ -0,0 +1,11 @@
+package org.carlspring.cloud.storage.s3fs;
+
+import java.util.concurrent.ConcurrentMap;
+
+/**
+ * This is an internal interface to allow for easier and more reliable testing.
+ */
+interface S3FileSystemRegistry
+{
+    ConcurrentMap<String, S3FileSystem> getFileSystems();
+}

src/main/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannel.java

@@ -139,7 +139,7 @@ public void close()
                 return;
             }
 
-            if (options.contains(StandardOpenOption.READ) && options.size() == 1)
+            if ((options.contains(StandardOpenOption.READ) && options.size() == 1) || options.isEmpty())
             {
                 return;
             }

src/test/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProviderMock.java

@@ -0,0 +1,57 @@
+package org.carlspring.cloud.storage.s3fs;
+
+import java.io.IOException;
+import java.net.URI;
+import java.nio.channels.SeekableByteChannel;
+import java.nio.file.FileSystem;
+import java.nio.file.OpenOption;
+import java.nio.file.Path;
+import java.nio.file.attribute.FileAttribute;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import static org.mockito.Mockito.spy;
+
+/**
+ * A Mock friendly version of the {@link S3FileSystemProvider} which allows a bit more flexibility for testing purposes.
+ */
+public class S3FileSystemProviderMock extends S3FileSystemProvider
+{
+
+    protected final ConcurrentHashMap<Path, S3SeekableByteChannel> byteChannelSpies = new ConcurrentHashMap<>();
+
+    public ConcurrentHashMap<Path, S3SeekableByteChannel> getByteChannelsSpies()
+    {
+        return byteChannelSpies;
+    }
+
+    @Override
+    public String getScheme()
+    {
+        return super.getScheme() + "mock";
+    }
+
+    @Override
+    public FileSystem newFileSystem(URI uri, Map<String, ?> env)
+    {
+        return super.newFileSystem(uri, env);
+    }
+
+    @Override
+    public SeekableByteChannel newByteChannel(Path path,
+                                              Set<? extends OpenOption> options,
+                                              FileAttribute<?>... attrs) throws IOException
+    {
+        S3SeekableByteChannel delegate = (S3SeekableByteChannel) super.newByteChannel(path, options, attrs);
+        S3SeekableByteChannel mock = spy(delegate);
+        byteChannelSpies.put(path, mock);
+        return mock;
+    }
+
+    @SuppressWarnings("unchecked")
+    protected S3FileSystemRegistryMock getFileSystemRegistry()
+    {
+        return S3FileSystemRegistryMock.getInstance();
+    }
+}

src/test/java/org/carlspring/cloud/storage/s3fs/S3FileSystemRegistryMock.java

@@ -0,0 +1,30 @@
+package org.carlspring.cloud.storage.s3fs;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+class S3FileSystemRegistryMock implements S3FileSystemRegistry
+{
+
+    private final ConcurrentMap<String, S3FileSystem> fileSystems = new ConcurrentHashMap<>();
+
+    private S3FileSystemRegistryMock()
+    {
+    }
+
+    private static class Holder
+    {
+        private static final S3FileSystemRegistryMock INSTANCE = new S3FileSystemRegistryMock();
+    }
+
+    public static S3FileSystemRegistryMock getInstance()
+    {
+        return Holder.INSTANCE;
+    }
+
+    public ConcurrentMap<String, S3FileSystem> getFileSystems()
+    {
+        return fileSystems;
+    }
+
+}

src/test/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannelTest.java

@@ -39,6 +39,7 @@ public void setup()
             throws IOException
     {
         fileSystem = FileSystems.newFileSystem(S3EndpointConstant.S3_GLOBAL_URI_TEST, null);
+        mockFileSystem = FileSystems.newFileSystem(S3EndpointConstant.S3_GLOBAL_URI_MOCK_TEST, null);
     }
 
     @ParameterizedTest
@@ -62,32 +63,24 @@ void constructor(final boolean tempFileRequired)
     }
 
     @Test
-    void readDontNeedToSyncTempFile()
+    void shouldNotCallSyncWhenOnlyReading()
             throws IOException
     {
         S3ClientMock client = S3MockFactory.getS3ClientMock();
         client.bucket("buck").file("file1");
+        client.bucket("buck").file("file2");
 
-        S3Path file1 = (S3Path) FileSystems.getFileSystem(S3EndpointConstant.S3_GLOBAL_URI_TEST).getPath("/buck/file1");
-        S3SeekableByteChannel channel = spy(new S3SeekableByteChannel(file1, EnumSet.of(StandardOpenOption.READ), true));
+        S3FileSystem fs = (S3FileSystem) FileSystems.getFileSystem(S3EndpointConstant.S3_GLOBAL_URI_MOCK_TEST);
+        S3FileSystemProviderMock mockFsProvider = (S3FileSystemProviderMock) fs.provider();
 
-        assertNotNull(channel);
+        S3Path file1 = fs.getPath("/buck/file1");
+        S3Path file2 = fs.getPath("/buck/file2");
 
-        channel.close();
+        Files.readAllBytes(file1);
 
-        verify(channel, never()).sync();
-    }
-
-    @Test
-    void tempFileRequiredFlagToFalseDontNeedToSyncTempFile()
-            throws IOException
-    {
-        S3ClientMock client = S3MockFactory.getS3ClientMock();
-        client.bucket("buck").file("file1");
-
-        S3Path file1 = (S3Path) FileSystems.getFileSystem(S3EndpointConstant.S3_GLOBAL_URI_TEST).getPath("/buck/file1");
-        S3SeekableByteChannel channel = spy(new S3SeekableByteChannel(file1, EnumSet.of(StandardOpenOption.READ), false));
+        verify(mockFsProvider.getByteChannelsSpies().get(file1), never()).sync();
 
+        S3SeekableByteChannel channel = spy(new S3SeekableByteChannel(file2, EnumSet.of(StandardOpenOption.READ), false));
         assertNotNull(channel);
 
         channel.close();

src/test/java/org/carlspring/cloud/storage/s3fs/S3UnitTestBase.java

@@ -5,7 +5,10 @@
 
 import java.io.IOException;
 import java.nio.file.FileSystem;
+import java.nio.file.spi.FileSystemProvider;
+import java.util.ArrayList;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -23,6 +26,7 @@ public abstract class S3UnitTestBase extends BaseTest
     protected S3FileSystemProvider s3fsProvider;
 
     protected FileSystem fileSystem;
+    protected FileSystem mockFileSystem;
 
     @BeforeEach
     public void setProperties()
@@ -49,16 +53,16 @@ public void closeMemory()
     {
         S3ClientMock client = S3MockFactory.getS3ClientMock();
         client.close();
-
-        for (S3FileSystem s3FileSystem : S3FileSystemProvider.getFilesystems().values())
+        ArrayList<FileSystemProvider> fileSystemProviders = FileSystemProvider.installedProviders()
+                                                                              .stream()
+                                                                              .filter(p -> p.getClass().isAssignableFrom(S3FileSystemProvider.class))
+                                                                              .collect(Collectors.toCollection(ArrayList::new));
+        for (FileSystemProvider provider : fileSystemProviders)
         {
-            try
-            {
-                s3FileSystem.close();
-            }
-            catch (Exception e)
+            if (S3FileSystemProvider.class.isAssignableFrom(provider.getClass()))
             {
-                //ignore
+                S3FileSystemProvider s3Provider = (S3FileSystemProvider) provider;
+                s3Provider.close();
             }
         }
     }
@@ -78,6 +82,10 @@ public void tearDown()
             {
                 fileSystem.close();
             }
+            if (mockFileSystem != null)
+            {
+                mockFileSystem.close();
+            }
         }
         catch (Throwable ignored)
         {

src/test/java/org/carlspring/cloud/storage/s3fs/util/S3EndpointConstant.java

@@ -7,6 +7,8 @@ public class S3EndpointConstant
 
     public static final URI S3_GLOBAL_URI_TEST = URI.create("s3://s3.test.amazonaws.com/");
 
+    public static final URI S3_GLOBAL_URI_MOCK_TEST = URI.create("s3mock://s3.test.amazonaws.com/");
+
     public static final String S3_REGION_URI_TEST = "s3://s3.test.%s.amazonaws.com/";
 
     public static final URI S3_GLOBAL_URI_IT = URI.create("s3://s3.amazonaws.com/");

src/test/resources/META-INF/services/java.nio.file.spi.FileSystemProvider

@@ -1,2 +1,3 @@
 org.carlspring.cloud.storage.s3fs.S3FileSystemProvider
+org.carlspring.cloud.storage.s3fs.S3FileSystemProviderMock
 com.github.marschall.memoryfilesystem.MemoryFileSystemProvider