Refactor requires validation, Fixes #142 (#144)

7 files changed, 119 insertions, 29 deletions

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<PluginDependency> dependencies;
     private String license;
@@ -74,20 +72,13 @@ 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.
      */
     public String getProvider() {
@@ -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));
+    }
+}