diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/DefaultS3FileSystemRegistry.java b/src/main/java/org/carlspring/cloud/storage/s3fs/DefaultS3FileSystemRegistry.java new file mode 100644 index 00000000..99d41f05 --- /dev/null +++ b/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 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 getFileSystems() { + return fileSystems; + } + +} diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProvider.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProvider.java index bed9ddfb..1ac9b7d2 100644 --- a/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProvider.java +++ b/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 fileSystems = new ConcurrentHashMap<>(); - private static final List 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 getFilesystems() + @SuppressWarnings("unchecked") + protected T getFileSystemRegistry() { - return fileSystems; + return (T) DefaultS3FileSystemRegistry.getInstance(); } + public ConcurrentMap getFileSystems() + { + return getFileSystemRegistry().getFileSystems(); + } } diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemRegistry.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemRegistry.java new file mode 100644 index 00000000..8937d428 --- /dev/null +++ b/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 getFileSystems(); +} diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannel.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannel.java index 5dba47d9..efb083da 100644 --- a/src/main/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannel.java +++ b/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; } diff --git a/src/test/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProviderMock.java b/src/test/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProviderMock.java new file mode 100644 index 00000000..f043e56e --- /dev/null +++ b/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 byteChannelSpies = new ConcurrentHashMap<>(); + + public ConcurrentHashMap getByteChannelsSpies() + { + return byteChannelSpies; + } + + @Override + public String getScheme() + { + return super.getScheme() + "mock"; + } + + @Override + public FileSystem newFileSystem(URI uri, Map env) + { + return super.newFileSystem(uri, env); + } + + @Override + public SeekableByteChannel newByteChannel(Path path, + Set 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(); + } +} diff --git a/src/test/java/org/carlspring/cloud/storage/s3fs/S3FileSystemRegistryMock.java b/src/test/java/org/carlspring/cloud/storage/s3fs/S3FileSystemRegistryMock.java new file mode 100644 index 00000000..afefaf03 --- /dev/null +++ b/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 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 getFileSystems() + { + return fileSystems; + } + +} diff --git a/src/test/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannelTest.java b/src/test/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannelTest.java index a3db6de8..a98f488f 100644 --- a/src/test/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannelTest.java +++ b/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(); diff --git a/src/test/java/org/carlspring/cloud/storage/s3fs/S3UnitTestBase.java b/src/test/java/org/carlspring/cloud/storage/s3fs/S3UnitTestBase.java index 0a9bfff0..b44b574c 100644 --- a/src/test/java/org/carlspring/cloud/storage/s3fs/S3UnitTestBase.java +++ b/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 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) { diff --git a/src/test/java/org/carlspring/cloud/storage/s3fs/util/S3EndpointConstant.java b/src/test/java/org/carlspring/cloud/storage/s3fs/util/S3EndpointConstant.java index 0539be89..aac47260 100644 --- a/src/test/java/org/carlspring/cloud/storage/s3fs/util/S3EndpointConstant.java +++ b/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/"); diff --git a/src/test/resources/META-INF/services/java.nio.file.spi.FileSystemProvider b/src/test/resources/META-INF/services/java.nio.file.spi.FileSystemProvider index a4bdda9e..4a953bff 100644 --- a/src/test/resources/META-INF/services/java.nio.file.spi.FileSystemProvider +++ b/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