From 287e6a3ee23f1da9f00a9c779557954e10d2c61e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jan=20H=C3=B8ydahl?= Date: Tue, 4 Apr 2017 14:46:59 +0200 Subject: [PATCH] Refactor of requires in PluginDescriptor (breaking change) (#138) --- CHANGELOG.md | 9 ++++++ .../fortsoft/pf4j/AbstractPluginManager.java | 8 ++--- .../ro/fortsoft/pf4j/PluginDescriptor.java | 32 ++++++++++--------- .../ManifestPluginDescriptorFinderTest.java | 4 +-- .../PropertiesPluginDescriptorFinderTest.java | 14 ++++---- 5 files changed, 38 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 991f367..9319bc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,19 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### [Unreleased][unreleased] #### Fixed +- [#129]: Properties Descriptor finder bug fixes and a test +- [#131]: Fix bug in loadJars(), did not add `/lib` to classloader +- [#134]: getVersion() use wrong class for calculating PF4J version +- [#135]: deletePlugin() failed to delete plugin folder with contents +- [#137]: The requires Expression does not print well #### Changed +- [#130]: Refactor validation of PluginDescriptors +- [#138]: Refactor of requires in PluginDescriptor (breaking change) #### Added +- [#133]: Support for adding license information to the plugins +- [#136]: Delete plugin zip on uninstall #### Removed diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java index 96f2c15..6014717 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java @@ -697,17 +697,15 @@ public abstract class AbstractPluginManager implements PluginManager { } protected boolean isPluginValid(PluginWrapper pluginWrapper) { - Expression requires = pluginWrapper.getDescriptor().getRequires(); - Version system = getSystemVersion(); - if (requires.interpret(system)) { + if (pluginWrapper.getDescriptor().validFor(getSystemVersion())) { return true; } log.warn("Plugin '{}:{}' requires a minimum system version of {}, and you have {}", pluginWrapper.getPluginId(), pluginWrapper.getDescriptor().getVersion(), - pluginWrapper.getDescriptor().getRequiresString(), - system); + pluginWrapper.getDescriptor().getRequires(), + getSystemVersion()); return false; } diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java index 17f60af..6f15d58 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java @@ -36,14 +36,12 @@ public class PluginDescriptor { private String pluginDescription; private String pluginClass; private Version version; - private Expression requires; - private String requiresString; + private String requires = "*"; private String provider; private List dependencies; private String license; public PluginDescriptor() { - requires = gte("0.0.0"); // Any dependencies = new ArrayList<>(); } @@ -76,17 +74,18 @@ public class PluginDescriptor { } /** - * Returns the requires of this plugin. + * Returns string version of requires + * @return String with requires expression */ - public Expression getRequires() { + public String getRequires() { return requires; } /** - * Returns the requires of this plugin as a string. + * Returns the requires expression of this plugin. */ - public String getRequiresString() { - return requiresString; + public Expression getRequiresExpression() { + return ExpressionParser.newInstance().parse(requires); } /** @@ -116,10 +115,19 @@ public class PluginDescriptor { return "PluginDescriptor [pluginId=" + pluginId + ", pluginClass=" + pluginClass + ", version=" + version + ", provider=" + provider + ", dependencies=" + dependencies + ", description=" - + pluginDescription + ", requires=" + requiresString + ", license=" + + pluginDescription + ", requires=" + requires + ", license=" + 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; } @@ -141,12 +149,6 @@ public class PluginDescriptor { } void setRequires(String requires) { - requiresString = requires; - Parser parser = ExpressionParser.newInstance(); - setRequires(parser.parse(requires)); - } - - void setRequires(Expression requires) { this.requires = requires; } diff --git a/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java b/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java index b76b50e..d86cefd 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinderTest.java @@ -91,7 +91,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.getRequires().interpret(Version.valueOf("1.0.0"))); + assertTrue(plugin1.validFor(Version.valueOf("1.0.0"))); assertEquals("test-plugin-2", plugin2.getPluginId()); assertEquals("", plugin2.getPluginDescription()); @@ -99,7 +99,7 @@ public class ManifestPluginDescriptorFinderTest { assertEquals(Version.valueOf("0.0.1"), plugin2.getVersion()); assertEquals("Decebal Suiu", plugin2.getProvider()); assertEquals(0, plugin2.getDependencies().size()); - assertTrue(plugin2.getRequires().interpret(Version.valueOf("1.0.0"))); + assertTrue(plugin2.validFor(Version.valueOf("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 ddd839b..42c01e0 100644 --- a/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java +++ b/pf4j/src/test/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinderTest.java @@ -28,8 +28,7 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class PropertiesPluginDescriptorFinderTest { @@ -79,8 +78,9 @@ public class PropertiesPluginDescriptorFinderTest { assertEquals("test-plugin-3", plugin1.getDependencies().get(1).getPluginId()); assertEquals("~1.0", plugin1.getDependencies().get(1).getPluginVersionSupport()); assertEquals("Apache-2.0", plugin1.getLicense()); - assertEquals("*", plugin1.getRequiresString()); - assertTrue(plugin1.getRequires().interpret(Version.valueOf("1.0.0"))); + assertEquals(">=1", plugin1.getRequires()); + assertTrue(plugin1.validFor(Version.valueOf("1.0.0"))); + assertFalse(plugin1.validFor(Version.valueOf("0.1.0"))); assertEquals("test-plugin-2", plugin2.getPluginId()); assertEquals("", plugin2.getPluginDescription()); @@ -88,7 +88,8 @@ public class PropertiesPluginDescriptorFinderTest { assertEquals(Version.valueOf("0.0.1"), plugin2.getVersion()); assertEquals("Decebal Suiu", plugin2.getProvider()); assertEquals(0, plugin2.getDependencies().size()); - assertTrue(plugin2.getRequires().interpret(Version.valueOf("1.0.0"))); + assertEquals("*", plugin2.getRequires()); // Default is * + assertTrue(plugin2.validFor(Version.valueOf("1.0.0"))); } @Test(expected = PluginException.class) @@ -105,7 +106,7 @@ public class PropertiesPluginDescriptorFinderTest { + "plugin.provider=Decebal Suiu\n" + "plugin.class=ro.fortsoft.pf4j.plugin.TestPlugin\n" + "plugin.dependencies=test-plugin-2,test-plugin-3@~1.0\n" - + "plugin.requires=*\n" + + "plugin.requires=>=1\n" + "plugin.license=Apache-2.0\n" + "\n" + "" @@ -121,7 +122,6 @@ public class PropertiesPluginDescriptorFinderTest { + "plugin.provider=Decebal Suiu\n" + "plugin.class=ro.fortsoft.pf4j.plugin.TestPlugin\n" + "plugin.dependencies=\n" - + "plugin.requires=*\n" + "\n" + "" }; -- 2.39.5