From 9f284467b3660298de7786be448f27577769b0b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Sat, 1 Apr 2017 07:39:43 +0200 Subject: [PATCH] Refactor validation of PluginDescriptors (#130) --- .../fortsoft/pf4j/AbstractPluginManager.java | 27 ++++++--- .../pf4j/ManifestPluginDescriptorFinder.java | 18 +----- .../PropertiesPluginDescriptorFinder.java | 20 +------ .../pf4j/DefaultPluginManagerTest.java | 60 +++++++++++++++++++ .../ManifestPluginDescriptorFinderTest.java | 27 --------- .../PropertiesPluginDescriptorFinderTest.java | 18 ------ 6 files changed, 83 insertions(+), 87 deletions(-) create mode 100644 pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java index 6087ec4..bd7c921 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java @@ -19,19 +19,14 @@ import com.github.zafarkhaja.semver.Version; import com.github.zafarkhaja.semver.expr.Expression; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import ro.fortsoft.pf4j.util.StringUtils; import java.io.Closeable; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; /** * This class implements the boilerplate plugin code that any {@link PluginManager} @@ -751,6 +746,7 @@ public abstract class AbstractPluginManager implements PluginManager { // retrieves the plugin descriptor log.debug("Find plugin descriptor '{}'", pluginPath); PluginDescriptor pluginDescriptor = getPluginDescriptorFinder().find(pluginPath); + validatePluginDescriptor(pluginDescriptor); log.debug("Descriptor {}", pluginDescriptor); String pluginClassName = pluginDescriptor.getPluginClass(); log.debug("Class '{}' for plugin '{}'", pluginClassName, pluginPath); @@ -792,6 +788,23 @@ public abstract class AbstractPluginManager implements PluginManager { return pluginWrapper; } + /** + * Override this to change the validation criteria + * @param descriptor the plugin descriptor to validate + * @throws PluginException if validation fails + */ + protected void validatePluginDescriptor(PluginDescriptor descriptor) throws PluginException { + if (StringUtils.isEmpty(descriptor.getPluginId())) { + throw new PluginException("id cannot be empty"); + } + if (StringUtils.isEmpty(descriptor.getPluginClass())) { + throw new PluginException("class cannot be empty"); + } + if (descriptor.getVersion() == null) { + throw new PluginException("version cannot be empty"); + } + } + // TODO add this method in PluginManager as default method for Java 8. protected boolean isDevelopment() { return RuntimeMode.DEVELOPMENT.equals(getRuntimeMode()); diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java b/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java index f9ef927..de0f84e 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java @@ -33,10 +33,7 @@ public abstract class ManifestPluginDescriptorFinder implements PluginDescriptor public PluginDescriptor find(Path pluginPath) throws PluginException { Manifest manifest = readManifest(pluginPath); - PluginDescriptor pluginDescriptor = createPluginDescriptor(manifest); - validatePluginDescriptor(pluginDescriptor); - - return pluginDescriptor; + return createPluginDescriptor(manifest); } public abstract Manifest readManifest(Path pluginPath) throws PluginException; @@ -82,17 +79,4 @@ public abstract class ManifestPluginDescriptorFinder implements PluginDescriptor protected PluginDescriptor createPluginDescriptorInstance() { return new PluginDescriptor(); } - - protected void validatePluginDescriptor(PluginDescriptor pluginDescriptor) throws PluginException { - if (StringUtils.isEmpty(pluginDescriptor.getPluginId())) { - throw new PluginException("Plugin-Id cannot be empty"); - } - if (StringUtils.isEmpty(pluginDescriptor.getPluginClass())) { - throw new PluginException("Plugin-Class cannot be empty"); - } - if (pluginDescriptor.getVersion() == null) { - throw new PluginException("Plugin-Version cannot be empty"); - } - } - } diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java b/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java index ee47562..8af4269 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java @@ -45,17 +45,14 @@ public class PropertiesPluginDescriptorFinder implements PluginDescriptorFinder } public PropertiesPluginDescriptorFinder(String propertiesFileName) { - this.propertiesFileName = propertiesFileName; + this.propertiesFileName = propertiesFileName; } @Override public PluginDescriptor find(Path pluginPath) throws PluginException { Properties properties = readProperties(pluginPath); - PluginDescriptor pluginDescriptor = createPluginDescriptor(properties); - validatePluginDescriptor(pluginDescriptor); - - return pluginDescriptor; + return createPluginDescriptor(properties); } protected Properties readProperties(Path pluginPath) throws PluginException { @@ -116,17 +113,4 @@ public class PropertiesPluginDescriptorFinder implements PluginDescriptorFinder protected PluginDescriptor createPluginDescriptorInstance() { return new PluginDescriptor(); } - - protected void validatePluginDescriptor(PluginDescriptor pluginDescriptor) throws PluginException { - if (StringUtils.isEmpty(pluginDescriptor.getPluginId())) { - throw new PluginException("plugin.id cannot be empty"); - } - if (StringUtils.isEmpty(pluginDescriptor.getPluginClass())) { - throw new PluginException("plugin.class cannot be empty"); - } - if (pluginDescriptor.getVersion() == null) { - throw new PluginException("plugin.version cannot be empty"); - } - } - } diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java new file mode 100644 index 0000000..cde124b --- /dev/null +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2015 Decebal Suiu + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ro.fortsoft.pf4j; + +import com.github.zafarkhaja.semver.Version; +import org.junit.Before; +import org.junit.Test; + +public class DefaultPluginManagerTest { + private PluginDescriptor pd1 = null; + private DefaultPluginManager pluginManager = new DefaultPluginManager(); + + @Before + public void init() { + pd1 = new PluginDescriptor(); + pd1.setPluginId("myPlugin"); + pd1.setPluginVersion(Version.valueOf("1.2.3")); + pd1.setPluginClass("foo"); + pd1.setPluginDescription("My plugin"); + pd1.setDependencies("bar, baz"); + pd1.setProvider("Me"); + pd1.setRequires("5.0.0"); + } + + @Test + public void validateOK() throws PluginException { + pluginManager.validatePluginDescriptor(pd1); + } + + @Test(expected = PluginException.class) + public void validateFailsOnId() throws PluginException { + pd1.setPluginId(""); + pluginManager.validatePluginDescriptor(pd1); + } + + @Test(expected = PluginException.class) + public void validateFailsOnVersion() throws PluginException { + pd1.setPluginVersion(null); + pluginManager.validatePluginDescriptor(pd1); + } + + @Test(expected = PluginException.class) + public void validateFailsOnClass() throws PluginException { + pd1.setPluginClass(null); + pluginManager.validatePluginDescriptor(pd1); + } +} diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java index 1ccaa22..b76b50e 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java @@ -111,33 +111,6 @@ public class ManifestPluginDescriptorFinderTest { instance.find(getPluginsRoot().resolve("test-plugin-3")); } - /** - * Test of {@link DefaultPluginDescriptorFinder#find(Path)} method. - */ - @Test(expected = PluginException.class) - public void testFindMissingPluginClass() throws Exception { - PluginDescriptorFinder instance = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath()); - instance.find(getPluginsRoot().resolve("test-plugin-4")); - } - - /** - * Test of {@link DefaultPluginDescriptorFinder#find(Path)} method. - */ - @Test(expected = PluginException.class) - public void testFindMissingPluginVersion() throws Exception { - PluginDescriptorFinder instance = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath()); - instance.find(getPluginsRoot().resolve("test-plugin-5")); - } - - /** - * Test of {@link DefaultPluginDescriptorFinder#find(Path)} method. - */ - @Test(expected = PluginException.class) - public void testFindMissingPluginId() throws Exception { - PluginDescriptorFinder instance = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath()); - instance.find(getPluginsRoot().resolve("test-plugin-6")); - } - private List getPlugin1Manifest() { String[] lines = new String[] { "Manifest-Version: 1.0\n" diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java index 9afb636..ca62770 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java @@ -96,24 +96,6 @@ public class PropertiesPluginDescriptorFinderTest { instance.find(getPluginsRoot().resolve("test-plugin-3")); } - @Test(expected = PluginException.class) - public void testFindMissingPluginClass() throws Exception { - PluginDescriptorFinder instance = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath()); - instance.find(getPluginsRoot().resolve("test-plugin-4")); - } - - @Test(expected = PluginException.class) - public void testFindMissingPluginVersion() throws Exception { - PluginDescriptorFinder instance = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath()); - instance.find(getPluginsRoot().resolve("test-plugin-5")); - } - - @Test(expected = PluginException.class) - public void testFindMissingPluginId() throws Exception { - PluginDescriptorFinder instance = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath()); - instance.find(getPluginsRoot().resolve("test-plugin-6")); - } - private List getPlugin1Properties() { String[] lines = new String[] { "plugin.id=test-plugin-1\n"