From 1d78b8b372f15d89f10fd32cb0227a6a7966de3c Mon Sep 17 00:00:00 2001 From: James Moger <james.moger@gitblit.com> Date: Thu, 17 Apr 2014 23:08:07 -0400 Subject: [PATCH] [findbugs] Clarify class used for resource loading --- src/main/java/com/gitblit/transport/ssh/commands/PluginDispatcher.java | 100 +++++++++++++++++++++++++++++++------------------ 1 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/gitblit/transport/ssh/commands/PluginDispatcher.java b/src/main/java/com/gitblit/transport/ssh/commands/PluginDispatcher.java index 313d731..6f8f95a 100644 --- a/src/main/java/com/gitblit/transport/ssh/commands/PluginDispatcher.java +++ b/src/main/java/com/gitblit/transport/ssh/commands/PluginDispatcher.java @@ -27,6 +27,7 @@ import ro.fortsoft.pf4j.PluginDescriptor; import ro.fortsoft.pf4j.PluginState; import ro.fortsoft.pf4j.PluginWrapper; +import ro.fortsoft.pf4j.Version; import com.gitblit.manager.IGitblit; import com.gitblit.models.PluginRegistry.InstallState; @@ -35,6 +36,7 @@ import com.gitblit.models.UserModel; import com.gitblit.utils.FlipTable; import com.gitblit.utils.FlipTable.Borders; +import com.gitblit.utils.StringUtils; import com.google.common.base.Joiner; /** @@ -75,7 +77,7 @@ protected void asTable(List<PluginWrapper> list) { String[] headers; if (verbose) { - String [] h = { "#", "Id", "Description", "Version", "Requires", "State", "Path", "Provider"}; + String [] h = { "#", "Id", "Description", "Version", "Requires", "State", "Path" }; headers = h; } else { String [] h = { "#", "Id", "Version", "State", "Path"}; @@ -86,7 +88,7 @@ PluginWrapper p = list.get(i); PluginDescriptor d = p.getDescriptor(); if (verbose) { - data[i] = new Object[] { "" + (i + 1), d.getPluginId(), null/*d.getDescription()*/, d.getVersion(), null/*d.getRequires()*/, p.getPluginState(), p.getPluginPath(), d.getProvider() }; + data[i] = new Object[] { "" + (i + 1), d.getPluginId(), d.getPluginDescription(), d.getVersion(), d.getRequires(), p.getPluginState(), p.getPluginPath() }; } else { data[i] = new Object[] { "" + (i + 1), d.getPluginId(), d.getVersion(), p.getPluginState(), p.getPluginPath() }; } @@ -100,7 +102,7 @@ for (PluginWrapper pw : list) { PluginDescriptor d = pw.getDescriptor(); if (verbose) { - outTabbed(d.getPluginId(), null/*d.getDescription()*/, d.getVersion(), null/*d.getRequires()*/, pw.getPluginState(), pw.getPluginPath(), d.getProvider()); + outTabbed(d.getPluginId(), d.getPluginDescription(), d.getVersion(), d.getRequires(), pw.getPluginState(), pw.getPluginPath()); } else { outTabbed(d.getPluginId(), d.getVersion(), pw.getPluginState(), pw.getPluginPath()); } @@ -138,7 +140,7 @@ @CommandMetaData(name = "start", description = "Start a plugin") public static class StartPlugin extends PluginCommand { - @Argument(index = 0, required = true, metaVar = "ALL|<id>", usage = "the plugin to start") + @Argument(index = 0, required = true, metaVar = "ALL|<ID>|<INDEX>", usage = "the plugin to start") protected String id; @Override @@ -157,7 +159,7 @@ if (PluginState.STARTED.equals(state)) { stdout.println(String.format("Started %s", pluginWrapper.getPluginId())); } else { - throw new Failure(1, String.format("Failed to start %s", pluginWrapper.getPluginId())); + throw new UnloggedFailure(1, String.format("Failed to start %s", pluginWrapper.getPluginId())); } } } @@ -166,7 +168,7 @@ @CommandMetaData(name = "stop", description = "Stop a plugin") public static class StopPlugin extends PluginCommand { - @Argument(index = 0, required = true, metaVar = "ALL|<id>", usage = "the plugin to stop") + @Argument(index = 0, required = true, metaVar = "ALL|<ID>|<INDEX>", usage = "the plugin to stop") protected String id; @Override @@ -185,7 +187,7 @@ if (PluginState.STOPPED.equals(state)) { stdout.println(String.format("Stopped %s", pluginWrapper.getPluginId())); } else { - throw new Failure(1, String.format("Failed to stop %s", pluginWrapper.getPluginId())); + throw new UnloggedFailure(1, String.format("Failed to stop %s", pluginWrapper.getPluginId())); } } } @@ -194,7 +196,7 @@ @CommandMetaData(name = "enable", description = "Enable a plugin") public static class EnablePlugin extends PluginCommand { - @Argument(index = 0, required = true, metaVar = "<id>", usage = "the plugin id to enable") + @Argument(index = 0, required = true, metaVar = "<ID>|<INDEX>", usage = "the plugin to enable") protected String id; @Override @@ -208,7 +210,7 @@ if (gitblit.enablePlugin(pluginWrapper.getPluginId())) { stdout.println(String.format("Enabled %s", pluginWrapper.getPluginId())); } else { - throw new Failure(1, String.format("Failed to enable %s", pluginWrapper.getPluginId())); + throw new UnloggedFailure(1, String.format("Failed to enable %s", pluginWrapper.getPluginId())); } } } @@ -216,7 +218,7 @@ @CommandMetaData(name = "disable", description = "Disable a plugin") public static class DisablePlugin extends PluginCommand { - @Argument(index = 0, required = true, metaVar = "<id>", usage = "the plugin to disable") + @Argument(index = 0, required = true, metaVar = "<ID>|<INDEX>", usage = "the plugin to disable") protected String id; @Override @@ -230,7 +232,7 @@ if (gitblit.disablePlugin(pluginWrapper.getPluginId())) { stdout.println(String.format("Disabled %s", pluginWrapper.getPluginId())); } else { - throw new Failure(1, String.format("Failed to disable %s", pluginWrapper.getPluginId())); + throw new UnloggedFailure(1, String.format("Failed to disable %s", pluginWrapper.getPluginId())); } } } @@ -238,7 +240,7 @@ @CommandMetaData(name = "show", description = "Show the details of a plugin") public static class ShowPlugin extends PluginCommand { - @Argument(index = 0, required = true, metaVar = "<id>", usage = "the plugin to show") + @Argument(index = 0, required = true, metaVar = "<ID>|<INDEX>", usage = "the plugin to show") protected String id; @Override @@ -258,9 +260,9 @@ protected String buildFieldTable(PluginWrapper pw, PluginRegistration reg) { final String id = pw == null ? reg.id : pw.getPluginId(); - final String description = reg == null ? ""/*pw.getDescriptor().getDescription()*/ : reg.description; + final String description = reg == null ? pw.getDescriptor().getPluginDescription() : reg.description; final String version = pw == null ? reg.getCurrentRelease().version : pw.getDescriptor().getVersion().toString(); - final String requires = pw == null ? reg.getCurrentRelease().requires : ""/*pw.getDescriptor().getRequires().toString()*/; + final String requires = pw == null ? reg.getCurrentRelease().requires : pw.getDescriptor().getRequires().toString(); final String provider = pw == null ? reg.provider : pw.getDescriptor().getProvider(); final String registry = reg == null ? "" : reg.registry; final String path = pw == null ? "" : pw.getPluginPath(); @@ -498,8 +500,8 @@ @CommandMetaData(name = "install", description = "Download and installs a plugin") public static class InstallPlugin extends SshCommand { - @Argument(index = 0, required = true, metaVar = "<URL>|<ID>|<NAME>", usage = "the id, name, or the url of the plugin to download and install") - protected String urlOrIdOrName; + @Argument(index = 0, required = true, metaVar = "<URL>|<ID>", usage = "the id or the url of the plugin to download and install") + protected String urlOrId; @Option(name = "--version", usage = "The specific version to install") private String version; @@ -511,27 +513,40 @@ public void run() throws Failure { IGitblit gitblit = getContext().getGitblit(); try { - String ulc = urlOrIdOrName.toLowerCase(); + String ulc = urlOrId.toLowerCase(); if (ulc.startsWith("http://") || ulc.startsWith("https://")) { - if (gitblit.installPlugin(urlOrIdOrName, !disableChecksum)) { - stdout.println(String.format("Installed %s", urlOrIdOrName)); + if (gitblit.installPlugin(urlOrId, !disableChecksum)) { + stdout.println(String.format("Installed %s", urlOrId)); } else { - new Failure(1, String.format("Failed to install %s", urlOrIdOrName)); + throw new UnloggedFailure(1, String.format("Failed to install %s", urlOrId)); } } else { - PluginRelease pv = gitblit.lookupRelease(urlOrIdOrName, version); - if (pv == null) { - throw new Failure(1, String.format("Plugin \"%s\" is not in the registry!", urlOrIdOrName)); + PluginRelease pr = gitblit.lookupRelease(urlOrId, version); + if (pr == null) { + throw new UnloggedFailure(1, String.format("Plugin \"%s\" is not in the registry!", urlOrId)); } - if (gitblit.installPlugin(pv.url, !disableChecksum)) { - stdout.println(String.format("Installed %s", urlOrIdOrName)); + + // enforce minimum system requirement + if (!StringUtils.isEmpty(pr.requires)) { + Version requires = Version.createVersion(pr.requires); + Version system = gitblit.getSystemVersion(); + boolean isValid = system.isZero() || system.atLeast(requires); + if (!isValid) { + String msg = String.format("Plugin \"%s:%s\" requires Gitblit %s", + urlOrId, pr.version, pr.requires); + throw new UnloggedFailure(1, msg); + } + } + + if (gitblit.installPlugin(pr.url, !disableChecksum)) { + stdout.println(String.format("Installed %s", urlOrId)); } else { - throw new Failure(1, String.format("Failed to install %s", urlOrIdOrName)); + throw new UnloggedFailure(1, String.format("Failed to install %s", urlOrId)); } } } catch (IOException e) { - log.error("Failed to install " + urlOrIdOrName, e); - throw new Failure(1, String.format("Failed to install %s", urlOrIdOrName), e); + log.error("Failed to install " + urlOrId, e); + throw new UnloggedFailure(1, String.format("Failed to install %s", urlOrId), e); } } } @@ -556,20 +571,31 @@ throw new UnloggedFailure("Invalid plugin specified!"); } - PluginRelease pv = gitblit.lookupRelease(pluginWrapper.getPluginId(), version); - if (pv == null) { - throw new Failure(1, String.format("Plugin \"%s\" is not in the registry!", pluginWrapper.getPluginId())); + PluginRelease pr = gitblit.lookupRelease(pluginWrapper.getPluginId(), version); + if (pr == null) { + throw new UnloggedFailure(1, String.format("Plugin \"%s\" is not in the registry!", pluginWrapper.getPluginId())); + } + + // enforce minimum system requirement + if (!StringUtils.isEmpty(pr.requires)) { + Version requires = Version.createVersion(pr.requires); + Version system = gitblit.getSystemVersion(); + boolean isValid = system.isZero() || system.atLeast(requires); + if (!isValid) { + throw new Failure(1, String.format("Plugin \"%s:%s\" requires Gitblit %s", + pluginWrapper.getPluginId(), pr.version, pr.requires)); + } } try { - if (gitblit.upgradePlugin(pluginWrapper.getPluginId(), pv.url, !disableChecksum)) { + if (gitblit.upgradePlugin(pluginWrapper.getPluginId(), pr.url, !disableChecksum)) { stdout.println(String.format("Upgraded %s", pluginWrapper.getPluginId())); } else { - throw new Failure(1, String.format("Failed to upgrade %s", pluginWrapper.getPluginId())); + throw new UnloggedFailure(1, String.format("Failed to upgrade %s", pluginWrapper.getPluginId())); } } catch (IOException e) { log.error("Failed to upgrade " + pluginWrapper.getPluginId(), e); - throw new Failure(1, String.format("Failed to upgrade %s", pluginWrapper.getPluginId()), e); + throw new UnloggedFailure(1, String.format("Failed to upgrade %s", pluginWrapper.getPluginId()), e); } } } @@ -577,7 +603,7 @@ @CommandMetaData(name = "uninstall", aliases = { "rm", "del" }, description = "Uninstall a plugin") public static class UninstallPlugin extends PluginCommand { - @Argument(index = 0, required = true, metaVar = "<id>", usage = "the plugin to uninstall") + @Argument(index = 0, required = true, metaVar = "<ID>|<INDEX>", usage = "the plugin to uninstall") protected String id; @Override @@ -588,10 +614,10 @@ throw new UnloggedFailure(String.format("Plugin %s is not installed!", id)); } - if (gitblit.deletePlugin(pluginWrapper.getPluginId())) { + if (gitblit.uninstallPlugin(pluginWrapper.getPluginId())) { stdout.println(String.format("Uninstalled %s", pluginWrapper.getPluginId())); } else { - throw new Failure(1, String.format("Failed to uninstall %s", pluginWrapper.getPluginId())); + throw new UnloggedFailure(1, String.format("Failed to uninstall %s", pluginWrapper.getPluginId())); } } } -- Gitblit v1.9.1