From cd8e12c0bc7afe2d71855147fbffd54f32f5cc08 Mon Sep 17 00:00:00 2001 From: Dmitry Timofeev Date: Sat, 2 Mar 2019 22:10:05 +0200 Subject: [PATCH] Check no plugin with same pluginId is loaded (#287) --- pf4j/pom.xml | 20 ++++ .../java/org/pf4j/AbstractPluginManager.java | 38 +++++--- .../test/java/org/pf4j/LoadPluginsTest.java | 93 +++++++++++++++++-- pom.xml | 1 + 4 files changed, 130 insertions(+), 22 deletions(-) diff --git a/pf4j/pom.xml b/pf4j/pom.xml index a1be7a4..061412d 100644 --- a/pf4j/pom.xml +++ b/pf4j/pom.xml @@ -60,6 +60,26 @@ ${asm.version} true + + + org.hamcrest + hamcrest + ${hamcrest.version} + test + + + + + org.hamcrest + hamcrest-core + ${hamcrest.version} + test + + junit junit diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java index 22cf7e3..c17fdab 100644 --- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java @@ -15,10 +15,6 @@ */ package org.pf4j; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.pf4j.util.StringUtils; - import java.io.Closeable; import java.io.IOException; import java.nio.file.Files; @@ -31,6 +27,9 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import org.pf4j.util.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class implements the boilerplate plugin code that any {@link PluginManager} @@ -51,17 +50,17 @@ public abstract class AbstractPluginManager implements PluginManager { private PluginDescriptorFinder pluginDescriptorFinder; - /* + /** * A map of plugins this manager is responsible for (the key is the 'pluginId'). */ protected Map plugins; - /* + /** * A map of plugin class loaders (the key is the 'pluginId'). */ private Map pluginClassLoaders; - /* + /** * A list with unresolved plugins (unresolved dependency). */ private List unresolvedPlugins; @@ -71,23 +70,23 @@ public abstract class AbstractPluginManager implements PluginManager { */ private List resolvedPlugins; - /* + /** * A list with started plugins. */ private List startedPlugins; - /* + /** * The registered {@link PluginStateListener}s. */ private List pluginStateListeners; - /* + /** * Cache value for the runtime mode. * No need to re-read it because it wont change at runtime. */ private RuntimeMode runtimeMode; - /* + /** * The system version used for comparisons to the plugin requires attribute. */ private String systemVersion = "0.0.0"; @@ -822,18 +821,31 @@ public abstract class AbstractPluginManager implements PluginManager { } protected PluginWrapper loadPluginFromPath(Path pluginPath) throws PluginException { - // test for plugin duplication + // Test for plugin path duplication String pluginId = idForPath(pluginPath); if (pluginId != null) { throw new PluginAlreadyLoadedException(pluginId, pluginPath); } - // retrieves the plugin descriptor + // Retrieve and validate the plugin descriptor PluginDescriptorFinder pluginDescriptorFinder = getPluginDescriptorFinder(); log.debug("Use '{}' to find plugins descriptors", pluginDescriptorFinder); log.debug("Finding plugin descriptor for plugin '{}'", pluginPath); PluginDescriptor pluginDescriptor = pluginDescriptorFinder.find(pluginPath); validatePluginDescriptor(pluginDescriptor); + + // Check there are no loaded plugins with the retrieved id + pluginId = pluginDescriptor.getPluginId(); + if (plugins.containsKey(pluginId)) { + PluginWrapper loadedPlugin = getPlugin(pluginId); + throw new PluginException("There is an already loaded plugin ({}) " + + "with the same id ({}) as the plugin at path '{}'. Simultaneous loading " + + "of plugins with the same PluginId is not currently supported.\n" + + "As a workaround you may include PluginVersion and PluginProvider " + + "in PluginId.", + loadedPlugin, pluginId, pluginPath); + } + log.debug("Found descriptor {}", pluginDescriptor); String pluginClassName = pluginDescriptor.getPluginClass(); log.debug("Class '{}' for plugin '{}'", pluginClassName, pluginPath); diff --git a/pf4j/src/test/java/org/pf4j/LoadPluginsTest.java b/pf4j/src/test/java/org/pf4j/LoadPluginsTest.java index 8b6af11..c2917e8 100644 --- a/pf4j/src/test/java/org/pf4j/LoadPluginsTest.java +++ b/pf4j/src/test/java/org/pf4j/LoadPluginsTest.java @@ -15,20 +15,26 @@ */ package org.pf4j; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.pf4j.plugin.PluginZip; - -import java.nio.file.Files; -import java.nio.file.Paths; - +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.pf4j.plugin.PluginZip; public class LoadPluginsTest { @@ -73,6 +79,75 @@ public class LoadPluginsTest { assertNull(pluginManager.loadPluginFromPath(pluginZip.path())); } + @Test + public void loadPluginWithSameIdDifferentPathFails() throws Exception { + String pluginId = "myPlugin"; + String pluginVersion = "1.2.3"; + File plugin1Path = pluginsFolder.newFile("my-plugin-1.2.3.zip"); + PluginZip plugin1 = new PluginZip.Builder(plugin1Path, pluginId) + .pluginVersion(pluginVersion) + .build(); + + File plugin2Path = pluginsFolder.newFile("my-plugin-1.2.3-renamed.zip"); + PluginZip plugin2 = new PluginZip.Builder(plugin2Path, pluginId) + .pluginVersion(pluginVersion) + .build(); + + // Verify the first plugin with the given id is loaded + assertNotNull(pluginManager.loadPluginFromPath(plugin1.path())); + Path loadedPlugin1Path = pluginManager.getPlugin(pluginId).getPluginPath(); + + try { + // Verify the second plugin is not loaded as it has the same metadata + pluginManager.loadPluginFromPath(plugin2.path()); + fail("Expected loadPluginFromPath to fail"); + } catch (PluginException e) { + // Check the path of the loaded plugin remains the same + PluginWrapper loadedPlugin = pluginManager.getPlugin(pluginId); + assertThat(loadedPlugin.getPluginPath(), equalTo(loadedPlugin1Path)); + // Check the message includes relevant information + String message = e.getMessage(); + assertThat(message, startsWith("There is an already loaded plugin")); + assertThat(message, containsString(pluginId)); + assertThat(message, containsString("my-plugin-1.2.3-renamed")); + } + } + + /** + * This test verifies the behaviour as of PF4J 2.x, where plugins of different + * versions but with the pluginId cannot be loaded correctly because the API + * uses pluginId as the unique identifier of the loaded plugin. + */ + @Test + public void loadPluginWithSameIdDifferentVersionsFails() throws Exception { + String pluginId = "myPlugin"; + String plugin1Version = "1.2.3"; + File plugin1Path = pluginsFolder.newFile("my-plugin-1.2.3.zip"); + PluginZip plugin1 = new PluginZip.Builder(plugin1Path, pluginId) + .pluginVersion(plugin1Version) + .build(); + + String plugin2Version = "2.0.0"; + File plugin2Path = pluginsFolder.newFile("my-plugin-2.0.0.zip"); + PluginZip plugin2 = new PluginZip.Builder(plugin2Path, pluginId) + .pluginVersion(plugin2Version) + .build(); + + // Verify the first plugin with the given id is loaded + assertNotNull(pluginManager.loadPluginFromPath(plugin1.path())); + Path loadedPlugin1Path = pluginManager.getPlugin(pluginId).getPluginPath(); + try { + // Verify the second plugin is not loaded as it has the same pluginId + pluginManager.loadPluginFromPath(plugin2.path()); + fail("Expected loadPluginFromPath to fail"); + } catch (PluginException e) { + // Check the path and version of the loaded plugin remain the same + PluginWrapper loadedPlugin = pluginManager.getPlugin(pluginId); + assertThat(loadedPlugin.getPluginPath(), equalTo(loadedPlugin1Path)); + assertThat(loadedPlugin.getDescriptor().getVersion(), equalTo(plugin1Version)); + } + } + @Test public void loadUnloadLoad() throws Exception { PluginZip pluginZip = new PluginZip.Builder(pluginsFolder.newFile("my-plugin-1.2.3.zip"), "myPlugin") diff --git a/pom.xml b/pom.xml index 6de3eef..dd40519 100644 --- a/pom.xml +++ b/pom.xml @@ -48,6 +48,7 @@ 7.0 4.12 + 2.1 2.24.0 2.7 3.1.0 -- 2.39.5