From: Decebal Suiu <decebal.suiu@gmail.com>
Date: Mon, 5 Aug 2019 19:28:14 +0000 (+0300)
Subject: Resolve #335
X-Git-Tag: release-3.1.0~5
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=f66859f71c0e9c043a14ca32f26b459de54317aa;p=pf4j.git

Resolve #335
---

diff --git a/pf4j/src/main/java/org/pf4j/DefaultPluginStatusProvider.java b/pf4j/src/main/java/org/pf4j/DefaultPluginStatusProvider.java
index 0b88fe8..37e84b9 100644
--- a/pf4j/src/main/java/org/pf4j/DefaultPluginStatusProvider.java
+++ b/pf4j/src/main/java/org/pf4j/DefaultPluginStatusProvider.java
@@ -20,6 +20,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
 
@@ -45,11 +46,11 @@ public class DefaultPluginStatusProvider implements PluginStatusProvider {
 
         try {
             // create a list with plugin identifiers that should be only accepted by this manager (whitelist from plugins/enabled.txt file)
-            enabledPlugins = FileUtils.readLines(pluginsRoot.resolve("enabled.txt"), true);
+            enabledPlugins = FileUtils.readLines(getEnabledFilePath(), true);
             log.info("Enabled plugins: {}", enabledPlugins);
 
             // create a list with plugin identifiers that should not be accepted by this manager (blacklist from plugins/disabled.txt file)
-            disabledPlugins = FileUtils.readLines(pluginsRoot.resolve("disabled.txt"), true);
+            disabledPlugins = FileUtils.readLines(getDisabledFilePath(), true);
             log.info("Disabled plugins: {}", disabledPlugins);
         } catch (IOException e) {
             log.error(e.getMessage(), e);
@@ -67,22 +68,70 @@ public class DefaultPluginStatusProvider implements PluginStatusProvider {
 
     @Override
     public void disablePlugin(String pluginId) {
-        disabledPlugins.add(pluginId);
-        try {
-            FileUtils.writeLines(disabledPlugins, pluginsRoot.resolve("disabled.txt").toFile());
-        } catch (IOException e) {
-            throw new PluginRuntimeException(e);
+        if (isPluginDisabled(pluginId)) {
+            // do nothing
+            return;
+        }
+
+        if (Files.exists(getEnabledFilePath())) {
+            enabledPlugins.remove(pluginId);
+
+            try {
+                FileUtils.writeLines(enabledPlugins, getEnabledFilePath());
+            } catch (IOException e) {
+                throw new PluginRuntimeException(e);
+            }
+        } else {
+            disabledPlugins.add(pluginId);
+
+            try {
+                FileUtils.writeLines(disabledPlugins, getDisabledFilePath());
+            } catch (IOException e) {
+                throw new PluginRuntimeException(e);
+            }
         }
     }
 
     @Override
     public void enablePlugin(String pluginId) {
-        disabledPlugins.remove(pluginId);
-        try {
-            FileUtils.writeLines(disabledPlugins, pluginsRoot.resolve("disabled.txt").toFile());
-        } catch (IOException e) {
-            throw new PluginRuntimeException(e);
+        if (!isPluginDisabled(pluginId)) {
+            // do nothing
+            return;
+        }
+
+        if (Files.exists(getEnabledFilePath())) {
+            enabledPlugins.add(pluginId);
+
+            try {
+                FileUtils.writeLines(enabledPlugins, getEnabledFilePath());
+            } catch (IOException e) {
+                throw new PluginRuntimeException(e);
+            }
+        } else {
+            disabledPlugins.remove(pluginId);
+
+            try {
+                FileUtils.writeLines(disabledPlugins, getDisabledFilePath());
+            } catch (IOException e) {
+                throw new PluginRuntimeException(e);
+            }
         }
     }
 
+    public Path getEnabledFilePath() {
+        return getEnabledFilePath(pluginsRoot);
+    }
+
+    public Path getDisabledFilePath() {
+        return getDisabledFilePath(pluginsRoot);
+    }
+
+    public static Path getEnabledFilePath(Path pluginsRoot) {
+        return pluginsRoot.resolve("enabled.txt");
+    }
+
+    public static Path getDisabledFilePath(Path pluginsRoot) {
+        return pluginsRoot.resolve("disabled.txt");
+    }
+
 }
diff --git a/pf4j/src/main/java/org/pf4j/util/FileUtils.java b/pf4j/src/main/java/org/pf4j/util/FileUtils.java
index 0cb8784..ba9dba1 100644
--- a/pf4j/src/main/java/org/pf4j/util/FileUtils.java
+++ b/pf4j/src/main/java/org/pf4j/util/FileUtils.java
@@ -66,8 +66,16 @@ public class FileUtils {
         return lines;
     }
 
+    /**
+     * Use {@link #writeLines(Collection, Path)} instead.
+     */
+    @Deprecated
     public static void writeLines(Collection<String> lines, File file) throws IOException {
-        Files.write(file.toPath(), lines, StandardCharsets.UTF_8);
+        writeLines(lines, file.toPath());
+    }
+
+    public static void writeLines(Collection<String> lines, Path path) throws IOException {
+        Files.write(path, lines, StandardCharsets.UTF_8);
     }
 
     /**
diff --git a/pf4j/src/test/java/org/pf4j/DefaultPluginStatusProviderTest.java b/pf4j/src/test/java/org/pf4j/DefaultPluginStatusProviderTest.java
index ca5b091..b0a0e8e 100644
--- a/pf4j/src/test/java/org/pf4j/DefaultPluginStatusProviderTest.java
+++ b/pf4j/src/test/java/org/pf4j/DefaultPluginStatusProviderTest.java
@@ -20,6 +20,7 @@ import org.junit.jupiter.api.io.TempDir;
 import org.pf4j.util.FileUtils;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
@@ -74,31 +75,67 @@ public class DefaultPluginStatusProviderTest {
 
     @Test
     public void testDisablePluginWithEnableEmpty() throws Exception {
+        // scenario with "disabled.txt"
         createDisabledFile();
 
-        PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
+        DefaultPluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
         statusProvider.disablePlugin("plugin-1");
 
         assertTrue(statusProvider.isPluginDisabled("plugin-1"));
         assertTrue(statusProvider.isPluginDisabled("plugin-2"));
         assertFalse(statusProvider.isPluginDisabled("plugin-3"));
 
-        List<String> disabledPlugins = FileUtils.readLines(pluginsPath.resolve("disabled.txt"), true);
+        List<String> disabledPlugins = FileUtils.readLines(statusProvider.getDisabledFilePath(), true);
         assertTrue(disabledPlugins.contains("plugin-1"));
+
+        assertTrue(Files.notExists(statusProvider.getEnabledFilePath()));
+
+        // scenario with "enabled.txt"
+        Files.delete(statusProvider.getDisabledFilePath());
+        assertTrue(Files.notExists(statusProvider.getDisabledFilePath()));
+
+        createEnabledFile();
+
+        statusProvider = new DefaultPluginStatusProvider(pluginsPath);
+        statusProvider.disablePlugin("plugin-1");
+
+        assertTrue(statusProvider.isPluginDisabled("plugin-1"));
+        assertFalse(statusProvider.isPluginDisabled("plugin-2"));
+
+        List<String> enabledPlugins = FileUtils.readLines(statusProvider.getEnabledFilePath(), true);
+        assertFalse(enabledPlugins.contains("plugin-1"));
     }
 
     @Test
     public void testEnablePlugin() throws Exception {
+        // scenario with "enabled.txt"
         createEnabledFile();
 
-        PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
+        DefaultPluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
         statusProvider.enablePlugin("plugin-2");
 
         assertFalse(statusProvider.isPluginDisabled("plugin-1"));
         assertFalse(statusProvider.isPluginDisabled("plugin-2"));
         assertTrue(statusProvider.isPluginDisabled("plugin-3"));
 
-        List<String> disabledPlugins = FileUtils.readLines(pluginsPath.resolve("disabled.txt"), true);
+        List<String> enabledPlugins = FileUtils.readLines(statusProvider.getEnabledFilePath(), true);
+        assertTrue(enabledPlugins.contains("plugin-2"));
+
+        assertTrue(Files.notExists(statusProvider.getDisabledFilePath()));
+
+        // scenario with "disabled.txt"
+        Files.delete(statusProvider.getEnabledFilePath());
+        assertTrue(Files.notExists(statusProvider.getEnabledFilePath()));
+
+        createDisabledFile();
+
+        statusProvider = new DefaultPluginStatusProvider(pluginsPath);
+        statusProvider.enablePlugin("plugin-2");
+
+        assertFalse(statusProvider.isPluginDisabled("plugin-1"));
+        assertFalse(statusProvider.isPluginDisabled("plugin-2"));
+
+        List<String> disabledPlugins = FileUtils.readLines(statusProvider.getDisabledFilePath(), true);
         assertFalse(disabledPlugins.contains("plugin-2"));
     }
 
@@ -126,7 +163,7 @@ public class DefaultPluginStatusProviderTest {
         List<String> disabledPlugins = new ArrayList<>();
         disabledPlugins.add("plugin-2");
 
-        FileUtils.writeLines(disabledPlugins, pluginsPath.resolve("disabled.txt").toFile());
+        FileUtils.writeLines(disabledPlugins, DefaultPluginStatusProvider.getDisabledFilePath(pluginsPath));
     }
 
     private void createEnabledFile() throws IOException {
@@ -134,7 +171,7 @@ public class DefaultPluginStatusProviderTest {
         enabledPlugins.add("plugin-1");
         enabledPlugins.add("plugin-2");
 
-        FileUtils.writeLines(enabledPlugins, pluginsPath.resolve("enabled.txt").toFile());
+        FileUtils.writeLines(enabledPlugins, DefaultPluginStatusProvider.getEnabledFilePath(pluginsPath));
     }
 
 }