From c6d1c2ca6b1ef9c3f5db02cfd72f98602de3a5bc Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 18 Nov 2020 13:05:56 +0100 Subject: [PATCH] added MacVolumeMountChooser and refactored "priority" of mount point choosers: now all priorities are set in MountPointChooserModule (as a map key) --- .../AvailableDriveLetterChooser.java | 9 +-- .../mountpoint/CustomDriveLetterChooser.java | 9 +-- .../mountpoint/CustomMountPointChooser.java | 8 +-- .../mountpoint/MacVolumeMountChooser.java | 64 +++++++++++++++++++ .../common/mountpoint/MountPointChooser.java | 33 +--------- .../mountpoint/MountPointChooserModule.java | 34 ++++++---- .../TemporaryMountPointChooser.java | 15 +---- .../common/vaults/AbstractVolume.java | 29 ++------- .../common/vaults/DokanyVolume.java | 2 +- .../cryptomator/common/vaults/FuseVolume.java | 2 +- 10 files changed, 101 insertions(+), 104 deletions(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java index 7634601fa..d0fc04f3c 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java @@ -8,9 +8,7 @@ import javax.inject.Inject; import java.nio.file.Path; import java.util.Optional; -public class AvailableDriveLetterChooser implements MountPointChooser { - - public static final int PRIORITY = 200; +class AvailableDriveLetterChooser implements MountPointChooser { private final WindowsDriveLetters windowsDriveLetters; @@ -28,9 +26,4 @@ public class AvailableDriveLetterChooser implements MountPointChooser { public Optional chooseMountPoint(Volume caller) { return this.windowsDriveLetters.getAvailableDriveLetterPath(); } - - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java index ac9e5b716..1a42aa5ad 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java @@ -9,9 +9,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -public class CustomDriveLetterChooser implements MountPointChooser { - - public static final int PRIORITY = 100; +class CustomDriveLetterChooser implements MountPointChooser { private final VaultSettings vaultSettings; @@ -29,9 +27,4 @@ public class CustomDriveLetterChooser implements MountPointChooser { public Optional chooseMountPoint(Volume caller) { return this.vaultSettings.getWinDriveLetter().map(letter -> letter.charAt(0) + ":\\").map(Paths::get); } - - @Override - public int getPriority() { - return PRIORITY; - } } 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 6a86c654a..5f1a7fedd 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 @@ -20,9 +20,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -public class CustomMountPointChooser implements MountPointChooser { - - public static final int PRIORITY = 0; +class CustomMountPointChooser implements MountPointChooser { private static final Logger LOG = LoggerFactory.getLogger(CustomMountPointChooser.class); @@ -94,8 +92,4 @@ public class CustomMountPointChooser implements MountPointChooser { } } - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java new file mode 100644 index 000000000..d6ebb4da5 --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java @@ -0,0 +1,64 @@ +package org.cryptomator.common.mountpoint; + +import org.apache.commons.lang3.SystemUtils; +import org.cryptomator.common.settings.VaultSettings; +import org.cryptomator.common.vaults.Volume; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Inject; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; + +class MacVolumeMountChooser implements MountPointChooser { + + private static final Logger LOG = LoggerFactory.getLogger(MacVolumeMountChooser.class); + private static final int MAX_MOUNTPOINT_CREATION_RETRIES = 10; + private static final Path VOLUME_PATH = Path.of("/Volumes"); + + private final VaultSettings vaultSettings; + + @Inject + public MacVolumeMountChooser(VaultSettings vaultSettings) { + this.vaultSettings = vaultSettings; + } + + @Override + public boolean isApplicable(Volume caller) { + return SystemUtils.IS_OS_MAC; + } + + @Override + public Optional chooseMountPoint(Volume caller) { + String basename = this.vaultSettings.mountName().get(); + // regular + Path mountPoint = VOLUME_PATH.resolve(basename); + if (Files.notExists(mountPoint)) { + return Optional.of(mountPoint); + } + // with id + mountPoint = VOLUME_PATH.resolve(basename + " (" + vaultSettings.getId() + ")"); + if (Files.notExists(mountPoint)) { + return Optional.of(mountPoint); + } + // with id and count + for (int i = 1; i < MAX_MOUNTPOINT_CREATION_RETRIES; i++) { + mountPoint = VOLUME_PATH.resolve(basename + "_(" + vaultSettings.getId() + ")_" + i); + if (Files.notExists(mountPoint)) { + return Optional.of(mountPoint); + } + } + LOG.error("Failed to find feasible mountpoint at /Volumes/{}_x. Giving up after {} attempts.", basename, MAX_MOUNTPOINT_CREATION_RETRIES); + return Optional.empty(); + } + + @Override + public boolean prepare(Volume caller, Path mountPoint) { + // https://github.com/osxfuse/osxfuse/issues/306#issuecomment-245114592: + // In order to allow non-admin users to mount FUSE volumes in `/Volumes`, + // starting with version 3.5.0, FUSE will create non-existent mount points automatically. + // Therefore we don't need to prepare anything. + return false; + } +} diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java index 2c40bb74a..3a4d82cf3 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java @@ -47,7 +47,7 @@ import java.util.SortedSet; * If the preparation succeeds {@link #cleanup(Volume, Path)} can be used after unmount to do any * remaining cleanup. */ -public interface MountPointChooser extends Comparable { +public interface MountPointChooser { /** * Called by the {@link Volume} to determine whether this MountPointChooser is @@ -135,35 +135,4 @@ public interface MountPointChooser extends Comparable { //NO-OP } - /** - * Called by the {@link MountPointChooserModule} to sort the available MPCs - * and determine their execution order. - * The priority must be defined by the developer to reflect a useful execution order. - * MPCs with lower priorities will be placed at lower indices in the resulting - * {@link SortedSet} and will be executed with higher probability.
- * A specific priority must not be assigned to more than one MPC at a time; - * the result of having two MPCs with equal priority is undefined. - * - * @return the priority of this MPC. - */ - int getPriority(); - - /** - * Called by the {@link Volume} to determine the execution order of the registered MPCs. - * Implementations usually may not override this method. This default implementation - * sorts the MPCs in ascending order of their {@link #getPriority() priority.}
- *
- * Original description: - *

{@inheritDoc} - * - * @implNote This default implementation sorts the MPCs in ascending order - * of their {@link #getPriority() priority.} - */ - @Override - default int compareTo(MountPointChooser other) { - Preconditions.checkNotNull(other, "Other must not be null!"); - - //Sort by priority (ascending order) - return Integer.compare(this.getPriority(), other.getPriority()); - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java index 09789dacb..cfb7b68ec 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java @@ -1,15 +1,17 @@ package org.cryptomator.common.mountpoint; -import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Iterables; import dagger.Binds; import dagger.Module; import dagger.Provides; -import dagger.multibindings.IntoSet; +import dagger.multibindings.IntKey; +import dagger.multibindings.IntoMap; import org.cryptomator.common.vaults.PerVault; import javax.inject.Named; -import java.util.Set; -import java.util.SortedSet; +import java.util.Map; +import java.util.SortedMap; +import java.util.TreeMap; /** * Dagger-Module for {@link MountPointChooser MountPointChoosers.}
@@ -21,30 +23,40 @@ import java.util.SortedSet; public abstract class MountPointChooserModule { @Binds - @IntoSet + @IntoMap + @IntKey(0) @PerVault public abstract MountPointChooser bindCustomMountPointChooser(CustomMountPointChooser chooser); @Binds - @IntoSet + @IntoMap + @IntKey(100) @PerVault public abstract MountPointChooser bindCustomDriveLetterChooser(CustomDriveLetterChooser chooser); @Binds - @IntoSet + @IntoMap + @IntKey(101) + @PerVault + public abstract MountPointChooser bindMacVolumeMountChooser(MacVolumeMountChooser chooser); + + @Binds + @IntoMap + @IntKey(200) @PerVault public abstract MountPointChooser bindAvailableDriveLetterChooser(AvailableDriveLetterChooser chooser); @Binds - @IntoSet + @IntoMap + @IntKey(999) @PerVault public abstract MountPointChooser bindTemporaryMountPointChooser(TemporaryMountPointChooser chooser); @Provides @PerVault @Named("orderedMountPointChoosers") - public static SortedSet provideOrderedMountPointChoosers(Set choosers) { - //Sort by natural order. The natural order is defined by MountPointChooser#compareTo - return ImmutableSortedSet.copyOf(choosers); + public static Iterable provideOrderedMountPointChoosers(Map choosers) { + SortedMap sortedChoosers = new TreeMap<>(choosers); + return Iterables.unmodifiableIterable(sortedChoosers.values()); } } 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 17c960b1e..9b2d502d9 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 @@ -15,9 +15,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -public class TemporaryMountPointChooser implements MountPointChooser { - - public static final int PRIORITY = 300; +class TemporaryMountPointChooser implements MountPointChooser { private static final Logger LOG = LoggerFactory.getLogger(TemporaryMountPointChooser.class); private static final int MAX_TMPMOUNTPOINT_CREATION_RETRIES = 10; @@ -70,13 +68,6 @@ public class TemporaryMountPointChooser implements MountPointChooser { @Override public boolean prepare(Volume caller, Path mountPoint) throws InvalidMountPointException { - // https://github.com/osxfuse/osxfuse/issues/306#issuecomment-245114592: - // In order to allow non-admin users to mount FUSE volumes in `/Volumes`, - // starting with version 3.5.0, FUSE will create non-existent mount points automatically. - if (SystemUtils.IS_OS_MAC && mountPoint.getParent().equals(Paths.get("/Volumes"))) { - return false; - } - try { switch (caller.getMountPointRequirement()) { case PARENT_NO_MOUNT_POINT -> { @@ -114,8 +105,4 @@ public class TemporaryMountPointChooser implements MountPointChooser { } } - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java index 91c6e22e4..6a09683ca 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java @@ -1,50 +1,35 @@ package org.cryptomator.common.vaults; -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import org.cryptomator.common.mountpoint.InvalidMountPointException; import org.cryptomator.common.mountpoint.MountPointChooser; import java.nio.file.Path; import java.util.Optional; -import java.util.SortedSet; -import java.util.TreeSet; public abstract class AbstractVolume implements Volume { - private final SortedSet choosers; + private final Iterable choosers; protected Path mountPoint; - - //Cleanup private boolean cleanupRequired; private MountPointChooser usedChooser; - public AbstractVolume(SortedSet choosers) { + public AbstractVolume(Iterable choosers) { this.choosers = choosers; } protected Path determineMountPoint() throws InvalidMountPointException { - SortedSet checkedChoosers = new TreeSet<>(); //Natural order - for (MountPointChooser chooser : this.choosers) { - if (!chooser.isApplicable(this)) { - continue; - } - + for (var chooser : Iterables.filter(choosers, c -> c.isApplicable(this))) { Optional chosenPath = chooser.chooseMountPoint(this); - checkedChoosers.add(chooser); //Consider a chooser checked if it's #chooseMountPoint() method was called - if (chosenPath.isEmpty()) { - //Chooser was applicable, but couldn't find a feasible mountpoint + if (chosenPath.isEmpty()) { // chooser couldn't find a feasible mountpoint continue; } - this.cleanupRequired = chooser.prepare(this, chosenPath.get()); //Fail entirely if an Exception occurs + this.cleanupRequired = chooser.prepare(this, chosenPath.get()); this.usedChooser = chooser; return chosenPath.get(); } - //SortedSet#stream() should return a sorted stream (that's what it's docs and the docs of #spliterator() say, even if they are not 100% clear for me.) - //We want to keep that order, that's why we use ImmutableSet#toImmutableSet() to collect (even if it doesn't implement SortedSet, it's docs promise use encounter ordering.) - String checked = Joiner.on(", ").join(checkedChoosers.stream().map((mpc) -> mpc.getClass().getTypeName()).collect(ImmutableSet.toImmutableSet())); - throw new InvalidMountPointException(String.format("No feasible MountPoint found! Checked %s", checked.isBlank() ? "" : checked)); + throw new InvalidMountPointException("No feasible MountPoint found!"); } protected void cleanupMountPoint() { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index e91c9f86d..89fd73203 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -28,7 +28,7 @@ public class DokanyVolume extends AbstractVolume { private Mount mount; @Inject - public DokanyVolume(VaultSettings vaultSettings, ExecutorService executorService, @Named("orderedMountPointChoosers") SortedSet choosers) { + public DokanyVolume(VaultSettings vaultSettings, ExecutorService executorService, @Named("orderedMountPointChoosers") Iterable choosers) { super(choosers); this.vaultSettings = vaultSettings; this.mountFactory = new MountFactory(executorService); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 76a83983b..74db3bd54 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -27,7 +27,7 @@ public class FuseVolume extends AbstractVolume { private Mount mount; @Inject - public FuseVolume(@Named("orderedMountPointChoosers") SortedSet choosers) { + public FuseVolume(@Named("orderedMountPointChoosers") Iterable choosers) { super(choosers); }