From 880d2e70b1a8a7573679cafbd21c2d9c2574c1c4 Mon Sep 17 00:00:00 2001 From: Decebal Suiu Date: Tue, 21 Mar 2023 09:29:22 +0200 Subject: [PATCH] Fix #520 --- .../java/org/pf4j/AbstractPluginManager.java | 47 ++++++---- .../org/pf4j/AbstractPluginManagerTest.java | 90 +++++++++++++++++-- 2 files changed, 111 insertions(+), 26 deletions(-) diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java index f56a94f..b860cbb 100644 --- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java @@ -271,6 +271,8 @@ public abstract class AbstractPluginManager implements PluginManager { } protected boolean unloadPlugin(String pluginId, boolean unloadDependents) { + checkPluginId(pluginId); + try { if (unloadDependents) { List dependents = dependencyResolver.getDependents(pluginId); @@ -451,18 +453,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", getPluginLabel(pluginId)); + return getPlugin(pluginId).getPluginState(); } if (stopDependents) { @@ -474,22 +469,33 @@ 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; } + /** + * This method throws an {@link IllegalArgumentException} if there is no plugin with this id. + */ protected void checkPluginId(String pluginId) { if (!plugins.containsKey(pluginId)) { throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId)); } } + /** + * This method returns {@code true} if the plugin state is equals with the value passed for parameter {@code pluginState}. + */ + protected boolean checkPluginState(String pluginId, PluginState pluginState) { + return getPlugin(pluginId).getPluginState() == pluginState; + } + @Override public boolean disablePlugin(String pluginId) { checkPluginId(pluginId); @@ -879,15 +885,14 @@ public abstract class AbstractPluginManager implements PluginManager { } /** - * creates the plugin wrapper. override this if you want to prevent plugins having full access to the plugin manager - * - * @return + * Creates the plugin wrapper. override this if you want to prevent plugins having full access to the plugin manager. */ protected PluginWrapper createPluginWrapper(PluginDescriptor pluginDescriptor, Path pluginPath, ClassLoader pluginClassLoader) { // create the plugin wrapper log.debug("Creating wrapper for plugin '{}'", pluginPath); PluginWrapper pluginWrapper = new PluginWrapper(this, pluginDescriptor, pluginPath, pluginClassLoader); pluginWrapper.setPluginFactory(getPluginFactory()); + return pluginWrapper; } @@ -946,8 +951,12 @@ public abstract class AbstractPluginManager implements PluginManager { return versionManager; } + protected String getPluginLabel(String pluginId) { + return getPluginLabel(getPlugin(pluginId).getDescriptor()); + } + /** - * The plugin label is used in logging and it's a string in format {@code pluginId@pluginVersion}. + * The plugin label is used in logging, and it's a string in format {@code pluginId@pluginVersion}. */ protected String getPluginLabel(PluginDescriptor pluginDescriptor) { return pluginDescriptor.getPluginId() + "@" + pluginDescriptor.getVersion(); diff --git a/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java index c43bc63..16821ea 100644 --- a/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java +++ b/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java @@ -15,28 +15,50 @@ */ package org.pf4j; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.pf4j.test.TestExtension; import org.pf4j.test.TestExtensionPoint; +import java.io.IOException; +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 { +class AbstractPluginManagerTest { - @Test - public void getExtensionsByType() { - AbstractPluginManager pluginManager = mock(AbstractPluginManager.class, CALLS_REAL_METHODS); + private AbstractPluginManager pluginManager; + + @BeforeEach + void setUp() { + pluginManager = mock(AbstractPluginManager.class, withSettings().useConstructor().defaultAnswer(CALLS_REAL_METHODS)); + } + @AfterEach + void tearDown() { + pluginManager = null; + } + + @Test + void getExtensionsByType() { ExtensionFinder extensionFinder = mock(ExtensionFinder.class); List> extensionList = new ArrayList<>(1); extensionList.add(new ExtensionWrapper<>(new ExtensionDescriptor(0, TestExtension.class), new DefaultExtensionFactory())); @@ -46,11 +68,65 @@ public class AbstractPluginManagerTest { List extensions = pluginManager.getExtensions(TestExtensionPoint.class); assertEquals(1, extensions.size()); } - + @Test - public void getVersion() { - AbstractPluginManager pluginManager = mock(AbstractPluginManager.class, CALLS_REAL_METHODS); + void getVersion() { 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; + } + } -- 2.39.5