Fix #223

pf4j/src/main/java/org/pf4j/AbstractPluginManager.java

@@ -130,8 +130,6 @@ public abstract class AbstractPluginManager implements PluginManager {
 
     /**
      * Returns a copy of plugins.
-     *
-     * @return
      */
     @Override
     public List<PluginWrapper> getPlugins() {
@@ -140,9 +138,6 @@ public abstract class AbstractPluginManager implements PluginManager {
 
     /**
      * Returns a copy of plugins with that state.
-     *
-     * @param pluginState
-     * @return
      */
     @Override
     public List<PluginWrapper> getPlugins(PluginState pluginState) {
@@ -786,7 +781,9 @@ public abstract class AbstractPluginManager implements PluginManager {
             PluginWrapper pluginWrapper = plugins.get(pluginId);
             if (unresolvedPlugins.remove(pluginWrapper)) {
                 PluginState pluginState = pluginWrapper.getPluginState();
+                if (pluginState != PluginState.DISABLED) {
                     pluginWrapper.setPluginState(PluginState.RESOLVED);
+                }
 
                 resolvedPlugins.add(pluginWrapper);
                 log.info("Plugin '{}' resolved", getPluginLabel(pluginWrapper.getDescriptor()));
@@ -839,7 +836,7 @@ public abstract class AbstractPluginManager implements PluginManager {
 
         // validate the plugin
         if (!isPluginValid(pluginWrapper)) {
-            log.info("Plugin '{}' is disabled", pluginPath);
+            log.warn("Plugin '{}' is invalid and it will be disabled", pluginPath);
             pluginWrapper.setPluginState(PluginState.DISABLED);
         }
 
@@ -919,9 +916,6 @@ public abstract class AbstractPluginManager implements PluginManager {
 
     /**
      * The plugin label is used in logging and it's a string in format {@code pluginId@pluginVersion}.
-     *
-     * @param pluginDescriptor
-     * @return
      */
     protected String getPluginLabel(PluginDescriptor pluginDescriptor) {
         return pluginDescriptor.getPluginId() + "@" + pluginDescriptor.getVersion();

pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java

@@ -15,71 +15,93 @@
  */
 package org.pf4j;
 
+import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.pf4j.plugin.PluginZip;
 
 import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.Path;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class DefaultPluginManagerTest {
 
-    private DefaultPluginDescriptor pd1 = null;
+    private DefaultPluginManager pluginManager;
-    private DefaultPluginManager pluginManager = new DefaultPluginManager();
+    private DefaultPluginDescriptor pluginDescriptor;
-    private PluginWrapper pw1;
+    private PluginWrapper pluginWrapper;
+    private Path pluginsPath;
+
+    @Rule
+    public TemporaryFolder pluginsFolder = new TemporaryFolder();
 
     @Before
-    public void init() throws IOException {
+    public void setUp() throws IOException {
-        pd1 = new DefaultPluginDescriptor();
+        pluginsPath = pluginsFolder.getRoot().toPath();
-        pd1.setPluginId("myPlugin");
+        pluginManager = new DefaultPluginManager(pluginsPath);
-        pd1.setPluginVersion("1.2.3");
+
-        pd1.setPluginDescription("My plugin");
+        pluginDescriptor = new DefaultPluginDescriptor();
-        pd1.setDependencies("bar, baz");
+        pluginDescriptor.setPluginId("myPlugin");
-        pd1.setProvider("Me");
+        pluginDescriptor.setPluginVersion("1.2.3");
-        pd1.setRequires("5.0.0");
+        pluginDescriptor.setPluginDescription("My plugin");
-
+        pluginDescriptor.setDependencies("bar, baz");
-        pw1 = new PluginWrapper(pluginManager, pd1, Files.createTempDirectory("test"), getClass().getClassLoader());
+        pluginDescriptor.setProvider("Me");
+        pluginDescriptor.setRequires("5.0.0");
+
+        pluginWrapper = new PluginWrapper(pluginManager, pluginDescriptor, Files.createTempDirectory("test"), getClass().getClassLoader());
+    }
+
+    @After
+    public void tearDown() {
+        pluginManager = null;
+        pluginDescriptor = null;
+        pluginWrapper = null;
     }
 
     @Test
     public void validateOK() throws PluginException {
-        pluginManager.validatePluginDescriptor(pd1);
+        pluginManager.validatePluginDescriptor(pluginDescriptor);
     }
 
     @Test(expected = PluginException.class)
     public void validateFailsOnId() throws PluginException {
-        pd1.setPluginId("");
+        pluginDescriptor.setPluginId("");
-        pluginManager.validatePluginDescriptor(pd1);
+        pluginManager.validatePluginDescriptor(pluginDescriptor);
     }
 
     @Test(expected = PluginException.class)
     public void validateFailsOnVersion() throws PluginException {
-        pd1.setPluginVersion(null);
+        pluginDescriptor.setPluginVersion(null);
-        pluginManager.validatePluginDescriptor(pd1);
+        pluginManager.validatePluginDescriptor(pluginDescriptor);
     }
 
     @Test
     public void validateNoPluginClass() throws PluginException {
-        pluginManager.validatePluginDescriptor(pd1);
+        pluginManager.validatePluginDescriptor(pluginDescriptor);
-        assertEquals(Plugin.class.getName(), pd1.getPluginClass());
+        assertEquals(Plugin.class.getName(), pluginDescriptor.getPluginClass());
     }
 
     @Test
     public void isPluginValid() {
         // By default accept all since system version not given
-        assertTrue(pluginManager.isPluginValid(pw1));
+        assertTrue(pluginManager.isPluginValid(pluginWrapper));
 
         pluginManager.setSystemVersion("1.0.0");
-        assertFalse(pluginManager.isPluginValid(pw1));
+        assertFalse(pluginManager.isPluginValid(pluginWrapper));
 
         pluginManager.setSystemVersion("5.0.0");
-        assertTrue(pluginManager.isPluginValid(pw1));
+        assertTrue(pluginManager.isPluginValid(pluginWrapper));
 
         pluginManager.setSystemVersion("6.0.0");
-        assertTrue(pluginManager.isPluginValid(pw1));
+        assertTrue(pluginManager.isPluginValid(pluginWrapper));
     }
 
     @Test
@@ -87,21 +109,52 @@ public class DefaultPluginManagerTest {
         pluginManager.setExactVersionAllowed(true);
 
         // By default accept all since system version not given
-        assertTrue(pluginManager.isPluginValid(pw1));
+        assertTrue(pluginManager.isPluginValid(pluginWrapper));
 
         pluginManager.setSystemVersion("1.0.0");
-        assertFalse(pluginManager.isPluginValid(pw1));
+        assertFalse(pluginManager.isPluginValid(pluginWrapper));
 
         pluginManager.setSystemVersion("5.0.0");
-        assertTrue(pluginManager.isPluginValid(pw1));
+        assertTrue(pluginManager.isPluginValid(pluginWrapper));
 
         pluginManager.setSystemVersion("6.0.0");
-        assertFalse(pluginManager.isPluginValid(pw1));
+        assertFalse(pluginManager.isPluginValid(pluginWrapper));
     }
 
     @Test
     public void testDefaultExactVersionAllowed() {
-        assertEquals(false, pluginManager.isExactVersionAllowed());
+        assertFalse(pluginManager.isExactVersionAllowed());
+    }
+
+    /**
+     * Test that a disabled plugin doesn't start.
+     * See https://github.com/pf4j/pf4j/issues/223.
+     */
+    @Test
+    public void testPluginDisabledNoStart() throws IOException {
+        new PluginZip.Builder(pluginsFolder.newFile("my-plugin-1.2.3.zip"), "myPlugin")
+            .pluginVersion("1.2.3")
+            .build();
+
+        final PluginStatusProvider statusProvider = mock(PluginStatusProvider.class);
+        when(statusProvider.isPluginDisabled("myPlugin")).thenReturn(true);
+
+        PluginManager pluginManager = new DefaultPluginManager(pluginsPath) {
+
+            protected PluginStatusProvider createPluginStatusProvider() {
+                return statusProvider;
+            }
+
+        };
+
+        pluginManager.loadPlugins();
+        pluginManager.startPlugins();
+
+        assertEquals(1, pluginManager.getPlugins().size());
+        assertEquals(0, pluginManager.getStartedPlugins().size());
+
+        PluginWrapper plugin = pluginManager.getPlugin("myPlugin");
+        assertSame(PluginState.DISABLED, plugin.getPluginState());
     }
 
 }