diff --git a/flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/CleanFrontendMojo.java b/flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/CleanFrontendMojo.java index 91501f70f8e..99f999004be 100644 --- a/flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/CleanFrontendMojo.java +++ b/flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/CleanFrontendMojo.java @@ -26,6 +26,8 @@ import org.apache.maven.plugins.annotations.LifecyclePhase; import org.apache.maven.plugins.annotations.Mojo; +import com.vaadin.flow.server.frontend.FrontendUtils; + import elemental.json.Json; import elemental.json.JsonObject; import elemental.json.impl.JsonUtil; @@ -103,15 +105,12 @@ public void execute() throws MojoFailureException { private void removeNodeModules() { // Remove node_modules folder File nodeModules = new File(npmFolder(), "node_modules"); - if (nodeModules.exists()) { - try { - FileUtils.deleteDirectory(nodeModules); - } catch (IOException exception) { - getLog().debug("Exception removing node_modules", exception); - getLog().error( - "Failed to remove '" + nodeModules.getAbsolutePath() - + "'. Please remove it manually."); - } + try { + FrontendUtils.deleteNodeModules(nodeModules); + } catch (IOException exception) { + getLog().debug("Exception removing node_modules", exception); + getLog().error("Failed to remove '" + nodeModules.getAbsolutePath() + + "'. Please remove it manually."); } } diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java index 4459c68cefc..0eb7677a6b0 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java @@ -25,16 +25,19 @@ import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.time.LocalDateTime; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Scanner; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.io.FileUtils; @@ -1215,4 +1218,39 @@ public static void console(String format, Object message) { System.out.print(format(format, message)); } + /** + * Try to remove the {@code node_modules} directory, if it exists inside the + * given base directory. Note that pnpm uses symlinks internally, so delete + * utilities that follow symlinks when deleting and/or modifying permissions + * may not work as intended. + * + * @param nodeModules + * the {@code node_modules} directory + * @throws IOException + * on failure to delete any one file, or if the directory name + * is not {@code node_modules} + */ + public static void deleteNodeModules(File nodeModules) throws IOException { + if (!nodeModules.exists()) { + return; + } + + if (!nodeModules.isDirectory() + || !nodeModules.getName().equals("node_modules")) { + throw new IOException(nodeModules.getAbsolutePath() + + " does not look like a node_modules directory"); + } + + Path nodeModulesPath = nodeModules.toPath(); + try (Stream walk = Files.walk(nodeModulesPath)) { + String undeletable = walk.sorted(Comparator.reverseOrder()) + .map(Path::toFile).filter(file -> !file.delete()) + .map(File::getAbsolutePath) + .collect(Collectors.joining(", ")); + + if (!undeletable.isEmpty()) { + throw new IOException("Unable to delete files: " + undeletable); + } + } + } } diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskRunNpmInstall.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskRunNpmInstall.java index 72e7da59b3d..af9e4f26d48 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskRunNpmInstall.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskRunNpmInstall.java @@ -531,7 +531,7 @@ private void cleanUp() throws ExecutionFailedException { private void deleteNodeModules(File nodeModulesFolder) throws ExecutionFailedException { try { - FileUtils.forceDelete(nodeModulesFolder); + FrontendUtils.deleteNodeModules(nodeModulesFolder); } catch (IOException exception) { Logger log = packageUpdater.log(); log.debug("Exception removing node_modules", exception); diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java index f9f7b165aa6..a360d3c34f0 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; import java.util.List; @@ -316,6 +317,56 @@ public void clearCachedStatsContent_clearsCache() service.getContext().getAttribute(CACHE_KEY)); } + @Test + public void deleteNodeModules_nopIfNotExists() throws IOException { + File nodeModules = new File(tmpDir.getRoot(), "node_modules"); + FrontendUtils.deleteNodeModules(nodeModules); + } + + @Test(expected = IOException.class) + public void deleteNodeModules_throwsIfNotNamedNodeModules() + throws IOException { + File myModules = new File(tmpDir.getRoot(), "my_modules"); + myModules.mkdirs(); + FrontendUtils.deleteNodeModules(myModules); + } + + @Test + public void deleteNodeModules_canDeleteSymlinksAndNotFollowThem() + throws IOException { + File externalDir = new File(tmpDir.getRoot(), "external"); + File externalLicense = new File(externalDir, "LICENSE"); + + externalLicense.getParentFile().mkdirs(); + externalLicense.createNewFile(); + + File nodeModules = new File(tmpDir.getRoot(), "node_modules"); + File containing = new File(nodeModules, ".pnpm/a/node_modules/dep"); + containing.mkdirs(); + File license = new File(containing, "LICENSE"); + license.createNewFile(); + + File linking = new File(nodeModules, ".pnpm/b/node_modules/dep"); + linking.getParentFile().mkdirs(); + Files.createSymbolicLink(linking.toPath(), + new File("../../a/node_modules/dep").toPath()); + + File linkingExternal = new File(nodeModules, + ".pnpm/b/node_modules/external"); + Files.createSymbolicLink(linkingExternal.toPath(), + new File("../../../../external").toPath()); + + Assert.assertTrue(nodeModules.exists()); + Assert.assertTrue(linking.exists()); + Assert.assertTrue(new File(linking, "LICENSE").exists()); + Assert.assertTrue(new File(linkingExternal, "LICENSE").exists()); + + FrontendUtils.deleteNodeModules(nodeModules); + + Assert.assertFalse(nodeModules.exists()); + Assert.assertTrue(externalLicense.exists()); + } + private ResourceProvider mockResourceProvider(VaadinService service) { DeploymentConfiguration config = Mockito .mock(DeploymentConfiguration.class);