From a77245a0da5bd69de8da3deb801fe1de0bd0b771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Mon, 3 Apr 2017 14:49:05 +0200 Subject: [PATCH] `deletePlugin(id)` which is called by `UpdateManager.uninstallPlugin(id)` failed since the `PluginRepository.deletePluginPath()` call used `Files.deleteIfExists(path)` which is not able to delete recursively. (#135) --- .../fortsoft/pf4j/AbstractPluginManager.java | 4 +- .../fortsoft/pf4j/BasePluginRepository.java | 9 +++- .../pf4j/DefaultPluginRepository.java | 2 +- .../java/ro/fortsoft/pf4j/util/FileUtils.java | 47 ++++++++++--------- .../pf4j/DefaultPluginRepositoryTest.java | 6 ++- 5 files changed, 38 insertions(+), 30 deletions(-) diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java index ff716d7..c8cb41d 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java @@ -304,9 +304,7 @@ public abstract class AbstractPluginManager implements PluginManager { Path pluginPath = pluginWrapper.getPluginPath(); - pluginRepository.deletePluginPath(pluginPath); - - return true; + return pluginRepository.deletePluginPath(pluginPath); } /** diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/BasePluginRepository.java b/pf4j/src/main/java/ro/fortsoft/pf4j/BasePluginRepository.java index dd9fc80..e9ee47f 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/BasePluginRepository.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/BasePluginRepository.java @@ -15,10 +15,12 @@ */ package ro.fortsoft.pf4j; +import ro.fortsoft.pf4j.util.FileUtils; + import java.io.File; import java.io.FileFilter; import java.io.IOException; -import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; @@ -65,7 +67,10 @@ public class BasePluginRepository implements PluginRepository { @Override public boolean deletePluginPath(Path pluginPath) { try { - return Files.deleteIfExists(pluginPath); + FileUtils.delete(pluginPath); + return true; + } catch (NoSuchFileException nsf) { + return false; // Return false on not found to be compatible with previous API } catch (IOException e) { throw new RuntimeException(e); } diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginRepository.java b/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginRepository.java index 6efb205..953bc4e 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginRepository.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginRepository.java @@ -103,7 +103,7 @@ public class DefaultPluginRepository extends BasePluginRepository { // do not overwrite an old version, remove it if (pluginDirectory.exists()) { - FileUtils.delete(pluginDirectory); + FileUtils.delete(pluginDirectory.toPath()); } // create root for plugin diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/util/FileUtils.java b/pf4j/src/main/java/ro/fortsoft/pf4j/util/FileUtils.java index 8cf29e4..24f67fc 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/util/FileUtils.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/util/FileUtils.java @@ -16,8 +16,11 @@ package ro.fortsoft.pf4j.util; import java.io.*; +import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -67,29 +70,28 @@ public class FileUtils { } } - /** - * Delete a file or recursively delete a folder. - * - * @param fileOrFolder - * @return true, if successful - */ - public static boolean delete(File fileOrFolder) { - boolean success = false; - if (fileOrFolder.isDirectory()) { - File [] files = fileOrFolder.listFiles(); - if (files != null) { - for (File file : files) { - if (file.isDirectory()) { - success |= delete(file); - } else { - success |= file.delete(); - } - } - } - } - success |= fileOrFolder.delete(); + /** + * Delete a file or recursively delete a folder, do not follow symlinks. + * + * @param path the file or folder to delete + * @throws IOException if something goes wrong + */ + public static void delete(Path path) throws IOException { + Files.walkFileTree(path, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException { + if (!attrs.isSymbolicLink()) { + Files.delete(path); + } + return FileVisitResult.CONTINUE; + } - return success; + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + Files.delete(dir); + return FileVisitResult.CONTINUE; + } + }); } public static List getJars(Path folder) { @@ -116,5 +118,4 @@ public class FileUtils { } } } - } diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginRepositoryTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginRepositoryTest.java index 2b1dc32..0e85bdc 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginRepositoryTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginRepositoryTest.java @@ -21,7 +21,9 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import static org.junit.Assert.assertEquals; @@ -40,6 +42,8 @@ public class DefaultPluginRepositoryTest { @Before public void setUp() throws IOException { testFolder.newFolder("plugin-1"); + // Prove that we can delete a folder with a file inside + Files.createFile(Paths.get(testFolder.getRoot().getAbsolutePath()).resolve("plugin-1").resolve("myfile")); testFolder.newFolder("plugin-2"); testFolder.newFolder("plugin-3"); } @@ -81,7 +85,7 @@ public class DefaultPluginRepositoryTest { } private void assertPathExists(List paths, Path path) { - assertTrue("The directory must contains the file " + path, paths.contains(path)); + assertTrue("The directory must contain the file " + path, paths.contains(path)); } private Path getPluginsRoot() {