Quellcode durchsuchen

Further improvements:
* make PARENT_NO_MOUNTPOINT preps system agnostic
* add unit tests for cleanup

Armin Schrenk vor 3 Jahren
Ursprung
Commit
4f4c992493

+ 11 - 10
src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java

@@ -72,13 +72,10 @@ class CustomMountPointChooser implements MountPointChooser {
 		}
 	}
 
+	//This the case on Windows when using FUSE
+	//See https://github.com/billziss-gh/winfsp/issues/320
 	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);
-
 		var mpExists = Files.exists(mountPoint);
 		var hideExists = Files.exists(hideaway);
 
@@ -104,11 +101,13 @@ class CustomMountPointChooser implements MountPointChooser {
 				throw new InvalidMountPointException(new NotDirectoryException(mountPoint.toString()));
 			}
 			try {
-				if(Files.list(mountPoint).findFirst().isPresent()) {
+				if (Files.list(mountPoint).findFirst().isPresent()) {
 					throw new InvalidMountPointException(new DirectoryNotEmptyException(mountPoint.toString()));
 				}
 				Files.move(mountPoint, hideaway);
-				Files.setAttribute(hideaway, "dos:hidden", true);
+				if (SystemUtils.IS_OS_WINDOWS) {
+					Files.setAttribute(hideaway, "dos:hidden", true);
+				}
 			} catch (IOException e) {
 				throw new InvalidMountPointException(e);
 			}
@@ -131,13 +130,15 @@ class CustomMountPointChooser implements MountPointChooser {
 
 	@Override
 	public void cleanup(Volume caller, Path mountPoint) {
-		if (VolumeImpl.FUSE == caller.getImplementationType() && MountPointRequirement.PARENT_NO_MOUNT_POINT == caller.getMountPointRequirement()) {
+		if (caller.getMountPointRequirement() == MountPointRequirement.PARENT_NO_MOUNT_POINT) {
 			Path hideaway = getHideaway(mountPoint);
 			try {
 				Files.move(hideaway, mountPoint);
-				Files.setAttribute(mountPoint, "dos:hidden", false);
+				if (SystemUtils.IS_OS_WINDOWS) {
+					Files.setAttribute(mountPoint, "dos:hidden", false);
+				}
 			} catch (IOException e) {
-				LOG.error("Unable to clean up mountpoint {} for Winfsp mounting.");
+				LOG.error("Unable to clean up mountpoint {} for Winfsp mounting.", mountPoint, e);
 			}
 		}
 	}

+ 48 - 5
src/test/java/org/cryptomator/common/mountpoint/CustomMountPointChooserTest.java

@@ -1,7 +1,10 @@
 package org.cryptomator.common.mountpoint;
 
+import org.apache.commons.lang3.SystemUtils;
 import org.cryptomator.common.Environment;
 import org.cryptomator.common.settings.VaultSettings;
+import org.cryptomator.common.vaults.MountPointRequirement;
+import org.cryptomator.common.vaults.Volume;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeEach;
@@ -10,6 +13,7 @@ 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.MockedStatic;
 import org.mockito.Mockito;
 
 import java.io.IOException;
@@ -21,12 +25,14 @@ public class CustomMountPointChooserTest {
 	//--- Mocks ---
 	VaultSettings vaultSettings;
 	Environment environment;
+	Volume volume;
 
 	CustomMountPointChooser customMpc;
 
 
 	@BeforeEach
 	public void init() {
+		this.volume = Mockito.mock(Volume.class);
 		this.vaultSettings = Mockito.mock(VaultSettings.class);
 		this.environment = Mockito.mock(Environment.class);
 		this.customMpc = new CustomMountPointChooser(vaultSettings, environment);
@@ -36,7 +42,7 @@ public class CustomMountPointChooserTest {
 	class WinfspPreperations {
 
 		@Test
-		@DisplayName("Test MP preparation for winfsp, if only mountpoint is present")
+		@DisplayName("PARENT_NO_MOUNTPOINT preparations succeeds, if only mountpoint is present")
 		public void testPrepareParentNoMountpointOnlyMountpoint(@TempDir Path tmpDir) throws IOException {
 			//prepare
 			var mntPoint = tmpDir.resolve("mntPoint");
@@ -58,7 +64,7 @@ public class CustomMountPointChooserTest {
 		}
 
 		@Test
-		@DisplayName("Test MP preparation for winfsp, if only non-empty mountpoint is present")
+		@DisplayName("PARENT_NO_MOUNTPOINT preparations fail, if only non-empty mountpoint is present")
 		public void testPrepareParentNoMountpointOnlyNonEmptyMountpoint(@TempDir Path tmpDir) throws IOException {
 			//prepare
 			var mntPoint = tmpDir.resolve("mntPoint");
@@ -73,7 +79,7 @@ public class CustomMountPointChooserTest {
 		}
 
 		@Test
-		@DisplayName("Test MP preparation for Winfsp, if for any reason only hideaway dir is present")
+		@DisplayName("PARENT_NO_MOUNTPOINT preparation succeeds, if for any reason only hideaway dir is present")
 		public void testPrepareParentNoMountpointOnlyHideaway(@TempDir Path tmpDir) throws IOException {
 			//prepare
 			var mntPoint = tmpDir.resolve("mntPoint");
@@ -93,7 +99,7 @@ public class CustomMountPointChooserTest {
 		}
 
 		@Test
-		@DisplayName("Test Winfsp MP preparation, if mountpoint and hideaway dirs are present")
+		@DisplayName("PARENT_NO_MOUNTPOINT preparation fails, if mountpoint and hideaway dirs are present")
 		public void testPrepareParentNoMountpointMountPointAndHideaway(@TempDir Path tmpDir) throws IOException {
 			//prepare
 			var mntPoint = tmpDir.resolve("mntPoint");
@@ -113,7 +119,7 @@ public class CustomMountPointChooserTest {
 		}
 
 		@Test
-		@DisplayName("Test Winfsp MP preparation, if neither mountpoint nor hideaway dir is present")
+		@DisplayName("PARENT_NO_MOUNTPOINT preparation fails, if neither mountpoint nor hideaway dir is present")
 		public void testPrepareParentNoMountpointNothing(@TempDir Path tmpDir) {
 			//prepare
 			var mntPoint = tmpDir.resolve("mntPoint");
@@ -127,6 +133,43 @@ public class CustomMountPointChooserTest {
 			Assertions.assertTrue(Files.notExists(mntPoint));
 		}
 
+		@Test
+		@DisplayName("Normal Cleanup for PARENT_NO_MOUNTPOINT")
+		public void testCleanupSuccess(@TempDir Path tmpDir) throws IOException {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			var hideaway = customMpc.getHideaway(mntPoint);
+
+			Files.createDirectory(hideaway);
+			Mockito.when(volume.getMountPointRequirement()).thenReturn(MountPointRequirement.PARENT_NO_MOUNT_POINT);
+
+			//execute
+			Assertions.assertDoesNotThrow(() -> customMpc.cleanup(volume, mntPoint));
+
+			//evaluate
+			Assertions.assertTrue(Files.exists(mntPoint));
+			Assertions.assertTrue(Files.notExists(hideaway));
+
+			Assumptions.assumeTrue(OS.WINDOWS.isCurrentOs());
+			Assertions.assertFalse((Boolean) Files.getAttribute(mntPoint, "dos:hidden"));
+		}
+
+		@Test
+		@DisplayName("On IOException cleanup for PARENT_NO_MOUNTPOINT exits normally")
+		public void testCleanupIOFailure(@TempDir Path tmpDir) throws IOException {
+			//prepare
+			var mntPoint = tmpDir.resolve("mntPoint");
+			var hideaway = customMpc.getHideaway(mntPoint);
+
+			Files.createDirectory(hideaway);
+			Mockito.when(volume.getMountPointRequirement()).thenReturn(MountPointRequirement.PARENT_NO_MOUNT_POINT);
+			try (MockedStatic<Files> filesMock = Mockito.mockStatic(Files.class)) {
+				filesMock.when(() -> Files.move(Mockito.any(), Mockito.any(), Mockito.any())).thenThrow(new IOException("error"));
+				//execute
+				Assertions.assertDoesNotThrow(() -> customMpc.cleanup(volume, mntPoint));
+			}
+		}
+
 	}