Merge pull request #16359 from rjernst/cli_errors

CliTool: Allow unexpected exceptions to propagate
This commit is contained in:
Ryan Ernst 2016-02-01 17:38:24 -08:00
commit 1152e79ec8
15 changed files with 125 additions and 207 deletions

View File

@ -27,6 +27,7 @@ import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolConfig;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.monitor.jvm.JvmInfo;
@ -103,7 +104,7 @@ final class BootstrapCLIParser extends CliTool {
// TODO: don't use system properties as a way to do this, its horrible...
@SuppressForbidden(reason = "Sets system properties passed as CLI parameters")
public static Command parse(Terminal terminal, CommandLine cli) {
public static Command parse(Terminal terminal, CommandLine cli) throws UserError {
if (cli.hasOption("V")) {
return Version.parse(terminal, cli);
}
@ -132,11 +133,11 @@ final class BootstrapCLIParser extends CliTool {
String arg = iterator.next();
if (!arg.startsWith("--")) {
if (arg.startsWith("-D") || arg.startsWith("-d") || arg.startsWith("-p")) {
throw new IllegalArgumentException(
throw new UserError(ExitStatus.USAGE,
"Parameter [" + arg + "] starting with \"-D\", \"-d\" or \"-p\" must be before any parameters starting with --"
);
} else {
throw new IllegalArgumentException("Parameter [" + arg + "]does not start with --");
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "]does not start with --");
}
}
// if there is no = sign, we have to get the next argu
@ -150,11 +151,11 @@ final class BootstrapCLIParser extends CliTool {
if (iterator.hasNext()) {
String value = iterator.next();
if (value.startsWith("--")) {
throw new IllegalArgumentException("Parameter [" + arg + "] needs value");
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
}
System.setProperty("es." + arg, value);
} else {
throw new IllegalArgumentException("Parameter [" + arg + "] needs value");
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
}
}
}

View File

@ -19,9 +19,14 @@
package org.elasticsearch.common.cli;
import org.apache.commons.cli.AlreadySelectedException;
import org.apache.commons.cli.AmbiguousOptionException;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.MissingArgumentException;
import org.apache.commons.cli.MissingOptionException;
import org.apache.commons.cli.UnrecognizedOptionException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.node.internal.InternalSettingsPreparer;
@ -50,7 +55,7 @@ import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
public abstract class CliTool {
// based on sysexits.h
public static enum ExitStatus {
public enum ExitStatus {
OK(0),
OK_AND_EXIT(0),
USAGE(64), /* command line usage error */
@ -69,23 +74,13 @@ public abstract class CliTool {
final int status;
private ExitStatus(int status) {
ExitStatus(int status) {
this.status = status;
}
public int status() {
return status;
}
public static ExitStatus fromStatus(int status) {
for (ExitStatus exitStatus : values()) {
if (exitStatus.status() == status) {
return exitStatus;
}
}
return null;
}
}
protected final Terminal terminal;
@ -108,7 +103,7 @@ public abstract class CliTool {
settings = env.settings();
}
public final ExitStatus execute(String... args) {
public final ExitStatus execute(String... args) throws Exception {
// first lets see if the user requests tool help. We're doing it only if
// this is a multi-command tool. If it's a single command tool, the -h/--help
@ -146,23 +141,11 @@ public abstract class CliTool {
}
}
Command command = null;
try {
command = parse(cmd, args);
return command.execute(settings, env);
} catch (IOException ioe) {
terminal.printError(ioe);
return ExitStatus.IO_ERROR;
} catch (IllegalArgumentException ilae) {
terminal.printError(ilae);
return ExitStatus.USAGE;
} catch (Throwable t) {
terminal.printError(t);
if (command == null) {
return ExitStatus.USAGE;
}
return ExitStatus.CODE_ERROR;
return parse(cmd, args).execute(settings, env);
} catch (UserError error) {
terminal.printError(error.getMessage());
return error.exitStatus;
}
}
@ -177,7 +160,13 @@ public abstract class CliTool {
if (cli.hasOption("h")) {
return helpCmd(cmd);
}
cli = parser.parse(cmd.options(), args, cmd.isStopAtNonOption());
try {
cli = parser.parse(cmd.options(), args, cmd.isStopAtNonOption());
} catch (AlreadySelectedException|MissingArgumentException|MissingOptionException|UnrecognizedOptionException e) {
// intentionally drop the stack trace here as these are really user errors,
// the stack trace into cli parsing lib is not important
throw new UserError(ExitStatus.USAGE, e.toString());
}
Terminal.Verbosity verbosity = Terminal.Verbosity.resolve(cli);
terminal.verbosity(verbosity);
return parse(cmd.name(), cli);

View File

@ -35,8 +35,6 @@ import java.util.Locale;
@SuppressForbidden(reason = "System#out")
public abstract class Terminal {
public static final String DEBUG_SYSTEM_PROPERTY = "es.cli.debug";
public static final Terminal DEFAULT = ConsoleTerminal.supported() ? new ConsoleTerminal() : new SystemTerminal();
public static enum Verbosity {
@ -64,7 +62,6 @@ public abstract class Terminal {
}
private Verbosity verbosity = Verbosity.NORMAL;
private final boolean isDebugEnabled;
public Terminal() {
this(Verbosity.NORMAL);
@ -72,7 +69,6 @@ public abstract class Terminal {
public Terminal(Verbosity verbosity) {
this.verbosity = verbosity;
this.isDebugEnabled = "true".equals(System.getProperty(DEBUG_SYSTEM_PROPERTY, "false"));
}
public void verbosity(Verbosity verbosity) {
@ -119,13 +115,6 @@ public abstract class Terminal {
println(Verbosity.SILENT, "ERROR: " + msg, args);
}
public void printError(Throwable t) {
printError("%s", t.toString());
if (isDebugEnabled) {
printStackTrace(t);
}
}
public void printWarn(String msg, Object... args) {
println(Verbosity.SILENT, "WARN: " + msg, args);
}

View File

@ -0,0 +1,35 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.common.cli;
/**
* An exception representing a user fixable problem in {@link CliTool} usage.
*/
public class UserError extends Exception {
/** The exist status the cli should use when catching this user error. */
public final CliTool.ExitStatus exitStatus;
/** Constructs a UserError with an exit status and message to show the user. */
public UserError(CliTool.ExitStatus exitStatus, String msg) {
super(msg);
this.exitStatus = exitStatus;
}
}

View File

@ -330,31 +330,4 @@ public class Environment {
public static FileStore getFileStore(Path path) throws IOException {
return ESFileStore.getMatchingFileStore(path, fileStores);
}
/**
* Returns true if the path is writable.
* Acts just like {@link Files#isWritable(Path)}, except won't
* falsely return false for paths on SUBST'd drive letters
* See https://bugs.openjdk.java.net/browse/JDK-8034057
* Note this will set the file modification time (to its already-set value)
* to test access.
*/
@SuppressForbidden(reason = "works around https://bugs.openjdk.java.net/browse/JDK-8034057")
public static boolean isWritable(Path path) throws IOException {
boolean v = Files.isWritable(path);
if (v || Constants.WINDOWS == false) {
return v;
}
// isWritable returned false on windows, the hack begins!!!!!!
// resetting the modification time is the least destructive/simplest
// way to check for both files and directories, and fails early just
// in getting the current value if file doesn't exist, etc
try {
Files.setLastModifiedTime(path, Files.getLastModifiedTime(path));
return true;
} catch (Throwable e) {
return false;
}
}
}

View File

@ -48,6 +48,7 @@ import org.elasticsearch.Version;
import org.elasticsearch.bootstrap.JarHell;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.hash.MessageDigests;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.settings.Settings;
@ -136,10 +137,6 @@ class InstallPluginCommand extends CliTool.Command {
Files.createDirectory(env.pluginsFile());
}
if (Environment.isWritable(env.pluginsFile()) == false) {
throw new IOException("Plugins directory is read only: " + env.pluginsFile());
}
Path pluginZip = download(pluginId, env.tmpFile());
Path extractedZip = unzip(pluginZip, env.pluginsFile());
install(extractedZip, env);
@ -148,7 +145,7 @@ class InstallPluginCommand extends CliTool.Command {
}
/** Downloads the plugin and returns the file it was downloaded to. */
private Path download(String pluginId, Path tmpDir) throws IOException {
private Path download(String pluginId, Path tmpDir) throws Exception {
if (OFFICIAL_PLUGINS.contains(pluginId)) {
final String version = Version.CURRENT.toString();
final String url;
@ -189,7 +186,7 @@ class InstallPluginCommand extends CliTool.Command {
}
/** Downloads a zip from the url, as well as a SHA1 checksum, and checks the checksum. */
private Path downloadZipAndChecksum(String urlString, Path tmpDir) throws IOException {
private Path downloadZipAndChecksum(String urlString, Path tmpDir) throws Exception {
Path zip = downloadZip(urlString, tmpDir);
URL checksumUrl = new URL(urlString + ".sha1");
@ -198,14 +195,14 @@ class InstallPluginCommand extends CliTool.Command {
BufferedReader checksumReader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8));
expectedChecksum = checksumReader.readLine();
if (checksumReader.readLine() != null) {
throw new IllegalArgumentException("Invalid checksum file at " + urlString.toString());
throw new UserError(CliTool.ExitStatus.IO_ERROR, "Invalid checksum file at " + checksumUrl);
}
}
byte[] zipbytes = Files.readAllBytes(zip);
String gotChecksum = MessageDigests.toHexString(MessageDigests.sha1().digest(zipbytes));
if (expectedChecksum.equals(gotChecksum) == false) {
throw new IllegalStateException("SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum);
throw new UserError(CliTool.ExitStatus.IO_ERROR, "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum);
}
return zip;
@ -250,7 +247,7 @@ class InstallPluginCommand extends CliTool.Command {
// don't let luser install plugin as a module...
// they might be unavoidably in maven central and are packaged up the same way)
if (MODULES.contains(info.getName())) {
throw new IOException("plugin '" + info.getName() + "' cannot be installed like this, it is a system module");
throw new UserError(CliTool.ExitStatus.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module");
}
// check for jar hell before any copying
@ -306,7 +303,7 @@ class InstallPluginCommand extends CliTool.Command {
final Path destination = env.pluginsFile().resolve(info.getName());
if (Files.exists(destination)) {
throw new IOException("plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command");
throw new UserError(CliTool.ExitStatus.USAGE, "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command");
}
Path tmpBinDir = tmpRoot.resolve("bin");
@ -337,9 +334,9 @@ class InstallPluginCommand extends CliTool.Command {
}
/** Copies the files from {@code tmpBinDir} into {@code destBinDir}, along with permissions from dest dirs parent. */
private void installBin(PluginInfo info, Path tmpBinDir, Path destBinDir) throws IOException {
private void installBin(PluginInfo info, Path tmpBinDir, Path destBinDir) throws Exception {
if (Files.isDirectory(tmpBinDir) == false) {
throw new IOException("bin in plugin " + info.getName() + " is not a directory");
throw new UserError(CliTool.ExitStatus.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory");
}
Files.createDirectory(destBinDir);
@ -357,7 +354,7 @@ class InstallPluginCommand extends CliTool.Command {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpBinDir)) {
for (Path srcFile : stream) {
if (Files.isDirectory(srcFile)) {
throw new IOException("Directories not allowed in bin dir for plugin " + info.getName());
throw new UserError(CliTool.ExitStatus.DATA_ERROR, "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName());
}
Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile));
@ -376,9 +373,9 @@ class InstallPluginCommand extends CliTool.Command {
* Copies the files from {@code tmpConfigDir} into {@code destConfigDir}.
* Any files existing in both the source and destination will be skipped.
*/
private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws IOException {
private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws Exception {
if (Files.isDirectory(tmpConfigDir) == false) {
throw new IOException("config in plugin " + info.getName() + " is not a directory");
throw new UserError(CliTool.ExitStatus.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
}
// create the plugin's config dir "if necessary"
@ -387,7 +384,7 @@ class InstallPluginCommand extends CliTool.Command {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpConfigDir)) {
for (Path srcFile : stream) {
if (Files.isDirectory(srcFile)) {
throw new IOException("Directories not allowed in config dir for plugin " + info.getName());
throw new UserError(CliTool.ExitStatus.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName());
}
Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));

View File

@ -55,7 +55,7 @@ public class PluginCli extends CliTool {
.cmds(LIST_CMD, INSTALL_CMD, REMOVE_CMD)
.build();
public static void main(String[] args) {
public static void main(String[] args) throws Exception {
// initialize default for es.logger.level because we will not read the logging.yml
String loggerLevel = System.getProperty("es.logger.level", "INFO");
// Set the appender for all potential log files to terminal so that other components that use the logger print out the

View File

@ -29,6 +29,7 @@ import org.apache.lucene.util.IOUtils;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
@ -51,7 +52,7 @@ class RemovePluginCommand extends CliTool.Command {
Path pluginDir = env.pluginsFile().resolve(pluginName);
if (Files.exists(pluginDir) == false) {
throw new IllegalArgumentException("Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins.");
throw new UserError(CliTool.ExitStatus.USAGE, "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins.");
}
List<Path> pluginPaths = new ArrayList<>();
@ -59,7 +60,7 @@ class RemovePluginCommand extends CliTool.Command {
Path pluginBinDir = env.binFile().resolve(pluginName);
if (Files.exists(pluginBinDir)) {
if (Files.isDirectory(pluginBinDir) == false) {
throw new IllegalStateException("Bin dir for " + pluginName + " is not a directory");
throw new UserError(CliTool.ExitStatus.IO_ERROR, "Bin dir for " + pluginName + " is not a directory");
}
pluginPaths.add(pluginBinDir);
terminal.println(VERBOSE, "Removing: %s", pluginBinDir);

View File

@ -46,24 +46,6 @@ public class TerminalTests extends CliToolTestCase {
assertPrinted(terminal, Terminal.Verbosity.VERBOSE, "text");
}
public void testError() throws Exception {
try {
// actually throw so we have a stacktrace
throw new NoSuchFileException("/path/to/some/file");
} catch (NoSuchFileException e) {
CaptureOutputTerminal terminal = new CaptureOutputTerminal(Terminal.Verbosity.NORMAL);
terminal.printError(e);
List<String> output = terminal.getTerminalOutput();
assertFalse(output.isEmpty());
assertTrue(output.get(0), output.get(0).contains("NoSuchFileException")); // exception class
assertTrue(output.get(0), output.get(0).contains("/path/to/some/file")); // message
assertEquals(1, output.size());
// TODO: we should test stack trace is printed in debug mode...except debug is a sysprop instead of
// a command line param...maybe it should be VERBOSE instead of a separate debug prop?
}
}
private void assertPrinted(CaptureOutputTerminal logTerminal, Terminal.Verbosity verbosity, String text) {
logTerminal.print(verbosity, text);
assertThat(logTerminal.getTerminalOutput(), hasSize(1));

View File

@ -19,21 +19,15 @@
package org.elasticsearch.plugins;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolTestCase;
import java.io.IOException;
import java.net.MalformedURLException;
import java.nio.file.Path;
import static org.elasticsearch.common.cli.CliTool.ExitStatus.IO_ERROR;
import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.is;
public class PluginCliTests extends CliToolTestCase {
public void testHelpWorks() throws IOException {
public void testHelpWorks() throws Exception {
CliToolTestCase.CaptureOutputTerminal terminal = new CliToolTestCase.CaptureOutputTerminal();
assertThat(new PluginCli(terminal).execute(args("--help")), is(OK_AND_EXIT));
assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin.help");
@ -53,12 +47,4 @@ public class PluginCliTests extends CliToolTestCase {
assertThat(new PluginCli(terminal).execute(args("list -h")), is(OK_AND_EXIT));
assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-list.help");
}
public void testUrlSpacesInPath() throws MalformedURLException {
CliToolTestCase.CaptureOutputTerminal terminal = new CliToolTestCase.CaptureOutputTerminal();
Path tmpDir = createTempDir().resolve("foo deps");
String finalDir = tmpDir.toAbsolutePath().toUri().toURL().toString();
CliTool.ExitStatus execute = new PluginCli(terminal).execute("install", finalDir);
assertThat(execute.status(), is(IO_ERROR.status()));
}
}

View File

@ -139,14 +139,10 @@ public class StandaloneRunner extends CliTool {
}
public static byte[] copyToBytes(Path path) throws IOException {
try (InputStream is = Files.newInputStream(path)) {
if (is == null) {
throw new FileNotFoundException("Resource [" + path + "] not found in classpath");
}
try (BytesStreamOutput out = new BytesStreamOutput()) {
copy(is, out);
return out.bytes().toBytes();
}
try (InputStream is = Files.newInputStream(path);
BytesStreamOutput out = new BytesStreamOutput()) {
copy(is, out);
return out.bytes().toBytes();
}
}
@ -177,7 +173,7 @@ public class StandaloneRunner extends CliTool {
}
public static void main(String[] args) {
public static void main(String[] args) throws Exception {
StandaloneRunner pluginManager = new StandaloneRunner();
pluginManager.execute(args);
}

View File

@ -24,6 +24,7 @@ import org.elasticsearch.Version;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.cli.CliTool.ExitStatus;
import org.elasticsearch.common.cli.CliToolTestCase;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.hamcrest.Matcher;
@ -167,7 +168,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
assertThatTerminalOutput(containsString("Parameter [network.host] needs value"));
}
public void testParsingErrors() {
public void testParsingErrors() throws Exception {
BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
// unknown params
@ -229,12 +230,10 @@ public class BootstrapCliParserTests extends CliToolTestCase {
public void testThatHelpfulErrorMessageIsGivenWhenParametersAreOutOfOrder() throws Exception {
BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
try {
parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"});
fail("expected IllegalArgumentException for out-of-order parameters");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("must be before any parameters starting with --"));
}
UserError e = expectThrows(UserError.class, () -> {
parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"});
});
assertThat(e.getMessage(), containsString("must be before any parameters starting with --"));
}
private void registerProperties(String ... systemProperties) {

View File

@ -71,9 +71,9 @@ public class CliToolTests extends CliToolTestCase {
final AtomicReference<Boolean> executed = new AtomicReference<>(false);
final NamedCommand cmd = new NamedCommand("cmd", terminal) {
@Override
public CliTool.ExitStatus execute(Settings settings, Environment env) {
public CliTool.ExitStatus execute(Settings settings, Environment env) throws UserError {
executed.set(true);
return CliTool.ExitStatus.USAGE;
throw new UserError(CliTool.ExitStatus.USAGE, "bad usage");
}
};
SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
@ -82,39 +82,7 @@ public class CliToolTests extends CliToolTestCase {
assertCommandHasBeenExecuted(executed);
}
public void testIOError() throws Exception {
Terminal terminal = new MockTerminal();
final AtomicReference<Boolean> executed = new AtomicReference<>(false);
final NamedCommand cmd = new NamedCommand("cmd", terminal) {
@Override
public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
executed.set(true);
throw new IOException("io error");
}
};
SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
CliTool.ExitStatus status = tool.execute();
assertStatus(status, CliTool.ExitStatus.IO_ERROR);
assertCommandHasBeenExecuted(executed);
}
public void testCodeError() throws Exception {
Terminal terminal = new MockTerminal();
final AtomicReference<Boolean> executed = new AtomicReference<>(false);
final NamedCommand cmd = new NamedCommand("cmd", terminal) {
@Override
public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
executed.set(true);
throw new Exception("random error");
}
};
SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
CliTool.ExitStatus status = tool.execute();
assertStatus(status, CliTool.ExitStatus.CODE_ERROR);
assertCommandHasBeenExecuted(executed);
}
public void testMultiCommand() {
public void testMultiCommand() throws Exception {
Terminal terminal = new MockTerminal();
int count = randomIntBetween(2, 7);
List<AtomicReference<Boolean>> executed = new ArrayList<>(count);
@ -141,7 +109,7 @@ public class CliToolTests extends CliToolTestCase {
}
}
public void testMultiCommandUnknownCommand() {
public void testMultiCommandUnknownCommand() throws Exception {
Terminal terminal = new MockTerminal();
int count = randomIntBetween(2, 7);
List<AtomicReference<Boolean>> executed = new ArrayList<>(count);
@ -184,7 +152,7 @@ public class CliToolTests extends CliToolTestCase {
assertThat(terminal.getTerminalOutput(), hasItem(containsString("cmd1 help")));
}
public void testMultiCommandToolHelp() {
public void testMultiCommandToolHelp() throws Exception {
CaptureOutputTerminal terminal = new CaptureOutputTerminal();
NamedCommand[] cmds = new NamedCommand[2];
cmds[0] = new NamedCommand("cmd0", terminal) {
@ -206,7 +174,7 @@ public class CliToolTests extends CliToolTestCase {
assertThat(terminal.getTerminalOutput(), hasItem(containsString("tool help")));
}
public void testMultiCommandCmdHelp() {
public void testMultiCommandCmdHelp() throws Exception {
CaptureOutputTerminal terminal = new CaptureOutputTerminal();
NamedCommand[] cmds = new NamedCommand[2];
cmds[0] = new NamedCommand("cmd0", terminal) {
@ -228,31 +196,19 @@ public class CliToolTests extends CliToolTestCase {
assertThat(terminal.getTerminalOutput(), hasItem(containsString("cmd1 help")));
}
public void testThatThrowExceptionCanBeLogged() throws Exception {
public void testNonUserErrorPropagates() throws Exception {
CaptureOutputTerminal terminal = new CaptureOutputTerminal();
NamedCommand cmd = new NamedCommand("cmd", terminal) {
@Override
public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
throw new ElasticsearchException("error message");
throw new IOException("error message");
}
};
SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
assertStatus(tool.execute(), CliTool.ExitStatus.CODE_ERROR);
assertThat(terminal.getTerminalOutput(), hasSize(1));
assertThat(terminal.getTerminalOutput(), hasItem(containsString("error message")));
// set env... and log stack trace
try {
System.setProperty(Terminal.DEBUG_SYSTEM_PROPERTY, "true");
terminal = new CaptureOutputTerminal();
assertStatus(new SingleCmdTool("tool", terminal, cmd).execute(), CliTool.ExitStatus.CODE_ERROR);
assertThat(terminal.getTerminalOutput(), hasSize(2));
assertThat(terminal.getTerminalOutput(), hasItem(containsString("error message")));
// This class must be part of the stack strace
assertThat(terminal.getTerminalOutput(), hasItem(containsString(getClass().getName())));
} finally {
System.clearProperty(Terminal.DEBUG_SYSTEM_PROPERTY);
}
IOException e = expectThrows(IOException.class, () -> {
tool.execute();
});
assertEquals("error message", e.getMessage());
}
public void testMultipleLaunch() throws Exception {

View File

@ -19,8 +19,8 @@
package org.elasticsearch.plugins;
import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.DirectoryStream;
import java.nio.file.FileAlreadyExistsException;
@ -28,6 +28,7 @@ import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFileAttributes;
@ -43,6 +44,7 @@ import org.elasticsearch.Version;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolTestCase;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
@ -193,6 +195,16 @@ public class InstallPluginCommandTests extends ESTestCase {
assertPlugin("fake", pluginDir, env);
}
public void testSpaceInUrl() throws Exception {
Environment env = createEnv();
Path pluginDir = createTempDir();
String pluginZip = createPlugin("fake", pluginDir);
Path pluginZipWithSpaces = createTempFile("foo bar", ".zip");
Files.copy(new URL(pluginZip).openStream(), pluginZipWithSpaces, StandardCopyOption.REPLACE_EXISTING);
installPlugin(pluginZipWithSpaces.toUri().toURL().toString(), env);
assertPlugin("fake", pluginDir, env);
}
public void testPluginsDirMissing() throws Exception {
Environment env = createEnv();
Files.delete(env.pluginsFile());
@ -211,7 +223,7 @@ public class InstallPluginCommandTests extends ESTestCase {
IOException e = expectThrows(IOException.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("Plugins directory is read only"));
assertTrue(e.getMessage(), e.getMessage().contains(env.pluginsFile().toString()));
}
assertInstallCleaned(env);
}
@ -219,7 +231,7 @@ public class InstallPluginCommandTests extends ESTestCase {
public void testBuiltinModule() throws Exception {
Environment env = createEnv();
String pluginZip = createPlugin("lang-groovy", createTempDir());
IOException e = expectThrows(IOException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("is a system module"));
@ -288,7 +300,7 @@ public class InstallPluginCommandTests extends ESTestCase {
Environment env = createEnv();
String pluginZip = createPlugin("fake", createTempDir());
installPlugin(pluginZip, env);
IOException e = expectThrows(IOException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("already exists"));
@ -312,7 +324,7 @@ public class InstallPluginCommandTests extends ESTestCase {
Path binDir = pluginDir.resolve("bin");
Files.createFile(binDir);
String pluginZip = createPlugin("fake", pluginDir);
IOException e = expectThrows(IOException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("not a directory"));
@ -326,7 +338,7 @@ public class InstallPluginCommandTests extends ESTestCase {
Files.createDirectories(dirInBinDir);
Files.createFile(dirInBinDir.resolve("somescript"));
String pluginZip = createPlugin("fake", pluginDir);
IOException e = expectThrows(IOException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("Directories not allowed in bin dir for plugin"));
@ -401,7 +413,7 @@ public class InstallPluginCommandTests extends ESTestCase {
Path configDir = pluginDir.resolve("config");
Files.createFile(configDir);
String pluginZip = createPlugin("fake", pluginDir);
IOException e = expectThrows(IOException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("not a directory"));
@ -415,7 +427,7 @@ public class InstallPluginCommandTests extends ESTestCase {
Files.createDirectories(dirInConfigDir);
Files.createFile(dirInConfigDir.resolve("myconfig.yml"));
String pluginZip = createPlugin("fake", pluginDir);
IOException e = expectThrows(IOException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
installPlugin(pluginZip, env);
});
assertTrue(e.getMessage(), e.getMessage().contains("Directories not allowed in config dir for plugin"));

View File

@ -28,6 +28,7 @@ import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolTestCase;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
@ -66,7 +67,7 @@ public class RemovePluginCommandTests extends ESTestCase {
public void testMissing() throws Exception {
Environment env = createEnv();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
removePlugin("dne", env);
});
assertTrue(e.getMessage(), e.getMessage().contains("Plugin dne not found"));
@ -101,9 +102,10 @@ public class RemovePluginCommandTests extends ESTestCase {
public void testBinNotDir() throws Exception {
Environment env = createEnv();
Files.createDirectories(env.pluginsFile().resolve("elasticsearch"));
IllegalStateException e = expectThrows(IllegalStateException.class, () -> {
UserError e = expectThrows(UserError.class, () -> {
removePlugin("elasticsearch", env);
});
assertTrue(e.getMessage(), e.getMessage().contains("not a directory"));
assertTrue(Files.exists(env.pluginsFile().resolve("elasticsearch"))); // did not remove
assertTrue(Files.exists(env.binFile().resolve("elasticsearch")));
assertRemoveCleaned(env);