`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)

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<Path>() {
+           @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<File> 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<Path> 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() {