Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MCLEAN-126] Replaced old File API with new Path equivalent where possible #62

Merged
merged 3 commits into from
Jan 19, 2025
Merged

[MCLEAN-126] Replaced old File API with new Path equivalent where possible #62

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99b4b8d
MCLEAN-126 Replaced old File API with new Path equivalent where possible
peterdemaeyer Nov 10, 2024
df5cd5a
Amendment
peterdemaeyer Nov 14, 2024
0a212c8
Amendment
peterdemaeyer Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions src/main/java/org/apache/maven/plugins/clean/CleanMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -197,7 +199,7 @@ public class CleanMojo extends AbstractMojo {
* waiting for all files to be deleted when the session ends, <code>at-end</code> to indicate that the actual
* deletion should be performed synchronously when the session ends, or <code>defer</code> to specify that
* the actual file deletion should be started in the background when the session ends (this should only be used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

period inside parentheses

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. Maybe even drop the parentheses altogether?

*
* @see #fast
* @since 3.2
Expand All @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -292,13 +301,22 @@ private boolean isVerbose() {
*
* @return The directories to clean or an empty array if none, never <code>null</code>.
*/
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;
}
}
81 changes: 40 additions & 41 deletions src/main/java/org/apache/maven/plugins/clean/Cleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -56,7 +60,7 @@ class Cleaner {
*/
private final MavenSession session;

private final File fastDir;
private final Path fastDir;

private final String fastMode;

Expand All @@ -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.
Expand All @@ -96,10 +100,10 @@ class Cleaner {
* @throws IOException If a file/directory could not be deleted and <code>failOnError</code> is <code>true</code>.
*/
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);
}
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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()) {
Expand All @@ -198,7 +200,7 @@ private boolean fastDelete(File baseDirFile) {
* @throws IOException If a file/directory could not be deleted and <code>failOnError</code> is <code>true</code>.
*/
private Result delete(
File file,
Path file,
String pathname,
Selector selector,
boolean followSymlinks,
Expand All @@ -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<Path> children = newDirectoryStream(canonical)) {
for (Path child : children) {
String filename = child.getFileName().toString();
result.update(delete(
child, prefix + filename, selector, followSymlinks, failOnError, retryOnError));
}
Expand All @@ -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;
Expand Down Expand Up @@ -272,7 +272,7 @@ private boolean isSymbolicLink(Path path) throws IOException {
* @return <code>0</code> if the file was deleted, <code>1</code> otherwise.
* @throws IOException If a file/directory could not be deleted and <code>failOnError</code> is <code>true</code>.
*/
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;
Expand All @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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<File> filesToDelete = new ArrayDeque<>();
private final Deque<Path> 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);
Expand All @@ -368,7 +368,7 @@ static void sessionEnd() {

public void run() {
while (true) {
File basedir = pollNext();
Path basedir = pollNext();
if (basedir == null) {
break;
}
Expand All @@ -380,24 +380,23 @@ 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<Path> children = newDirectoryStream(fastDir)) {
for (Path child : children) {
doDelete(child);
}
}
}
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;
Expand All @@ -409,7 +408,7 @@ synchronized File pollNext() {
return basedir;
}

synchronized boolean doDelete(File dir) {
synchronized boolean doDelete(Path dir) {
if (status == STOPPED) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/apache/maven/plugins/clean/CleanerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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 =
Expand All @@ -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());
Expand All @@ -116,7 +116,7 @@ void deleteLogsWarningWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws
final Set<PosixFilePermission> 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<AccessDeniedException> cause1 = ArgumentCaptor.forClass(AccessDeniedException.class);
Expand All @@ -137,7 +137,7 @@ void deleteDoesNotLogAnythingWhenNoPermissionAndWarnDisabled(@TempDir Path tempD
final Set<PosixFilePermission> 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));
}
}