Improve VersionManager (prepare pf4j-update release)

7 files changed, 59 insertions, 27 deletions

diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
index 9f3471e..f247998 100644
--- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
+++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
@@ -186,11 +186,9 @@ public abstract class AbstractPluginManager implements PluginManager {
 
         try {
             PluginWrapper pluginWrapper = loadPluginFromPath(pluginPath);
-            // TODO uninstalled plugin dependencies?
-            getUnresolvedPlugins().remove(pluginWrapper);
-            getResolvedPlugins().add(pluginWrapper);
 
-            firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, null));
+            // try to resolve  the loaded plugin together with other possible plugins that depend on this plugin
+            resolvePlugins();
 
             return pluginWrapper.getDescriptor().getPluginId();
         } catch (PluginException e) {
@@ -224,7 +222,6 @@ public abstract class AbstractPluginManager implements PluginManager {
         log.debug("Found {} possible plugins: {}", pluginPaths.size(), pluginPaths);
 
         // load plugins from plugin paths
-        // TODO
         for (Path pluginPath : pluginPaths) {
             try {
                 loadPluginFromPath(pluginPath);
@@ -233,7 +230,7 @@ public abstract class AbstractPluginManager implements PluginManager {
             }
         }
 
-        // resolve 'unresolvedPlugins'
+        // resolve plugins
         try {
             resolvePlugins();
         } catch (PluginException e) {
@@ -739,7 +736,7 @@ public abstract class AbstractPluginManager implements PluginManager {
             // If exact versions are not allowed in requires, rewrite to >= expression
             requires = ">=" + requires;
         }
-        if (systemVersion.equals("0.0.0") || versionManager.satisfies(requires, systemVersion)) {
+        if (systemVersion.equals("0.0.0") || versionManager.checkVersionConstraint(systemVersion, requires)) {
             return true;
         }
 
@@ -757,9 +754,9 @@ public abstract class AbstractPluginManager implements PluginManager {
     }
 
     protected void resolvePlugins() throws PluginException {
-        // retrieves the  plugins descriptors from "unresolvedPlugins" list
+        // retrieves the  plugins descriptors
         List<PluginDescriptor> descriptors = new ArrayList<>();
-        for (PluginWrapper plugin : unresolvedPlugins) {
+        for (PluginWrapper plugin : plugins.values()) {
             descriptors.add(plugin.getDescriptor());
         }
 
@@ -784,9 +781,10 @@ public abstract class AbstractPluginManager implements PluginManager {
         // move plugins from "unresolved" to "resolved"
         for (String pluginId : sortedPlugins) {
             PluginWrapper pluginWrapper = plugins.get(pluginId);
-            unresolvedPlugins.remove(pluginWrapper);
-            resolvedPlugins.add(pluginWrapper);
-            log.info("Plugin '{}' resolved", getPluginLabel(pluginWrapper.getDescriptor()));
+            if (unresolvedPlugins.remove(pluginWrapper)) {
+                resolvedPlugins.add(pluginWrapper);
+                log.info("Plugin '{}' resolved", getPluginLabel(pluginWrapper.getDescriptor()));
+            }
         }
     }
 
@@ -918,6 +916,7 @@ public abstract class AbstractPluginManager implements PluginManager {
 
     /**
      * The plugin label is used in logging and it's a string in format {@code pluginId@pluginVersion}.
+     *
      * @param pluginDescriptor
      * @return
      */
diff --git a/pf4j/src/main/java/org/pf4j/DefaultVersionManager.java b/pf4j/src/main/java/org/pf4j/DefaultVersionManager.java
index 96adf56..5ce6135 100644
--- a/pf4j/src/main/java/org/pf4j/DefaultVersionManager.java
+++ b/pf4j/src/main/java/org/pf4j/DefaultVersionManager.java
@@ -16,6 +16,8 @@
 package org.pf4j;
 
 import com.github.zafarkhaja.semver.Version;
+import com.github.zafarkhaja.semver.expr.Expression;
+import org.pf4j.util.StringUtils;
 
 /**
  * Default implementation for {@link VersionManager}.
@@ -25,9 +27,24 @@ import com.github.zafarkhaja.semver.Version;
  */
 public class DefaultVersionManager implements VersionManager {
 
+    /**
+     * Checks if a version satisfies the specified SemVer {@link Expression} string.
+     * If the constraint is empty or null then the method returns true.
+     * Constraint examples: {@code >2.0.0} (simple), {@code ">=1.4.0 & <1.6.0"} (range).
+     * See https://github.com/zafarkhaja/jsemver#semver-expressions-api-ranges for more info.
+     *
+     * @param version
+     * @param constraint
+     * @return
+     */
     @Override
-    public boolean satisfies(String constraint, String version) {
-        return Version.valueOf(version).satisfies(constraint);
+    public boolean checkVersionConstraint(String version, String constraint) {
+        return StringUtils.isNullOrEmpty(constraint) || Version.valueOf(version).satisfies(constraint);
+    }
+
+    @Override
+    public int compareVersions(String v1, String v2) {
+        return Version.valueOf(v1).compareTo(Version.valueOf(v2));
     }
 
 }
diff --git a/pf4j/src/main/java/org/pf4j/DependencyResolver.java b/pf4j/src/main/java/org/pf4j/DependencyResolver.java
index 05e8d11..215a433 100644
--- a/pf4j/src/main/java/org/pf4j/DependencyResolver.java
+++ b/pf4j/src/main/java/org/pf4j/DependencyResolver.java
@@ -133,7 +133,7 @@ public class DependencyResolver {
      * @return
      */
     protected boolean checkDependencyVersion(String requiredVersion, String existingVersion) {
-        return versionManager.satisfies(requiredVersion, existingVersion);
+        return versionManager.checkVersionConstraint(existingVersion, requiredVersion);
     }
 
     private void addPlugin(PluginDescriptor descriptor) {
diff --git a/pf4j/src/main/java/org/pf4j/VersionManager.java b/pf4j/src/main/java/org/pf4j/VersionManager.java
index ae9187b..855afea 100644
--- a/pf4j/src/main/java/org/pf4j/VersionManager.java
+++ b/pf4j/src/main/java/org/pf4j/VersionManager.java
@@ -15,6 +15,8 @@
  */
 package org.pf4j;
 
+import java.util.Comparator;
+
 /**
  * Manager responsible for versions of plugins.
  *
@@ -24,11 +26,20 @@ public interface VersionManager {
 
     /**
      * Check if a {@code constraint} and a {@code version} match.
+     * A possible constrain can be {@code >=1.0.0 & <2.0.0}.
      *
-     * @param constraint
      * @param version
+     * @param constraint
      * @return
      */
-    boolean satisfies(String constraint, String version);
+    boolean checkVersionConstraint(String version, String constraint);
+
+    /**
+     * Compare two versions. It's similar with {@link Comparator#compare(Object, Object)}.
+     *
+     * @param v1
+     * @param v2
+     */
+    int compareVersions(String v1, String v2);
 
 }
diff --git a/pf4j/src/test/java/org/pf4j/DefaultVersionManagerTest.java b/pf4j/src/test/java/org/pf4j/DefaultVersionManagerTest.java
index aaa5fa9..68a75a8 100644
--- a/pf4j/src/test/java/org/pf4j/DefaultVersionManagerTest.java
+++ b/pf4j/src/test/java/org/pf4j/DefaultVersionManagerTest.java
@@ -34,19 +34,24 @@ public class DefaultVersionManagerTest {
     }
 
     @Test
-    public void satisfies() {
-        assertFalse(versionManager.satisfies(">2.0.0", "1.4.3")); // simple
-        assertTrue(versionManager.satisfies(">=1.4.0 & <1.6.0", "1.4.3")); // range
+    public void checkVersionConstraint() {
+        assertFalse(versionManager.checkVersionConstraint("1.4.3", ">2.0.0")); // simple
+        assertTrue(versionManager.checkVersionConstraint("1.4.3", ">=1.4.0 & <1.6.0")); // range
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void nullOrEmptyVersion() {
-        assertFalse(versionManager.satisfies(">2.0.0", null));
+        assertFalse(versionManager.checkVersionConstraint(null, ">2.0.0"));
     }
 
     @Test(expected = ParseException.class)
     public void invalidVersion() {
-        assertFalse(versionManager.satisfies(">2.0.0", "1.0"));
+        assertFalse(versionManager.checkVersionConstraint("1.0", ">2.0.0"));
+    }
+
+    @Test
+    public void compareVersions() {
+        assertTrue(versionManager.compareVersions("1.1.0", "1.0.0") > 0);
     }
 
 }
diff --git a/pf4j/src/test/java/org/pf4j/ManifestPluginDescriptorFinderTest.java b/pf4j/src/test/java/org/pf4j/ManifestPluginDescriptorFinderTest.java
index fcd0035..c073c88 100644
--- a/pf4j/src/test/java/org/pf4j/ManifestPluginDescriptorFinderTest.java
+++ b/pf4j/src/test/java/org/pf4j/ManifestPluginDescriptorFinderTest.java
@@ -93,7 +93,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(versionManager.satisfies(plugin1.getRequires(), "1.0.0"));
+        assertTrue(versionManager.checkVersionConstraint("1.0.0", plugin1.getRequires()));
 
         assertEquals("test-plugin-2", plugin2.getPluginId());
         assertEquals("", plugin2.getPluginDescription());
@@ -101,7 +101,7 @@ public class ManifestPluginDescriptorFinderTest {
         assertEquals("0.0.1", plugin2.getVersion());
         assertEquals("Decebal Suiu", plugin2.getProvider());
         assertEquals(0, plugin2.getDependencies().size());
-        assertTrue(versionManager.satisfies(plugin2.getRequires(), "1.0.0"));
+        assertTrue(versionManager.checkVersionConstraint("1.0.0", plugin2.getRequires()));
     }
 
     /**
diff --git a/pf4j/src/test/java/org/pf4j/PropertiesPluginDescriptorFinderTest.java b/pf4j/src/test/java/org/pf4j/PropertiesPluginDescriptorFinderTest.java
index a3e3c68..cf36b4f 100644
--- a/pf4j/src/test/java/org/pf4j/PropertiesPluginDescriptorFinderTest.java
+++ b/pf4j/src/test/java/org/pf4j/PropertiesPluginDescriptorFinderTest.java
@@ -83,8 +83,8 @@ public class PropertiesPluginDescriptorFinderTest {
         assertEquals("~1.0", plugin1.getDependencies().get(1).getPluginVersionSupport());
         assertEquals("Apache-2.0", plugin1.getLicense());
         assertEquals(">=1", plugin1.getRequires());
-        assertTrue(versionManager.satisfies(plugin1.getRequires(), "1.0.0"));
-        assertFalse(versionManager.satisfies(plugin1.getRequires(), "0.1.0"));
+        assertTrue(versionManager.checkVersionConstraint("1.0.0", plugin1.getRequires()));
+        assertFalse(versionManager.checkVersionConstraint("0.1.0", plugin1.getRequires()));
 
         assertEquals("test-plugin-2", plugin2.getPluginId());
         assertEquals("", plugin2.getPluginDescription());
@@ -93,7 +93,7 @@ public class PropertiesPluginDescriptorFinderTest {
         assertEquals("Decebal Suiu", plugin2.getProvider());
         assertEquals(0, plugin2.getDependencies().size());
         assertEquals("*", plugin2.getRequires()); // Default is *
-        assertTrue(versionManager.satisfies(plugin2.getRequires(), "1.0.0"));
+        assertTrue(versionManager.checkVersionConstraint("1.0.0", plugin2.getRequires()));
     }
 
     @Test(expected = PluginException.class)