Refactor winfsp mount preps and add unit tests

src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java

@@ -13,7 +13,9 @@ import javax.inject.Inject;
 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.NoSuchFileException;
 import java.nio.file.NotDirectoryException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -21,6 +23,8 @@ import java.util.Optional;
 
 class CustomMountPointChooser implements MountPointChooser {
 
+	private static final String HIDEAWAY_PREFIX = ".~$";
+	private static final String HIDEAWAY_SUFFIX = ".tmp";
 	private static final Logger LOG = LoggerFactory.getLogger(CustomMountPointChooser.class);
 
 	private final VaultSettings vaultSettings;
@@ -68,19 +72,41 @@ class CustomMountPointChooser implements MountPointChooser {
 		}
 	}
 
-	private void prepareParentNoMountPoint(Path mountPoint) throws InvalidMountPointException {
+	void prepareParentNoMountPoint(Path mountPoint) throws InvalidMountPointException {
 		//This the case on Windows when using FUSE
 		//See https://github.com/billziss-gh/winfsp/issues/320
 		assert SystemUtils.IS_OS_WINDOWS;
 
 		Path hideaway = getHideaway(mountPoint);
-		if (Files.exists(hideaway)) {
-			LOG.info("Mountpoint {} for winfsp mount seems to be not properly cleaned up. Will be fixed on unmount.");
-		} else if (!Files.isDirectory(mountPoint)) {
-			throw new InvalidMountPointException(new NotDirectoryException(mountPoint.toString())); //simulate we need a directory
+
+		var mpExists = Files.exists(mountPoint);
+		var hideExists = Files.exists(hideaway);
+
+		//both resources exist (whatever type)
+		//TODO: possible improvement by just deleting an _empty_ hideaway
+		if (mpExists && hideExists) {
+			throw new InvalidMountPointException(new FileAlreadyExistsException(hideaway.toString()));
+		} else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist
+			throw new InvalidMountPointException(new NoSuchFileException(mountPoint.toString()));
+		} else if (!mpExists) { //only hideaway exists
+
+			if (!Files.isDirectory(hideaway)) {
+				throw new InvalidMountPointException(new NotDirectoryException(hideaway.toString()));
+			}
+			LOG.info("Mountpoint {} for winfsp mount seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint);
+			try {
+				Files.setAttribute(hideaway, "dos:hidden", true);
+			} catch (IOException e) {
+				throw new InvalidMountPointException(e);
+			}
 		} else {
-			//TODO: should we require it to be empty?
+			if (!Files.isDirectory(mountPoint)) {
+				throw new InvalidMountPointException(new NotDirectoryException(mountPoint.toString()));
+			}
 			try {
+				if(Files.list(mountPoint).findFirst().isPresent()) {
+					throw new InvalidMountPointException(new DirectoryNotEmptyException(mountPoint.toString()));
+				}
 				Files.move(mountPoint, hideaway);
 				Files.setAttribute(hideaway, "dos:hidden", true);
 			} catch (IOException e) {
@@ -116,8 +142,9 @@ class CustomMountPointChooser implements MountPointChooser {
 		}
 	}
 
-	private Path getHideaway(Path mountPoint) {
-		return mountPoint.resolveSibling(mountPoint.getFileName().toString() + "_tmp");
+	//visible for testing
+	Path getHideaway(Path mountPoint) {
+		return mountPoint.resolveSibling(HIDEAWAY_PREFIX + mountPoint.getFileName().toString() + HIDEAWAY_SUFFIX);
 	}
 
 }

src/test/java/org/cryptomator/common/mountpoint/CustomMountPointChooserTest.java

@@ -0,0 +1,133 @@
+package org.cryptomator.common.mountpoint;
+
+import org.cryptomator.common.Environment;
+import org.cryptomator.common.settings.VaultSettings;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+public class CustomMountPointChooserTest {
+
+	//--- Mocks ---
+	VaultSettings vaultSettings;
+	Environment environment;
+
+	CustomMountPointChooser customMpc;
+
+
+	@BeforeEach
+	public void init() {
+		this.vaultSettings = Mockito.mock(VaultSettings.class);
+		this.environment = Mockito.mock(Environment.class);
+		this.customMpc = new CustomMountPointChooser(vaultSettings, environment);
+	}
+
+	@Nested
+	class WinfspPreperations {
+
+		@Test
+		@DisplayName("Test MP preparation for winfsp, if only mountpoint is present")
+		public void testPrepareParentNoMountpointOnlyMountpoint(@TempDir Path tmpDir) throws IOException {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			Files.createDirectory(mntPoint);
+
+			//execute
+			Assertions.assertDoesNotThrow(() -> customMpc.prepareParentNoMountPoint(mntPoint));
+
+			//evaluate
+			Assertions.assertTrue(Files.notExists(mntPoint));
+
+			Path hideaway = customMpc.getHideaway(mntPoint);
+			Assertions.assertTrue(Files.exists(hideaway));
+			Assertions.assertNotEquals(hideaway.getFileName().toString(), "mntPoint");
+			Assertions.assertTrue(hideaway.getFileName().toString().contains("mntPoint"));
+
+			Assumptions.assumeTrue(OS.WINDOWS.isCurrentOs());
+			Assertions.assertTrue((Boolean) Files.getAttribute(hideaway, "dos:hidden"));
+		}
+
+		@Test
+		@DisplayName("Test MP preparation for winfsp, if only non-empty mountpoint is present")
+		public void testPrepareParentNoMountpointOnlyNonEmptyMountpoint(@TempDir Path tmpDir) throws IOException {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			Files.createDirectory(mntPoint);
+			Files.createFile(mntPoint.resolve("foo"));
+
+			//execute
+			Assertions.assertThrows(InvalidMountPointException.class, () -> customMpc.prepareParentNoMountPoint(mntPoint));
+
+			//evaluate
+			Assertions.assertTrue(Files.exists(mntPoint.resolve("foo")));
+		}
+
+		@Test
+		@DisplayName("Test MP preparation for Winfsp, if for any reason only hideaway dir is present")
+		public void testPrepareParentNoMountpointOnlyHideaway(@TempDir Path tmpDir) throws IOException {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			var hideaway = customMpc.getHideaway(mntPoint);
+			Files.createDirectory(hideaway); //we explicitly do not set the file attributes here
+
+			//execute
+			Assertions.assertDoesNotThrow(() -> customMpc.prepareParentNoMountPoint(mntPoint));
+
+			//evaluate
+			Assertions.assertTrue(Files.exists(hideaway));
+			Assertions.assertNotEquals(hideaway.getFileName().toString(), "mntPoint");
+			Assertions.assertTrue(hideaway.getFileName().toString().contains("mntPoint"));
+
+			Assumptions.assumeTrue(OS.WINDOWS.isCurrentOs());
+			Assertions.assertTrue((Boolean) Files.getAttribute(hideaway, "dos:hidden"));
+		}
+
+		@Test
+		@DisplayName("Test Winfsp MP preparation, if mountpoint and hideaway dirs are present")
+		public void testPrepareParentNoMountpointMountPointAndHideaway(@TempDir Path tmpDir) throws IOException {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			var hideaway = customMpc.getHideaway(mntPoint);
+			Files.createDirectory(hideaway); //we explicitly do not set the file attributes here
+			Files.createDirectory(mntPoint);
+
+			//execute
+			Assertions.assertThrows(InvalidMountPointException.class, () -> customMpc.prepareParentNoMountPoint(mntPoint));
+
+			//evaluate
+			Assertions.assertTrue(Files.exists(hideaway));
+			Assertions.assertTrue(Files.exists(mntPoint));
+
+			Assumptions.assumeTrue(OS.WINDOWS.isCurrentOs());
+			Assertions.assertFalse((Boolean) Files.getAttribute(hideaway, "dos:hidden"));
+		}
+
+		@Test
+		@DisplayName("Test Winfsp MP preparation, if neither mountpoint nor hideaway dir is present")
+		public void testPrepareParentNoMountpointNothing(@TempDir Path tmpDir) {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			var hideaway = customMpc.getHideaway(mntPoint);
+
+			//execute
+			Assertions.assertThrows(InvalidMountPointException.class, () -> customMpc.prepareParentNoMountPoint(mntPoint));
+
+			//evaluate
+			Assertions.assertTrue(Files.notExists(hideaway));
+			Assertions.assertTrue(Files.notExists(mntPoint));
+		}
+
+	}
+
+
+}