desruisseaux commented on code in PR #328:
URL: 
https://github.com/apache/maven-clean-plugin/pull/328#discussion_r3702518521


##########
src/main/java/org/apache/maven/plugins/clean/BackgroundCleaner.java:
##########
@@ -111,44 +161,67 @@ final class BackgroundCleaner extends Cleaner implements 
Listener, Runnable {
     private boolean disabled;
 
     /**
-     * Creates a new cleaner to be executed in a background thread.
+     * Creates a new background cleaner service.
+     * Use {@link #getOrCreate} to obtain a session-scoped instance.
      *
-     * @param session         the Maven session to be used
-     * @param matcherFactory  the service to use for creating include and 
exclude filters.
-     * @param logger          the logger to use
-     * @param verbose         whether to perform verbose logging
-     * @param fastDir         the explicit configured directory or to be 
deleted in fast mode
-     * @param fastMode        the fast deletion mode
-     * @param followSymlinks  whether to follow symlinks
-     * @param force           whether to force the deletion of read-only files
-     * @param failOnError     whether to abort with an exception in case a 
selected file/directory could not be deleted
-     * @param retryOnError    whether to undertake additional delete attempts 
in case the first attempt failed
+     * @param session   the Maven session to be used
+     * @param logger    the logger to use
+     * @param fastDir   the directory where to temporarily move the files to 
delete
+     * @param fastMode  the fast deletion mode
      */
-    @SuppressWarnings("checkstyle:ParameterNumber")
-    BackgroundCleaner(
-            @Nonnull Session session,
-            @Nonnull PathMatcherFactory matcherFactory,
-            @Nonnull Log logger,
-            boolean verbose,
-            @Nonnull Path fastDir,
-            @Nonnull FastMode fastMode,
-            boolean followSymlinks,
-            boolean force,
-            boolean failOnError,
-            boolean retryOnError) {
-        super(matcherFactory, logger, verbose, followSymlinks, force, 
failOnError, retryOnError);
+    private BackgroundCleaner(
+            @Nonnull Session session, @Nonnull Log logger, @Nonnull Path 
fastDir, @Nonnull FastMode fastMode) {
         this.session = session;
+        this.logger = logger;
         this.fastDir = fastDir;
         this.fastMode = fastMode;
         filesToDeleteAtEnd = (fastMode != FastMode.BACKGROUND) ? new 
ArrayList<>() : null;
         directoriesToDeleteIfEmpty = new LinkedHashSet<>(); // Will need to 
delete in order.
         executor = Executors.newSingleThreadExecutor((task) -> new 
Thread(task, "mvn-background-cleaner"));
+        session.registerListener(this);
+        scanForLeftovers();
+    }
+
+    /**
+     * Returns the session-scoped {@code BackgroundCleaner}, creating it on 
first access.
+     * The instance is stored in {@link SessionData} so that all modules in a 
reactor
+     * share the same background thread and session listener.
+     *
+     * @param session   the Maven session to be used
+     * @param logger    the logger to use
+     * @param fastDir   the directory where to temporarily move the files to 
delete
+     * @param fastMode  the fast deletion mode
+     * @return the shared background cleaner instance for the session
+     */
+    static BackgroundCleaner getOrCreate(
+            @Nonnull Session session, @Nonnull Log logger, @Nonnull Path 
fastDir, @Nonnull FastMode fastMode) {
+        return session.getData().computeIfAbsent(KEY, () -> new 
BackgroundCleaner(session, logger, fastDir, fastMode));
+    }
+
+    /**
+     * Scans the fast directory for leftover directories from previous 
(possibly killed) builds
+     * and queues them for background deletion. This restores the cleanup 
behavior that was
+     * present in the singleton pattern of version 3.5.0 but was lost when 
switching to
+     * per-module instances.
+     */
+    private void scanForLeftovers() {
+        if (Files.isDirectory(fastDir)) {
+            try (DirectoryStream<Path> stream = 
Files.newDirectoryStream(fastDir)) {
+                for (Path child : stream) {
+                    if (Files.isDirectory(child)) {
+                        logger.debug("Cleaning leftover directory from 
previous build: " + child);
+                        executor.submit(() -> deleteInBackground(child, false, 
true));
+                    }
+                }
+            } catch (IOException e) {
+                logger.debug("Failed to scan for leftover directories in " + 
fastDir + ": " + e);
+            }
+        }
     }
 
     /**
      * Returns an error message to show to user if the fast delete failed.
      */
-    @Override
     String fastDeleteError(IOException e) {

Review Comment:
   If this method no longer override a method from the parent class, I presume 
that it can be made private. Then, if this method appears to be invoked in only 
one place, maybe it can be completely removed and replaced by moving this code 
inline at the invocation place, if that make the code easier to follow.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to