From d20ed779decb74ce55942358da52652160d3bbc0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jan=20H=C3=B8ydahl?= Date: Thu, 20 Apr 2017 08:00:48 +0200 Subject: [PATCH] Refactor requires validation, Fixes #142 (#144) --- README.md | 8 ++- .../fortsoft/pf4j/AbstractPluginManager.java | 29 ++++++++++- .../ro/fortsoft/pf4j/PluginDescriptor.java | 22 +-------- .../pf4j/DefaultPluginManagerTest.java | 49 ++++++++++++++++++- .../ManifestPluginDescriptorFinderTest.java | 5 +- .../PropertiesPluginDescriptorFinderTest.java | 7 +-- .../ro/fortsoft/pf4j/util/SemVerUtils.java | 28 +++++++++++ 7 files changed, 119 insertions(+), 29 deletions(-) create mode 100644 pf4j/src/test/java/ro/fortsoft/pf4j/util/SemVerUtils.java diff --git a/README.md b/README.md index fa4f8de..fcd3148 100644 --- a/README.md +++ b/README.md @@ -256,10 +256,14 @@ a requirement to specify a minimum system version for loading plugins. Loading & starting a newer plugin on an older system could result in runtime failures due to method signature changes or other class differences. + For this reason was added a manifest attribute (in PluginDescriptor) to specify a 'requires' version -which is a minimum system version. Also DefaultPluginManager contains a method to +which is a minimum system version on x.y.z format, or a +[SemVer Expression](https://github.com/zafarkhaja/jsemver#semver-expressions-api-ranges). +Also DefaultPluginManager contains a method to specify the system version of the plugin manager and the logic to disable -plugins on load if the system version is too old (if you want total control, please override `isPluginValid()`). This works for both `loadPlugins()` and `loadPlugin()`. +plugins on load if the system version is too old (if you want total control, +please override `isPluginValid()`). This works for both `loadPlugins()` and `loadPlugin()`. __PluginStateListener__ defines the interface for an object that listens to plugin state changes. You can use `addPluginStateListener()` and `removePluginStateListener()` from PluginManager if you want to add or remove a plugin state listener. diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java index 07d1043..4bd25fc 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java @@ -91,6 +91,7 @@ public abstract class AbstractPluginManager implements PluginManager { private PluginStatusProvider pluginStatusProvider; private DependencyResolver dependencyResolver; private PluginLoader pluginLoader; + private boolean exactVersionAllowed = false; /** * The plugins root is supplied by {@code System.getProperty("pf4j.pluginsDir", "plugins")}. @@ -688,8 +689,18 @@ public abstract class AbstractPluginManager implements PluginManager { return Paths.get(pluginsDir); } + /** + * Check if this plugin is valid (satisfies "requires" param) for a given system version + * @param pluginWrapper the plugin to check + * @return true if plugin satisfies the "requires" or if requires was left blank + */ protected boolean isPluginValid(PluginWrapper pluginWrapper) { - if (pluginWrapper.getDescriptor().validFor(getSystemVersion())) { + String requires = pluginWrapper.getDescriptor().getRequires().trim(); + if (!isExactVersionAllowed() && requires.matches("^\\d+\\.\\d+\\.\\d+$")) { + // If exact versions are not allowed in requires, rewrite to >= expression + requires = ">=" + requires; + } + if (systemVersion.equals(Version.forIntegers(0,0,0)) || systemVersion.satisfies(requires)) { return true; } @@ -814,4 +825,20 @@ public abstract class AbstractPluginManager implements PluginManager { return RuntimeMode.DEVELOPMENT.equals(getRuntimeMode()); } + /** + * @return true if exact versions in requires is allowed + */ + public boolean isExactVersionAllowed() { + return exactVersionAllowed; + } + + /** + * Set to true to allow requires expression to be exactly x.y.z. + * The default is false, meaning that using an exact version x.y.z will + * implicitly mean the same as >=x.y.z + * @param exactVersionAllowed set to true or false + */ + public void setExactVersionAllowed(boolean exactVersionAllowed) { + this.exactVersionAllowed = exactVersionAllowed; + } } diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java index 046adcd..65a1d43 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java @@ -16,8 +16,6 @@ package ro.fortsoft.pf4j; import com.github.zafarkhaja.semver.Version; -import com.github.zafarkhaja.semver.expr.Expression; -import com.github.zafarkhaja.semver.expr.ExpressionParser; import java.util.ArrayList; import java.util.Collections; @@ -35,7 +33,7 @@ public class PluginDescriptor { private String pluginDescription; private String pluginClass; private Version version; - private String requires = "*"; + private String requires = "*"; // SemVer format private String provider; private List dependencies; private String license; @@ -74,19 +72,12 @@ public class PluginDescriptor { /** * Returns string version of requires - * @return String with requires expression + * @return String with requires expression on SemVer format */ public String getRequires() { return requires; } - /** - * Returns the requires expression of this plugin. - */ - public Expression getRequiresExpression() { - return ExpressionParser.newInstance().parse(requires); - } - /** * Returns the provider name of this plugin. */ @@ -118,15 +109,6 @@ public class PluginDescriptor { + license + "]"; } - /** - * Check if this plugin is valid (satisfies "requires" param) for a given system version - * @param systemVersion the system (host) version to test - * @return true if plugin satisfies the "requires" or if requires is left blank - */ - public boolean validFor(Version systemVersion) { - return systemVersion.satisfies(getRequiresExpression()); - } - void setPluginId(String pluginId) { this.pluginId = pluginId; } diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java index cde124b..5f235be 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/DefaultPluginManagerTest.java @@ -19,12 +19,20 @@ import com.github.zafarkhaja.semver.Version; import org.junit.Before; import org.junit.Test; +import java.io.IOException; +import java.nio.file.Files; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + public class DefaultPluginManagerTest { private PluginDescriptor pd1 = null; private DefaultPluginManager pluginManager = new DefaultPluginManager(); + private PluginWrapper pw1; @Before - public void init() { + public void init() throws IOException { pd1 = new PluginDescriptor(); pd1.setPluginId("myPlugin"); pd1.setPluginVersion(Version.valueOf("1.2.3")); @@ -33,6 +41,8 @@ public class DefaultPluginManagerTest { pd1.setDependencies("bar, baz"); pd1.setProvider("Me"); pd1.setRequires("5.0.0"); + + pw1 = new PluginWrapper(pluginManager, pd1, Files.createTempDirectory("test"), getClass().getClassLoader()); } @Test @@ -57,4 +67,41 @@ public class DefaultPluginManagerTest { pd1.setPluginClass(null); pluginManager.validatePluginDescriptor(pd1); } + + @Test + public void isPluginValid() throws Exception { + // By default accept all since system version not given + assertTrue(pluginManager.isPluginValid(pw1)); + + pluginManager.setSystemVersion(Version.valueOf("1.0.0")); + assertFalse(pluginManager.isPluginValid(pw1)); + + pluginManager.setSystemVersion(Version.valueOf("5.0.0")); + assertTrue(pluginManager.isPluginValid(pw1)); + + pluginManager.setSystemVersion(Version.valueOf("6.0.0")); + assertTrue(pluginManager.isPluginValid(pw1)); + } + + @Test + public void isPluginValidAllowExact() throws Exception { + pluginManager.setExactVersionAllowed(true); + + // By default accept all since system version not given + assertTrue(pluginManager.isPluginValid(pw1)); + + pluginManager.setSystemVersion(Version.valueOf("1.0.0")); + assertFalse(pluginManager.isPluginValid(pw1)); + + pluginManager.setSystemVersion(Version.valueOf("5.0.0")); + assertTrue(pluginManager.isPluginValid(pw1)); + + pluginManager.setSystemVersion(Version.valueOf("6.0.0")); + assertFalse(pluginManager.isPluginValid(pw1)); + } + + @Test + public void testDefaultExactVersionAllowed() throws Exception { + assertEquals(false, pluginManager.isExactVersionAllowed()); + } } diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java index d86cefd..1d1b621 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java @@ -30,6 +30,7 @@ import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static ro.fortsoft.pf4j.util.SemVerUtils.versionMatches; /** * @author Mario Franco @@ -91,7 +92,7 @@ public class ManifestPluginDescriptorFinderTest { assertEquals("test-plugin-3", plugin1.getDependencies().get(1).getPluginId()); assertEquals("~1.0", plugin1.getDependencies().get(1).getPluginVersionSupport()); assertEquals("Apache-2.0", plugin1.getLicense()); - assertTrue(plugin1.validFor(Version.valueOf("1.0.0"))); + assertTrue(versionMatches(plugin1.getRequires(), "1.0.0")); assertEquals("test-plugin-2", plugin2.getPluginId()); assertEquals("", plugin2.getPluginDescription()); @@ -99,7 +100,7 @@ public class ManifestPluginDescriptorFinderTest { assertEquals(Version.valueOf("0.0.1"), plugin2.getVersion()); assertEquals("Decebal Suiu", plugin2.getProvider()); assertEquals(0, plugin2.getDependencies().size()); - assertTrue(plugin2.validFor(Version.valueOf("1.0.0"))); + assertTrue(versionMatches(plugin2.getRequires(),"1.0.0")); } /** diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java index 42c01e0..fa6666a 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java @@ -29,6 +29,7 @@ import java.util.Arrays; import java.util.List; import static org.junit.Assert.*; +import static ro.fortsoft.pf4j.util.SemVerUtils.versionMatches; public class PropertiesPluginDescriptorFinderTest { @@ -79,8 +80,8 @@ public class PropertiesPluginDescriptorFinderTest { assertEquals("~1.0", plugin1.getDependencies().get(1).getPluginVersionSupport()); assertEquals("Apache-2.0", plugin1.getLicense()); assertEquals(">=1", plugin1.getRequires()); - assertTrue(plugin1.validFor(Version.valueOf("1.0.0"))); - assertFalse(plugin1.validFor(Version.valueOf("0.1.0"))); + assertTrue(versionMatches(plugin1.getRequires(),"1.0.0")); + assertFalse(versionMatches(plugin1.getRequires(), "0.1.0")); assertEquals("test-plugin-2", plugin2.getPluginId()); assertEquals("", plugin2.getPluginDescription()); @@ -89,7 +90,7 @@ public class PropertiesPluginDescriptorFinderTest { assertEquals("Decebal Suiu", plugin2.getProvider()); assertEquals(0, plugin2.getDependencies().size()); assertEquals("*", plugin2.getRequires()); // Default is * - assertTrue(plugin2.validFor(Version.valueOf("1.0.0"))); + assertTrue(versionMatches(plugin2.getRequires(),"1.0.0")); } @Test(expected = PluginException.class) diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/util/SemVerUtils.java b/pf4j/src/test/java/ro/fortsoft/pf4j/util/SemVerUtils.java new file mode 100644 index 0000000..b5c94d1 --- /dev/null +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/util/SemVerUtils.java @@ -0,0 +1,28 @@ +/* + * Copyright 2017 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.util; + +import com.github.zafarkhaja.semver.Version; +import com.github.zafarkhaja.semver.expr.ExpressionParser; + +/** + * Utility for semantic version testing + */ +public class SemVerUtils { + public static boolean versionMatches(String expression, String systemVersion) { + return ExpressionParser.newInstance().parse(expression).interpret(Version.valueOf(systemVersion)); + } +} -- 2.39.5