From ae392b40142d3666aa458c2f6afbf7fd7e85b716 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 25 Jul 2023 13:34:39 +0200 Subject: [PATCH] Refactored "handleMountPointFolder" (now: "getMountPointState") See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273277804 --- .../common/mount/MountWithinParentUtil.java | 36 +++++++++++++------ .../mount/MountWithinParentUtilTest.java | 17 +++++---- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 92023279c..39d6a9479 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -22,18 +22,25 @@ public final class MountWithinParentUtil { static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException, IOException { Path hideaway = getHideaway(mountPoint); - var mpExists = handleMountPointFolder(mountPoint); //Handle residual (= broken) junction as not existing + var mpState = getMountPointState(mountPoint); var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); - if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist + if (mpState == MountPointState.JUNCTION) { + LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", mountPoint); + Files.delete(mountPoint); //Throws if mountPoint is also a non-empty folder + mpState = MountPointState.NOT_EXISTING; + } + + if (mpState == MountPointState.NOT_EXISTING && !hideExists) { //neither mountpoint nor hideaway exist throw new MountPointNotExistingException(mountPoint); - } else if (!mpExists) { //only hideaway exists + } else if (mpState == MountPointState.NOT_EXISTING) { //only hideaway exists checkIsHideawayDirectory(mountPoint, hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); if (SystemUtils.IS_OS_WINDOWS) { Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, true, LinkOption.NOFOLLOW_LINKS); } - } else { //mountpoint exists... + } else { + assert mpState == MountPointState.EMPTY_DIR; try { if (hideExists) { //... with hideaway removeResidualHideaway(mountPoint, hideaway); @@ -60,21 +67,30 @@ public final class MountWithinParentUtil { } //visible for testing - static boolean handleMountPointFolder(Path path) throws IOException { + static MountPointState getMountPointState(Path path) throws IOException { if (Files.notExists(path, LinkOption.NOFOLLOW_LINKS)) { - return false; + return MountPointState.NOT_EXISTING; } if (!Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { checkIsMountPointDirectory(path); checkIsMountPointEmpty(path); - return true; + return MountPointState.EMPTY_DIR; } if (Files.exists(path /* FOLLOW_LINKS */)) { //Both junction and target exist throw new MountPointInUseException(path); } - LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); - Files.delete(path); //Throws if path is also a non-empty folder - return false; + return MountPointState.JUNCTION; + } + + //visible for testing + enum MountPointState { + + NOT_EXISTING, + + EMPTY_DIR, + + JUNCTION; + } //visible for testing diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index b9d0820a9..aad33410b 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -15,14 +15,17 @@ import java.nio.file.Path; import java.util.Objects; import static java.nio.file.LinkOption.NOFOLLOW_LINKS; +import static org.cryptomator.common.mount.MountWithinParentUtil.MountPointState.EMPTY_DIR; +import static org.cryptomator.common.mount.MountWithinParentUtil.MountPointState.NOT_EXISTING; import static org.cryptomator.common.mount.MountWithinParentUtil.cleanup; import static org.cryptomator.common.mount.MountWithinParentUtil.getHideaway; -import static org.cryptomator.common.mount.MountWithinParentUtil.handleMountPointFolder; +import static org.cryptomator.common.mount.MountWithinParentUtil.getMountPointState; import static org.cryptomator.common.mount.MountWithinParentUtil.prepareParentNoMountPoint; import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualHideaway; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -128,7 +131,7 @@ class MountWithinParentUtilTest { @Test void testHandleMountPointFolderDoesNotExist(@TempDir Path parentDir) throws IOException { - assertFalse(handleMountPointFolder(parentDir.resolve("notExisting"))); + assertSame(getMountPointState(parentDir.resolve("notExisting")), NOT_EXISTING); } @Test @@ -137,9 +140,8 @@ class MountWithinParentUtilTest { Files.createFile(regularFile); assertThrows(MountPointNotEmptyDirectoryException.class, () -> { - handleMountPointFolder(regularFile); + getMountPointState(regularFile); }); - assertTrue(Files.exists(regularFile)); } @Test @@ -150,10 +152,8 @@ class MountWithinParentUtilTest { Files.createFile(dummyFile); assertThrows(MountPointNotEmptyDirectoryException.class, () -> { - handleMountPointFolder(regularFolder); + getMountPointState(regularFolder); }); - assertTrue(Files.exists(regularFolder)); - assertTrue(Files.exists(dummyFile)); } @Test @@ -162,8 +162,7 @@ class MountWithinParentUtilTest { var regularFolder = parentDir.resolve("regularFolder"); Files.createDirectory(regularFolder); - assertTrue(handleMountPointFolder(regularFolder)); - assertTrue(Files.exists(regularFolder)); + assertSame(getMountPointState(regularFolder), EMPTY_DIR); } @Test