[MSHARED-1285] use an up-to-date scanner instead the newscanner (#77)

* [MSHARED-1285] - exhibit incremental issue

Current incremental build does not honor isUptodate(), so changed or
removed target resources are not refreshed until sources are modified or
a full build is performed.

2 new testcases exhibits this issue (one for missing target resource,
one for modified target resource). As stub BuildContext provides
appropriate isUptodate results, build should refresh this resources.

Current implementation prevents BuildContext implementors to trigger
appropriate resource refresh by tweaking isUptodate implementation.

See javadoc in BuildContext#newScanner javadoc that advices that
incremental build may be performed with a full resource scanning and a
isUptodate call to refresh only needed resources.

* MSHARED-1285 use an up-to-date scanner instead the newscanner

Currently it could happen that the scanner misses changed files (because
they are not part of the delta) or copies files even if they have not
changed (e.g. because the output has changes).

This uses now a different approach, instead of only handling the delta
files, we scan all inputs and compare if they are up-to-date with the
output.

---------

Co-authored-by: Laurent Almeras <[email protected]>
Co-authored-by: Christoph Läubrich <[email protected]>

src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java

@@ -37,6 +37,7 @@
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.maven.model.Resource;
+import org.codehaus.plexus.util.DirectoryScanner;
 import org.codehaus.plexus.util.Scanner;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -189,20 +190,44 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr
             // as destination
             // see MNG-1345
             File outputDirectory = mavenResourcesExecution.getOutputDirectory();
-            boolean outputExists = outputDirectory.exists();
-            if (!outputExists && !outputDirectory.mkdirs()) {
+            if (!outputDirectory.mkdirs() && !outputDirectory.exists()) {
                 throw new MavenFilteringException("Cannot create resource output directory: " + outputDirectory);
             }
 
             if (resource.isFiltering()) {
                 isFilteringUsed = true;
             }
 
-            boolean ignoreDelta = !outputExists
-                    || buildContext.hasDelta(mavenResourcesExecution.getFileFilters())
-                    || buildContext.hasDelta(getRelativeOutputDirectory(mavenResourcesExecution));
-            LOGGER.debug("ignoreDelta " + ignoreDelta);
-            Scanner scanner = buildContext.newScanner(resourceDirectory, ignoreDelta);
+            boolean filtersFileChanged = buildContext.hasDelta(mavenResourcesExecution.getFileFilters());
+            Path resourcePath = resourceDirectory.toPath();
+            DirectoryScanner scanner = new DirectoryScanner() {
+                @Override
+                protected boolean isSelected(String name, File file) {
+                    if (filtersFileChanged) {
+                        // if the file filters file has a change we must assume everything is out of
+                        // date
+                        return true;
+                    }
+                    if (file.isFile()) {
+                        try {
+                            File targetFile = getTargetFile(file);
+                            if (targetFile.isFile() && buildContext.isUptodate(targetFile, file)) {
+                                return false;
+                            }
+                        } catch (MavenFilteringException e) {
+                            // can't really do anything than to assume we must copy the file...
+                        }
+                    }
+                    return true;
+                }
+
+                private File getTargetFile(File file) throws MavenFilteringException {
+                    Path relativize = resourcePath.relativize(file.toPath());
+                    return getDestinationFile(
+                            outputDirectory, targetPath, relativize.toString(), mavenResourcesExecution);
+                }
+            };
+            scanner.setBasedir(resourceDirectory);
 
             setupScanner(resource, scanner, mavenResourcesExecution.isAddDefaultExcludes());
 
@@ -276,13 +301,13 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr
 
             // deal with deleted source files
 
-            scanner = buildContext.newDeleteScanner(resourceDirectory);
+            Scanner deleteScanner = buildContext.newDeleteScanner(resourceDirectory);
 
-            setupScanner(resource, scanner, mavenResourcesExecution.isAddDefaultExcludes());
+            setupScanner(resource, deleteScanner, mavenResourcesExecution.isAddDefaultExcludes());
 
-            scanner.scan();
+            deleteScanner.scan();
 
-            for (String name : scanner.getIncludedFiles()) {
+            for (String name : deleteScanner.getIncludedFiles()) {
                 File destinationFile = getDestinationFile(outputDirectory, targetPath, name, mavenResourcesExecution);
 
                 destinationFile.delete();

src/test/java/org/apache/maven/shared/filtering/IncrementalResourceFilteringTest.java

@@ -124,6 +124,55 @@ public void testFilterChange() throws Exception {
         assertTrue(ctx.getRefreshFiles().contains(outputFile02));
     }
 
+    /**
+     * Check that missing targets are rebuilt even if source is not changed (MSHARED-1285)
+     */
+    public void testMissingTarget() throws Exception {
+        // run full build first
+        filter("time");
+
+        // erase target files
+        outputFile01.toFile().delete();
+        outputFile02.toFile().delete();
+        // report change only on one file
+        Set<Path> changedFiles = new HashSet<>();
+        changedFiles.add(inputFile01);
+        TestIncrementalBuildContext ctx = new TestIncrementalBuildContext(baseDirectory, changedFiles);
+        ThreadBuildContext.setThreadBuildContext(ctx);
+
+        filter("time");
+
+        assertTrue(ctx.getRefreshFiles().contains(outputFile01));
+        assertTrue(ctx.getRefreshFiles().contains(outputFile02));
+        assertTrue(outputFile01.toFile().exists());
+        assertTrue(outputFile02.toFile().exists());
+    }
+
+    /**
+     * Check that updated targets with unchanged sources are updated (MSHARED-1285)
+     */
+    public void testUpdatedTarget() throws Exception {
+        // run full build first
+        filter("time");
+
+        // touch target file02
+        FileUtils.touch(outputFile02.toFile());
+        Set<Path> changedFiles = new HashSet<>();
+        changedFiles.add(inputFile01);
+        // report change only on target file
+        changedFiles.add(outputFile02);
+        TestIncrementalBuildContext ctx = new TestIncrementalBuildContext(baseDirectory, changedFiles);
+        ThreadBuildContext.setThreadBuildContext(ctx);
+
+        // both files are updated
+        filter("notime");
+        assertTime("notime", "file01.txt");
+        assertTime("notime", "file02.txt");
+
+        assertTrue(ctx.getRefreshFiles().contains(outputFile01));
+        assertTrue(ctx.getRefreshFiles().contains(outputFile02));
+    }
+
     public void testFilterDeleted() throws Exception {
         // run full build first
         filter("time");