Procházet zdrojové kódy

Resolve code smells and a bug

Co-authored-by: sonarcloud <sonarcloud@users.noreply.github.com>
Armin Schrenk před 3 roky
rodič
revize
fba0df10f9

+ 27 - 24
src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java

@@ -12,7 +12,6 @@ import org.slf4j.LoggerFactory;
 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;
@@ -25,6 +24,7 @@ class CustomMountPointChooser implements MountPointChooser {
 
 	private static final String HIDEAWAY_PREFIX = ".~$";
 	private static final String HIDEAWAY_SUFFIX = ".tmp";
+	private static final String WIN_HIDDEN = "dos:hidden";
 	private static final Logger LOG = LoggerFactory.getLogger(CustomMountPointChooser.class);
 
 	private final VaultSettings vaultSettings;
@@ -79,34 +79,27 @@ class CustomMountPointChooser implements MountPointChooser {
 		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) {
+		if (mpExists && hideExists) { //both resources exist (whatever type)
 			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()));
-			}
+			isDirectory(hideaway);
 			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);
+				Files.setAttribute(hideaway, WIN_HIDDEN, true);
 			} catch (IOException e) {
 				throw new InvalidMountPointException(e);
 			}
-		} else {
-			if (!Files.isDirectory(mountPoint)) {
-				throw new InvalidMountPointException(new NotDirectoryException(mountPoint.toString()));
-			}
+		} else { //only mountpoint exists
 			try {
-				if (Files.list(mountPoint).findFirst().isPresent()) {
-					throw new InvalidMountPointException(new DirectoryNotEmptyException(mountPoint.toString()));
-				}
+				isDirectory(mountPoint);
+				isEmpty(mountPoint);
+
 				Files.move(mountPoint, hideaway);
 				if (SystemUtils.IS_OS_WINDOWS) {
-					Files.setAttribute(hideaway, "dos:hidden", true);
+					Files.setAttribute(hideaway, WIN_HIDDEN, true);
 				}
 			} catch (IOException e) {
 				throw new InvalidMountPointException(e);
@@ -116,13 +109,9 @@ class CustomMountPointChooser implements MountPointChooser {
 
 	private void prepareEmptyMountPoint(Path mountPoint) throws InvalidMountPointException {
 		//This is the case for Windows when using Dokany and for Linux and Mac
-		if (!Files.isDirectory(mountPoint)) {
-			throw new InvalidMountPointException(new NotDirectoryException(mountPoint.toString()));
-		}
-		try (DirectoryStream<Path> ds = Files.newDirectoryStream(mountPoint)) {
-			if (ds.iterator().hasNext()) {
-				throw new InvalidMountPointException(new DirectoryNotEmptyException(mountPoint.toString()));
-			}
+		isDirectory(mountPoint);
+		try {
+			isEmpty(mountPoint);
 		} catch (IOException exception) {
 			throw new InvalidMountPointException("IOException while checking folder content", exception);
 		}
@@ -135,7 +124,7 @@ class CustomMountPointChooser implements MountPointChooser {
 			try {
 				Files.move(hideaway, mountPoint);
 				if (SystemUtils.IS_OS_WINDOWS) {
-					Files.setAttribute(mountPoint, "dos:hidden", false);
+					Files.setAttribute(mountPoint, WIN_HIDDEN, false);
 				}
 			} catch (IOException e) {
 				LOG.error("Unable to clean up mountpoint {} for Winfsp mounting.", mountPoint, e);
@@ -143,6 +132,20 @@ class CustomMountPointChooser implements MountPointChooser {
 		}
 	}
 
+	private void isDirectory(Path toCheck) throws InvalidMountPointException {
+		if (!Files.isDirectory(toCheck)) {
+			throw new InvalidMountPointException(new NotDirectoryException(toCheck.toString()));
+		}
+	}
+
+	private void isEmpty(Path toCheck) throws InvalidMountPointException, IOException {
+		try (var dirStream = Files.list(toCheck)) {
+			if (dirStream.findFirst().isPresent()) {
+				throw new InvalidMountPointException(new DirectoryNotEmptyException(toCheck.toString()));
+			}
+		}
+	}
+
 	//visible for testing
 	Path getHideaway(Path mountPoint) {
 		return mountPoint.resolveSibling(HIDEAWAY_PREFIX + mountPoint.getFileName().toString() + HIDEAWAY_SUFFIX);

+ 2 - 2
src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java

@@ -183,7 +183,7 @@ public class MountOptionsController implements FxController {
 	}
 
 	public boolean isReadOnlySupported() {
-		return !(usedVolumeImpl == VolumeImpl.FUSE && isOsWindows()) ;
+		return !(usedVolumeImpl == VolumeImpl.FUSE && isOsWindows());
 	}
 
 	public StringProperty customMountPathProperty() {
@@ -191,7 +191,7 @@ public class MountOptionsController implements FxController {
 	}
 
 	public boolean isCustomMountOptionsSupported() {
-		return !(usedVolumeImpl == VolumeImpl.WEBDAV);
+		return usedVolumeImpl != VolumeImpl.WEBDAV;
 	}
 
 	public String getCustomMountPath() {

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

@@ -1,6 +1,5 @@
 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;
@@ -41,6 +40,19 @@ public class CustomMountPointChooserTest {
 	@Nested
 	public class WinfspPreperations {
 
+		@Test
+		@DisplayName("Hideaway name for PARENT_NO_MOUNTPOINT is not the same as mountpoint")
+		public void testGetHideaway() {
+			//prepare
+			Path mntPoint = Path.of("/foo/bar");
+			//execute
+			var hideaway = customMpc.getHideaway(mntPoint);
+			//eval
+			Assertions.assertNotEquals(hideaway.getFileName(), mntPoint.getFileName());
+			Assertions.assertEquals(hideaway.getParent(), mntPoint.getParent());
+			Assertions.assertTrue(hideaway.getFileName().toString().contains(mntPoint.getFileName().toString()));
+		}
+
 		@Test
 		@DisplayName("PARENT_NO_MOUNTPOINT preparations succeeds, if only mountpoint is present")
 		public void testPrepareParentNoMountpointOnlyMountpoint(@TempDir Path tmpDir) throws IOException {
@@ -56,8 +68,6 @@ public class CustomMountPointChooserTest {
 
 			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"));
@@ -91,8 +101,6 @@ public class CustomMountPointChooserTest {
 
 			//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"));