fix: Remove extra files from the jar-resources folder (#15714)

When you remove an add-on or change Vaadin version, you want the old files to be removed so they do not interfere with the compilation

Author: Artur-

flow-plugins/flow-maven-plugin/src/test/java/com/vaadin/flow/plugin/maven/BuildFrontendMojoTest.java

@@ -101,6 +101,7 @@ public class BuildFrontendMojoTest {
     private File openApiJsonFile;
     private File generatedTsFolder;
     private File tokenFile;
+    private File jarResourcesSource;
 
     private final BuildFrontendMojo mojo = Mockito.spy(new BuildFrontendMojo());
     private Lookup lookup;
@@ -141,6 +142,9 @@ public void setup() throws Exception {
         webpackOutputDirectory = new File(projectBase, VAADIN_WEBAPP_RESOURCES);
         resourceOutputDirectory = new File(projectBase,
                 VAADIN_SERVLET_RESOURCES);
+        jarResourcesSource = new File(projectBase,
+                "jar-resources-source/META-INF/frontend");
+        jarResourcesSource.mkdirs();
 
         projectFrontendResourcesDirectory = new File(npmFolder,
                 "flow_resources");
@@ -193,6 +197,8 @@ public void setup() throws Exception {
                 projectBase);
         ReflectionUtils.setVariableValueInObject(mojo, "projectBuildDir",
                 Paths.get(projectBase.toString(), "target").toString());
+        Mockito.when(mojo.getJarFiles()).thenReturn(
+                Set.of(jarResourcesSource.getParentFile().getParentFile()));
 
         generatedFolder.mkdirs();
 
@@ -640,10 +646,16 @@ private List<String> getExpectedImports() {
     private void createExpectedImports(File directoryWithImportsJs,
             File nodeModulesPath) throws IOException {
         for (String expectedImport : getExpectedImports()) {
-            File newFile = resolveImportFile(directoryWithImportsJs,
-                    nodeModulesPath, expectedImport);
-            newFile.getParentFile().mkdirs();
-            Assert.assertTrue(newFile.createNewFile());
+            if (expectedImport.startsWith("./generated/jar-resources/")) {
+                File newFile = new File(jarResourcesSource, expectedImport
+                        .substring("./generated/jar-resources/".length()));
+                Assert.assertTrue(newFile.createNewFile());
+            } else {
+                File newFile = resolveImportFile(directoryWithImportsJs,
+                        nodeModulesPath, expectedImport);
+                newFile.getParentFile().mkdirs();
+                Assert.assertTrue(newFile.createNewFile());
+            }
         }
     }
 

flow-server/src/main/java/com/vaadin/flow/server/frontend/JarContentsManager.java

@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.stream.Collectors;
@@ -190,6 +191,8 @@ private byte[] getJarEntryContents(JarFile jarFile, JarEntry entry) {
      * @param wildcardPathExclusions
      *            wildcard exclusions that are used to check each path against
      *            before copying
+     * @return names of the files that were either copied or already existed in
+     *         the output directory
      * @throws IllegalArgumentException
      *             if jar file specified is not a file or does not exist or if
      *             output directory is not a directory or does not exist
@@ -199,7 +202,7 @@ private byte[] getJarEntryContents(JarFile jarFile, JarEntry entry) {
      *             if {@link IOException} occurs during the operation, for
      *             instance, when jar file specified is not a jar file
      */
-    public void copyFilesFromJarTrimmingBasePath(File jar,
+    public Set<String> copyFilesFromJarTrimmingBasePath(File jar,
             String jarDirectoryToCopyFrom, File outputDirectory,
             String... wildcardPathExclusions) {
         requireFileExistence(jar);
@@ -213,13 +216,16 @@ public void copyFilesFromJarTrimmingBasePath(File jar,
         String basePath = normalizeJarBasePath(jarDirectoryToCopyFrom);
 
         try (JarFile jarFile = new JarFile(jar, false)) {
-            jarFile.stream().filter(file -> !file.isDirectory())
+            Set<String> handledFiles = jarFile.stream()
+                    .filter(file -> !file.isDirectory())
                     .filter(file -> file.getName().toLowerCase(Locale.ENGLISH)
                             .startsWith(basePath.toLowerCase(Locale.ENGLISH)))
                     .filter(file -> isFileIncluded(file,
                             wildcardPathExclusions))
-                    .forEach(jarEntry -> copyJarEntryTrimmingBasePath(jarFile,
-                            jarEntry, basePath, outputDirectory));
+                    .map(jarEntry -> copyJarEntryTrimmingBasePath(jarFile,
+                            jarEntry, basePath, outputDirectory))
+                    .collect(Collectors.toSet());
+            return handledFiles;
         } catch (IOException e) {
             throw new UncheckedIOException(String.format(
                     "Failed to extract files from jarFile '%s' to directory '%s'",
@@ -243,6 +249,8 @@ public void copyFilesFromJarTrimmingBasePath(File jar,
      * @param wildcardPathInclusions
      *            wildcard inclusions that are used to check each path against
      *            before copying
+     * @return names of the files that were either copied or already existed in
+     *         the output directory
      * @throws IllegalArgumentException
      *             if jar file specified is not a file or does not exist or if
      *             output directory is not a directory or does not exist
@@ -252,7 +260,7 @@ public void copyFilesFromJarTrimmingBasePath(File jar,
      *             if {@link IOException} occurs during the operation, for
      *             instance, when jar file specified is not a jar file
      */
-    public void copyIncludedFilesFromJarTrimmingBasePath(File jar,
+    public Set<String> copyIncludedFilesFromJarTrimmingBasePath(File jar,
             String jarDirectoryToCopyFrom, File outputDirectory,
             String... wildcardPathInclusions) {
         requireFileExistence(jar);
@@ -266,12 +274,15 @@ public void copyIncludedFilesFromJarTrimmingBasePath(File jar,
         String basePath = normalizeJarBasePath(jarDirectoryToCopyFrom);
 
         try (JarFile jarFile = new JarFile(jar, false)) {
-            jarFile.stream().filter(file -> !file.isDirectory())
+            Set<String> handledFiles = jarFile.stream()
+                    .filter(file -> !file.isDirectory())
                     .filter(file -> file.getName().toLowerCase(Locale.ENGLISH)
                             .startsWith(basePath.toLowerCase(Locale.ENGLISH)))
                     .filter(file -> includeFile(file, wildcardPathInclusions))
-                    .forEach(jarEntry -> copyJarEntryTrimmingBasePath(jarFile,
-                            jarEntry, basePath, outputDirectory));
+                    .map(jarEntry -> copyJarEntryTrimmingBasePath(jarFile,
+                            jarEntry, basePath, outputDirectory))
+                    .collect(Collectors.toSet());
+            return handledFiles;
         } catch (IOException e) {
             throw new UncheckedIOException(String.format(
                     "Failed to extract files from jarFile '%s' to directory '%s'",
@@ -300,7 +311,7 @@ private boolean includeFile(ZipEntry file, String... pathInclusions) {
                 .wildcardMatch(filePath, inclusionRule));
     }
 
-    private void copyJarEntryTrimmingBasePath(JarFile jarFile,
+    private String copyJarEntryTrimmingBasePath(JarFile jarFile,
             ZipEntry jarEntry, String basePath, File outputDirectory) {
         String fullPath = jarEntry.getName();
         String relativePath = fullPath
@@ -315,6 +326,7 @@ private void copyJarEntryTrimmingBasePath(JarFile jarFile,
                 FileUtils.copyInputStreamToFile(
                         jarFile.getInputStream(jarEntry), target);
             }
+            return relativePath;
         } catch (IOException e) {
             throw new UncheckedIOException(String.format(
                     "Failed to extract jar entry '%s' from jarFile", jarEntry),

flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskCopyFrontendFiles.java

@@ -15,17 +15,22 @@
  */
 package com.vaadin.flow.server.frontend;
 
+import static com.vaadin.flow.server.Constants.COMPATIBILITY_RESOURCES_FRONTEND_DEFAULT;
+import static com.vaadin.flow.server.Constants.RESOURCES_FRONTEND_DEFAULT;
+import static com.vaadin.flow.server.Constants.RESOURCES_JAR_DEFAULT;
+
 import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashSet;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static com.vaadin.flow.server.Constants.COMPATIBILITY_RESOURCES_FRONTEND_DEFAULT;
-import static com.vaadin.flow.server.Constants.RESOURCES_FRONTEND_DEFAULT;
-import static com.vaadin.flow.server.Constants.RESOURCES_JAR_DEFAULT;
-
 /**
  * Copies JavaScript and CSS files from JAR files into a given folder.
  * <p>
@@ -61,33 +66,60 @@ public void execute() {
         log().info("Copying frontend resources from jar files ...");
         File targetDirectory = options.getJarFrontendResourcesFolder();
         TaskCopyLocalFrontendFiles.createTargetFolder(targetDirectory);
+        Set<String> existingFiles;
+        try {
+            existingFiles = getFilesInDirectory(targetDirectory);
+        } catch (IOException e) {
+            // If we do not find the existing files, we will not delete anything
+            existingFiles = new HashSet<>();
+            log().error("Unable to list contents of the directory "
+                    + targetDirectory.getAbsolutePath());
+        }
         JarContentsManager jarContentsManager = new JarContentsManager();
+        Set<String> handledFiles = new HashSet<>();
         for (File location : resourceLocations) {
             if (location.isDirectory()) {
-                TaskCopyLocalFrontendFiles.copyLocalResources(
-                        new File(location, RESOURCES_FRONTEND_DEFAULT),
-                        targetDirectory);
-                TaskCopyLocalFrontendFiles.copyLocalResources(
-                        new File(location,
+                handledFiles
+                        .addAll(TaskCopyLocalFrontendFiles.copyLocalResources(
+                                new File(location, RESOURCES_FRONTEND_DEFAULT),
+                                targetDirectory));
+                handledFiles.addAll(TaskCopyLocalFrontendFiles
+                        .copyLocalResources(new File(location,
                                 COMPATIBILITY_RESOURCES_FRONTEND_DEFAULT),
-                        targetDirectory);
+                                targetDirectory));
             } else {
-                jarContentsManager.copyIncludedFilesFromJarTrimmingBasePath(
-                        location, RESOURCES_FRONTEND_DEFAULT, targetDirectory,
-                        WILDCARD_INCLUSIONS);
-                jarContentsManager.copyIncludedFilesFromJarTrimmingBasePath(
-                        location, COMPATIBILITY_RESOURCES_FRONTEND_DEFAULT,
-                        targetDirectory, WILDCARD_INCLUSIONS);
-                jarContentsManager.copyIncludedFilesFromJarTrimmingBasePath(
-                        location, RESOURCES_JAR_DEFAULT, targetDirectory,
-                        WILDCARD_INCLUSION_APP_THEME_JAR);
+                handledFiles.addAll(jarContentsManager
+                        .copyIncludedFilesFromJarTrimmingBasePath(location,
+                                RESOURCES_FRONTEND_DEFAULT, targetDirectory,
+                                WILDCARD_INCLUSIONS));
+                handledFiles.addAll(jarContentsManager
+                        .copyIncludedFilesFromJarTrimmingBasePath(location,
+                                COMPATIBILITY_RESOURCES_FRONTEND_DEFAULT,
+                                targetDirectory, WILDCARD_INCLUSIONS));
+                handledFiles.addAll(jarContentsManager
+                        .copyIncludedFilesFromJarTrimmingBasePath(location,
+                                RESOURCES_JAR_DEFAULT, targetDirectory,
+                                WILDCARD_INCLUSION_APP_THEME_JAR));
             }
         }
+        existingFiles.removeAll(handledFiles);
+        existingFiles.forEach(
+                filename -> new File(targetDirectory, filename).delete());
         long ms = (System.nanoTime() - start) / 1000000;
         log().info("Visited {} resources. Took {} ms.",
                 resourceLocations.size(), ms);
     }
 
+    static Set<String> getFilesInDirectory(File targetDirectory)
+            throws IOException {
+        try (Stream<Path> stream = Files.walk(targetDirectory.toPath())) {
+            return stream.filter(path -> path.toFile().isFile())
+                    .map(path -> targetDirectory.toPath().relativize(path)
+                            .toString())
+                    .collect(Collectors.toSet());
+        }
+    }
+
     private Logger log() {
         return LoggerFactory.getLogger(this.getClass());
     }

flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskCopyLocalFrontendFiles.java

@@ -21,7 +21,10 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Stream;
 
 import org.apache.commons.io.FileUtils;
@@ -65,18 +68,21 @@ public void execute() {
         }
     }
 
-    static void copyLocalResources(File source, File target) {
+    static Set<String> copyLocalResources(File source, File target) {
         if (!source.isDirectory() || !target.isDirectory()) {
-            return;
+            return Collections.emptySet();
         }
         try {
+            Set<String> handledFiles = new HashSet<>(
+                    TaskCopyFrontendFiles.getFilesInDirectory(source));
             FileUtils.copyDirectory(source, target);
             try (Stream<Path> fileStream = Files
                     .walk(Paths.get(target.getPath()))) {
                 // used with try-with-resources as defined in walk API note
                 fileStream.filter(file -> !Files.isWritable(file)).forEach(
                         filePath -> filePath.toFile().setWritable(true));
             }
+            return handledFiles;
         } catch (IOException e) {
             throw new UncheckedIOException(String.format(
                     "Failed to copy project frontend resources from '%s' to '%s'",

flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskCopyFrontendFilesTest.java

@@ -70,6 +70,16 @@ public void should_collectJsAndCssFilesFromJars_modernResourceFolder()
                 "dir-with-modern-frontend");
     }
 
+    @Test
+    public void should_collectJsAndCssFilesFromJars_removeExtraFiles()
+            throws IOException {
+        File dummy = new File(frontendDepsFolder, "dummy.ts");
+        frontendDepsFolder.mkdirs();
+        dummy.createNewFile();
+        should_collectJsAndCssFilesFromJars("jar-with-modern-frontend.jar",
+                "dir-with-modern-frontend");
+    }
+
     @Test
     public void should_createPackageJson() throws IOException {
         Options options = new Options(Mockito.mock(Lookup.class), npmFolder)