Move requires logic to isPluginValid(PluginWrapper), rename default version to ZERO

3 files changed, 35 insertions, 26 deletions

diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java
index 970ae2d..286b6ea 100644
--- a/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java
+++ b/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java
@@ -92,7 +92,7 @@ public class DefaultPluginManager implements PluginManager {
     /**
      * The system version used for comparisons to the plugin requires attribute.
      */
-    private PluginVersion systemVersion = PluginVersion.DEFAULT;
+    private PluginVersion systemVersion = PluginVersion.ZERO;
 
     /**
      * The plugins directory is supplied by System.getProperty("pf4j.pluginsDir", "plugins").
@@ -469,23 +469,20 @@ public class DefaultPluginManager implements PluginManager {
         }
 
         PluginWrapper pluginWrapper = getPlugin(pluginId);
+       	if (!isPluginValid(pluginWrapper)) {
+        	log.warn("Plugin '{}:{}' can not be enabled",
+        			pluginWrapper.getPluginId(),
+        			pluginWrapper.getDescriptor().getVersion());
+            return false;
+        }
+
         PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor();
         PluginState pluginState = pluginWrapper.getPluginState();
         if (PluginState.DISABLED != pluginState) {
-            log.debug("Plugin plugin '{}:{}' is not disabled", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion());
+            log.debug("Plugin '{}:{}' is not disabled", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion());
             return true;
         }
 
-        PluginVersion requires = pluginWrapper.getDescriptor().getRequires();
-       	PluginVersion system = getSystemVersion();
-       	if (!system.isDefault() && !system.atLeast(requires)) {
-       		log.warn(String.format("Failed to enable plugin `{}:{}` because it requires a minimum system version of %s",
-       				pluginWrapper.getPluginId(),
-       				pluginWrapper.getDescriptor().getVersion(),
-       				requires));
-            return false;
-        }
-
         try {
             if (disabledPlugins.remove(pluginId)) {
                 FileUtils.writeLines(disabledPlugins, new File(pluginsDirectory, "disabled.txt"));
@@ -659,6 +656,20 @@ public class DefaultPluginManager implements PluginManager {
     	return !enabledPlugins.contains(pluginId);
     }
 
+    protected boolean isPluginValid(PluginWrapper pluginWrapper) {
+    	PluginVersion requires = pluginWrapper.getDescriptor().getRequires();
+    	PluginVersion system = getSystemVersion();
+    	if (system.isZero() || system.atLeast(requires)) {
+    		return true;
+    	}
+
+    	log.warn(String.format("Plugin '%s:%s' requires a minimum system version of %s",
+    			pluginWrapper.getPluginId(),
+    			pluginWrapper.getDescriptor().getVersion(),
+    			requires));
+    	return false;
+    }
+
     protected FileFilter createHiddenPluginFilter() {
     	return new HiddenFilter();
     }
@@ -749,16 +760,10 @@ public class DefaultPluginManager implements PluginManager {
             pluginWrapper.setPluginState(PluginState.DISABLED);
         }
 
-        // optionally enforce minimum system version requirements
-        PluginVersion requires = pluginWrapper.getDescriptor().getRequires();
-       	PluginVersion system = getSystemVersion();
-       	if (!system.isDefault() && !system.atLeast(requires)) {
-       		log.warn(String.format("Disabling plugin '%s:%s' because it requires a minimum system version of %s",
-       				pluginWrapper.getPluginId(),
-       				pluginWrapper.getDescriptor().getVersion(),
-       				requires));
-            pluginWrapper.setPluginState(PluginState.DISABLED);
-            disabledPlugins.add(pluginWrapper.getPluginId());
+        // validate the plugin
+        if (!isPluginValid(pluginWrapper)) {
+        	log.info("Plugin '{}' is disabled", pluginPath);
+        	pluginWrapper.setPluginState(PluginState.DISABLED);
         }
 
         log.debug("Created wrapper '{}' for plugin '{}'", pluginWrapper, pluginPath);
diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java
index aa05b52..ae07fbb 100644
--- a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java
+++ b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginDescriptor.java
@@ -33,7 +33,7 @@ public class PluginDescriptor {
     private List<PluginDependency> dependencies;
 
     public PluginDescriptor() {
-    	requires = PluginVersion.DEFAULT;
+    	requires = PluginVersion.ZERO;
         dependencies = new ArrayList<PluginDependency>();
     }
 
diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginVersion.java b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginVersion.java
index 7cfc94b..c5d57fd 100644
--- a/pf4j/src/main/java/ro/fortsoft/pf4j/PluginVersion.java
+++ b/pf4j/src/main/java/ro/fortsoft/pf4j/PluginVersion.java
@@ -33,7 +33,7 @@ import ro.fortsoft.pf4j.util.StringUtils;
  */
 public class PluginVersion implements Comparable<PluginVersion> {
 
-	public static final PluginVersion DEFAULT = new PluginVersion(0, 0, 0);
+	public static final PluginVersion ZERO = new PluginVersion(0, 0, 0);
 
     private static final String FORMAT = "(\\d+)\\.(\\d+)(?:\\.)?(\\d*)(\\.|-|\\+)?([0-9A-Za-z-.]*)?";
     private static final Pattern PATTERN = Pattern.compile(FORMAT);
@@ -138,14 +138,18 @@ public class PluginVersion implements Comparable<PluginVersion> {
         return 0;
     }
 
-    public boolean isDefault() {
-    	return compareTo(DEFAULT) == 0;
+    public boolean isZero() {
+    	return compareTo(ZERO) == 0;
     }
 
     public boolean atLeast(PluginVersion v) {
     	return compareTo(v) <= 0;
     }
 
+    public boolean exceeds(PluginVersion v) {
+    	return compareTo(v) > 0;
+    }
+
     // for test only
     public static void main(String[] args) {
         PluginVersion v = PluginVersion.createVersion("1.2.3-SNAPSHOT");