diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java index 4336df9..17116ae 100644 --- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java +++ b/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 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 getPlugins(PluginState pluginState) { @@ -757,7 +752,7 @@ public abstract class AbstractPluginManager implements PluginManager { } protected void resolvePlugins() throws PluginException { - // retrieves the plugins descriptors + // retrieves the plugins descriptors List descriptors = new ArrayList<>(); for (PluginWrapper plugin : plugins.values()) { descriptors.add(plugin.getDescriptor()); @@ -786,7 +781,9 @@ public abstract class AbstractPluginManager implements PluginManager { PluginWrapper pluginWrapper = plugins.get(pluginId); if (unresolvedPlugins.remove(pluginWrapper)) { PluginState pluginState = pluginWrapper.getPluginState(); - pluginWrapper.setPluginState(PluginState.RESOLVED); + 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(); diff --git a/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java index 163a28c..3e6fd04 100644 --- a/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java +++ b/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 = new DefaultPluginManager(); - private PluginWrapper pw1; + private DefaultPluginManager pluginManager; + private DefaultPluginDescriptor pluginDescriptor; + private PluginWrapper pluginWrapper; + private Path pluginsPath; + + @Rule + public TemporaryFolder pluginsFolder = new TemporaryFolder(); @Before - public void init() throws IOException { - pd1 = new DefaultPluginDescriptor(); - pd1.setPluginId("myPlugin"); - pd1.setPluginVersion("1.2.3"); - pd1.setPluginDescription("My plugin"); - pd1.setDependencies("bar, baz"); - pd1.setProvider("Me"); - pd1.setRequires("5.0.0"); - - pw1 = new PluginWrapper(pluginManager, pd1, Files.createTempDirectory("test"), getClass().getClassLoader()); + public void setUp() throws IOException { + pluginsPath = pluginsFolder.getRoot().toPath(); + pluginManager = new DefaultPluginManager(pluginsPath); + + pluginDescriptor = new DefaultPluginDescriptor(); + pluginDescriptor.setPluginId("myPlugin"); + pluginDescriptor.setPluginVersion("1.2.3"); + pluginDescriptor.setPluginDescription("My plugin"); + pluginDescriptor.setDependencies("bar, baz"); + 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(""); - pluginManager.validatePluginDescriptor(pd1); + pluginDescriptor.setPluginId(""); + pluginManager.validatePluginDescriptor(pluginDescriptor); } @Test(expected = PluginException.class) public void validateFailsOnVersion() throws PluginException { - pd1.setPluginVersion(null); - pluginManager.validatePluginDescriptor(pd1); + pluginDescriptor.setPluginVersion(null); + pluginManager.validatePluginDescriptor(pluginDescriptor); } @Test public void validateNoPluginClass() throws PluginException { - pluginManager.validatePluginDescriptor(pd1); - assertEquals(Plugin.class.getName(), pd1.getPluginClass()); + pluginManager.validatePluginDescriptor(pluginDescriptor); + 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()); } }