Implemented MPC-System in FuseVolume and DokanyVolume, did refactoring

Implemented MPC-System in FuseVolume and DokanyVolume
Removed methods from FuseVol and DokanyVol
Renamed methods
Added chooser logic instead

Added imports for VolumeException to multiple classes
Added support for throwing and handling of InvalidMountPointException from Volume up to UnlockWorkflow (changed method signatures, added try-catchs, etc.)

main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java

@@ -1,7 +1,9 @@
 package org.cryptomator.common.vaults;
 
-import com.google.common.base.Strings;
-import org.cryptomator.common.Environment;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableSet;
+import org.cryptomator.common.mountpoint.InvalidMountPointException;
+import org.cryptomator.common.mountpoint.MountPointChooser;
 import org.cryptomator.common.settings.VaultSettings;
 import org.cryptomator.cryptofs.CryptoFileSystem;
 import org.cryptomator.frontend.dokany.Mount;
@@ -11,15 +13,10 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
-import java.io.File;
-import java.io.IOException;
-import java.nio.file.DirectoryNotEmptyException;
-import java.nio.file.DirectoryStream;
-import java.nio.file.Files;
-import java.nio.file.NotDirectoryException;
+import javax.inject.Named;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ExecutorService;
 
 public class DokanyVolume implements Volume {
@@ -31,18 +28,21 @@ public class DokanyVolume implements Volume {
 
 	private final VaultSettings vaultSettings;
 	private final MountFactory mountFactory;
-	private final Environment environment;
-	private final WindowsDriveLetters windowsDriveLetters;
+
+	private final Set<MountPointChooser> choosers;
+
 	private Mount mount;
 	private Path mountPoint;
-	private boolean createdTemporaryMountPoint;
+
+	//Cleanup
+	private boolean cleanupRequired;
+	private MountPointChooser usedChooser;
 
 	@Inject
-	public DokanyVolume(VaultSettings vaultSettings, Environment environment, ExecutorService executorService, WindowsDriveLetters windowsDriveLetters) {
+	public DokanyVolume(VaultSettings vaultSettings, ExecutorService executorService, @Named("orderedValidMountPointChoosers") Set<MountPointChooser> choosers) {
 		this.vaultSettings = vaultSettings;
-		this.environment = environment;
 		this.mountFactory = new MountFactory(executorService);
-		this.windowsDriveLetters = windowsDriveLetters;
+		this.choosers = choosers;
 	}
 
 	@Override
@@ -51,7 +51,7 @@ public class DokanyVolume implements Volume {
 	}
 
 	@Override
-	public void mount(CryptoFileSystem fs, String mountFlags) throws VolumeException, IOException {
+	public void mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException {
 		this.mountPoint = determineMountPoint();
 		String mountName = vaultSettings.mountName().get();
 		try {
@@ -64,60 +64,21 @@ public class DokanyVolume implements Volume {
 		}
 	}
 
-	private Path determineMountPoint() throws VolumeException, IOException {
-		Optional<String> optionalCustomMountPoint = vaultSettings.getCustomMountPath();
-		if (optionalCustomMountPoint.isPresent()) {
-			Path customMountPoint = Paths.get(optionalCustomMountPoint.get());
-			checkProvidedMountPoint(customMountPoint);
-			return customMountPoint;
-		}
-		if (!Strings.isNullOrEmpty(vaultSettings.winDriveLetter().get())) {
-			return Path.of(vaultSettings.winDriveLetter().get().charAt(0) + ":\\");
-		}
-
-		//auto assign drive letter
-		Optional<Path> optionalDriveLetter = windowsDriveLetters.getAvailableDriveLetterPath();
-		if (optionalDriveLetter.isPresent()) {
-			return optionalDriveLetter.get();
-		}
-
-		//Nothing has worked so far -> Choose and prepare a folder
-		mountPoint = prepareTemporaryMountPoint();
-		LOG.debug("Successfully created mount point: {}", mountPoint);
-		return mountPoint;
-	}
-
-	private void checkProvidedMountPoint(Path mountPoint) throws IOException {
-		if (!Files.isDirectory(mountPoint)) {
-			throw new NotDirectoryException(mountPoint.toString());
-		}
-		try (DirectoryStream<Path> ds = Files.newDirectoryStream(mountPoint)) {
-			if (ds.iterator().hasNext()) {
-				throw new DirectoryNotEmptyException(mountPoint.toString());
-			}
-		}
-	}
-
-	private Path chooseNonExistingTemporaryMountPoint() throws VolumeException {
-		Path parent = environment.getMountPointsDir().orElseThrow();
-		String basename = vaultSettings.getId(); //FIXME
-		for (int i = 0; i < MAX_TMPMOUNTPOINT_CREATION_RETRIES; i++) {
-			Path mountPoint = parent.resolve(basename + "_" + i);
-			if (Files.notExists(mountPoint)) {
-				return mountPoint;
+	private Path determineMountPoint() throws InvalidMountPointException {
+		for (MountPointChooser chooser : this.choosers) {
+			Optional<Path> chosenPath = chooser.chooseMountPoint();
+			if (chosenPath.isEmpty()) {
+				//Chooser was applicable, but couldn't find a feasible mountpoint
+				continue;
 			}
+			this.cleanupRequired = chooser.prepare(chosenPath.get()); //Fail entirely if an Exception occurs
+			this.usedChooser = chooser;
+			return chosenPath.get();
 		}
-		LOG.error("Failed to find feasible mountpoint at {}{}{}_x. Giving up after {} attempts.", parent, File.separator, basename, MAX_TMPMOUNTPOINT_CREATION_RETRIES);
-		throw new VolumeException("Did not find feasible mount point.");
-	}
-
-	private Path prepareTemporaryMountPoint() throws IOException, VolumeException {
-		Path mountPoint = chooseNonExistingTemporaryMountPoint();
-
-		Files.createDirectories(mountPoint);
-		this.createdTemporaryMountPoint = true;
-
-		return mountPoint;
+		String tried = Joiner.on(", ").join(this.choosers.stream()
+				.map((mpc) -> mpc.getClass().getTypeName())
+				.collect(ImmutableSet.toImmutableSet()));
+		throw new InvalidMountPointException(String.format("No feasible MountPoint found! Tried %s", tried));
 	}
 
 	@Override
@@ -131,17 +92,12 @@ public class DokanyVolume implements Volume {
 	@Override
 	public void unmount() {
 		mount.close();
-		cleanupTemporaryMountPoint();
+		cleanupMountPoint();
 	}
 
-	private void cleanupTemporaryMountPoint() {
-		if (createdTemporaryMountPoint) {
-			try {
-				Files.delete(mountPoint);
-				LOG.debug("Successfully deleted mount point: {}", mountPoint);
-			} catch (IOException e) {
-				LOG.warn("Could not delete mount point: {}", e.getMessage());
-			}
+	private void cleanupMountPoint() {
+		if (this.cleanupRequired) {
+			this.usedChooser.cleanup(this.mountPoint);
 		}
 	}
 

main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java

@@ -1,10 +1,11 @@
 package org.cryptomator.common.vaults;
 
+import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
-import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
 import org.apache.commons.lang3.SystemUtils;
-import org.cryptomator.common.Environment;
-import org.cryptomator.common.settings.VaultSettings;
+import org.cryptomator.common.mountpoint.InvalidMountPointException;
+import org.cryptomator.common.mountpoint.MountPointChooser;
 import org.cryptomator.cryptofs.CryptoFileSystem;
 import org.cryptomator.frontend.fuse.mount.CommandFailedException;
 import org.cryptomator.frontend.fuse.mount.EnvironmentVariables;
@@ -16,131 +17,51 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
-import java.io.File;
-import java.io.IOException;
-import java.nio.file.DirectoryNotEmptyException;
-import java.nio.file.DirectoryStream;
-import java.nio.file.FileAlreadyExistsException;
-import java.nio.file.Files;
-import java.nio.file.LinkOption;
-import java.nio.file.NotDirectoryException;
+import javax.inject.Named;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Optional;
+import java.util.Set;
 
 public class FuseVolume implements Volume {
 
 	private static final Logger LOG = LoggerFactory.getLogger(FuseVolume.class);
-	private static final int MAX_TMPMOUNTPOINT_CREATION_RETRIES = 10;
 
-	private final VaultSettings vaultSettings;
-	private final Environment environment;
-	private final WindowsDriveLetters windowsDriveLetters;
+	private final Set<MountPointChooser> choosers;
 
 	private Mount fuseMnt;
 	private Path mountPoint;
-	private boolean createdTemporaryMountPoint;
+
+	//Cleanup
+	private boolean cleanupRequired;
+	private MountPointChooser usedChooser;
 
 	@Inject
-	public FuseVolume(VaultSettings vaultSettings, Environment environment, WindowsDriveLetters windowsDriveLetters) {
-		this.vaultSettings = vaultSettings;
-		this.environment = environment;
-		this.windowsDriveLetters = windowsDriveLetters;
+	public FuseVolume(@Named("orderedValidMountPointChoosers") Set<MountPointChooser> choosers) {
+		this.choosers = choosers;
 	}
 
 	@Override
-	public void mount(CryptoFileSystem fs, String mountFlags) throws IOException, FuseNotSupportedException, VolumeException {
+	public void mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, FuseNotSupportedException, VolumeException {
 		this.mountPoint = determineMountPoint();
 
 		mount(fs.getPath("/"), mountFlags);
 	}
 
-	private Path determineMountPoint() throws IOException, VolumeException {
-		Path mountPoint;
-		Optional<String> optionalCustomMountPoint = vaultSettings.getCustomMountPath();
-		//Is there a custom mountpoint?
-		if (optionalCustomMountPoint.isPresent()) {
-			mountPoint = Paths.get(optionalCustomMountPoint.get());
-			checkProvidedMountPoint(mountPoint);
-			LOG.debug("Successfully checked custom mount point: {}", this.mountPoint);
-			return mountPoint;
-		}
-		//No custom mounpoint -> Are we on Windows?
-		if (SystemUtils.IS_OS_WINDOWS) {
-			//Is there a chosen Driveletter?
-			if (!Strings.isNullOrEmpty(vaultSettings.winDriveLetter().get())) {
-				mountPoint = Path.of(vaultSettings.winDriveLetter().get().charAt(0) + ":\\");
-				return mountPoint;
-			}
-
-			//No chosen Driveltter -> Is there a free Driveletter?
-			Optional<Path> optionalDriveLetter = windowsDriveLetters.getAvailableDriveLetterPath();
-			if(optionalDriveLetter.isPresent()) {
-				mountPoint = optionalDriveLetter.get();
-				return mountPoint;
-			}
-			//No free or chosen Driveletter -> Continue below
-		}
-		//Nothing has worked so far -> Choose and prepare a folder
-		mountPoint = prepareTemporaryMountPoint();
-		LOG.debug("Successfully created mount point: {}", mountPoint);
-		return mountPoint;
-	}
-
-	private void checkProvidedMountPoint(Path mountPoint) throws IOException {
-		//On Windows the target folder MUST NOT exist...
-		//https://github.com/billziss-gh/winfsp/issues/320
-		if (SystemUtils.IS_OS_WINDOWS) {
-			//We must use #notExists() here because notExists =/= !exists (see docs)
-			if (Files.notExists(mountPoint, LinkOption.NOFOLLOW_LINKS)) {
-				//File really doesn't exist
-				return;
-			}
-			//File exists OR can't be determined
-			throw new FileAlreadyExistsException(mountPoint.toString());
-		}
-
-		//... on Mac and Linux it's the opposite
-		if (!Files.isDirectory(mountPoint)) {
-			throw new NotDirectoryException(mountPoint.toString());
-		}
-		try (DirectoryStream<Path> ds = Files.newDirectoryStream(mountPoint)) {
-			if (ds.iterator().hasNext()) {
-				throw new DirectoryNotEmptyException(mountPoint.toString());
-			}
-		}
-	}
-
-	private Path prepareTemporaryMountPoint() throws IOException, VolumeException {
-		Path mountPoint = chooseNonExistingTemporaryMountPoint();
-		// 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 mountPoint;
-		}
-
-		//WinFSP needs the parent, but the acutal Mount Point must not exist...
-		if (SystemUtils.IS_OS_WINDOWS) {
-			Files.createDirectories(mountPoint.getParent());
-		} else {
-			Files.createDirectories(mountPoint);
-			this.createdTemporaryMountPoint = true;
-		}
-		return mountPoint;
-	}
-
-	private Path chooseNonExistingTemporaryMountPoint() throws VolumeException {
-		Path parent = environment.getMountPointsDir().orElseThrow();
-		String basename = vaultSettings.getId(); //FIXME
-		for (int i = 0; i < MAX_TMPMOUNTPOINT_CREATION_RETRIES; i++) {
-			Path mountPoint = parent.resolve(basename + "_" + i);
-			if (Files.notExists(mountPoint)) {
-				return mountPoint;
+	private Path determineMountPoint() throws InvalidMountPointException {
+		for (MountPointChooser chooser : this.choosers) {
+			Optional<Path> chosenPath = chooser.chooseMountPoint();
+			if (chosenPath.isEmpty()) {
+				//Chooser was applicable, but couldn't find a feasible mountpoint
+				continue;
 			}
+			this.cleanupRequired = chooser.prepare(chosenPath.get()); //Fail entirely if an Exception occurs
+			this.usedChooser = chooser;
+			return chosenPath.get();
 		}
-		LOG.error("Failed to find feasible mountpoint at {}{}{}_x. Giving up after {} attempts.", parent, File.separator, basename, MAX_TMPMOUNTPOINT_CREATION_RETRIES);
-		throw new VolumeException("Did not find feasible mount point.");
+		String tried = Joiner.on(", ").join(this.choosers.stream()
+				.map((mpc) -> mpc.getClass().getTypeName())
+				.collect(ImmutableSet.toImmutableSet()));
+		throw new InvalidMountPointException(String.format("No feasible MountPoint found! Tried %s", tried));
 	}
 
 	private void mount(Path root, String mountFlags) throws VolumeException {
@@ -182,7 +103,7 @@ public class FuseVolume implements Volume {
 		} catch (CommandFailedException e) {
 			throw new VolumeException(e);
 		}
-		cleanupTemporaryMountPoint();
+		cleanupMountPoint();
 	}
 
 	@Override
@@ -193,17 +114,12 @@ public class FuseVolume implements Volume {
 		} catch (CommandFailedException e) {
 			throw new VolumeException(e);
 		}
-		cleanupTemporaryMountPoint();
+		cleanupMountPoint();
 	}
 
-	private void cleanupTemporaryMountPoint() {
-		if (createdTemporaryMountPoint) {
-			try {
-				Files.delete(mountPoint);
-				LOG.debug("Successfully deleted mount point: {}", mountPoint);
-			} catch (IOException e) {
-				LOG.warn("Could not delete mount point: {}", e.getMessage());
-			}
+	private void cleanupMountPoint() {
+		if (this.cleanupRequired) {
+			this.usedChooser.cleanup(this.mountPoint);
 		}
 	}
 

main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java

@@ -16,6 +16,7 @@ import javafx.beans.binding.StringBinding;
 import javafx.beans.property.ObjectProperty;
 import org.apache.commons.lang3.SystemUtils;
 import org.cryptomator.common.LazyInitializer;
+import org.cryptomator.common.mountpoint.InvalidMountPointException;
 import org.cryptomator.common.settings.VaultSettings;
 import org.cryptomator.common.vaults.Volume.VolumeException;
 import org.cryptomator.cryptofs.CryptoFileSystem;
@@ -121,7 +122,7 @@ public class Vault {
 		return CryptoFileSystemProvider.newFileSystem(getPath(), fsProps);
 	}
 
-	public synchronized void unlock(CharSequence passphrase) throws CryptoException, IOException, VolumeException {
+	public synchronized void unlock(CharSequence passphrase) throws CryptoException, IOException, VolumeException, InvalidMountPointException {
 		if (vaultSettings.useCustomMountPath().get() && Strings.isNullOrEmpty(vaultSettings.customMountPath().get())) {
 			throw new NotDirectoryException("");
 		}

main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java

@@ -1,5 +1,6 @@
 package org.cryptomator.common.vaults;
 
+import org.cryptomator.common.mountpoint.InvalidMountPointException;
 import org.cryptomator.common.settings.VolumeImpl;
 import org.cryptomator.cryptofs.CryptoFileSystem;
 
@@ -24,7 +25,7 @@ public interface Volume {
 	 * @param fs
 	 * @throws IOException
 	 */
-	void mount(CryptoFileSystem fs, String mountFlags) throws IOException, VolumeException;
+	void mount(CryptoFileSystem fs, String mountFlags) throws IOException, VolumeException, InvalidMountPointException;
 
 	void reveal() throws VolumeException;
 

main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java

@@ -6,10 +6,11 @@ import javafx.concurrent.Task;
 import javafx.scene.Scene;
 import javafx.stage.Stage;
 import javafx.stage.Window;
+import org.cryptomator.common.mountpoint.InvalidMountPointException;
 import org.cryptomator.common.vaults.MountPointRequirement;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.VaultState;
-import org.cryptomator.common.vaults.Volume;
+import org.cryptomator.common.vaults.Volume.VolumeException;
 import org.cryptomator.cryptolib.api.InvalidPassphraseException;
 import org.cryptomator.keychain.KeychainAccessException;
 import org.cryptomator.keychain.KeychainManager;
@@ -28,8 +29,6 @@ import javax.inject.Named;
 import java.io.IOException;
 import java.nio.CharBuffer;
 import java.nio.file.DirectoryNotEmptyException;
-import java.nio.file.FileAlreadyExistsException;
-import java.nio.file.FileSystemException;
 import java.nio.file.NotDirectoryException;
 import java.util.Arrays;
 import java.util.Optional;
@@ -76,7 +75,7 @@ public class UnlockWorkflow extends Task<Boolean> {
 	}
 
 	@Override
-	protected Boolean call() throws InterruptedException, IOException, Volume.VolumeException {
+	protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException {
 		try {
 			if (attemptUnlock()) {
 				handleSuccess();
@@ -85,7 +84,11 @@ public class UnlockWorkflow extends Task<Boolean> {
 				cancel(false); // set Tasks state to cancelled
 				return false;
 			}
-		} catch (FileAlreadyExistsException | NotDirectoryException | DirectoryNotEmptyException e) {
+		} catch (NotDirectoryException | DirectoryNotEmptyException thrown) {
+			InvalidMountPointException e = new InvalidMountPointException(thrown);
+			handleInvalidMountPoint(e);
+			throw e; // rethrow to trigger correct exception handling in Task
+		} catch (InvalidMountPointException e) {
 			handleInvalidMountPoint(e);
 			throw e; // rethrow to trigger correct exception handling in Task
 		} catch (Exception e) {
@@ -97,7 +100,7 @@ public class UnlockWorkflow extends Task<Boolean> {
 		}
 	}
 
-	private boolean attemptUnlock() throws InterruptedException, IOException, Volume.VolumeException {
+	private boolean attemptUnlock() throws InterruptedException, IOException, VolumeException, InvalidMountPointException {
 		boolean proceed = password.get() != null || askForPassword(false) == PasswordEntry.PASSWORD_ENTERED;
 		while (proceed) {
 			try {
@@ -156,7 +159,7 @@ public class UnlockWorkflow extends Task<Boolean> {
 		}
 	}
 
-	private void handleInvalidMountPoint(FileSystemException e) {
+	private void handleInvalidMountPoint(InvalidMountPointException e) {
 		MountPointRequirement requirement = vault.getMountPointRequirement();
 		assert requirement != MountPointRequirement.NONE; //An invalid MountPoint with no required MountPoint doesn't seem sensible