From 172d8b9188748d5b6522dcf8324f2d24892664e7 Mon Sep 17 00:00:00 2001 From: Decebal Suiu Date: Sat, 27 Jan 2018 20:22:06 +0200 Subject: [PATCH] Improve VersionManager (prepare pf4j-update release) --- .../java/org/pf4j/AbstractPluginManager.java | 23 +++++++++---------- .../java/org/pf4j/DefaultVersionManager.java | 21 +++++++++++++++-- .../java/org/pf4j/DependencyResolver.java | 2 +- .../main/java/org/pf4j/VersionManager.java | 15 ++++++++++-- .../org/pf4j/DefaultVersionManagerTest.java | 15 ++++++++---- .../ManifestPluginDescriptorFinderTest.java | 4 ++-- .../PropertiesPluginDescriptorFinderTest.java | 6 ++--- 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 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)