Refactor of requires in PluginDescriptor (breaking change) (#138)

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
 

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;
     }

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<PluginDependency> 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<Expression> parser = ExpressionParser.newInstance();
-        setRequires(parser.parse(requires));
-    }
-
-    void setRequires(Expression requires) {
         this.requires = requires;
     }
 

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")));
     }
 
     /**

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"
             + ""
         };