diff options
3 files changed, 152 insertions, 36 deletions
diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java index beac7c8..c3e8c0f 100644 --- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java @@ -91,7 +91,7 @@ public abstract class AbstractPluginManager implements PluginManager { /** * Cache value for the runtime mode. - * No need to re-read it because it wont change at runtime. + * No need to re-read it because it won't change at runtime. */ protected RuntimeMode runtimeMode; @@ -455,6 +455,9 @@ public abstract class AbstractPluginManager implements PluginManager { } catch (PluginRuntimeException e) { log.error(e.getMessage(), e); } + } else { + // do nothing + log.debug("Plugin '{}' is not started, nothing to stop", getPluginLabel(pluginWrapper.getDescriptor())); } } } @@ -477,18 +480,11 @@ public abstract class AbstractPluginManager implements PluginManager { protected PluginState stopPlugin(String pluginId, boolean stopDependents) { checkPluginId(pluginId); - PluginWrapper pluginWrapper = getPlugin(pluginId); - PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); - PluginState pluginState = pluginWrapper.getPluginState(); - if (PluginState.STOPPED == pluginState) { - log.debug("Already stopped plugin '{}'", getPluginLabel(pluginDescriptor)); - return PluginState.STOPPED; - } - - // test for disabled plugin - if (PluginState.DISABLED == pluginState) { + // test for started plugin + if (!checkPluginState(pluginId, PluginState.STARTED)) { // do nothing - return pluginState; + log.debug("Plugin '{}' is not started, nothing to stop", getPluginLabel(pluginId)); + return getPlugin(pluginId).getPluginState(); } if (stopDependents) { @@ -500,14 +496,15 @@ public abstract class AbstractPluginManager implements PluginManager { } } - log.info("Stop plugin '{}'", getPluginLabel(pluginDescriptor)); + log.info("Stop plugin '{}'", getPluginLabel(pluginId)); + PluginWrapper pluginWrapper = getPlugin(pluginId); pluginWrapper.getPlugin().stop(); pluginWrapper.setPluginState(PluginState.STOPPED); - startedPlugins.remove(pluginWrapper); + getStartedPlugins().remove(pluginWrapper); - firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState)); + firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, PluginState.STOPPED)); - return pluginWrapper.getPluginState(); + return PluginState.STOPPED; } /** @@ -522,6 +519,16 @@ public abstract class AbstractPluginManager implements PluginManager { } } + /** + * Check if the plugin state is equals with the value passed for parameter {@code pluginState}. + * + * @param pluginId the pluginId to check + * @return {@code true} if the plugin state is equals with the value passed for parameter {@code pluginState}, otherwise {@code false} + */ + protected boolean checkPluginState(String pluginId, PluginState pluginState) { + return getPlugin(pluginId).getPluginState() == pluginState; + } + @Override public boolean disablePlugin(String pluginId) { checkPluginId(pluginId); @@ -532,20 +539,21 @@ public abstract class AbstractPluginManager implements PluginManager { if (PluginState.DISABLED == pluginState) { log.debug("Already disabled plugin '{}'", getPluginLabel(pluginDescriptor)); return true; + } else if (PluginState.STARTED == pluginState) { + if (PluginState.STOPPED == stopPlugin(pluginId)) { + log.error("Failed to stop plugin '{}' on disable", getPluginLabel(pluginDescriptor)); + return false; + } } - if (PluginState.STOPPED == stopPlugin(pluginId)) { - pluginWrapper.setPluginState(PluginState.DISABLED); + pluginWrapper.setPluginState(PluginState.DISABLED); - firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, PluginState.STOPPED)); + firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState)); - pluginStatusProvider.disablePlugin(pluginId); - log.info("Disabled plugin '{}'", getPluginLabel(pluginDescriptor)); + pluginStatusProvider.disablePlugin(pluginId); + log.info("Disabled plugin '{}'", getPluginLabel(pluginDescriptor)); - return true; - } - - return false; + return true; } @Override @@ -812,10 +820,9 @@ public abstract class AbstractPluginManager implements PluginManager { */ protected void resolvePlugins() { // retrieves the plugins descriptors - List<PluginDescriptor> descriptors = new ArrayList<>(); - for (PluginWrapper plugin : plugins.values()) { - descriptors.add(plugin.getDescriptor()); - } + List<PluginDescriptor> descriptors = plugins.values().stream() + .map(PluginWrapper::getDescriptor) + .collect(Collectors.toList()); DependencyResolver.Result result = dependencyResolver.resolve(descriptors); @@ -1022,6 +1029,16 @@ public abstract class AbstractPluginManager implements PluginManager { return pluginDescriptor.getPluginId() + "@" + pluginDescriptor.getVersion(); } + /** + * Shortcut for {@code getPluginLabel(getPlugin(pluginId).getDescriptor())}. + * + * @param pluginId the pluginId + * @return the plugin label + */ + protected String getPluginLabel(String pluginId) { + return getPluginLabel(getPlugin(pluginId).getDescriptor()); + } + @SuppressWarnings("unchecked") protected <T> List<Class<? extends T>> getExtensionClasses(List<ExtensionWrapper<T>> extensionsWrapper) { List<Class<? extends T>> extensionClasses = new ArrayList<>(extensionsWrapper.size()); diff --git a/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java index c43bc63..70a9312 100644 --- a/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java +++ b/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java @@ -15,28 +15,54 @@ */ package org.pf4j; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.pf4j.test.TestExtension; import org.pf4j.test.TestExtensionPoint; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; /** * @author Decebal Suiu */ public class AbstractPluginManagerTest { + private AbstractPluginManager pluginManager; + + @BeforeEach + void setUp() { + pluginManager = mock(AbstractPluginManager.class, withSettings() + .useConstructor() + .defaultAnswer(CALLS_REAL_METHODS)); + } + + @AfterEach + void tearDown() { + pluginManager = null; + } + @Test public void getExtensionsByType() { - AbstractPluginManager pluginManager = mock(AbstractPluginManager.class, CALLS_REAL_METHODS); - ExtensionFinder extensionFinder = mock(ExtensionFinder.class); List<ExtensionWrapper<TestExtensionPoint>> extensionList = new ArrayList<>(1); extensionList.add(new ExtensionWrapper<>(new ExtensionDescriptor(0, TestExtension.class), new DefaultExtensionFactory())); @@ -46,11 +72,65 @@ public class AbstractPluginManagerTest { List<TestExtensionPoint> extensions = pluginManager.getExtensions(TestExtensionPoint.class); assertEquals(1, extensions.size()); } - + @Test public void getVersion() { - AbstractPluginManager pluginManager = mock(AbstractPluginManager.class, CALLS_REAL_METHODS); assertNotEquals("0.0.0", pluginManager.getVersion()); } + @Test + void stopStartedPlugin() throws IOException { + String pluginId = "plugin1"; + PluginWrapper pluginWrapper = createPluginWrapper(pluginId); + pluginWrapper.setPluginState(PluginState.STARTED); + + doReturn(pluginWrapper).when(pluginManager).getPlugin(pluginId); + doNothing().when(pluginManager).checkPluginId(pluginId); + doReturn(new ArrayList<>(Arrays.asList(pluginWrapper))).when(pluginManager).getStartedPlugins(); + + PluginState pluginState = pluginManager.stopPlugin(pluginId, false); + verify(pluginWrapper.getPlugin()).stop(); + assertSame(PluginState.STOPPED, pluginState); + } + + @Test + void stopCreatedPlugin() { + String pluginId = "plugin1"; + PluginWrapper pluginWrapper = createPluginWrapper(pluginId); + + doReturn(pluginWrapper).when(pluginManager).getPlugin(pluginId); + doNothing().when(pluginManager).checkPluginId(pluginId); + doReturn(new ArrayList<>(Arrays.asList(pluginWrapper))).when(pluginManager).getStartedPlugins(); + + PluginState pluginState = pluginManager.stopPlugin(pluginId, false); + verify(pluginWrapper.getPlugin(), never()).stop(); + assertSame(PluginState.CREATED, pluginState); + } + + @Test + void checkExistedPluginId() { + String pluginId = "plugin1"; + PluginWrapper pluginWrapper = createPluginWrapper(pluginId); + + pluginManager.plugins.put("plugin1", pluginWrapper); + pluginManager.checkPluginId("plugin1"); + } + + @Test + void checkNotExistedPluginId() { + assertThrows(IllegalArgumentException.class, () -> pluginManager.checkPluginId("plugin1")); + } + + private PluginWrapper createPluginWrapper(String pluginId) { + DefaultPluginDescriptor pluginDescriptor = new DefaultPluginDescriptor(); + pluginDescriptor.setPluginId(pluginId); + pluginDescriptor.setPluginVersion("1.2.3"); + + PluginWrapper pluginWrapper = new PluginWrapper(pluginManager, pluginDescriptor, Paths.get("plugin1"), getClass().getClassLoader()); + Plugin plugin= mock(Plugin.class); + pluginWrapper.setPluginFactory(wrapper -> plugin); + + return pluginWrapper; + } + } diff --git a/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java index fab1a02..3edb1c5 100644 --- a/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java +++ b/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java @@ -28,6 +28,8 @@ import java.nio.file.Path; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -90,7 +92,7 @@ public class DefaultPluginManagerTest { @Test public void isPluginValid() { - // By default accept all since system version not given + // By default, accept all since system version not given assertTrue(pluginManager.isPluginValid(pluginWrapper)); pluginManager.setSystemVersion("1.0.0"); @@ -107,7 +109,7 @@ public class DefaultPluginManagerTest { public void isPluginValidAllowExact() { pluginManager.setExactVersionAllowed(true); - // By default accept all since system version not given + // By default, accept all since system version not given assertTrue(pluginManager.isPluginValid(pluginWrapper)); pluginManager.setSystemVersion("1.0.0"); @@ -127,7 +129,7 @@ public class DefaultPluginManagerTest { /** * Test that a disabled plugin doesn't start. - * See https://github.com/pf4j/pf4j/issues/223. + * See <a href="https://github.com/pf4j/pf4j/issues/223">#223</a>. */ @Test public void testPluginDisabledNoStart() throws IOException { @@ -190,4 +192,21 @@ public class DefaultPluginManagerTest { assertFalse(pluginJar.file().exists()); } + @Test + public void loadedPluginWithMissingDependencyCanBeUnloaded() throws IOException { + PluginZip pluginZip = new PluginZip.Builder(pluginsPath.resolve("my-plugin-1.1.1.zip"), "myPlugin") + .pluginVersion("1.1.1") + .pluginDependencies("myBasePlugin") + .build(); + + pluginManager.loadPlugins(); + pluginManager.startPlugins(); + + PluginWrapper myPlugin = pluginManager.getPlugin(pluginZip.pluginId()); + assertNotNull(myPlugin); + assertNotEquals(PluginState.STARTED, myPlugin.getPluginState()); + + assertTrue(pluginManager.unloadPlugin(pluginZip.pluginId())); + } + } |