devonfw#139: clean up

cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java

@@ -59,23 +59,22 @@ public interface FileAccess {
   void move(Path source, Path targetDir);
 
   /**
-   * Creates a symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a symbolic
-   * link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback,
-   * which must point to absolute paths. The created link will therefore be absolute in such case.
+   * Creates a symbolic link. If the given {@code targetLink} already exists and is a symbolic link or a Windows
+   * junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback, which must
+   * point to absolute paths. Therefore, the created link will be absolute instead of relative.
    * 
-   * @param source the source {@link Path} to link to.
+   * @param source the source {@link Path} to link to, may be relative or absolute.
    * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}.
    * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute.
    */
   void symlink(Path source, Path targetLink, boolean relative);
 
   /**
-   * Creates a relative symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a
-   * symbolic link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be
-   * used as fallback, which must point to absolute paths. Hence, the created link will be absolute instead of relative
-   * in such case.
+   * Creates a relative symbolic link. If the given {@code targetLink} already exists and is a symbolic link or a
+   * Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback,
+   * which must point to absolute paths. Therefore, the created link will be absolute instead of relative.
    * 
-   * @param source the source {@link Path file or folder} to link to.
+   * @param source the source {@link Path} to link to, may be relative or absolute.
    * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}.
    */
   default void symlink(Path source, Path targetLink) {

cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java

@@ -312,9 +312,11 @@ private void deleteLinkIfExists(Path path) throws IOException {
         isJunction = attr.isOther() && attr.isDirectory();
       } catch (NoSuchFileException e) {
         // ignore, since there is no previous file at the location, so nothing to delete
+        return;
       }
     }
-    exists = exists || Files.exists(path); // since broken junctions are not detected by Files.exists(brokenJunction)
+    exists = exists || Files.exists(path); // "||" since broken junctions are not detected by
+                                           // Files.exists(brokenJunction)
     boolean isSymlink = exists && Files.isSymbolicLink(path);
 
     assert !(isSymlink && isJunction);
@@ -332,20 +334,23 @@ private void deleteLinkIfExists(Path path) throws IOException {
 
   /**
    * Adapts the given {@link Path} to be relative or absolute depending on the given {@code relative} flag.
+   * Additionally, {@link Path#toRealPath(LinkOption...)} is applied to {@code source}.
    *
    * @param source the {@link Path} to adapt.
    * @param targetLink the {@link Path} used to calculate the relative path to the {@code source} if {@code relative} is
    *        set to {@code true}.
    * @param relative the {@code relative} flag.
    * @return the adapted {@link Path}.
+   * @see FileAccessImpl#symlink(Path, Path, boolean)
    */
   private Path adaptPath(Path source, Path targetLink, boolean relative) throws IOException {
 
     if (source.isAbsolute()) {
       try {
         source = source.toRealPath(LinkOption.NOFOLLOW_LINKS); // to transform ../d1/../d2 to ../d2
       } catch (IOException e) {
-        throw new IOException("source.toRealPath() failed for source " + source, e);
+        throw new IOException(
+            "Calling toRealPath() on the source (" + source + ") in method FileAccessImpl.adaptPath() failed.", e);
       }
       if (relative) {
         source = targetLink.getParent().relativize(source);
@@ -363,38 +368,45 @@ private Path adaptPath(Path source, Path targetLink, boolean relative) throws IO
         try {
           source = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS);
         } catch (IOException e) {
-          throw new IOException(
-              "targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS) failed for source " + source
-                  + " and target link " + targetLink,
-              e);
+          throw new IOException("Calling toRealPath() on " + targetLink + ".resolveSibling(" + source
+              + ") in method FileAccessImpl.adaptPath() failed.", e);
         }
       }
     }
     return source;
   }
 
+  /**
+   * Creates a Windows junction at {@code targetLink} pointing to {@code source}.
+   * 
+   * @param source must be another Windows junction or a directory.
+   * @param targetLink the location of the Windows junction.
+   */
   private void createWindowsJunction(Path source, Path targetLink) {
 
-    Path fallbackPath = null;
+    this.context.trace("Creating a Windows junction at " + targetLink + " with " + source + " as source.");
+    Path fallbackPath;
     if (!source.isAbsolute()) {
-      try {
-        fallbackPath = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS);
-      } catch (IOException ioe) {
-        throw new IllegalStateException("Failed to create fallback symlink at " + fallbackPath, ioe);
-      }
       this.context.warning(
           "You are on Windows and you do not have permissions to create symbolic links. Junctions are used as an "
               + "alternative, however, these can not point to relative paths. So the source (" + source
-              + ") is interpreted as " + "absolute path (" + fallbackPath + ").");
+              + ") is interpreted as an absolute path.");
+      try {
+        fallbackPath = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS);
+      } catch (IOException e) {
+        throw new IllegalStateException(
+            "Since Windows junctions are used, the source must be an absolute path. The transformation of the passed "
+                + "source (" + source + ") to an absolute path failed.",
+            e);
+      }
 
     } else {
       fallbackPath = source;
     }
     if (!Files.isDirectory(fallbackPath)) { // if source is a junction. This returns true as well.
-      // TODO this if does not recognize broken junctions
       throw new IllegalStateException(
           "These junctions can only point to directories or other junctions. Please make sure that the source ("
-              + source.toAbsolutePath() + ") is one of these.");
+              + fallbackPath + ") is one of these.");
     }
     this.context.newProcess().executable("cmd")
         .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), fallbackPath.toString()).run();

cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java

@@ -20,25 +20,36 @@
  */
 public class FileAccessImplTest extends AbstractIdeContextTest {
 
+  /**
+   * Checks if Windows junctions are used.
+   *
+   * @param context the {@link IdeContext} to get system info and file access from.
+   * @param dir the {@link Path} to the directory which is used as temp directory.
+   * @return {@code true} if Windows junctions are used, {@code false} otherwise.
+   */
   private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) {
 
+    if (!context.getSystemInfo().isWindows()) {
+      return false;
+    }
+
     Path source = dir.resolve("checkIfWindowsJunctionsAreUsed");
     Path link = dir.resolve("checkIfWindowsJunctionsAreUsedLink");
     context.getFileAccess().mkdirs(source);
     try {
       Files.createSymbolicLink(link, source);
+      return false;
     } catch (IOException e) {
-      return context.getSystemInfo().isWindows();
+      return true;
     }
-    return false;
   }
 
   /**
    * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Passing absolute paths as
-   * source
+   * source.
    */
   @Test
-  public void testSymlinkAbsoluteLinks(@TempDir Path tempDir) {
+  public void testSymlinkAbsolute(@TempDir Path tempDir) {
 
     // relative links are checked in testRelativeLinksWorkAfterMoving
 
@@ -60,13 +71,11 @@ public void testSymlinkAbsoluteLinks(@TempDir Path tempDir) {
 
   /**
    * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Passing relative paths as
-   * source
+   * source.
    */
   @Test
   public void testSymlinkAbsolutePassingRelativeSource(@TempDir Path tempDir) {
 
-    // relative links are checked in testRelativeLinksWorkAfterMoving
-
     // arrange
     IdeContext context = IdeTestContextMock.get();
     FileAccess fileAccess = new FileAccessImpl(context);
@@ -220,6 +229,10 @@ public void testSymlinkWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir)
     assertThat(e1).hasMessageContaining("These junctions can only point to directories or other junctions");
   }
 
+  /**
+   * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} and whether the source paths are simplified correctly
+   * by {@link Path#toRealPath(LinkOption...)}.
+   */
   @Test
   public void testSymlinkShortcutPaths(@TempDir Path tempDir) {
 
@@ -257,6 +270,14 @@ private void createDirs(FileAccess fileAccess, Path dir) {
     fileAccess.mkdirs(dir.resolve("d2/d22/d222"));
   }
 
+  /**
+   * Creates the symlinks with passing relative paths as source. This is used by the tests of
+   * {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   *
+   * @param fa the {@link FileAccess} to use.
+   * @param dir the {@link Path} to the directory where the symlinks shall be created.
+   * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute.
+   */
   private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, boolean relative) {
 
     fa.symlink(Path.of("."), dir.resolve("d1/d11/link_to_d1"), relative);
@@ -275,6 +296,14 @@ private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, bool
     fa.symlink(Path.of("d2/another_link_to_link_to_d1"), dir.resolve("link_to_another_link_to_link_to_d1"), relative);
   }
 
+  /**
+   * Creates the symlinks with passing absolute paths as source. This is used by the tests of
+   * {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   *
+   * @param fa the {@link FileAccess} to use.
+   * @param dir the {@link Path} to the directory where the symlinks shall be created.
+   * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute.
+   */
   private void createSymlinks(FileAccess fa, Path dir, boolean relative) {
 
     fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link_to_d1"), relative);
@@ -294,6 +323,11 @@ private void createSymlinks(FileAccess fa, Path dir, boolean relative) {
         relative);
   }
 
+  /**
+   * Checks if the symlinks exist. This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   * 
+   * @param dir the {@link Path} to the directory where the symlinks are expected.
+   */
   private void assertSymlinksExist(Path dir) {
 
     assertThat(dir.resolve("d1/d11/link_to_d1")).existsNoFollowLinks();
@@ -308,6 +342,14 @@ private void assertSymlinksExist(Path dir) {
     assertThat(dir.resolve("link_to_another_link_to_link_to_d1")).existsNoFollowLinks();
   }
 
+  /**
+   * Checks if the symlinks are broken. This is used by the tests of
+   * {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   *
+   * @param dir the {@link Path} to the directory where the symlinks are expected.
+   * @param readLinks - {@code true} if the symbolic link shall be read with {@link Files#readSymbolicLink(Path)}, this
+   *        does not work for Windows junctions.
+   */
   private void assertSymlinksAreBroken(Path dir, boolean readLinks) throws IOException {
 
     assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d1"), readLinks);
@@ -322,6 +364,13 @@ private void assertSymlinksAreBroken(Path dir, boolean readLinks) throws IOExcep
     assertSymlinkIsBroken(dir.resolve("link_to_another_link_to_link_to_d1"), readLinks);
   }
 
+  /**
+   * Checks if the symlink is broken. This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   *
+   * @param link the {@link Path} to the link.
+   * @param readLinks - {@code true} if the symbolic link shall be read with {@link Files#readSymbolicLink(Path)}, this
+   *        does not work for Windows junctions.
+   */
   private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOException {
 
     try {
@@ -340,7 +389,13 @@ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOExcept
     }
   }
 
-  // only pass readLinks = true when junctions are not used.
+  /**
+   * Checks if the symlinks work. This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   *
+   * @param dir the {@link Path} to the directory where the symlinks are expected.
+   * @param readLinks - {@code true} if the symbolic link shall be read with {@link Files#readSymbolicLink(Path)}, this
+   *        does not work for Windows junctions.
+   */
   private void assertSymlinksWork(Path dir, boolean readLinks) {
 
     assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d1"), dir.resolve("d1"));
@@ -369,35 +424,49 @@ private void assertSymlinksWork(Path dir, boolean readLinks) {
     }
   }
 
+  /**
+   * Checks if the symlink works by checking {@link Path#toRealPath(LinkOption...)}} against the {@code trueTarget}. .
+   * This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}.
+   *
+   * @param link the {@link Path} to the link.
+   * @param trueTarget the {@link Path} to the true target.
+   */
   private void assertSymlinkToRealPath(Path link, Path trueTarget) {
 
     Path realPath = null;
     try {
       realPath = link.toRealPath();
     } catch (IOException e) {
-      fail("Could not call toRealPath on link: " + link, e);
+      fail("In method assertSymlinkToRealPath() could not call toRealPath() on link " + link, e);
     }
     assertThat(realPath).exists();
     assertThat(realPath).existsNoFollowLinks();
     assertThat(realPath).isEqualTo(trueTarget);
   }
 
+  /**
+   * Checks if the symlink works by checking {@link Files#readSymbolicLink(Path)} against the {@code trueTarget}. This
+   * is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}. Only call this method if junctions are
+   * not used, since junctions can not be read with {@link Files#readSymbolicLink(Path)}.
+   *
+   * @param link the {@link Path} to the link.
+   * @param trueTarget the {@link Path} to the true target.
+   */
   private void assertSymlinkRead(Path link, Path trueTarget) {
 
-    // only call this method if junctions are not used
-
     Path readPath = null;
     try {
       readPath = Files.readSymbolicLink(link);
     } catch (IOException e) {
-      fail("Could not call Files.readSymbolicLink on link: " + link, e);
+      fail("In method assertSymlinkRead() could not call readSymbolicLink() on link " + link, e);
     }
     assertThat(link.resolveSibling(readPath)).existsNoFollowLinks();
     assertThat(link.resolveSibling(readPath)).exists();
     try {
       assertThat(link.resolveSibling(readPath).toRealPath(LinkOption.NOFOLLOW_LINKS)).isEqualTo(trueTarget);
     } catch (IOException e) {
-      fail("Couldn't link.resolveSibling(readPath).toRealPath() in assertSymlinkRead:", e);
+      fail("In method assertSymlinkRead() could not call toRealPath() on link.resolveSibling(readPath) for link " + link
+          + " and readPath " + readPath, e);
     }
   }
 }