From 99b4b8d509e8a94d89d47350d2bd1e11a3bd6eb3 Mon Sep 17 00:00:00 2001 From: Peter De Maeyer Date: Sun, 10 Nov 2024 21:35:55 +0100 Subject: [PATCH 1/3] MCLEAN-126 Replaced old File API with new Path equivalent where possible --- .../apache/maven/plugins/clean/CleanMojo.java | 38 ++++++--- .../apache/maven/plugins/clean/Cleaner.java | 81 +++++++++---------- .../maven/plugins/clean/CleanMojoTest.java | 4 +- .../maven/plugins/clean/CleanerTest.java | 10 +-- 4 files changed, 75 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 4f77c00..c7a2206 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -20,6 +20,8 @@ import java.io.File; import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Path; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.AbstractMojo; @@ -197,7 +199,7 @@ public class CleanMojo extends AbstractMojo { * waiting for all files to be deleted when the session ends, at-end to indicate that the actual * deletion should be performed synchronously when the session ends, or defer to specify that * the actual file deletion should be started in the background when the session ends (this should only be used - * when maven is embedded in a long running process). + * when maven is embedded in a long-running process). * * @see #fast * @since 3.2 @@ -223,11 +225,14 @@ public void execute() throws MojoExecutionException { String multiModuleProjectDirectory = session != null ? session.getSystemProperties().getProperty("maven.multiModuleProjectDirectory") : null; - File fastDir; + Path fastDir; if (fast && this.fastDir != null) { - fastDir = this.fastDir; + fastDir = toNullablePath(this.fastDir); } else if (fast && multiModuleProjectDirectory != null) { - fastDir = new File(multiModuleProjectDirectory, "target/.clean"); + fastDir = FileSystems.getDefault() + .getPath(multiModuleProjectDirectory) + .resolve("target") + .resolve(".clean"); } else { fastDir = null; if (fast) { @@ -247,7 +252,7 @@ public void execute() throws MojoExecutionException { Cleaner cleaner = new Cleaner(session, getLog(), isVerbose(), fastDir, fastMode); try { - for (File directoryItem : getDirectories()) { + for (Path directoryItem : getDirectories()) { if (directoryItem != null) { cleaner.delete(directoryItem, null, followSymLinks, failOnError, retryOnError); } @@ -270,7 +275,11 @@ public void execute() throws MojoExecutionException { selector = null; } cleaner.delete( - fileset.getDirectory(), selector, fileset.isFollowSymlinks(), failOnError, retryOnError); + fileset.getDirectory().toPath(), + selector, + fileset.isFollowSymlinks(), + failOnError, + retryOnError); } } } catch (IOException e) { @@ -292,13 +301,22 @@ private boolean isVerbose() { * * @return The directories to clean or an empty array if none, never null. */ - private File[] getDirectories() { - File[] directories; + private Path[] getDirectories() { + Path[] directories; if (excludeDefaultDirectories) { - directories = new File[0]; + directories = new Path[0]; } else { - directories = new File[] {directory, outputDirectory, testOutputDirectory, reportDirectory}; + directories = new Path[] { + toNullablePath(directory), + toNullablePath(outputDirectory), + toNullablePath(testOutputDirectory), + toNullablePath(reportDirectory) + }; } return directories; } + + private static Path toNullablePath(File file) { + return file != null ? file.toPath() : null; + } } diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 52f1e25..ef05038 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -23,6 +23,7 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; +import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; @@ -37,6 +38,9 @@ import org.codehaus.plexus.util.Os; import org.eclipse.aether.SessionData; +import static java.nio.file.Files.exists; +import static java.nio.file.Files.isDirectory; +import static java.nio.file.Files.newDirectoryStream; import static org.apache.maven.plugins.clean.CleanMojo.FAST_MODE_BACKGROUND; import static org.apache.maven.plugins.clean.CleanMojo.FAST_MODE_DEFER; @@ -56,7 +60,7 @@ class Cleaner { */ private final MavenSession session; - private final File fastDir; + private final Path fastDir; private final String fastMode; @@ -73,7 +77,7 @@ class Cleaner { * @param fastDir The explicit configured directory or to be deleted in fast mode. * @param fastMode The fast deletion mode. */ - Cleaner(MavenSession session, final Log log, boolean verbose, File fastDir, String fastMode) { + Cleaner(MavenSession session, final Log log, boolean verbose, Path fastDir, String fastMode) { this.session = session; // This can't be null as the Cleaner gets it from the CleanMojo which gets it from AbstractMojo class, where it // is never null. @@ -96,10 +100,10 @@ class Cleaner { * @throws IOException If a file/directory could not be deleted and failOnError is true. */ public void delete( - File basedir, Selector selector, boolean followSymlinks, boolean failOnError, boolean retryOnError) + Path basedir, Selector selector, boolean followSymlinks, boolean failOnError, boolean retryOnError) throws IOException { - if (!basedir.isDirectory()) { - if (!basedir.exists()) { + if (!isDirectory(basedir)) { + if (!exists(basedir)) { if (log.isDebugEnabled()) { log.debug("Skipping non-existing directory " + basedir); } @@ -112,7 +116,7 @@ public void delete( log.info("Deleting " + basedir + (selector != null ? " (" + selector + ")" : "")); } - File file = followSymlinks ? basedir : basedir.getCanonicalFile(); + Path file = followSymlinks ? basedir : basedir.toRealPath(); if (selector == null && !followSymlinks && fastDir != null && session != null) { // If anything wrong happens, we'll just use the usual deletion mechanism @@ -124,9 +128,7 @@ public void delete( delete(file, "", selector, followSymlinks, failOnError, retryOnError); } - private boolean fastDelete(File baseDirFile) { - Path baseDir = baseDirFile.toPath(); - Path fastDir = this.fastDir.toPath(); + private boolean fastDelete(Path baseDir) { // Handle the case where we use ${maven.multiModuleProjectDirectory}/target/.clean for example if (fastDir.toAbsolutePath().startsWith(baseDir.toAbsolutePath())) { try { @@ -135,7 +137,7 @@ private boolean fastDelete(File baseDirFile) { try { Files.move(baseDir, tmpDir, StandardCopyOption.REPLACE_EXISTING); if (session != null) { - session.getRepositorySession().getData().set(LAST_DIRECTORY_TO_DELETE, baseDir.toFile()); + session.getRepositorySession().getData().set(LAST_DIRECTORY_TO_DELETE, baseDir); } baseDir = tmpDir; } catch (IOException e) { @@ -151,7 +153,7 @@ private boolean fastDelete(File baseDirFile) { } // Create fastDir and the needed parents if needed try { - if (!Files.isDirectory(fastDir)) { + if (!isDirectory(fastDir)) { Files.createDirectories(fastDir); } } catch (IOException e) { @@ -172,7 +174,7 @@ private boolean fastDelete(File baseDirFile) { // or any other exception occurs, an exception will be thrown in which case // the method will return false and the usual deletion will be performed. Files.move(baseDir, dstDir, StandardCopyOption.ATOMIC_MOVE); - BackgroundCleaner.delete(this, tmpDir.toFile(), fastMode); + BackgroundCleaner.delete(this, tmpDir, fastMode); return true; } catch (IOException e) { if (log.isDebugEnabled()) { @@ -198,7 +200,7 @@ private boolean fastDelete(File baseDirFile) { * @throws IOException If a file/directory could not be deleted and failOnError is true. */ private Result delete( - File file, + Path file, String pathname, Selector selector, boolean followSymlinks, @@ -207,18 +209,16 @@ private Result delete( throws IOException { Result result = new Result(); - boolean isDirectory = file.isDirectory(); + boolean isDirectory = isDirectory(file); if (isDirectory) { if (selector == null || selector.couldHoldSelected(pathname)) { - if (followSymlinks || !isSymbolicLink(file.toPath())) { - File canonical = followSymlinks ? file : file.getCanonicalFile(); - String[] filenames = canonical.list(); - if (filenames != null) { - String prefix = pathname.length() > 0 ? pathname + File.separatorChar : ""; - for (int i = filenames.length - 1; i >= 0; i--) { - String filename = filenames[i]; - File child = new File(canonical, filename); + if (followSymlinks || !isSymbolicLink(file)) { + Path canonical = followSymlinks ? file : file.toRealPath(); + String prefix = pathname.length() > 0 ? pathname + File.separatorChar : ""; + try (DirectoryStream children = newDirectoryStream(canonical)) { + for (Path child : children) { + String filename = child.getFileName().toString(); result.update(delete( child, prefix + filename, selector, followSymlinks, failOnError, retryOnError)); } @@ -235,7 +235,7 @@ private Result delete( String logmessage; if (isDirectory) { logmessage = "Deleting directory " + file; - } else if (file.exists()) { + } else if (exists(file)) { logmessage = "Deleting file " + file; } else { logmessage = "Deleting dangling symlink " + file; @@ -272,7 +272,7 @@ private boolean isSymbolicLink(Path path) throws IOException { * @return 0 if the file was deleted, 1 otherwise. * @throws IOException If a file/directory could not be deleted and failOnError is true. */ - private int delete(File file, boolean failOnError, boolean retryOnError) throws IOException { + private int delete(Path file, boolean failOnError, boolean retryOnError) throws IOException { IOException failure = delete(file); if (failure != null) { boolean deleted = false; @@ -290,10 +290,10 @@ private int delete(File file, boolean failOnError, boolean retryOnError) throws } catch (InterruptedException e) { // ignore } - deleted = delete(file) == null || !file.exists(); + deleted = delete(file) == null || !exists(file); } } else { - deleted = !file.exists(); + deleted = !exists(file); } if (!deleted) { @@ -311,9 +311,9 @@ private int delete(File file, boolean failOnError, boolean retryOnError) throws return 0; } - private static IOException delete(File file) { + private static IOException delete(Path file) { try { - Files.delete(file.toPath()); + Files.delete(file); } catch (IOException e) { return e; } @@ -338,19 +338,19 @@ private static class BackgroundCleaner extends Thread { private static final int RUNNING = 1; private static final int STOPPED = 2; private static BackgroundCleaner instance; - private final Deque filesToDelete = new ArrayDeque<>(); + private final Deque filesToDelete = new ArrayDeque<>(); private final Cleaner cleaner; private final String fastMode; private int status = NEW; - private BackgroundCleaner(Cleaner cleaner, File dir, String fastMode) { + private BackgroundCleaner(Cleaner cleaner, Path dir, String fastMode) throws IOException { super("mvn-background-cleaner"); this.cleaner = cleaner; this.fastMode = fastMode; init(cleaner.fastDir, dir); } - public static void delete(Cleaner cleaner, File dir, String fastMode) { + public static void delete(Cleaner cleaner, Path dir, String fastMode) throws IOException { synchronized (BackgroundCleaner.class) { if (instance == null || !instance.doDelete(dir)) { instance = new BackgroundCleaner(cleaner, dir, fastMode); @@ -368,7 +368,7 @@ static void sessionEnd() { public void run() { while (true) { - File basedir = pollNext(); + Path basedir = pollNext(); if (basedir == null) { break; } @@ -380,11 +380,10 @@ public void run() { } } - synchronized void init(File fastDir, File dir) { - if (fastDir.isDirectory()) { - File[] children = fastDir.listFiles(); - if (children != null && children.length > 0) { - for (File child : children) { + synchronized void init(Path fastDir, Path dir) throws IOException { + if (isDirectory(fastDir)) { + try (DirectoryStream children = newDirectoryStream(fastDir)) { + for (Path child : children) { doDelete(child); } } @@ -392,12 +391,12 @@ synchronized void init(File fastDir, File dir) { doDelete(dir); } - synchronized File pollNext() { - File basedir = filesToDelete.poll(); + synchronized Path pollNext() { + Path basedir = filesToDelete.poll(); if (basedir == null) { if (cleaner.session != null) { SessionData data = cleaner.session.getRepositorySession().getData(); - File lastDir = (File) data.get(LAST_DIRECTORY_TO_DELETE); + Path lastDir = (Path) data.get(LAST_DIRECTORY_TO_DELETE); if (lastDir != null) { data.set(LAST_DIRECTORY_TO_DELETE, null); return lastDir; @@ -409,7 +408,7 @@ synchronized File pollNext() { return basedir; } - synchronized boolean doDelete(File dir) { + synchronized boolean doDelete(Path dir) { if (status == STOPPED) { return false; } diff --git a/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java b/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java index 22287d7..06b7297 100644 --- a/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java @@ -263,7 +263,7 @@ private void testSymlink(LinkCreator linkCreator) throws Exception { Files.write(file, Collections.singleton("Hello world")); linkCreator.createLink(jctDir, orgDir); // delete - cleaner.delete(dirWithLnk.toFile(), null, false, true, false); + cleaner.delete(dirWithLnk, null, false, true, false); // verify assertTrue(Files.exists(file)); assertFalse(Files.exists(jctDir)); @@ -276,7 +276,7 @@ private void testSymlink(LinkCreator linkCreator) throws Exception { Files.write(file, Collections.singleton("Hello world")); linkCreator.createLink(jctDir, orgDir); // delete - cleaner.delete(dirWithLnk.toFile(), null, true, true, false); + cleaner.delete(dirWithLnk, null, true, true, false); // verify assertFalse(Files.exists(file)); assertFalse(Files.exists(jctDir)); diff --git a/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java b/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java index e958664..d7d42cc 100644 --- a/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java +++ b/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java @@ -65,7 +65,7 @@ void deleteSucceedsDeeply(@TempDir Path tempDir) throws Exception { final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); final Path file = createFile(basedir.resolve("file")); final Cleaner cleaner = new Cleaner(null, log, false, null, null); - cleaner.delete(basedir.toFile(), null, false, true, false); + cleaner.delete(basedir, null, false, true, false); assertFalse(exists(basedir)); assertFalse(exists(file)); } @@ -81,7 +81,7 @@ void deleteFailsWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws Excep setPosixFilePermissions(basedir, permissions); final Cleaner cleaner = new Cleaner(null, log, false, null, null); final IOException exception = - assertThrows(IOException.class, () -> cleaner.delete(basedir.toFile(), null, false, true, false)); + assertThrows(IOException.class, () -> cleaner.delete(basedir, null, false, true, false)); verify(log, never()).warn(any(CharSequence.class), any(Throwable.class)); assertEquals("Failed to delete " + basedir, exception.getMessage()); final DirectoryNotEmptyException cause = @@ -99,7 +99,7 @@ void deleteFailsAfterRetryWhenNoPermission(@TempDir Path tempDir) throws Excepti setPosixFilePermissions(basedir, permissions); final Cleaner cleaner = new Cleaner(null, log, false, null, null); final IOException exception = - assertThrows(IOException.class, () -> cleaner.delete(basedir.toFile(), null, false, true, true)); + assertThrows(IOException.class, () -> cleaner.delete(basedir, null, false, true, true)); assertEquals("Failed to delete " + basedir, exception.getMessage()); final DirectoryNotEmptyException cause = assertInstanceOf(DirectoryNotEmptyException.class, exception.getCause()); @@ -116,7 +116,7 @@ void deleteLogsWarningWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws final Set permissions = PosixFilePermissions.fromString("r-xr-xr-x"); setPosixFilePermissions(basedir, permissions); final Cleaner cleaner = new Cleaner(null, log, false, null, null); - assertDoesNotThrow(() -> cleaner.delete(basedir.toFile(), null, false, false, false)); + assertDoesNotThrow(() -> cleaner.delete(basedir, null, false, false, false)); verify(log, times(2)).warn(any(CharSequence.class), any(Throwable.class)); InOrder inOrder = inOrder(log); ArgumentCaptor cause1 = ArgumentCaptor.forClass(AccessDeniedException.class); @@ -137,7 +137,7 @@ void deleteDoesNotLogAnythingWhenNoPermissionAndWarnDisabled(@TempDir Path tempD final Set permissions = PosixFilePermissions.fromString("r-xr-xr-x"); setPosixFilePermissions(basedir, permissions); final Cleaner cleaner = new Cleaner(null, log, false, null, null); - assertDoesNotThrow(() -> cleaner.delete(basedir.toFile(), null, false, false, false)); + assertDoesNotThrow(() -> cleaner.delete(basedir, null, false, false, false)); verify(log, never()).warn(any(CharSequence.class), any(Throwable.class)); } } From df5cd5aa002ac43891eb101e4a1bb85c96f54a63 Mon Sep 17 00:00:00 2001 From: Peter De Maeyer Date: Thu, 14 Nov 2024 08:15:49 +0100 Subject: [PATCH 2/3] Amendment --- src/main/java/org/apache/maven/plugins/clean/CleanMojo.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index c7a2206..f8133d7 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -198,8 +198,8 @@ public class CleanMojo extends AbstractMojo { * Mode to use when using fast clean. Values are: background to start deletion immediately and * waiting for all files to be deleted when the session ends, at-end to indicate that the actual * deletion should be performed synchronously when the session ends, or defer to specify that - * the actual file deletion should be started in the background when the session ends (this should only be used - * when maven is embedded in a long-running process). + * the actual file deletion should be started in the background when the session ends. This should only be used + * when maven is embedded in a long-running process. * * @see #fast * @since 3.2 From 0a212c8e243c5429621ee9f87f01c08175a0cbc0 Mon Sep 17 00:00:00 2001 From: Peter De Maeyer Date: Thu, 14 Nov 2024 08:17:35 +0100 Subject: [PATCH 3/3] Amendment --- src/main/java/org/apache/maven/plugins/clean/CleanMojo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index f8133d7..0c3ee3a 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -185,7 +185,7 @@ public class CleanMojo extends AbstractMojo { * ${maven.multiModuleProjectDirectory}/target/.clean directory will be used. If the * ${build.directory} has been modified, you'll have to adjust this property explicitly. * In order for fast clean to work correctly, this directory and the various directories that will be deleted - * should usually reside on the same volume. The exact conditions are system dependant though, but if an atomic + * should usually reside on the same volume. The exact conditions are system-dependent though, but if an atomic * move is not supported, the standard deletion mechanism will be used. * * @see #fast