honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

PluginManager: tests for missing plugin name when passing --url 

Adding code to test for unset plugin names to fail fast with descriptive error messages. Also simplified the series of `if` statements checking for the commands by using a `switch` (now that it's using Java 7), added tests, and updated random exceptions with the up-to-date flag names (e.g., "--verbose" instead of "-verbose").

Closes #5976.
Closes #6013.
This commit is contained in:
Chris Earle 2014-05-01 23:47:11 -04:00 committed by David Pilato
parent 04c573104f
commit 312eb2b8b6
2 changed files with 137 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

127
src/main/java/org/elasticsearch/plugins/PluginManager.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.plugins;
import com.google.common.base.Strings;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.ElasticsearchIllegalStateException;
import org.elasticsearch.ElasticsearchTimeoutException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.collect.Tuple;
@ -104,6 +105,9 @@ public class PluginManager {
}
public void downloadAndExtract(String name) throws IOException {
if (name == null) {
throw new ElasticsearchIllegalArgumentException("plugin name must be supplied with --install [name].");
}
HttpDownloadHelper downloadHelper = new HttpDownloadHelper();
boolean downloaded = false;
HttpDownloadHelper.DownloadProgress progress;
@ -123,7 +127,7 @@ public class PluginManager {
// extract the plugin
File extractLocation = pluginHandle.extractedDir(environment);
if (extractLocation.exists()) {
throw new IOException("plugin directory " + extractLocation.getAbsolutePath() + " already exists. To update the plugin, uninstall it first using -remove " + name + " command");
throw new IOException("plugin directory " + extractLocation.getAbsolutePath() + " already exists. To update the plugin, uninstall it first using --remove " + name + " command");
}
// first, try directly from the URL provided
@ -158,7 +162,7 @@ public class PluginManager {
}
if (!downloaded) {
throw new IOException("failed to download out of all possible locations..., use -verbose to get detailed information");
throw new IOException("failed to download out of all possible locations..., use --verbose to get detailed information");
}
ZipFile zipFile = null;
@ -231,6 +235,9 @@ public class PluginManager {
}
public void removePlugin(String name) throws IOException {
if (name == null) {
throw new ElasticsearchIllegalArgumentException("plugin name must be supplied with --remove [name].");
}
PluginHandle pluginHandle = PluginHandle.parse(name);
boolean removed = false;
@ -329,36 +336,66 @@ public class PluginManager {
try {
for (int c = 0; c < args.length; c++) {
String command = args[c];
if ("-u".equals(command) || "--url".equals(command)
// Deprecated commands
|| "url".equals(command) || "-url".equals(command)) {
url = args[++c];
} else if ("-v".equals(command) || "--verbose".equals(command)
|| "verbose".equals(command) || "-verbose".equals(command)) {
outputMode = OutputMode.VERBOSE;
} else if ("-s".equals(command) || "--silent".equals(command)
|| "silent".equals(command) || "-silent".equals(command)) {
outputMode = OutputMode.SILENT;
} else if (command.equals("-i") || command.equals("--install")
// Deprecated commands
|| command.equals("install") || command.equals("-install")) {
pluginName = args[++c];
switch (command) {
case "-u":
case "--url":
// deprecated versions:
case "url":
case "-url":
url = getCommandValue(args, ++c, "--url");
// Until update is supported, then supplying a URL implies installing
// By specifying this action, we also avoid silently failing without
// dubious checks.
action = ACTION.INSTALL;
} else if (command.equals("-t") || command.equals("--timeout")
|| command.equals("timeout") || command.equals("-timeout")) {
timeout = TimeValue.parseTimeValue(args[++c], DEFAULT_TIMEOUT);
} else if (command.equals("-r") || command.equals("--remove")
// Deprecated commands
|| command.equals("remove") || command.equals("-remove")) {
pluginName = args[++c];
break;
case "-v":
case "--verbose":
// deprecated versions:
case "verbose":
case "-verbose":
outputMode = OutputMode.VERBOSE;
break;
case "-s":
case "--silent":
// deprecated versions:
case "silent":
case "-silent":
outputMode = OutputMode.SILENT;
break;
case "-i":
case "--install":
// deprecated versions:
case "install":
case "-install":
pluginName = getCommandValue(args, ++c, "--install");
action = ACTION.INSTALL;
break;
case "-r":
case "--remove":
// deprecated versions:
case "remove":
case "-remove":
pluginName = getCommandValue(args, ++c, "--remove");
action = ACTION.REMOVE;
} else if (command.equals("-l") || command.equals("--list")) {
break;
case "-t":
case "--timeout":
// deprecated versions:
case "timeout":
case "-timeout":
String timeoutValue = getCommandValue(args, ++c, "--timeout");
timeout = TimeValue.parseTimeValue(timeoutValue, DEFAULT_TIMEOUT);
break;
case "-l":
case "--list":
action = ACTION.LIST;
} else if (command.equals("-h") || command.equals("--help")) {
break;
case "-h":
case "--help":
displayHelp(null);
} else {
displayHelp("Command [" + args[c] + "] unknown.");
break;
default:
displayHelp("Command [" + command + "] unknown.");
// Unknown command. We break...
System.exit(EXIT_CODE_CMD_USAGE);
}
@ -375,7 +412,7 @@ public class PluginManager {
switch (action) {
case ACTION.INSTALL:
try {
pluginManager.log("-> Installing " + pluginName + "...");
pluginManager.log("-> Installing " + Strings.nullToEmpty(pluginName) + "...");
pluginManager.downloadAndExtract(pluginName);
exitCode = EXIT_CODE_OK;
} catch (IOException e) {
@ -389,7 +426,7 @@ public class PluginManager {
break;
case ACTION.REMOVE:
try {
pluginManager.log("-> Removing " + pluginName + " ");
pluginManager.log("-> Removing " + Strings.nullToEmpty(pluginName) + "...");
pluginManager.removePlugin(pluginName);
exitCode = EXIT_CODE_OK;
} catch (ElasticsearchIllegalArgumentException e) {
@ -423,6 +460,36 @@ public class PluginManager {
}
}
/**
* Get the value for the {@code flag} at the specified {@code arg} of the command line {@code args}.
* <p />
* This is useful to avoid having to check for multiple forms of unset (e.g., " " versus "" versus {@code null}).
* @param args Incoming command line arguments.
* @param arg Expected argument containing the value.
* @param flag The flag whose value is being retrieved.
* @return Never {@code null}. The trimmed value.
* @throws NullPointerException if {@code args} is {@code null}.
* @throws ArrayIndexOutOfBoundsException if {@code arg} is negative.
* @throws ElasticsearchIllegalStateException if {@code arg} is &gt;= {@code args.length}.
* @throws ElasticsearchIllegalArgumentException if the value evaluates to blank ({@code null} or only whitespace)
*/
private static String getCommandValue(String[] args, int arg, String flag) {
if (arg >= args.length) {
throw new ElasticsearchIllegalStateException("missing value for " + flag + ". Usage: " + flag + " [value]");
}
// avoid having to interpret multiple forms of unset
String trimmedValue = Strings.emptyToNull(args[arg].trim());
// If we had a value that is blank, then fail immediately
if (trimmedValue == null) {
throw new ElasticsearchIllegalArgumentException(
"value for " + flag + "('" + args[arg] + "') must be set. Usage: " + flag + " [value]");
}
return trimmedValue;
}
private static void displayHelp(String message) {
System.out.println("Usage:");
System.out.println(" -u, --url [plugin location] : Set exact URL to download the plugin from");

56
src/test/java/org/elasticsearch/plugin/PluginManagerTests.java
View File

@ -71,12 +71,16 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
deletePluginsFolder();
}
@Test(expected = ElasticsearchIllegalArgumentException.class)
public void testDownloadAndExtract_NullName_ThrowsException() throws IOException {
pluginManager(getPluginUrlForResource("plugin_single_folder.zip")).downloadAndExtract(null);
}
@Test
public void testLocalPluginInstallSingleFolder() throws Exception {
//When we have only a folder in top-level (no files either) we remove that folder while extracting
String pluginName = "plugin-test";
URI uri = URI.create(PluginManagerTests.class.getResource("plugin_single_folder.zip").toString());
downloadAndExtract(pluginName, "file://" + uri.getPath());
downloadAndExtract(pluginName, getPluginUrlForResource("plugin_single_folder.zip"));
internalCluster().startNode(SETTINGS);
@ -89,10 +93,9 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
//When we have only a folder in top-level (no files either) but it's called _site, we make it work
//we can either remove the folder while extracting and then re-add it manually or just leave it as it is
String pluginName = "plugin-test";
URI uri = URI.create(PluginManagerTests.class.getResource("plugin_folder_site.zip").toString());
downloadAndExtract(pluginName, "file://" + uri.getPath());
downloadAndExtract(pluginName, getPluginUrlForResource("plugin_folder_site.zip"));
String nodeName = internalCluster().startNode(SETTINGS);
internalCluster().startNode(SETTINGS);
assertPluginLoaded(pluginName);
assertPluginAvailable(pluginName);
@ -102,8 +105,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
public void testLocalPluginWithoutFolders() throws Exception {
//When we don't have folders at all in the top-level, but only files, we don't modify anything
String pluginName = "plugin-test";
URI uri = URI.create(PluginManagerTests.class.getResource("plugin_without_folders.zip").toString());
downloadAndExtract(pluginName, "file://" + uri.getPath());
downloadAndExtract(pluginName, getPluginUrlForResource("plugin_without_folders.zip"));
internalCluster().startNode(SETTINGS);
@ -115,8 +117,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
public void testLocalPluginFolderAndFile() throws Exception {
//When we have a single top-level folder but also files in the top-level, we don't modify anything
String pluginName = "plugin-test";
URI uri = URI.create(PluginManagerTests.class.getResource("plugin_folder_file.zip").toString());
downloadAndExtract(pluginName, "file://" + uri.getPath());
downloadAndExtract(pluginName, getPluginUrlForResource("plugin_folder_file.zip"));
internalCluster().startNode(SETTINGS);
@ -127,8 +128,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
@Test(expected = IllegalArgumentException.class)
public void testSitePluginWithSourceThrows() throws Exception {
String pluginName = "plugin-with-source";
URI uri = URI.create(PluginManagerTests.class.getResource("plugin_with_sourcefiles.zip").toString());
downloadAndExtract(pluginName, "file://" + uri.getPath());
downloadAndExtract(pluginName, getPluginUrlForResource("plugin_with_sourcefiles.zip"));
}
/**
@ -202,14 +202,13 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
@Test(expected = IOException.class)
public void testInstallPluginNull() throws IOException {
pluginManager(null).downloadAndExtract("");
pluginManager(null).downloadAndExtract("plugin-test");
}
@Test
public void testInstallPlugin() throws IOException {
PluginManager pluginManager = pluginManager("file://".concat(
URI.create(PluginManagerTests.class.getResource("plugin_with_classfile.zip").toString()).getPath()));
PluginManager pluginManager = pluginManager(getPluginUrlForResource("plugin_with_classfile.zip"));
pluginManager.downloadAndExtract("plugin");
File[] plugins = pluginManager.getListInstalledPlugins();
@ -219,8 +218,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
@Test
public void testInstallSitePlugin() throws IOException {
PluginManager pluginManager = pluginManager("file://".concat(
URI.create(PluginManagerTests.class.getResource("plugin_without_folders.zip").toString()).getPath()));
PluginManager pluginManager = pluginManager(getPluginUrlForResource("plugin_without_folders.zip"));
pluginManager.downloadAndExtract("plugin-site");
File[] plugins = pluginManager.getListInstalledPlugins();
@ -314,16 +312,18 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
@Test
public void testRemovePlugin() throws Exception {
// We want to remove plugin with plugin short name
singlePluginInstallAndRemove("plugintest", "file://".concat(
URI.create(PluginManagerTests.class.getResource("plugin_without_folders.zip").toString()).getPath()));
singlePluginInstallAndRemove("plugintest", getPluginUrlForResource("plugin_without_folders.zip"));
// We want to remove plugin with groupid/artifactid/version form
singlePluginInstallAndRemove("groupid/plugintest/1.0.0", "file://".concat(
URI.create(PluginManagerTests.class.getResource("plugin_without_folders.zip").toString()).getPath()));
singlePluginInstallAndRemove("groupid/plugintest/1.0.0", getPluginUrlForResource("plugin_without_folders.zip"));
// We want to remove plugin with groupid/artifactid form
singlePluginInstallAndRemove("groupid/plugintest", "file://".concat(
URI.create(PluginManagerTests.class.getResource("plugin_without_folders.zip").toString()).getPath()));
singlePluginInstallAndRemove("groupid/plugintest", getPluginUrlForResource("plugin_without_folders.zip"));
}
@Test(expected = ElasticsearchIllegalArgumentException.class)
public void testRemovePlugin_NullName_ThrowsException() throws IOException {
pluginManager(getPluginUrlForResource("plugin_single_folder.zip")).removePlugin(null);
}
@Test(expected = ElasticsearchIllegalArgumentException.class)
@ -331,4 +331,16 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
PluginManager pluginManager = pluginManager(null);
pluginManager.removePlugin("file://whatever");
}
/**
* Retrieve a URL string that represents the resource with the given {@code resourceName}.
* @param resourceName The resource name relative to {@link PluginManagerTests}.
* @return Never {@code null}.
* @throws NullPointerException if {@code resourceName} does not point to a valid resource.
*/
private String getPluginUrlForResource(String resourceName) {
URI uri = URI.create(PluginManagerTests.class.getResource(resourceName).toString());
return "file://" + uri.getPath();
}
}