This is an automated email from the ASF dual-hosted git repository.
elharo pushed a commit to branch maven-clean-plugin-3.x
in repository https://gitbox.apache.org/repos/asf/maven-clean-plugin.git
The following commit(s) were added to refs/heads/maven-clean-plugin-3.x by this
push:
new cb2127f [MCLEAN-124] Leverage Files.delete(Path) API to provide more
accurate reason in case of failure (#60)
cb2127f is described below
commit cb2127f8ddb531edb67de4d9ec99555470b39e81
Author: Peter De Maeyer <peter.de.mae...@gmail.com>
AuthorDate: Sat Nov 9 16:19:42 2024 +0100
[MCLEAN-124] Leverage Files.delete(Path) API to provide more accurate
reason in case of failure (#60)
* MCLEAN-124 Leverage Files.delete(Path) API to provide more accurate
reason in case of failure
* Use Mockito + fix unit tests for Windows
* Renamed variable error -> failure
* Removed the resetting of permissions in tests, relying on JUnit @TempDir
to take care of permissions issues when clearing the temporary directory
* Fixed typo a -> an
* Simplified the setting of permissions in tests
---
pom.xml | 6 +
.../org/apache/maven/plugins/clean/Cleaner.java | 18 ++-
.../apache/maven/plugins/clean/CleanerTest.java | 143 +++++++++++++++++++++
3 files changed, 163 insertions(+), 4 deletions(-)
diff --git a/pom.xml b/pom.xml
index 73c3c72..40bb82e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -108,6 +108,12 @@ under the License.
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>4.11.0</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-testing</artifactId>
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 f5c25d2..52f1e25 100644
--- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java
+++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java
@@ -273,7 +273,8 @@ class Cleaner {
* @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 {
- if (!file.delete()) {
+ IOException failure = delete(file);
+ if (failure != null) {
boolean deleted = false;
if (retryOnError) {
@@ -289,7 +290,7 @@ class Cleaner {
} catch (InterruptedException e) {
// ignore
}
- deleted = file.delete() || !file.exists();
+ deleted = delete(file) == null || !file.exists();
}
} else {
deleted = !file.exists();
@@ -297,10 +298,10 @@ class Cleaner {
if (!deleted) {
if (failOnError) {
- throw new IOException("Failed to delete " + file);
+ throw new IOException("Failed to delete " + file, failure);
} else {
if (log.isWarnEnabled()) {
- log.warn("Failed to delete " + file);
+ log.warn("Failed to delete " + file, failure);
}
return 1;
}
@@ -310,6 +311,15 @@ class Cleaner {
return 0;
}
+ private static IOException delete(File file) {
+ try {
+ Files.delete(file.toPath());
+ } catch (IOException e) {
+ return e;
+ }
+ return null;
+ }
+
private static class Result {
private int failures;
diff --git a/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java
b/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java
new file mode 100644
index 0000000..e958664
--- /dev/null
+++ b/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.plugins.clean;
+
+import java.io.IOException;
+import java.nio.file.AccessDeniedException;
+import java.nio.file.DirectoryNotEmptyException;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Set;
+
+import org.apache.maven.plugin.logging.Log;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
+
+import static java.nio.file.Files.createDirectory;
+import static java.nio.file.Files.createFile;
+import static java.nio.file.Files.exists;
+import static java.nio.file.Files.setPosixFilePermissions;
+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.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+class CleanerTest {
+
+ private static final boolean POSIX_COMPLIANT =
+
FileSystems.getDefault().supportedFileAttributeViews().contains("posix");
+
+ private final Log log = mock();
+
+ @Test
+ void deleteSucceedsDeeply(@TempDir Path tempDir) throws Exception {
+ assumeTrue(POSIX_COMPLIANT);
+ 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);
+ assertFalse(exists(basedir));
+ assertFalse(exists(file));
+ }
+
+ @Test
+ void deleteFailsWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws
Exception {
+ assumeTrue(POSIX_COMPLIANT);
+ when(log.isWarnEnabled()).thenReturn(true);
+ final Path basedir =
createDirectory(tempDir.resolve("target")).toRealPath();
+ createFile(basedir.resolve("file"));
+ // Remove the executable flag to prevent directory listing, which will
result in a DirectoryNotEmptyException.
+ final Set<PosixFilePermission> permissions =
PosixFilePermissions.fromString("rw-rw-r--");
+ 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));
+ verify(log, never()).warn(any(CharSequence.class),
any(Throwable.class));
+ assertEquals("Failed to delete " + basedir, exception.getMessage());
+ final DirectoryNotEmptyException cause =
+ assertInstanceOf(DirectoryNotEmptyException.class,
exception.getCause());
+ assertEquals(basedir.toString(), cause.getMessage());
+ }
+
+ @Test
+ void deleteFailsAfterRetryWhenNoPermission(@TempDir Path tempDir) throws
Exception {
+ assumeTrue(POSIX_COMPLIANT);
+ final Path basedir =
createDirectory(tempDir.resolve("target")).toRealPath();
+ createFile(basedir.resolve("file"));
+ // Remove the executable flag to prevent directory listing, which will
result in a DirectoryNotEmptyException.
+ final Set<PosixFilePermission> permissions =
PosixFilePermissions.fromString("rw-rw-r--");
+ 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));
+ assertEquals("Failed to delete " + basedir, exception.getMessage());
+ final DirectoryNotEmptyException cause =
+ assertInstanceOf(DirectoryNotEmptyException.class,
exception.getCause());
+ assertEquals(basedir.toString(), cause.getMessage());
+ }
+
+ @Test
+ void deleteLogsWarningWithoutRetryWhenNoPermission(@TempDir Path tempDir)
throws Exception {
+ assumeTrue(POSIX_COMPLIANT);
+ when(log.isWarnEnabled()).thenReturn(true);
+ final Path basedir =
createDirectory(tempDir.resolve("target")).toRealPath();
+ final Path file = createFile(basedir.resolve("file"));
+ // Remove the writable flag to prevent deletion of the file, which
will result in an AccessDeniedException.
+ 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));
+ verify(log, times(2)).warn(any(CharSequence.class),
any(Throwable.class));
+ InOrder inOrder = inOrder(log);
+ ArgumentCaptor<AccessDeniedException> cause1 =
ArgumentCaptor.forClass(AccessDeniedException.class);
+ inOrder.verify(log).warn(eq("Failed to delete " + file),
cause1.capture());
+ assertEquals(file.toString(), cause1.getValue().getMessage());
+ ArgumentCaptor<DirectoryNotEmptyException> cause2 =
ArgumentCaptor.forClass(DirectoryNotEmptyException.class);
+ inOrder.verify(log).warn(eq("Failed to delete " + basedir),
cause2.capture());
+ assertEquals(basedir.toString(), cause2.getValue().getMessage());
+ }
+
+ @Test
+ void deleteDoesNotLogAnythingWhenNoPermissionAndWarnDisabled(@TempDir Path
tempDir) throws Exception {
+ assumeTrue(POSIX_COMPLIANT);
+ when(log.isWarnEnabled()).thenReturn(false);
+ final Path basedir =
createDirectory(tempDir.resolve("target")).toRealPath();
+ createFile(basedir.resolve("file"));
+ // Remove the writable flag to prevent deletion of the file, which
will result in an AccessDeniedException.
+ 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));
+ verify(log, never()).warn(any(CharSequence.class),
any(Throwable.class));
+ }
+}
