From b5efe39eb88a99c8d3266d36dc63f8fd5036d265 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Wed, 7 Oct 2020 18:06:05 +0200 Subject: [PATCH] Refactored Vault, MPCs, Unlock to integrate better with latest changes Removed delegate to Volume#getMountPointRequirement() from Vault Added getter for the Vault's Volume (#getVolume()) Changed CustomMountPointChooser to use VaultSettings instead of Vault for the constructor/field declaration Updated CustomMountPointChooser#isApplicable() to be disabled when using FUSE on Windows (without useExperimentalFuse) Updated CustomMountPointChooser to call Volume#getMountPointRequirement() directly Replaced OS-Check in TemporaryMountPointChooser with MPR-Check Replaced call to Vault#getMountPointRequirement() with call to Vault#getVolume() (and Volume#getMountPointRequirement()) in UnlockInvalidMountPointController and UnlockWorkflow Cleaned up UnlockWorkflow --- .../mountpoint/CustomMountPointChooser.java | 20 ++++++++----- .../TemporaryMountPointChooser.java | 30 ++++++++++++------- .../org/cryptomator/common/vaults/Vault.java | 5 ++-- .../UnlockInvalidMountPointController.java | 2 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 12 ++++---- 5 files changed, 43 insertions(+), 26 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java index b39baba23..6a86c654a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java @@ -1,6 +1,9 @@ package org.cryptomator.common.mountpoint; -import org.cryptomator.common.vaults.Vault; +import org.apache.commons.lang3.SystemUtils; +import org.cryptomator.common.Environment; +import org.cryptomator.common.settings.VaultSettings; +import org.cryptomator.common.settings.VolumeImpl; import org.cryptomator.common.vaults.Volume; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -23,27 +26,30 @@ public class CustomMountPointChooser implements MountPointChooser { private static final Logger LOG = LoggerFactory.getLogger(CustomMountPointChooser.class); - private final Vault vault; + private final VaultSettings vaultSettings; + private final Environment environment; @Inject - public CustomMountPointChooser(Vault vault) { - this.vault = vault; + public CustomMountPointChooser(VaultSettings vaultSettings, Environment environment) { + this.vaultSettings = vaultSettings; + this.environment = environment; } @Override public boolean isApplicable(Volume caller) { - return true; + //Disable if useExperimentalFuse is required (Win + Fuse), but set to false + return caller.getImplementationType() != VolumeImpl.FUSE || !SystemUtils.IS_OS_WINDOWS || environment.useExperimentalFuse(); } @Override public Optional chooseMountPoint(Volume caller) { //VaultSettings#getCustomMountPath already checks whether the saved custom mountpoint should be used - return this.vault.getVaultSettings().getCustomMountPath().map(Paths::get); + return this.vaultSettings.getCustomMountPath().map(Paths::get); } @Override public boolean prepare(Volume caller, Path mountPoint) throws InvalidMountPointException { - switch (this.vault.getMountPointRequirement()) { + switch (caller.getMountPointRequirement()) { case PARENT_NO_MOUNT_POINT -> prepareParentNoMountPoint(mountPoint); case EMPTY_MOUNT_POINT -> prepareEmptyMountPoint(mountPoint); case NONE -> { diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java index fa3ab5ab2..60991b97e 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java @@ -67,16 +67,26 @@ public class TemporaryMountPointChooser implements MountPointChooser { } try { - //WinFSP needs the parent, but the actual Mountpoint must not exist... - if (SystemUtils.IS_OS_WINDOWS) { - Files.createDirectories(mountPoint.getParent()); - LOG.debug("Successfully created folder for mount point: {}", mountPoint); - return false; - } else { - //For Linux and Mac the actual Mountpoint must exist - Files.createDirectories(mountPoint); - LOG.debug("Successfully created mount point: {}", mountPoint); - return true; + switch (caller.getMountPointRequirement()) { + case PARENT_NO_MOUNT_POINT -> { + Files.createDirectories(mountPoint.getParent()); + LOG.debug("Successfully created folder for mount point: {}", mountPoint); + return false; + } + case EMPTY_MOUNT_POINT -> { + Files.createDirectories(mountPoint); + LOG.debug("Successfully created mount point: {}", mountPoint); + return true; + } + case NONE -> { + //Requirement "NONE" doesn't make any sense here. + //No need to prepare/verify a Mountpoint without requiring one... + throw new InvalidMountPointException(new IllegalStateException("Illegal MountPointRequirement")); + } + default -> { + //Currently the case for "PARENT_OPT_MOUNT_POINT" + throw new InvalidMountPointException(new IllegalStateException("Not implemented")); + } } } catch (IOException exception) { throw new InvalidMountPointException("IOException while preparing mountpoint", exception); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index d63ab0330..90e099ae0 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -39,6 +39,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.EnumSet; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; @@ -316,8 +317,8 @@ public class Vault { return vaultSettings.getId(); } - public MountPointRequirement getMountPointRequirement() { - return volume.getMountPointRequirement(); + public Optional getVolume() { + return Optional.ofNullable(this.volume); } // ****************************************************************************** diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index a70f58311..7b0af6042 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -38,7 +38,7 @@ public class UnlockInvalidMountPointController implements FxController { } public boolean getMustExist() { - MountPointRequirement requirement = vault.getMountPointRequirement(); + MountPointRequirement requirement = vault.getVolume().orElseThrow(() -> new IllegalStateException("Invalid Mountpoint without a Volume?!")).getMountPointRequirement(); assert requirement != MountPointRequirement.NONE; //An invalid MountPoint with no required MountPoint doesn't seem sensible assert requirement != MountPointRequirement.PARENT_OPT_MOUNT_POINT; //Not implemented anywhere (yet) diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index ea692f783..c339cb344 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -157,7 +157,7 @@ public class UnlockWorkflow extends Task { } private void handleInvalidMountPoint(InvalidMountPointException impExc) { - MountPointRequirement requirement = vault.getMountPointRequirement(); + MountPointRequirement requirement = vault.getVolume().orElseThrow(() -> new IllegalStateException("Invalid Mountpoint without a Volume?!", impExc)).getMountPointRequirement(); assert requirement != MountPointRequirement.NONE; //An invalid MountPoint with no required MountPoint doesn't seem sensible assert requirement != MountPointRequirement.PARENT_OPT_MOUNT_POINT; //Not implemented anywhere (yet) @@ -165,14 +165,14 @@ public class UnlockWorkflow extends Task { //Cause is either null (cause the IMPE was thrown directly, e.g. because no MPC succeeded) //or the cause was not an Exception (but some other kind of Throwable) //Either way: Handle as generic error - if(!(cause instanceof Exception)) { + if (!(cause instanceof Exception)) { handleGenericError(impExc); return; } //From here on handle the cause, not the caught exception - if(cause instanceof NotDirectoryException) { - if(requirement == MountPointRequirement.PARENT_NO_MOUNT_POINT) { + if (cause instanceof NotDirectoryException) { + if (requirement == MountPointRequirement.PARENT_NO_MOUNT_POINT) { LOG.error("Unlock failed. Parent folder is missing: {}", cause.getMessage()); } else { LOG.error("Unlock failed. Mountpoint doesn't exist (needs to be a folder): {}", cause.getMessage()); @@ -181,13 +181,13 @@ public class UnlockWorkflow extends Task { return; } - if(cause instanceof FileAlreadyExistsException) { + if (cause instanceof FileAlreadyExistsException) { LOG.error("Unlock failed. Mountpoint already exists: {}", cause.getMessage()); showInvalidMountPointScene(); return; } - if(cause instanceof DirectoryNotEmptyException) { + if (cause instanceof DirectoryNotEmptyException) { LOG.error("Unlock failed. Mountpoint not an empty directory: {}", cause.getMessage()); showInvalidMountPointScene(); return;