Merge pull request #159 from cryptomator/feature/use-dagger-factories

Feature: Use dagger factories instead of builders
cryptomator · Mar 15, 2023 · 2594595 · 2594595
2 parents d61d57a + 62d7e9e
commit 2594595
Show file tree

Hide file tree

Showing 17 changed files with 100 additions and 251 deletions.
diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemComponent.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemComponent.java
@@ -12,25 +12,13 @@ public interface CryptoFileSystemComponent {
 
 	CryptoFileSystemImpl cryptoFileSystem();
 
-	@Subcomponent.Builder
-	interface Builder {
-
-		@BindsInstance
-		Builder cryptor(Cryptor cryptor);
-
-		@BindsInstance
-		Builder vaultConfig(VaultConfig vaultConfig);
-
-		@BindsInstance
-		Builder provider(CryptoFileSystemProvider provider);
-
-		@BindsInstance
-		Builder pathToVault(@PathToVault Path pathToVault);
-
-		@BindsInstance
-		Builder properties(CryptoFileSystemProperties cryptoFileSystemProperties);
-
-		CryptoFileSystemComponent build();
+	@Subcomponent.Factory
+	interface Factory {
+		CryptoFileSystemComponent create(@BindsInstance Cryptor cryptor, //
+										 @BindsInstance VaultConfig vaultConfig, //
+										 @BindsInstance CryptoFileSystemProvider provider, //
+										 @BindsInstance @PathToVault Path pathToVault, //
+										 @BindsInstance CryptoFileSystemProperties cryptoFileSystemProperties);
 	}
 
 }
diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java
@@ -33,13 +33,13 @@ class CryptoFileSystems {
 	private static final Logger LOG = LoggerFactory.getLogger(CryptoFileSystems.class);
 
 	private final ConcurrentMap<Path, CryptoFileSystemImpl> fileSystems = new ConcurrentHashMap<>();
-	private final CryptoFileSystemComponent.Builder cryptoFileSystemComponentBuilder; // sharing reusable builder via synchronized
+	private final CryptoFileSystemComponent.Factory cryptoFileSystemComponentFactory;
 	private final FileSystemCapabilityChecker capabilityChecker;
 	private final SecureRandom csprng;
 
 	@Inject
-	public CryptoFileSystems(CryptoFileSystemComponent.Builder cryptoFileSystemComponentBuilder, FileSystemCapabilityChecker capabilityChecker, SecureRandom csprng) {
-		this.cryptoFileSystemComponentBuilder = cryptoFileSystemComponentBuilder;
+	public CryptoFileSystems(CryptoFileSystemComponent.Factory cryptoFileSystemComponentFactory, FileSystemCapabilityChecker capabilityChecker, SecureRandom csprng) {
+		this.cryptoFileSystemComponentFactory = cryptoFileSystemComponentFactory;
 		this.capabilityChecker = capabilityChecker;
 		this.csprng = csprng;
 	}
@@ -59,7 +59,7 @@ public CryptoFileSystemImpl create(CryptoFileSystemProvider provider, Path pathT
 				checkVaultRootExistence(pathToVault, cryptor);
 				return fileSystems.compute(normalizedPathToVault, (path, fs) -> {
 					if (fs == null) {
-						return create(provider, normalizedPathToVault, adjustedProperties, cryptor, config);
+						return cryptoFileSystemComponentFactory.create(cryptor, config, provider, normalizedPathToVault, adjustedProperties).cryptoFileSystem();
 					} else {
 						throw new FileSystemAlreadyExistsException();
 					}
@@ -86,18 +86,6 @@ private void checkVaultRootExistence(Path pathToVault, Cryptor cryptor) throws C
 		}
 	}
 
-	// synchronized access to non-threadsafe cryptoFileSystemComponentBuilder required
-	private synchronized CryptoFileSystemImpl create(CryptoFileSystemProvider provider, Path pathToVault, CryptoFileSystemProperties properties, Cryptor cryptor, VaultConfig config) {
-		return cryptoFileSystemComponentBuilder //
-				.cryptor(cryptor) //
-				.vaultConfig(config) //
-				.pathToVault(pathToVault) //
-				.properties(properties) //
-				.provider(provider) //
-				.build() //
-				.cryptoFileSystem();
-	}
-
 	/**
 	 * Attempts to read a vault config file
 	 *

diff --git a/src/main/java/org/cryptomator/cryptofs/attr/AttributeComponent.java b/src/main/java/org/cryptomator/cryptofs/attr/AttributeComponent.java
@@ -24,18 +24,12 @@ default <T extends BasicFileAttributes> T attributes(Class<T> type) {
 		}
 	}
 
-	@Subcomponent.Builder
-	interface Builder {
+	@Subcomponent.Factory
+	interface Factory {
 
-		@BindsInstance
-		Builder ciphertextPath(Path ciphertextPath);
+		AttributeComponent create(@BindsInstance Path ciphertextPath, //
+								  @BindsInstance CiphertextFileType ciphertextFileType, //
+								  @BindsInstance @Named("ciphertext") BasicFileAttributes ciphertextAttributes);
 
-		@BindsInstance
-		Builder ciphertextFileType(CiphertextFileType ciphertextFileType);
-
-		@BindsInstance
-		Builder ciphertextAttributes(@Named("ciphertext") BasicFileAttributes ciphertextAttributes);
-
-		AttributeComponent build();
 	}
 }
diff --git a/src/main/java/org/cryptomator/cryptofs/attr/AttributeProvider.java b/src/main/java/org/cryptomator/cryptofs/attr/AttributeProvider.java
@@ -16,7 +16,6 @@
 import org.cryptomator.cryptofs.common.CiphertextFileType;
 
 import javax.inject.Inject;
-import javax.inject.Provider;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
@@ -26,13 +25,13 @@
 @CryptoFileSystemScoped
 public class AttributeProvider {
 
-	private final Provider<AttributeComponent.Builder> attributeComponentBuilderProvider;
+	private final AttributeComponent.Factory attributeComponentFactory;
 	private final CryptoPathMapper pathMapper;
 	private final Symlinks symlinks;
 
 	@Inject
-	AttributeProvider(Provider<AttributeComponent.Builder> attributeComponentBuilderProvider, CryptoPathMapper pathMapper, Symlinks symlinks) {
-		this.attributeComponentBuilderProvider = attributeComponentBuilderProvider;
+	AttributeProvider(AttributeComponent.Factory attributeComponentFactory, CryptoPathMapper pathMapper, Symlinks symlinks) {
+		this.attributeComponentFactory = attributeComponentFactory;
 		this.pathMapper = pathMapper;
 		this.symlinks = symlinks;
 	}
@@ -45,13 +44,10 @@ public <A extends BasicFileAttributes> A readAttributes(CryptoPath cleartextPath
 		}
 		Path ciphertextPath = getCiphertextPath(cleartextPath, ciphertextFileType);
 		A ciphertextAttrs = Files.readAttributes(ciphertextPath, type);
-		AttributeComponent.Builder builder = attributeComponentBuilderProvider.get();
-		return builder  //
-				.ciphertextFileType(ciphertextFileType) //
-				.ciphertextPath(ciphertextPath) //
-				.ciphertextAttributes(ciphertextAttrs) //
-				.build() //
-				.attributes(type);
+		return attributeComponentFactory.create(ciphertextPath, //
+						ciphertextFileType, //
+						ciphertextAttrs) //
+				.attributes(type); //
 	}
 
 	private Path getCiphertextPath(CryptoPath path, CiphertextFileType type) throws IOException {

diff --git a/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewComponent.java b/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewComponent.java
@@ -14,19 +14,11 @@ public interface AttributeViewComponent {
 
 	Optional<FileAttributeView> attributeView();
 
-	@Subcomponent.Builder
-	interface Builder {
+	@Subcomponent.Factory
+	interface Factory {
 
-		@BindsInstance
-		Builder cleartextPath(CryptoPath cleartextPath);
+		AttributeViewComponent create(@BindsInstance CryptoPath cleartextPath, @BindsInstance Class<? extends FileAttributeView> type, @BindsInstance LinkOption[] linkOptions);
 
-		@BindsInstance
-		Builder viewType(Class<? extends FileAttributeView> type);
-
-		@BindsInstance
-		Builder linkOptions(LinkOption[] linkOptions);
-
-		AttributeViewComponent build();
 	}
 
 }

diff --git a/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewProvider.java b/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewProvider.java
@@ -14,7 +14,6 @@
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
-import javax.inject.Provider;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.attribute.FileAttributeView;
@@ -25,27 +24,21 @@ public class AttributeViewProvider {
 
 	private static final Logger LOG = LoggerFactory.getLogger(AttributeViewProvider.class);
 
-	private final Provider<AttributeViewComponent.Builder> attrViewComponentBuilderProvider;
+	private final AttributeViewComponent.Factory attrViewComponentFactory;
 
 	@Inject
-	AttributeViewProvider(Provider<AttributeViewComponent.Builder> attrViewComponentBuilderProvider) {
-		this.attrViewComponentBuilderProvider = attrViewComponentBuilderProvider;
+	AttributeViewProvider(AttributeViewComponent.Factory attrViewComponentFactory) {
+		this.attrViewComponentFactory = attrViewComponentFactory;
 	}
 
 	/**
 	 * @param cleartextPath the unencrypted path to the file
-	 * @param type          the Class object corresponding to the file attribute view
+	 * @param type the Class object corresponding to the file attribute view
 	 * @return a file attribute view of the specified type, or <code>null</code> if the attribute view type is not available
 	 * @see Files#getFileAttributeView(java.nio.file.Path, Class, java.nio.file.LinkOption...)
 	 */
 	public <A extends FileAttributeView> A getAttributeView(CryptoPath cleartextPath, Class<A> type, LinkOption... options) {
-		AttributeViewComponent.Builder builder = attrViewComponentBuilderProvider.get();
-		Optional<FileAttributeView> view = builder //
-				.cleartextPath(cleartextPath) //
-				.viewType(type) //
-				.linkOptions(options) //
-				.build() //
-				.attributeView();
+		Optional<FileAttributeView> view = attrViewComponentFactory.create(cleartextPath, type, options).attributeView();
 		if (view.isPresent() && type.isInstance(view.get())) {
 			return type.cast(view.get());
 		} else {

diff --git a/src/main/java/org/cryptomator/cryptofs/ch/ChannelComponent.java b/src/main/java/org/cryptomator/cryptofs/ch/ChannelComponent.java
@@ -13,25 +13,14 @@ public interface ChannelComponent {
 
 	CleartextFileChannel channel();
 
-	@Subcomponent.Builder
-	interface Builder {
-
-		@BindsInstance
-		Builder openOptions(EffectiveOpenOptions options);
-
-		@BindsInstance
-		Builder onClose(ChannelCloseListener listener);
-
-		@BindsInstance
-		Builder ciphertextChannel(FileChannel ciphertextChannel);
-
-		@BindsInstance
-		Builder mustWriteHeader(@MustWriteHeader boolean mustWriteHeader);
-
-		@BindsInstance
-		Builder fileHeader(FileHeader fileHeader);
-
-		ChannelComponent build();
+	@Subcomponent.Factory
+	interface Factory {
+
+		ChannelComponent create(@BindsInstance FileChannel ciphertextChannel, //
+								@BindsInstance FileHeader fileHeader, //
+								@BindsInstance @MustWriteHeader boolean mustWriteHeader, //
+								@BindsInstance EffectiveOpenOptions options, //
+								@BindsInstance ChannelCloseListener listener); //
 	}
 
 }
diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamComponent.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamComponent.java
@@ -14,25 +14,14 @@ public interface DirectoryStreamComponent {
 
 	CryptoDirectoryStream directoryStream();
 
-	@Subcomponent.Builder
-	interface Builder {
-
-		@BindsInstance
-		Builder cleartextPath(@Named("cleartextPath") Path cleartextPath);
-
-		@BindsInstance
-		Builder dirId(@Named("dirId") String dirId);
-
-		@BindsInstance
-		Builder ciphertextDirectoryStream(DirectoryStream<Path> ciphertextDirectoryStream);
-
-		@BindsInstance
-		Builder filter(DirectoryStream.Filter<? super Path> filter);
-
-		@BindsInstance
-		Builder onClose(Consumer<CryptoDirectoryStream> onClose);
-
-		DirectoryStreamComponent build();
+	@Subcomponent.Factory
+	interface Factory {
+
+		DirectoryStreamComponent create(@BindsInstance @Named("cleartextPath") Path cleartextPath, //
+										@BindsInstance @Named("dirId") String dirId, //
+										@BindsInstance DirectoryStream<Path> ciphertextDirectoryStream, //
+										@BindsInstance DirectoryStream.Filter<? super Path> filter, //
+										@BindsInstance Consumer<CryptoDirectoryStream> onClose);
 	}
 
 }

diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java
@@ -20,31 +20,25 @@
 public class DirectoryStreamFactory {
 
 	private final CryptoPathMapper cryptoPathMapper;
-	private final DirectoryStreamComponent.Builder directoryStreamComponentBuilder; // sharing reusable builder via synchronized
+	private final DirectoryStreamComponent.Factory directoryStreamComponentFactory;
 	private final Map<CryptoDirectoryStream, DirectoryStream<Path>> streams = new HashMap<>();
 
 	private volatile boolean closed = false;
 
 	@Inject
-	public DirectoryStreamFactory(CryptoPathMapper cryptoPathMapper, DirectoryStreamComponent.Builder directoryStreamComponentBuilder) {
+	public DirectoryStreamFactory(CryptoPathMapper cryptoPathMapper, DirectoryStreamComponent.Factory directoryStreamComponentFactory) {
 		this.cryptoPathMapper = cryptoPathMapper;
-		this.directoryStreamComponentBuilder = directoryStreamComponentBuilder;
+		this.directoryStreamComponentFactory = directoryStreamComponentFactory;
 	}
 
+	//TODO: is synchronized still needed? One reason was, that a dagger builder was used (replaced by thread safe factory)
 	public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter<? super Path> filter) throws IOException {
 		if (closed) {
 			throw new ClosedFileSystemException();
 		}
 		CiphertextDirectory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir);
 		DirectoryStream<Path> ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path, this::matchesEncryptedContentPattern);
-		CryptoDirectoryStream cleartextDirStream = directoryStreamComponentBuilder //
-				.dirId(ciphertextDir.dirId) //
-				.ciphertextDirectoryStream(ciphertextDirStream) //
-				.cleartextPath(cleartextDir) //
-				.filter(filter) //
-				.onClose(streams::remove) //
-				.build() //
-				.directoryStream();
+		var cleartextDirStream = directoryStreamComponentFactory.create(cleartextDir, ciphertextDir.dirId, ciphertextDirStream, filter, streams::remove).directoryStream();
 		streams.put(cleartextDirStream, ciphertextDirStream);
 		return cleartextDirStream;
 	}

diff --git a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java
@@ -9,7 +9,6 @@
 package org.cryptomator.cryptofs.fh;
 
 import org.cryptomator.cryptofs.EffectiveOpenOptions;
-import org.cryptomator.cryptofs.ch.ChannelComponent;
 import org.cryptomator.cryptofs.ch.CleartextFileChannel;
 import org.cryptomator.cryptolib.api.Cryptor;
 import org.cryptomator.cryptolib.api.FileHeader;
@@ -87,14 +86,9 @@ public synchronized FileChannel newFileChannel(EffectiveOpenOptions options, Fil
 				isNewHeader = false;
 			}
 			initFileSize(ciphertextFileChannel);
-			ChannelComponent channelComponent = component.newChannelComponent() //
-					.ciphertextChannel(ciphertextFileChannel) //
-					.openOptions(options) //
-					.onClose(this::channelClosed) //
-					.mustWriteHeader(isNewHeader) //
-					.fileHeader(header) //
-					.build();
-			cleartextFileChannel = channelComponent.channel();
+			cleartextFileChannel = component.newChannelComponent() //
+					.create(ciphertextFileChannel, header, isNewHeader, options, this::channelClosed) //
+					.channel();
 		} finally {
 			if (cleartextFileChannel == null) { // i.e. something didn't work
 				closeQuietly(ciphertextFileChannel);

diff --git a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFileComponent.java b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFileComponent.java
@@ -12,18 +12,13 @@ public interface OpenCryptoFileComponent {
 
 	OpenCryptoFile openCryptoFile();
 
-	ChannelComponent.Builder newChannelComponent();
+	ChannelComponent.Factory newChannelComponent();
 
-	@Subcomponent.Builder
-	interface Builder {
+	@Subcomponent.Factory
+	interface Factory {
 
-		@BindsInstance
-		Builder path(@OriginalOpenFilePath Path path);
-
-		@BindsInstance
-		Builder onClose(FileCloseListener listener);
-
-		OpenCryptoFileComponent build();
+		OpenCryptoFileComponent create(@BindsInstance @OriginalOpenFilePath Path path, //
+									   @BindsInstance FileCloseListener onCloseListener);
 	}
 
 }