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
This commit is contained in:
JaniruTEC
2020-10-07 18:06:05 +02:00
parent 9329311491
commit b5efe39eb8
5 changed files with 43 additions and 26 deletions

View File

@@ -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<Path> 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 -> {

View File

@@ -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);

View File

@@ -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<Volume> getVolume() {
return Optional.ofNullable(this.volume);
}
// ******************************************************************************

View File

@@ -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)

View File

@@ -157,7 +157,7 @@ public class UnlockWorkflow extends Task<Boolean> {
}
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<Boolean> {
//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<Boolean> {
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;