From 98590ecec532197e92b7bfc5169a8c1ab598d60a Mon Sep 17 00:00:00 2001 From: Jan-Peter Klein Date: Fri, 1 Dec 2023 09:11:43 +0100 Subject: [PATCH] refactoring --- .../common/mount/ActualMountService.java | 6 --- .../org/cryptomator/common/mount/Mounter.java | 48 +++++++++++++++--- .../org/cryptomator/common/vaults/Vault.java | 33 ++---------- .../common/vaults/VaultListManager.java | 10 ++-- .../common/vaults/VaultModule.java | 50 ------------------- .../vaultoptions/MountOptionsController.java | 13 +++-- 6 files changed, 55 insertions(+), 105 deletions(-) delete mode 100644 src/main/java/org/cryptomator/common/mount/ActualMountService.java diff --git a/src/main/java/org/cryptomator/common/mount/ActualMountService.java b/src/main/java/org/cryptomator/common/mount/ActualMountService.java deleted file mode 100644 index a96cc8e37..000000000 --- a/src/main/java/org/cryptomator/common/mount/ActualMountService.java +++ /dev/null @@ -1,6 +0,0 @@ -package org.cryptomator.common.mount; - -import org.cryptomator.integrations.mount.MountService; - -public record ActualMountService(MountService service, boolean isDesired) { -} diff --git a/src/main/java/org/cryptomator/common/mount/Mounter.java b/src/main/java/org/cryptomator/common/mount/Mounter.java index 0cd838b9e..cf5f2b678 100644 --- a/src/main/java/org/cryptomator/common/mount/Mounter.java +++ b/src/main/java/org/cryptomator/common/mount/Mounter.java @@ -1,6 +1,7 @@ package org.cryptomator.common.mount; import org.cryptomator.common.Environment; +import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.integrations.mount.Mount; import org.cryptomator.integrations.mount.MountBuilder; @@ -8,11 +9,13 @@ import org.cryptomator.integrations.mount.MountFailedException; import org.cryptomator.integrations.mount.MountService; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Singleton; -import javafx.beans.value.ObservableValue; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import static org.cryptomator.integrations.mount.MountCapability.MOUNT_AS_DRIVE_LETTER; import static org.cryptomator.integrations.mount.MountCapability.MOUNT_TO_EXISTING_DIR; @@ -23,13 +26,24 @@ import static org.cryptomator.integrations.mount.MountCapability.UNMOUNT_FORCED; @Singleton public class Mounter { + private static final List problematicFuseMountServices = List.of("org.cryptomator.frontend.fuse.mount.MacFuseMountProvider", "org.cryptomator.frontend.fuse.mount.FuseTMountProvider"); + private final Environment env; private final WindowsDriveLetters driveLetters; + private final Settings settings; + private final List mountProviders; + private final AtomicReference firstUsedProblematicFuseMountService; @Inject - public Mounter(Environment env, WindowsDriveLetters driveLetters) { + public Mounter(Environment env, // + WindowsDriveLetters driveLetters, // + Settings settings, List mountProviders, // + @Named("FUPFMS") AtomicReference firstUsedProblematicFuseMountService) { this.env = env; this.driveLetters = driveLetters; + this.settings = settings; + this.mountProviders = mountProviders; + this.firstUsedProblematicFuseMountService = firstUsedProblematicFuseMountService; } private class SettledMounter { @@ -124,12 +138,32 @@ public class Mounter { } - public MountHandle mount(VaultSettings vaultSettings, Path cryptoFsRoot, ObservableValue actualMountService) throws IOException, MountFailedException { - var mountService = actualMountService.getValue().service(); - var builder = mountService.forFileSystem(cryptoFsRoot); - var internal = new SettledMounter(mountService, builder, vaultSettings); + public MountHandle mount(VaultSettings vaultSettings, Path cryptoFsRoot) throws IOException, MountFailedException { + var fallbackProvider = mountProviders.stream().findFirst().orElse(null); + var defMntServ = mountProviders.stream().filter(s -> s.getClass().getName().equals(settings.mountService.getValue())).findFirst().orElse(fallbackProvider); + var selMntServ = mountProviders.stream().filter(s -> s.getClass().getName().equals(vaultSettings.mountService.getValue())).findFirst().orElse(defMntServ); + + var targetIsProblematicFuse = isProblematicFuseService(selMntServ); + if (targetIsProblematicFuse && firstUsedProblematicFuseMountService.get() == null) { + firstUsedProblematicFuseMountService.set(selMntServ); + } + + var fuseRestartRequired = firstUsedProblematicFuseMountService.get() != null // + && isProblematicFuseService(selMntServ) // + && !firstUsedProblematicFuseMountService.get().equals(selMntServ); + + if (fuseRestartRequired) { + throw new FuseRestartRequiredException("fuseRestartRequired"); + } + + var builder = selMntServ.forFileSystem(cryptoFsRoot); + var internal = new SettledMounter(selMntServ, builder, vaultSettings); var cleanup = internal.prepare(); - return new MountHandle(builder.mount(), mountService.hasCapability(UNMOUNT_FORCED), cleanup); + return new MountHandle(builder.mount(), selMntServ.hasCapability(UNMOUNT_FORCED), cleanup); + } + + public static boolean isProblematicFuseService(MountService service) { + return problematicFuseMountServices.contains(service.getClass().getName()); } public record MountHandle(Mount mountObj, boolean supportsUnmountForced, Runnable specialCleanup) { diff --git a/src/main/java/org/cryptomator/common/vaults/Vault.java b/src/main/java/org/cryptomator/common/vaults/Vault.java index d617a5db3..d741ae5c4 100644 --- a/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -10,10 +10,7 @@ package org.cryptomator.common.vaults; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.Constants; -import org.cryptomator.common.mount.ActualMountService; -import org.cryptomator.common.mount.FuseRestartRequiredException; import org.cryptomator.common.mount.Mounter; -import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.cryptofs.CryptoFileSystem; import org.cryptomator.cryptofs.CryptoFileSystemProperties; @@ -24,7 +21,6 @@ import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.MasterkeyLoader; import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.integrations.mount.MountFailedException; -import org.cryptomator.integrations.mount.MountService; import org.cryptomator.integrations.mount.Mountpoint; import org.cryptomator.integrations.mount.UnmountFailedException; import org.slf4j.Logger; @@ -41,12 +37,10 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyStringProperty; import javafx.beans.property.SimpleBooleanProperty; -import javafx.beans.value.ObservableValue; import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.EnumSet; -import java.util.List; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; @@ -58,9 +52,7 @@ public class Vault { private static final Path HOME_DIR = Paths.get(SystemUtils.USER_HOME); private static final int UNLIMITED_FILENAME_LENGTH = Integer.MAX_VALUE; - private final Settings settings; private final VaultSettings vaultSettings; - private final List mountProviders; private final AtomicReference cryptoFileSystem; private final VaultState state; private final ObjectProperty lastKnownException; @@ -76,28 +68,20 @@ public class Vault { private final ObjectBinding mountPoint; private final Mounter mounter; private final BooleanProperty showingStats; - private final ObservableValue actualMountService; - private final AtomicReference firstUsedProblematicFuseMountService; private final AtomicReference mountHandle = new AtomicReference<>(null); @Inject - Vault(Settings settings, // - VaultSettings vaultSettings, // + Vault(VaultSettings vaultSettings, // VaultConfigCache configCache, // AtomicReference cryptoFileSystem, // - List mountProviders, // VaultState state, // @Named("lastKnownException") ObjectProperty lastKnownException, // VaultStats stats, // - Mounter mounter, // - @Named("vaultMountService") ObservableValue actualMountService, // - @Named("FUPFMS") AtomicReference firstUsedProblematicFuseMountService) { - this.settings = settings; + Mounter mounter) { this.vaultSettings = vaultSettings; this.configCache = configCache; this.cryptoFileSystem = cryptoFileSystem; - this.mountProviders = mountProviders; this.state = state; this.lastKnownException = lastKnownException; this.stats = stats; @@ -111,8 +95,6 @@ public class Vault { this.mountPoint = Bindings.createObjectBinding(this::getMountPoint, state); this.mounter = mounter; this.showingStats = new SimpleBooleanProperty(false); - this.actualMountService = actualMountService; - this.firstUsedProblematicFuseMountService = firstUsedProblematicFuseMountService; } // ****************************************************************************** @@ -165,22 +147,13 @@ public class Vault { if (cryptoFileSystem.get() != null) { throw new IllegalStateException("Already unlocked."); } - var fallbackProvider = mountProviders.stream().findFirst().orElse(null); - var defMntServ = mountProviders.stream().filter(s -> s.getClass().getName().equals(settings.mountService.getValue())).findFirst().orElse(fallbackProvider); - var selMntServ = mountProviders.stream().filter(s -> s.getClass().getName().equals(vaultSettings.mountService.getValue())).findFirst().orElse(defMntServ); - var fuseRestartRequired = firstUsedProblematicFuseMountService.get() != null // - && VaultModule.isProblematicFuseService(selMntServ) // - && !firstUsedProblematicFuseMountService.get().equals(selMntServ); - if (fuseRestartRequired) { - throw new FuseRestartRequiredException("fuseRestartRequired"); - } CryptoFileSystem fs = createCryptoFileSystem(keyLoader); boolean success = false; try { cryptoFileSystem.set(fs); var rootPath = fs.getRootDirectories().iterator().next(); - var mountHandle = mounter.mount(vaultSettings, rootPath, actualMountService); + var mountHandle = mounter.mount(vaultSettings, rootPath); success = this.mountHandle.compareAndSet(null, mountHandle); } finally { if (!success) { diff --git a/src/main/java/org/cryptomator/common/vaults/VaultListManager.java b/src/main/java/org/cryptomator/common/vaults/VaultListManager.java index cbcc281ac..6f0e30153 100644 --- a/src/main/java/org/cryptomator/common/vaults/VaultListManager.java +++ b/src/main/java/org/cryptomator/common/vaults/VaultListManager.java @@ -37,19 +37,19 @@ public class VaultListManager { private static final Logger LOG = LoggerFactory.getLogger(VaultListManager.class); - private final AutoLocker autoLocker; private final VaultComponent.Factory vaultComponentFactory; private final ObservableList vaultList; private final String defaultVaultName; - private final Settings settings; + @Inject - public VaultListManager(ObservableList vaultList, AutoLocker autoLocker, VaultComponent.Factory vaultComponentFactory, ResourceBundle resourceBundle, Settings settings) { + public VaultListManager(ObservableList vaultList, // + AutoLocker autoLocker, // + VaultComponent.Factory vaultComponentFactory, // + ResourceBundle resourceBundle, Settings settings) { this.vaultList = vaultList; - this.autoLocker = autoLocker; this.vaultComponentFactory = vaultComponentFactory; this.defaultVaultName = resourceBundle.getString("defaults.vault.vaultName"); - this.settings = settings; addAll(settings.directories); vaultList.addListener(new VaultListChangeListener(settings.directories)); diff --git a/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/src/main/java/org/cryptomator/common/vaults/VaultModule.java index b172a1e03..276af9f96 100644 --- a/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -8,72 +8,22 @@ package org.cryptomator.common.vaults; import dagger.Module; import dagger.Provides; import org.cryptomator.common.Nullable; -import org.cryptomator.common.ObservableUtil; -import org.cryptomator.common.mount.ActualMountService; -import org.cryptomator.common.settings.Settings; -import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.cryptofs.CryptoFileSystem; -import org.cryptomator.integrations.mount.MountService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import javax.inject.Named; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; -import javafx.beans.value.ObservableValue; -import java.util.List; import java.util.concurrent.atomic.AtomicReference; @Module public class VaultModule { - private static final AtomicReference formerSelectedMountService = new AtomicReference<>(null); - private static final List problematicFuseMountServices = List.of("org.cryptomator.frontend.fuse.mount.MacFuseMountProvider", "org.cryptomator.frontend.fuse.mount.FuseTMountProvider"); - private static final Logger LOG = LoggerFactory.getLogger(VaultModule.class); - @Provides @PerVault public AtomicReference provideCryptoFileSystemReference() { return new AtomicReference<>(); } - @Provides - @Named("vaultMountService") - @PerVault - static ObservableValue provideMountService(Settings settings, VaultSettings vaultSettings, List serviceImpls, @Named("FUPFMS") AtomicReference fupfms) { - var fallbackProvider = serviceImpls.stream().findFirst().orElse(null); - var defaultMountService = ObservableUtil.mapWithDefault(settings.mountService, serviceName -> serviceImpls.stream().filter(s -> s.getClass().getName().equals(serviceName)).findFirst().orElse(fallbackProvider), fallbackProvider); - return ObservableUtil.mapWithDefault(vaultSettings.mountService, // - desiredServiceImpl -> { // - var serviceFromSettings = serviceImpls.stream().filter(serviceImpl -> serviceImpl.getClass().getName().equals(desiredServiceImpl)).findAny(); // - var targetedService = serviceFromSettings.orElse(defaultMountService.getValue()); - return applyWorkaroundForProblematicFuse(targetedService, serviceFromSettings.isPresent(), fupfms); - }, // - () -> applyWorkaroundForProblematicFuse(defaultMountService.getValue(), true, fupfms) - ); - } - - //see https://github.com/cryptomator/cryptomator/issues/2786 - private synchronized static ActualMountService applyWorkaroundForProblematicFuse(MountService targetedService, boolean isDesired, AtomicReference firstUsedProblematicFuseMountService) { - //set the first used problematic fuse service if applicable - var targetIsProblematicFuse = isProblematicFuseService(targetedService); - if (targetIsProblematicFuse && firstUsedProblematicFuseMountService.get() == null) { - firstUsedProblematicFuseMountService.set(targetedService); - } - - //do not use the targeted mount service and fallback to former one, if the service is problematic _and_ not the first problematic one used. - if (targetIsProblematicFuse && !firstUsedProblematicFuseMountService.get().equals(targetedService)) { - return new ActualMountService(formerSelectedMountService.get(), false); - } else { - formerSelectedMountService.set(targetedService); - return new ActualMountService(targetedService, isDesired); - } - } - - public static boolean isProblematicFuseService(MountService service) { - return problematicFuseMountServices.contains(service.getClass().getName()); - } - @Provides @Named("lastKnownException") @PerVault diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java b/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java index 9d28d8d9d..0583e4fbb 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java @@ -3,11 +3,11 @@ package org.cryptomator.ui.vaultoptions; import com.google.common.base.Strings; import dagger.Lazy; import org.cryptomator.common.ObservableUtil; +import org.cryptomator.common.mount.Mounter; import org.cryptomator.common.mount.WindowsDriveLetters; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.common.vaults.VaultModule; import org.cryptomator.integrations.mount.MountCapability; import org.cryptomator.integrations.mount.MountService; import org.cryptomator.ui.common.FxController; @@ -102,11 +102,10 @@ public class MountOptionsController implements FxController { fallbackProvider); this.selectedMountService = Bindings.createObjectBinding(this::reselectMountService, defaultMountService, vaultSettings.mountService); this.fuseRestartRequired = selectedMountService.map(s -> { - return firstUsedProblematicFuseMountService.get() != null // - && VaultModule.isProblematicFuseService(s) // - && !firstUsedProblematicFuseMountService.get().equals(s); - } - ); + return firstUsedProblematicFuseMountService.get() != null // + && Mounter.isProblematicFuseService(s) // + && !firstUsedProblematicFuseMountService.get().equals(s); + }); this.loopbackPortSupported = BooleanExpression.booleanExpression(selectedMountService.map(s -> s.hasCapability(MountCapability.LOOPBACK_PORT))); this.defaultMountFlags = selectedMountService.map(s -> { @@ -130,7 +129,7 @@ public class MountOptionsController implements FxController { @FXML public void initialize() { - defaultMountService.addListener((_,_,_) -> vaultVolumeTypeChoiceBox.setConverter(new MountServiceConverter())); + defaultMountService.addListener((_, _, _) -> vaultVolumeTypeChoiceBox.setConverter(new MountServiceConverter())); // readonly: readOnlyCheckbox.selectedProperty().bindBidirectional(vaultSettings.usesReadOnlyMode);