diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java index 1fbc350..4d0cca6 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java @@ -287,9 +287,7 @@ public abstract class AbstractPluginManager implements PluginManager { @Override public boolean deletePlugin(String pluginId) { - if (!plugins.containsKey(pluginId)) { - throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId)); - } + checkPluginId(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId); PluginState pluginState = stopPlugin(pluginId); @@ -317,8 +315,7 @@ public abstract class AbstractPluginManager implements PluginManager { PluginState pluginState = pluginWrapper.getPluginState(); if ((PluginState.DISABLED != pluginState) && (PluginState.STARTED != pluginState)) { try { - PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); - log.info("Start plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.info("Start plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor())); pluginWrapper.getPlugin().start(); pluginWrapper.setPluginState(PluginState.STARTED); startedPlugins.add(pluginWrapper); @@ -336,15 +333,13 @@ public abstract class AbstractPluginManager implements PluginManager { */ @Override public PluginState startPlugin(String pluginId) { - if (!plugins.containsKey(pluginId)) { - throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId)); - } + checkPluginId(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId); PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); PluginState pluginState = pluginWrapper.getPluginState(); if (PluginState.STARTED == pluginState) { - log.debug("Already started plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.debug("Already started plugin '{}'", getPluginLabel(pluginDescriptor)); return PluginState.STARTED; } @@ -360,7 +355,7 @@ public abstract class AbstractPluginManager implements PluginManager { } try { - log.info("Start plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.info("Start plugin '{}'", getPluginLabel(pluginDescriptor)); pluginWrapper.getPlugin().start(); pluginWrapper.setPluginState(PluginState.STARTED); startedPlugins.add(pluginWrapper); @@ -386,8 +381,7 @@ public abstract class AbstractPluginManager implements PluginManager { PluginState pluginState = pluginWrapper.getPluginState(); if (PluginState.STARTED == pluginState) { try { - PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); - log.info("Stop plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.info("Stop plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor())); pluginWrapper.getPlugin().stop(); pluginWrapper.setPluginState(PluginState.STOPPED); itr.remove(); @@ -409,9 +403,7 @@ public abstract class AbstractPluginManager implements PluginManager { } private PluginState stopPlugin(String pluginId, boolean stopDependents) { - if (!plugins.containsKey(pluginId)) { - throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId)); - } + checkPluginId(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId); PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); @@ -450,17 +442,21 @@ public abstract class AbstractPluginManager implements PluginManager { return pluginWrapper.getPluginState(); } - @Override - public boolean disablePlugin(String pluginId) { + private void checkPluginId(String pluginId) { if (!plugins.containsKey(pluginId)) { throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId)); } + } + + @Override + public boolean disablePlugin(String pluginId) { + checkPluginId(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId); PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); PluginState pluginState = pluginWrapper.getPluginState(); if (PluginState.DISABLED == pluginState) { - log.debug("Already disabled plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.debug("Already disabled plugin '{}'", getPluginLabel(pluginDescriptor)); return true; } @@ -473,7 +469,7 @@ public abstract class AbstractPluginManager implements PluginManager { return false; } - log.info("Disabled plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.info("Disabled plugin '{}'", getPluginLabel(pluginDescriptor)); return true; } @@ -483,21 +479,18 @@ public abstract class AbstractPluginManager implements PluginManager { @Override public boolean enablePlugin(String pluginId) { - if (!plugins.containsKey(pluginId)) { - throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId)); - } + checkPluginId(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId); if (!isPluginValid(pluginWrapper)) { - log.warn("Plugin '{}:{}' can not be enabled", pluginWrapper.getPluginId(), - pluginWrapper.getDescriptor().getVersion()); + log.warn("Plugin '{}' can not be enabled", getPluginLabel(pluginWrapper.getDescriptor())); return false; } PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); PluginState pluginState = pluginWrapper.getPluginState(); if (PluginState.DISABLED != pluginState) { - log.debug("Plugin '{}:{}' is not disabled", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.debug("Plugin '{}' is not disabled", getPluginLabel(pluginDescriptor)); return true; } @@ -509,7 +502,7 @@ public abstract class AbstractPluginManager implements PluginManager { firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState)); - log.info("Enabled plugin '{}:{}'", pluginDescriptor.getPluginId(), pluginDescriptor.getVersion()); + log.info("Enabled plugin '{}'", getPluginLabel(pluginDescriptor)); return true; } @@ -714,9 +707,9 @@ public abstract class AbstractPluginManager implements PluginManager { return true; } - log.warn("Plugin '{}:{}' requires a minimum system version of {}, and you have {}", - pluginWrapper.getPluginId(), - pluginWrapper.getDescriptor().getVersion(), + PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); + log.warn("Plugin '{}' requires a minimum system version of {}, and you have {}", + getPluginLabel(pluginDescriptor), pluginWrapper.getDescriptor().getRequires(), getSystemVersion()); @@ -728,7 +721,7 @@ public abstract class AbstractPluginManager implements PluginManager { } protected void resolvePlugins() throws PluginException { - // extract plugins descriptors from "unresolvedPlugins" list + // retrieves the plugins descriptors from "unresolvedPlugins" list List descriptors = new ArrayList<>(); for (PluginWrapper plugin : unresolvedPlugins) { descriptors.add(plugin.getDescriptor()); @@ -757,7 +750,7 @@ public abstract class AbstractPluginManager implements PluginManager { PluginWrapper pluginWrapper = plugins.get(pluginId); unresolvedPlugins.remove(pluginWrapper); resolvedPlugins.add(pluginWrapper); - log.info("Plugin '{}' resolved", pluginId); + log.info("Plugin '{}' resolved", getPluginLabel(pluginWrapper.getDescriptor())); } } @@ -770,16 +763,17 @@ public abstract class AbstractPluginManager implements PluginManager { protected PluginWrapper loadPluginFromPath(Path pluginPath) throws PluginException { // test for plugin duplication - if (idForPath(pluginPath) != null) { - log.warn("Plugin {} already loaded", idForPath(pluginPath)); + String pluginId = idForPath(pluginPath); + if (pluginId != null) { + log.warn("Plugin '{}' already loaded with id '{}'", pluginPath, pluginId); return null; } // retrieves the plugin descriptor - log.debug("Find plugin descriptor '{}'", pluginPath); + log.debug("Finding plugin descriptor for plugin '{}'", pluginPath); PluginDescriptor pluginDescriptor = getPluginDescriptorFinder().find(pluginPath); validatePluginDescriptor(pluginDescriptor); - log.debug("Descriptor {}", pluginDescriptor); + log.debug("Found descriptor {}", pluginDescriptor); String pluginClassName = pluginDescriptor.getPluginClass(); log.debug("Class '{}' for plugin '{}'", pluginClassName, pluginPath); @@ -808,7 +802,7 @@ public abstract class AbstractPluginManager implements PluginManager { log.debug("Created wrapper '{}' for plugin '{}'", pluginWrapper, pluginPath); - String pluginId = pluginDescriptor.getPluginId(); + pluginId = pluginDescriptor.getPluginId(); // add plugin to the list with plugins plugins.put(pluginId, pluginWrapper); @@ -844,13 +838,15 @@ public abstract class AbstractPluginManager implements PluginManager { */ protected void validatePluginDescriptor(PluginDescriptor descriptor) throws PluginException { if (StringUtils.isEmpty(descriptor.getPluginId())) { - throw new PluginException("id cannot be empty"); + throw new PluginException("Field 'id' cannot be empty"); } + if (StringUtils.isEmpty(descriptor.getPluginClass())) { - throw new PluginException("class cannot be empty"); + throw new PluginException("Field 'class' cannot be empty"); } + if (descriptor.getVersion() == null) { - throw new PluginException("version cannot be empty"); + throw new PluginException("Field 'version' cannot be empty"); } } @@ -882,4 +878,13 @@ public abstract class AbstractPluginManager implements PluginManager { return versionManager; } + /** + * The plugin label is used in logging and it's a string in format {@code pluginId@pluginVersion}. + * @param pluginDescriptor + * @return + */ + protected String getPluginLabel(PluginDescriptor pluginDescriptor) { + return pluginDescriptor.getPluginId() + "@" + pluginDescriptor.getVersion(); + } + } diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java b/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java index 9e24e01..a7aaac5 100644 --- a/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java +++ b/pf4j/src/main/java/ro/fortsoft/pf4j/DefaultPluginManager.java @@ -47,7 +47,7 @@ public class DefaultPluginManager extends AbstractPluginManager { } /** - * By default if {@link DefaultPluginManager#isDevelopment()} returns true + * By default if {@link DefaultPluginManager#isDevelopment()} returns {@code true} * than a {@link PropertiesPluginDescriptorFinder} is returned * else this method returns {@link DefaultPluginDescriptorFinder}. */