From 8cd919c6876f3b380137eb97bc7b37f0058e9c2f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 29 Feb 2016 19:52:42 -0800 Subject: [PATCH 01/16] Added jopt simple option parser and switched plugin cli to use it --- core/build.gradle | 3 +- .../bootstrap/BootstrapCLIParser.java | 11 +- .../java/org/elasticsearch/cli/Command.java | 111 ++++++++++++++++++ .../java/org/elasticsearch/cli/ExitCodes.java | 42 +++++++ .../org/elasticsearch/cli/MultiCommand.java | 73 ++++++++++++ .../org/elasticsearch/cli/TestCommand.java | 41 +++++++ .../{common => }/cli/UserError.java | 10 +- .../org/elasticsearch/common/cli/CliTool.java | 6 +- .../elasticsearch/common/cli/Terminal.java | 21 +++- .../plugins/InstallPluginCommand.java | 73 +++++++----- .../plugins/ListPluginsCommand.java | 17 ++- .../org/elasticsearch/plugins/PluginCli.java | 103 ++-------------- .../plugins/RemovePluginCommand.java | 51 +++++--- .../elasticsearch/plugins/PluginCliTests.java | 2 + .../bootstrap/BootstrapCliParserTests.java | 3 +- .../common/cli/CliToolTests.java | 4 +- .../plugins/InstallPluginCommandTests.java | 7 +- .../plugins/ListPluginsCommandTests.java | 6 +- .../plugins/RemovePluginCommandTests.java | 6 +- .../common/cli/CliToolTestCase.java | 6 + 20 files changed, 426 insertions(+), 170 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/cli/Command.java create mode 100644 core/src/main/java/org/elasticsearch/cli/ExitCodes.java create mode 100644 core/src/main/java/org/elasticsearch/cli/MultiCommand.java create mode 100644 core/src/main/java/org/elasticsearch/cli/TestCommand.java rename core/src/main/java/org/elasticsearch/{common => }/cli/UserError.java (79%) diff --git a/core/build.gradle b/core/build.gradle index e1511a9cdd1..f79c2a7623b 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -48,7 +48,8 @@ dependencies { compile 'org.elasticsearch:securesm:1.0' // utilities - compile 'commons-cli:commons-cli:1.3.1' + compile 'commons-cli:commons-cli:1.3.1' // nocommit: remove the old! + compile 'net.sf.jopt-simple:jopt-simple:4.9' compile 'com.carrotsearch:hppc:0.7.1' // time handling, remove with java 8 time diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java index ca67fc91132..ec11a773ccc 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java @@ -27,7 +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.cli.UserError; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.monitor.jvm.JvmInfo; @@ -37,7 +37,6 @@ import java.util.Iterator; import java.util.Locale; import java.util.Map; import java.util.Properties; -import java.util.Set; import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder; @@ -138,11 +137,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 UserError(ExitStatus.USAGE, + throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "] starting with \"-D\", \"-d\" or \"-p\" must be before any parameters starting with --" ); } else { - throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "]does not start with --"); + throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "]does not start with --"); } } // if there is no = sign, we have to get the next argu @@ -156,11 +155,11 @@ final class BootstrapCLIParser extends CliTool { if (iterator.hasNext()) { String value = iterator.next(); if (value.startsWith("--")) { - throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value"); + throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "] needs value"); } properties.put("es." + arg, value); } else { - throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value"); + throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "] needs value"); } } } diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java new file mode 100644 index 00000000000..bc44a8eb635 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cli/Command.java @@ -0,0 +1,111 @@ +/* + * 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.cli; + +import java.io.IOException; +import java.util.Arrays; + +import joptsimple.OptionException; +import joptsimple.OptionParser; +import joptsimple.OptionSet; +import joptsimple.OptionSpec; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.cli.Terminal; + +/** + * An action to execute within a cli. + */ +public abstract class Command { + + /** A description of the command, used in the help output. */ + protected final String description; + + /** The option parser for this command. */ + protected final OptionParser parser = new OptionParser(); + + private final OptionSpec helpOption = parser.acceptsAll(Arrays.asList("h", "help"), "show help").forHelp(); + private final OptionSpec silentOption = parser.acceptsAll(Arrays.asList("s", "silent"), "show minimal output"); + private final OptionSpec verboseOption = parser.acceptsAll(Arrays.asList("v", "verbose"), "show verbose output"); + + public Command(String description) { + this.description = description; + } + + /** Parses options for this command from args and executes it. */ + public final int main(String[] args, Terminal terminal) throws Exception { + + final OptionSet options; + try { + options = parser.parse(args); + } catch (OptionException e) { + printHelp(terminal); + terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + return ExitCodes.USAGE; + } + + if (options.has(helpOption)) { + printHelp(terminal); + return ExitCodes.OK; + } + + if (options.has(silentOption)) { + if (options.has(verboseOption)) { + // mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it + printHelp(terminal); + terminal.println(Terminal.Verbosity.SILENT, "ERROR: Cannot specify -s and -v together"); + return ExitCodes.USAGE; + } + terminal.setVerbosity(Terminal.Verbosity.SILENT); + } else if (options.has(verboseOption)) { + terminal.setVerbosity(Terminal.Verbosity.VERBOSE); + } else { + terminal.setVerbosity(Terminal.Verbosity.NORMAL); + } + + try { + return execute(terminal, options); + } catch (UserError e) { + terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + return e.exitCode; + } + } + + /** Prints a help message for the command to the terminal. */ + private void printHelp(Terminal terminal) throws IOException { + terminal.println(description); + terminal.println(""); + printAdditionalHelp(terminal); + parser.printHelpOn(terminal.getWriter()); + } + + /** Prints additional help information, specific to the command */ + protected void printAdditionalHelp(Terminal terminal) {} + + @SuppressForbidden(reason = "Allowed to exit explicitly from #main()") + protected static void exit(int status) { + System.exit(status); + } + + /** + * Executes this command. + * + * Any runtime user errors (like an input file that does not exist), should throw a {@link UserError}. */ + protected abstract int execute(Terminal terminal, OptionSet options) throws Exception; +} diff --git a/core/src/main/java/org/elasticsearch/cli/ExitCodes.java b/core/src/main/java/org/elasticsearch/cli/ExitCodes.java new file mode 100644 index 00000000000..d08deb8b1ad --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cli/ExitCodes.java @@ -0,0 +1,42 @@ +/* + * 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.cli; + +/** + * POSIX exit codes. + */ +public class ExitCodes { + public static final int OK = 0; + public static final int USAGE = 64; /* command line usage error */ + public static final int DATA_ERROR = 65; /* data format error */ + public static final int NO_INPUT = 66; /* cannot open input */ + public static final int NO_USER = 67; /* addressee unknown */ + public static final int NO_HOST = 68; /* host name unknown */ + public static final int UNAVAILABLE = 69; /* service unavailable */ + public static final int CODE_ERROR = 70; /* internal software error */ + public static final int CANT_CREATE = 73; /* can't create (user) output file */ + public static final int IO_ERROR = 74; /* input/output error */ + public static final int TEMP_FAILURE = 75; /* temp failure; user is invited to retry */ + public static final int PROTOCOL = 76; /* remote error in protocol */ + public static final int NOPERM = 77; /* permission denied */ + public static final int CONFIG = 78; /* configuration error */ + + private ExitCodes() { /* no instance, just constants */ } +} diff --git a/core/src/main/java/org/elasticsearch/cli/MultiCommand.java b/core/src/main/java/org/elasticsearch/cli/MultiCommand.java new file mode 100644 index 00000000000..94c403d57d0 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cli/MultiCommand.java @@ -0,0 +1,73 @@ +/* + * 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.cli; + +import java.io.IOException; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.Map; + +import joptsimple.NonOptionArgumentSpec; +import joptsimple.OptionSet; +import org.elasticsearch.common.cli.Terminal; + +/** + * A cli tool which is made up of multiple subcommands. + */ +public class MultiCommand extends Command { + + protected final Map subcommands = new LinkedHashMap<>(); + + private final NonOptionArgumentSpec arguments = parser.nonOptions("command"); + + public MultiCommand(String description) { + super(description); + parser.posixlyCorrect(true); + } + + @Override + protected void printAdditionalHelp(Terminal terminal) { + if (subcommands.isEmpty()) { + throw new IllegalStateException("No subcommands configured"); + } + terminal.println("Commands"); + terminal.println("--------"); + for (Map.Entry subcommand : subcommands.entrySet()) { + terminal.println(subcommand.getKey() + " - " + subcommand.getValue().description); + } + terminal.println(""); + } + + @Override + protected int execute(Terminal terminal, OptionSet options) throws Exception { + if (subcommands.isEmpty()) { + throw new IllegalStateException("No subcommands configured"); + } + String[] args = arguments.values(options).toArray(new String[0]); + if (args.length == 0) { + throw new UserError(ExitCodes.USAGE, "Missing command"); + } + Command subcommand = subcommands.get(args[0]); + if (subcommand == null) { + throw new UserError(ExitCodes.USAGE, "Unknown command [" + args[0] + "]"); + } + return subcommand.main(Arrays.copyOfRange(args, 1, args.length), terminal); + } +} diff --git a/core/src/main/java/org/elasticsearch/cli/TestCommand.java b/core/src/main/java/org/elasticsearch/cli/TestCommand.java new file mode 100644 index 00000000000..fe3fa5c6b8c --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cli/TestCommand.java @@ -0,0 +1,41 @@ +/* + * 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.cli; + +import joptsimple.OptionSet; +import org.elasticsearch.common.cli.Terminal; + +public class TestCommand extends Command { + + public static void main(String[] args) throws Exception { + exit(new TestCommand().main(args, Terminal.DEFAULT)); + } + + public TestCommand() { + super("some test cli"); + parser.accepts("foo", "some option"); + } + + @Override + protected int execute(Terminal terminal, OptionSet options) throws Exception { + terminal.println("running"); + return ExitCodes.OK; + } +} diff --git a/core/src/main/java/org/elasticsearch/common/cli/UserError.java b/core/src/main/java/org/elasticsearch/cli/UserError.java similarity index 79% rename from core/src/main/java/org/elasticsearch/common/cli/UserError.java rename to core/src/main/java/org/elasticsearch/cli/UserError.java index ad709830885..2a4f2bf1233 100644 --- a/core/src/main/java/org/elasticsearch/common/cli/UserError.java +++ b/core/src/main/java/org/elasticsearch/cli/UserError.java @@ -17,19 +17,19 @@ * under the License. */ -package org.elasticsearch.common.cli; +package org.elasticsearch.cli; /** - * An exception representing a user fixable problem in {@link CliTool} usage. + * An exception representing a user fixable problem in {@link Command} usage. */ public class UserError extends Exception { /** The exist status the cli should use when catching this user error. */ - public final CliTool.ExitStatus exitStatus; + public final int exitCode; /** Constructs a UserError with an exit status and message to show the user. */ - public UserError(CliTool.ExitStatus exitStatus, String msg) { + public UserError(int exitCode, String msg) { super(msg); - this.exitStatus = exitStatus; + this.exitCode = exitCode; } } diff --git a/core/src/main/java/org/elasticsearch/common/cli/CliTool.java b/core/src/main/java/org/elasticsearch/common/cli/CliTool.java index 2ea01f45068..ba2007813d5 100644 --- a/core/src/main/java/org/elasticsearch/common/cli/CliTool.java +++ b/core/src/main/java/org/elasticsearch/common/cli/CliTool.java @@ -26,6 +26,7 @@ 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.cli.UserError; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.node.internal.InternalSettingsPreparer; @@ -143,7 +144,8 @@ public abstract class CliTool { return parse(cmd, args).execute(settings, env); } catch (UserError error) { terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + error.getMessage()); - return error.exitStatus; + return ExitStatus.USAGE; + //return error.exitCode; } } @@ -163,7 +165,7 @@ public abstract class CliTool { } 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()); + throw new UserError(ExitStatus.USAGE.status(), e.toString()); } if (cli.hasOption("v")) { diff --git a/core/src/main/java/org/elasticsearch/common/cli/Terminal.java b/core/src/main/java/org/elasticsearch/common/cli/Terminal.java index 8d4a8036bdf..27dd2f7b87f 100644 --- a/core/src/main/java/org/elasticsearch/common/cli/Terminal.java +++ b/core/src/main/java/org/elasticsearch/common/cli/Terminal.java @@ -23,12 +23,14 @@ import java.io.BufferedReader; import java.io.Console; import java.io.IOException; import java.io.InputStreamReader; +import java.io.PrintWriter; +import java.io.Writer; import java.nio.charset.Charset; import org.elasticsearch.common.SuppressForbidden; /** - * A Terminal wraps access to reading input and writing output for a {@link CliTool}. + * A Terminal wraps access to reading input and writing output for a cli. * * The available methods are similar to those of {@link Console}, with the ability * to read either normal text or a password, and the ability to print a line @@ -53,7 +55,7 @@ public abstract class Terminal { private Verbosity verbosity = Verbosity.NORMAL; /** Sets the verbosity of the terminal. */ - void setVerbosity(Verbosity verbosity) { + public void setVerbosity(Verbosity verbosity) { this.verbosity = verbosity; } @@ -63,6 +65,9 @@ public abstract class Terminal { /** Reads password text from the terminal input. See {@link Console#readPassword()}}. */ public abstract char[] readSecret(String prompt); + /** Returns a Writer which can be used to write to the terminal directly. */ + public abstract PrintWriter getWriter(); + /** Print a message directly to the terminal. */ protected abstract void doPrint(String msg); @@ -86,6 +91,11 @@ public abstract class Terminal { return console != null; } + @Override + public PrintWriter getWriter() { + return console.writer(); + } + @Override public void doPrint(String msg) { console.printf("%s", msg); @@ -105,6 +115,8 @@ public abstract class Terminal { private static class SystemTerminal extends Terminal { + private static final PrintWriter writer = new PrintWriter(System.out); + @Override @SuppressForbidden(reason = "System#out") public void doPrint(String msg) { @@ -112,6 +124,11 @@ public abstract class Terminal { System.out.flush(); } + @Override + public PrintWriter getWriter() { + return writer; + } + @Override public String readText(String text) { doPrint(text); diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 767f6d42179..977d89a3418 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -19,16 +19,19 @@ package org.elasticsearch.plugins; +import joptsimple.OptionSet; +import joptsimple.OptionSpec; import org.apache.lucene.util.IOUtils; import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.bootstrap.JarHell; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.common.cli.UserError; +import org.elasticsearch.cli.UserError; import org.elasticsearch.common.hash.MessageDigests; import org.elasticsearch.common.io.FileSystemUtils; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import java.io.BufferedReader; @@ -88,7 +91,7 @@ import static org.elasticsearch.common.util.set.Sets.newHashSet; * elasticsearch config directory, using the name of the plugin. If any files to be installed * already exist, they will be skipped. */ -class InstallPluginCommand extends CliTool.Command { +class InstallPluginCommand extends Command { private static final String PROPERTY_SUPPORT_STAGING_URLS = "es.plugins.staging"; @@ -119,17 +122,33 @@ class InstallPluginCommand extends CliTool.Command { "repository-s3", "store-smb")); - private final String pluginId; - private final boolean batch; + private final Environment env; + private final OptionSpec batchOption; + private final OptionSpec arguments; - InstallPluginCommand(Terminal terminal, String pluginId, boolean batch) { - super(terminal); - this.pluginId = pluginId; - this.batch = batch; + InstallPluginCommand(Environment env) { + super("Install a plugin"); + this.env = env; + this.batchOption = parser.acceptsAll(Arrays.asList("b", "batch"), + "Enable batch mode explicitly, automatic confirmation of security permission"); + this.arguments = parser.nonOptions("plugin id"); } @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { + protected int execute(Terminal terminal, OptionSet options) throws Exception { + // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args + List args = arguments.values(options); + if (args.size() != 1) { + throw new UserError(ExitCodes.USAGE, "Must supply a single plugin id argument"); + } + String pluginId = args.get(0); + boolean isBatch = options.has(batchOption) || System.console() == null; + execute(terminal, pluginId, isBatch); + return ExitCodes.OK; + } + + // pkg private for testing + void execute(Terminal terminal, String pluginId, boolean isBatch) throws Exception { // TODO: remove this leniency!! is it needed anymore? if (Files.exists(env.pluginsFile()) == false) { @@ -137,15 +156,13 @@ class InstallPluginCommand extends CliTool.Command { Files.createDirectory(env.pluginsFile()); } - Path pluginZip = download(pluginId, env.tmpFile()); + Path pluginZip = download(terminal, pluginId, env.tmpFile()); Path extractedZip = unzip(pluginZip, env.pluginsFile()); - install(extractedZip, env); - - return CliTool.ExitStatus.OK; + install(terminal, isBatch, extractedZip); } /** Downloads the plugin and returns the file it was downloaded to. */ - private Path download(String pluginId, Path tmpDir) throws Exception { + private Path download(Terminal terminal, String pluginId, Path tmpDir) throws Exception { if (OFFICIAL_PLUGINS.contains(pluginId)) { final String version = Version.CURRENT.toString(); final String url; @@ -195,14 +212,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 UserError(CliTool.ExitStatus.IO_ERROR, "Invalid checksum file at " + checksumUrl); + throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "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 UserError(CliTool.ExitStatus.IO_ERROR, "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum); + throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum); } return zip; @@ -244,13 +261,13 @@ class InstallPluginCommand extends CliTool.Command { Files.delete(zip); if (hasEsDir == false) { IOUtils.rm(target); - throw new UserError(CliTool.ExitStatus.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip"); + throw new UserError(CliTool.ExitStatus.DATA_ERROR.status(), "`elasticsearch` directory is missing in the plugin zip"); } return target; } /** Load information about the plugin, and verify it can be installed with no errors. */ - private PluginInfo verify(Path pluginRoot, Environment env) throws Exception { + private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) throws Exception { // read and validate the plugin descriptor PluginInfo info = PluginInfo.readFromProperties(pluginRoot); terminal.println(VERBOSE, info.toString()); @@ -258,7 +275,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 UserError(CliTool.ExitStatus.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module"); + throw new UserError(CliTool.ExitStatus.USAGE.status(), "plugin '" + info.getName() + "' cannot be installed like this, it is a system module"); } // check for jar hell before any copying @@ -268,7 +285,7 @@ class InstallPluginCommand extends CliTool.Command { // if it exists, confirm or warn the user Path policy = pluginRoot.resolve(PluginInfo.ES_PLUGIN_POLICY); if (Files.exists(policy)) { - PluginSecurity.readPolicy(policy, terminal, env, batch); + PluginSecurity.readPolicy(policy, terminal, env, isBatch); } return info; @@ -305,16 +322,16 @@ class InstallPluginCommand extends CliTool.Command { * Installs the plugin from {@code tmpRoot} into the plugins dir. * If the plugin has a bin dir and/or a config dir, those are copied. */ - private void install(Path tmpRoot, Environment env) throws Exception { + private void install(Terminal terminal, boolean isBatch, Path tmpRoot) throws Exception { List deleteOnFailure = new ArrayList<>(); deleteOnFailure.add(tmpRoot); try { - PluginInfo info = verify(tmpRoot, env); + PluginInfo info = verify(terminal, tmpRoot, isBatch); final Path destination = env.pluginsFile().resolve(info.getName()); if (Files.exists(destination)) { - throw new UserError(CliTool.ExitStatus.USAGE, "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command"); + throw new UserError(CliTool.ExitStatus.USAGE.status(), "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command"); } Path tmpBinDir = tmpRoot.resolve("bin"); @@ -347,7 +364,7 @@ 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 Exception { if (Files.isDirectory(tmpBinDir) == false) { - throw new UserError(CliTool.ExitStatus.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory"); + throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "bin in plugin " + info.getName() + " is not a directory"); } Files.createDirectory(destBinDir); @@ -365,7 +382,7 @@ class InstallPluginCommand extends CliTool.Command { try (DirectoryStream stream = Files.newDirectoryStream(tmpBinDir)) { for (Path srcFile : stream) { if (Files.isDirectory(srcFile)) { - throw new UserError(CliTool.ExitStatus.DATA_ERROR, "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName()); + throw new UserError(CliTool.ExitStatus.DATA_ERROR.status(), "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName()); } Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile)); @@ -386,7 +403,7 @@ class InstallPluginCommand extends CliTool.Command { */ private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws Exception { if (Files.isDirectory(tmpConfigDir) == false) { - throw new UserError(CliTool.ExitStatus.IO_ERROR, "config in plugin " + info.getName() + " is not a directory"); + throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "config in plugin " + info.getName() + " is not a directory"); } // create the plugin's config dir "if necessary" @@ -395,7 +412,7 @@ class InstallPluginCommand extends CliTool.Command { try (DirectoryStream stream = Files.newDirectoryStream(tmpConfigDir)) { for (Path srcFile : stream) { if (Files.isDirectory(srcFile)) { - throw new UserError(CliTool.ExitStatus.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName()); + throw new UserError(CliTool.ExitStatus.DATA_ERROR.status(), "Directories not allowed in config dir for plugin " + info.getName()); } Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile)); diff --git a/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java b/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java index 6abed4e6bc2..142a18cbde5 100644 --- a/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java @@ -24,22 +24,27 @@ import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; +import joptsimple.OptionSet; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; /** * A command for the plugin cli to list plugins installed in elasticsearch. */ -class ListPluginsCommand extends CliTool.Command { +class ListPluginsCommand extends Command { - ListPluginsCommand(Terminal terminal) { - super(terminal); + private final Environment env; + + ListPluginsCommand(Environment env) { + super("Lists installed elasticsearch plugins"); + this.env = env; } @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { + public int execute(Terminal terminal, OptionSet options) throws Exception { if (Files.exists(env.pluginsFile()) == false) { throw new IOException("Plugins directory missing: " + env.pluginsFile()); } @@ -51,6 +56,6 @@ class ListPluginsCommand extends CliTool.Command { } } - return CliTool.ExitStatus.OK; + return ExitCodes.OK; } } diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginCli.java b/core/src/main/java/org/elasticsearch/plugins/PluginCli.java index 30a36501a61..9f2e432a438 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginCli.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginCli.java @@ -19,106 +19,29 @@ package org.elasticsearch.plugins; -import org.apache.commons.cli.CommandLine; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.CliToolConfig; +import org.apache.log4j.BasicConfigurator; +import org.apache.log4j.varia.NullAppender; +import org.elasticsearch.cli.MultiCommand; import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.common.logging.log4j.LogConfigurator; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.node.internal.InternalSettingsPreparer; -import java.util.Locale; - -import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; -import static org.elasticsearch.common.cli.CliToolConfig.Builder.option; - /** * A cli tool for adding, removing and listing plugins for elasticsearch. */ -public class PluginCli extends CliTool { +public class PluginCli extends MultiCommand { - // commands - private static final String LIST_CMD_NAME = "list"; - private static final String INSTALL_CMD_NAME = "install"; - private static final String REMOVE_CMD_NAME = "remove"; - - // usage config - private static final CliToolConfig.Cmd LIST_CMD = cmd(LIST_CMD_NAME, ListPluginsCommand.class).build(); - private static final CliToolConfig.Cmd INSTALL_CMD = cmd(INSTALL_CMD_NAME, InstallPluginCommand.class) - .options(option("b", "batch").required(false)) - .build(); - private static final CliToolConfig.Cmd REMOVE_CMD = cmd(REMOVE_CMD_NAME, RemovePluginCommand.class).build(); - - static final CliToolConfig CONFIG = CliToolConfig.config("plugin", PluginCli.class) - .cmds(LIST_CMD, INSTALL_CMD, REMOVE_CMD) - .build(); + public PluginCli(Environment env) { + super("A tool for managing installed elasticsearch plugins"); + subcommands.put("list", new ListPluginsCommand(env)); + subcommands.put("install", new InstallPluginCommand(env)); + subcommands.put("remove", new RemovePluginCommand(env)); + } 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 - // same terminal. - // The reason for this is that the plugin cli cannot be configured with a file appender because when the plugin command is - // executed there is no way of knowing where the logfiles should be placed. For example, if elasticsearch - // is run as service then the logs should be at /var/log/elasticsearch but when started from the tar they should be at es.home/logs. - // Therefore we print to Terminal. - Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.builder() - .put("appender.terminal.type", "terminal") - .put("rootLogger", "${es.logger.level}, terminal") - .put("es.logger.level", loggerLevel) - .build(), Terminal.DEFAULT); - // configure but do not read the logging conf file - LogConfigurator.configure(env.settings(), false); - int status = new PluginCli(Terminal.DEFAULT).execute(args).status(); - exit(status); - } - - @SuppressForbidden(reason = "Allowed to exit explicitly from #main()") - private static void exit(int status) { - System.exit(status); - } - - PluginCli(Terminal terminal) { - super(CONFIG, terminal); - } - - @Override - protected Command parse(String cmdName, CommandLine cli) throws Exception { - switch (cmdName.toLowerCase(Locale.ROOT)) { - case LIST_CMD_NAME: - return new ListPluginsCommand(terminal); - case INSTALL_CMD_NAME: - return parseInstallPluginCommand(cli); - case REMOVE_CMD_NAME: - return parseRemovePluginCommand(cli); - default: - assert false : "can't get here as cmd name is validated before this method is called"; - return exitCmd(ExitStatus.USAGE); - } - } - - private Command parseInstallPluginCommand(CommandLine cli) { - String[] args = cli.getArgs(); - if (args.length != 1) { - return exitCmd(ExitStatus.USAGE, terminal, "Must supply a single plugin id argument"); - } - - boolean batch = System.console() == null; - if (cli.hasOption("b")) { - batch = true; - } - - return new InstallPluginCommand(terminal, args[0], batch); - } - - private Command parseRemovePluginCommand(CommandLine cli) { - String[] args = cli.getArgs(); - if (args.length != 1) { - return exitCmd(ExitStatus.USAGE, terminal, "Must supply a single plugin name argument"); - } - - return new RemovePluginCommand(terminal, args[0]); + BasicConfigurator.configure(new NullAppender()); + Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, Terminal.DEFAULT); + exit(new PluginCli(env).main(args, Terminal.DEFAULT)); } } diff --git a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java index 8ce1056bbfd..10a73a0fc9a 100644 --- a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java @@ -19,40 +19,57 @@ package org.elasticsearch.plugins; -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; - import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.List; +import joptsimple.OptionSet; +import joptsimple.OptionSpec; +import org.apache.lucene.util.IOUtils; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.UserError; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.cli.CliTool; +import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.env.Environment; + import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE; /** * A command for the plugin cli to remove a plugin from elasticsearch. */ -class RemovePluginCommand extends CliTool.Command { - private final String pluginName; +class RemovePluginCommand extends Command { - public RemovePluginCommand(Terminal terminal, String pluginName) { - super(terminal); - this.pluginName = pluginName; + private final Environment env; + private final OptionSpec arguments; + + RemovePluginCommand(Environment env) { + super("Removes a plugin from elasticsearch"); + this.env = env; + this.arguments = parser.nonOptions("plugin name"); } @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { + public int execute(Terminal terminal, OptionSet options) throws Exception { + // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args + List args = arguments.values(options); + if (args.size() != 1) { + throw new UserError(ExitCodes.USAGE, "Must supply a single plugin id argument"); + } + execute(terminal, args.get(0)); + return ExitCodes.OK; + } + + // pkg private for testing + void execute(Terminal terminal, String pluginName) throws Exception { terminal.println("-> Removing " + Strings.coalesceToEmpty(pluginName) + "..."); Path pluginDir = env.pluginsFile().resolve(pluginName); if (Files.exists(pluginDir) == false) { - throw new UserError(CliTool.ExitStatus.USAGE, "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins."); + throw new UserError(CliTool.ExitStatus.USAGE.status(), "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins."); } List pluginPaths = new ArrayList<>(); @@ -60,7 +77,7 @@ class RemovePluginCommand extends CliTool.Command { Path pluginBinDir = env.binFile().resolve(pluginName); if (Files.exists(pluginBinDir)) { if (Files.isDirectory(pluginBinDir) == false) { - throw new UserError(CliTool.ExitStatus.IO_ERROR, "Bin dir for " + pluginName + " is not a directory"); + throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "Bin dir for " + pluginName + " is not a directory"); } pluginPaths.add(pluginBinDir); terminal.println(VERBOSE, "Removing: " + pluginBinDir); @@ -72,7 +89,5 @@ class RemovePluginCommand extends CliTool.Command { pluginPaths.add(tmpPluginDir); IOUtils.rm(pluginPaths.toArray(new Path[pluginPaths.size()])); - - return CliTool.ExitStatus.OK; } } diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java index 3a121590083..8973a9be3e2 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java @@ -28,6 +28,7 @@ import static org.hamcrest.Matchers.is; public class PluginCliTests extends CliToolTestCase { 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"); @@ -46,5 +47,6 @@ public class PluginCliTests extends CliToolTestCase { terminal.getTerminalOutput().clear(); assertThat(new PluginCli(terminal).execute(args("list -h")), is(OK_AND_EXIT)); assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-list.help"); + */ } } diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java index 012af99cef0..c1d894710a3 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java @@ -24,7 +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.cli.UserError; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.monitor.jvm.JvmInfo; import org.hamcrest.Matcher; @@ -37,7 +37,6 @@ import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Properties; import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK; import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java index 5033914632a..70d507853fa 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java @@ -20,7 +20,7 @@ package org.elasticsearch.common.cli; import org.apache.commons.cli.CommandLine; -import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.cli.UserError; import org.elasticsearch.common.Strings; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.settings.Settings; @@ -70,7 +70,7 @@ public class CliToolTests extends CliToolTestCase { @Override public CliTool.ExitStatus execute(Settings settings, Environment env) throws UserError { executed.set(true); - throw new UserError(CliTool.ExitStatus.USAGE, "bad usage"); + throw new UserError(CliTool.ExitStatus.USAGE.status(), "bad usage"); } }; SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 66dfa67ccbd..e51b13e969e 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -36,6 +36,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -44,10 +45,11 @@ import java.util.zip.ZipOutputStream; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.Version; +import org.elasticsearch.cli.ExitCodes; 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.cli.UserError; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; @@ -118,8 +120,7 @@ public class InstallPluginCommandTests extends ESTestCase { static CliToolTestCase.CaptureOutputTerminal installPlugin(String pluginUrl, Environment env) throws Exception { CliToolTestCase.CaptureOutputTerminal terminal = new CliToolTestCase.CaptureOutputTerminal(Terminal.Verbosity.NORMAL); - CliTool.ExitStatus status = new InstallPluginCommand(terminal, pluginUrl, true).execute(env.settings(), env); - assertEquals(CliTool.ExitStatus.OK, status); + new InstallPluginCommand(env).execute(terminal, pluginUrl, true); return terminal; } diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java index c68e207c0c3..7ffdd8545df 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.List; import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.CliToolTestCase; import org.elasticsearch.common.cli.Terminal; @@ -47,8 +48,9 @@ public class ListPluginsCommandTests extends ESTestCase { static CliToolTestCase.CaptureOutputTerminal listPlugins(Environment env) throws Exception { CliToolTestCase.CaptureOutputTerminal terminal = new CliToolTestCase.CaptureOutputTerminal(Terminal.Verbosity.NORMAL); - CliTool.ExitStatus status = new ListPluginsCommand(terminal).execute(env.settings(), env); - assertEquals(CliTool.ExitStatus.OK, status); + String[] args = {}; + int status = new ListPluginsCommand(env).main(args, terminal); + assertEquals(ExitCodes.OK, status); return terminal; } diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java index 10fbc3c2696..6ffe4168de1 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java @@ -25,10 +25,11 @@ import java.nio.file.Files; import java.nio.file.Path; import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.cli.ExitCodes; 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.cli.UserError; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; @@ -50,8 +51,7 @@ public class RemovePluginCommandTests extends ESTestCase { static CliToolTestCase.CaptureOutputTerminal removePlugin(String name, Environment env) throws Exception { CliToolTestCase.CaptureOutputTerminal terminal = new CliToolTestCase.CaptureOutputTerminal(Terminal.Verbosity.VERBOSE); - CliTool.ExitStatus status = new RemovePluginCommand(terminal, name).execute(env.settings(), env); - assertEquals(CliTool.ExitStatus.OK, status); + new RemovePluginCommand(env).execute(terminal, name); return terminal; } diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java index 6d6c176b27d..21a5e0228f6 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java @@ -28,6 +28,7 @@ import org.junit.After; import org.junit.Before; import java.io.IOException; +import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; @@ -73,6 +74,11 @@ public abstract class CliToolTestCase extends ESTestCase { public char[] readSecret(String prompt) { return new char[0]; } + + @Override + public PrintWriter getWriter() { + return null; + } } /** From 354ede717b870f7413b538ed893ab1bb80128cc8 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 1 Mar 2016 11:48:52 -0800 Subject: [PATCH 02/16] Removed old help files and improved plugin cli tests --- .../plugins/InstallPluginCommand.java | 14 +- .../elasticsearch/plugins/plugin-install.help | 59 --- .../elasticsearch/plugins/plugin-list.help | 12 - .../elasticsearch/plugins/plugin-remove.help | 12 - .../org/elasticsearch/plugins/plugin.help | 24 -- .../common/cli/CliToolTests.java | 365 ------------------ .../plugins/RemovePluginCommandTests.java | 4 +- .../common/cli/CliToolTestCase.java | 11 +- 8 files changed, 18 insertions(+), 483 deletions(-) delete mode 100644 core/src/main/resources/org/elasticsearch/plugins/plugin-install.help delete mode 100644 core/src/main/resources/org/elasticsearch/plugins/plugin-list.help delete mode 100644 core/src/main/resources/org/elasticsearch/plugins/plugin-remove.help delete mode 100644 core/src/main/resources/org/elasticsearch/plugins/plugin.help delete mode 100644 qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 977d89a3418..bbe00fddd8c 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -51,6 +51,7 @@ import java.nio.file.attribute.PosixFilePermission; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Set; @@ -101,7 +102,7 @@ class InstallPluginCommand extends Command { "lang-groovy")); // TODO: make this a resource file generated by gradle - static final Set OFFICIAL_PLUGINS = unmodifiableSet(newHashSet( + static final Set OFFICIAL_PLUGINS = unmodifiableSet(new LinkedHashSet<>(Arrays.asList( "analysis-icu", "analysis-kuromoji", "analysis-phonetic", @@ -120,7 +121,7 @@ class InstallPluginCommand extends Command { "repository-azure", "repository-hdfs", "repository-s3", - "store-smb")); + "store-smb"))); private final Environment env; private final OptionSpec batchOption; @@ -134,6 +135,15 @@ class InstallPluginCommand extends Command { this.arguments = parser.nonOptions("plugin id"); } + @Override + protected void printAdditionalHelp(Terminal terminal) { + terminal.println("The following official plugins may be installed by name:"); + for (String plugin : OFFICIAL_PLUGINS) { + terminal.println(" " + plugin); + } + terminal.println(""); + } + @Override protected int execute(Terminal terminal, OptionSet options) throws Exception { // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args diff --git a/core/src/main/resources/org/elasticsearch/plugins/plugin-install.help b/core/src/main/resources/org/elasticsearch/plugins/plugin-install.help deleted file mode 100644 index 7037974ede3..00000000000 --- a/core/src/main/resources/org/elasticsearch/plugins/plugin-install.help +++ /dev/null @@ -1,59 +0,0 @@ -NAME - - install - Install a plugin - -SYNOPSIS - - plugin install - -DESCRIPTION - - This command installs an elasticsearch plugin. It can be used as follows: - - Officially supported or commercial plugins require just the plugin name: - - plugin install analysis-icu - plugin install x-pack - - Plugins from Maven Central require 'groupId:artifactId:version': - - plugin install org.elasticsearch:mapper-attachments:3.0.0 - - Plugins can be installed from a custom URL or file location as follows: - - plugin install http://some.domain.name//my-plugin-1.0.0.zip - plugin install file:/path/to/my-plugin-1.0.0.zip - -OFFICIAL PLUGINS - - The following plugins are officially supported and can be installed by just referring to their name - - - analysis-icu - - analysis-kuromoji - - analysis-phonetic - - analysis-smartcn - - analysis-stempel - - delete-by-query - - discovery-azure - - discovery-ec2 - - discovery-gce - - ingest-geoip - - lang-javascript - - lang-painless - - lang-python - - mapper-attachments - - mapper-murmur3 - - mapper-size - - repository-azure - - repository-hdfs - - repository-s3 - - store-smb - - -OPTIONS - - -v,--verbose Verbose output - - -h,--help Shows this message - - -b,--batch Enable batch mode explicitly, automatic confirmation of security permissions diff --git a/core/src/main/resources/org/elasticsearch/plugins/plugin-list.help b/core/src/main/resources/org/elasticsearch/plugins/plugin-list.help deleted file mode 100644 index c13949e8cb6..00000000000 --- a/core/src/main/resources/org/elasticsearch/plugins/plugin-list.help +++ /dev/null @@ -1,12 +0,0 @@ -NAME - - list - List all plugins - -SYNOPSIS - - plugin list - -DESCRIPTION - - This command lists all installed elasticsearch plugins - diff --git a/core/src/main/resources/org/elasticsearch/plugins/plugin-remove.help b/core/src/main/resources/org/elasticsearch/plugins/plugin-remove.help deleted file mode 100644 index b708adf1f69..00000000000 --- a/core/src/main/resources/org/elasticsearch/plugins/plugin-remove.help +++ /dev/null @@ -1,12 +0,0 @@ -NAME - - remove - Remove a plugin - -SYNOPSIS - - plugin remove - -DESCRIPTION - - This command removes an elasticsearch plugin - diff --git a/core/src/main/resources/org/elasticsearch/plugins/plugin.help b/core/src/main/resources/org/elasticsearch/plugins/plugin.help deleted file mode 100644 index 5cba544627a..00000000000 --- a/core/src/main/resources/org/elasticsearch/plugins/plugin.help +++ /dev/null @@ -1,24 +0,0 @@ -NAME - - plugin - Manages plugins - -SYNOPSIS - - plugin - -DESCRIPTION - - Manage plugins - -COMMANDS - - install Install a plugin - - remove Remove a plugin - - list List installed plugins - -NOTES - - [*] For usage help on specific commands please type "plugin -h" - diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java deleted file mode 100644 index 70d507853fa..00000000000 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java +++ /dev/null @@ -1,365 +0,0 @@ -/* - * 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; - -import org.apache.commons.cli.CommandLine; -import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.elasticsearch.node.internal.InternalSettingsPreparer; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; - -import static java.util.Collections.unmodifiableMap; -import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK; -import static org.elasticsearch.common.cli.CliTool.ExitStatus.USAGE; -import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; -import static org.hamcrest.Matchers.arrayContaining; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; - -@SuppressForbidden(reason = "modifies system properties intentionally") -public class CliToolTests extends CliToolTestCase { - public void testOK() throws Exception { - Terminal terminal = new MockTerminal(); - final AtomicReference executed = new AtomicReference<>(false); - final NamedCommand cmd = new NamedCommand("cmd", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) { - executed.set(true); - return OK; - } - }; - SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd); - CliTool.ExitStatus status = tool.execute(); - assertStatus(status, OK); - assertCommandHasBeenExecuted(executed); - } - - public void testUsageError() throws Exception { - Terminal terminal = new MockTerminal(); - final AtomicReference executed = new AtomicReference<>(false); - final NamedCommand cmd = new NamedCommand("cmd", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws UserError { - executed.set(true); - throw new UserError(CliTool.ExitStatus.USAGE.status(), "bad usage"); - } - }; - SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd); - CliTool.ExitStatus status = tool.execute(); - assertStatus(status, CliTool.ExitStatus.USAGE); - assertCommandHasBeenExecuted(executed); - } - - public void testMultiCommand() throws Exception { - Terminal terminal = new MockTerminal(); - int count = randomIntBetween(2, 7); - List> executed = new ArrayList<>(count); - for (int i = 0; i < count; i++) { - executed.add(new AtomicReference<>(false)); - } - NamedCommand[] cmds = new NamedCommand[count]; - for (int i = 0; i < count; i++) { - final int index = i; - cmds[i] = new NamedCommand("cmd" + index, terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - executed.get(index).set(true); - return OK; - } - }; - } - MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds); - int cmdIndex = randomIntBetween(0, count-1); - CliTool.ExitStatus status = tool.execute("cmd" + cmdIndex); - assertThat(status, is(OK)); - for (int i = 0; i < count; i++) { - assertThat(executed.get(i).get(), is(i == cmdIndex)); - } - } - - public void testMultiCommandUnknownCommand() throws Exception { - Terminal terminal = new MockTerminal(); - int count = randomIntBetween(2, 7); - List> executed = new ArrayList<>(count); - for (int i = 0; i < count; i++) { - executed.add(new AtomicReference<>(false)); - } - NamedCommand[] cmds = new NamedCommand[count]; - for (int i = 0; i < count; i++) { - final int index = i; - cmds[i] = new NamedCommand("cmd" + index, terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - executed.get(index).set(true); - return OK; - } - }; - } - MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds); - CliTool.ExitStatus status = tool.execute("cmd" + count); // "cmd" + count doesn't exist - assertThat(status, is(CliTool.ExitStatus.USAGE)); - for (int i = 0; i < count; i++) { - assertThat(executed.get(i).get(), is(false)); - } - } - - public void testSingleCommandToolHelp() throws Exception { - CaptureOutputTerminal terminal = new CaptureOutputTerminal(); - final AtomicReference executed = new AtomicReference<>(false); - final NamedCommand cmd = new NamedCommand("cmd1", 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(args("-h")); - assertStatus(status, CliTool.ExitStatus.OK_AND_EXIT); - assertThat(terminal.getTerminalOutput(), hasSize(3)); - assertThat(terminal.getTerminalOutput(), hasItem(containsString("cmd1 help"))); - } - - public void testMultiCommandToolHelp() throws Exception { - CaptureOutputTerminal terminal = new CaptureOutputTerminal(); - NamedCommand[] cmds = new NamedCommand[2]; - cmds[0] = new NamedCommand("cmd0", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - return OK; - } - }; - cmds[1] = new NamedCommand("cmd1", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - return OK; - } - }; - MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds); - CliTool.ExitStatus status = tool.execute(args("-h")); - assertStatus(status, CliTool.ExitStatus.OK_AND_EXIT); - assertThat(terminal.getTerminalOutput(), hasSize(3)); - assertThat(terminal.getTerminalOutput(), hasItem(containsString("tool help"))); - } - - public void testMultiCommandCmdHelp() throws Exception { - CaptureOutputTerminal terminal = new CaptureOutputTerminal(); - NamedCommand[] cmds = new NamedCommand[2]; - cmds[0] = new NamedCommand("cmd0", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - return OK; - } - }; - cmds[1] = new NamedCommand("cmd1", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - return OK; - } - }; - MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds); - CliTool.ExitStatus status = tool.execute(args("cmd1 -h")); - assertStatus(status, CliTool.ExitStatus.OK_AND_EXIT); - assertThat(terminal.getTerminalOutput(), hasSize(3)); - assertThat(terminal.getTerminalOutput(), hasItem(containsString("cmd1 help"))); - } - - 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 IOException("error message"); - } - }; - SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd); - IOException e = expectThrows(IOException.class, () -> { - tool.execute(); - }); - assertEquals("error message", e.getMessage()); - } - - public void testMultipleLaunch() throws Exception { - Terminal terminal = new MockTerminal(); - final AtomicReference executed = new AtomicReference<>(false); - final NamedCommand cmd = new NamedCommand("cmd", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) { - executed.set(true); - return OK; - } - }; - SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd); - tool.parse("cmd", Strings.splitStringByCommaToArray("--verbose")); - tool.parse("cmd", Strings.splitStringByCommaToArray("--silent")); - tool.parse("cmd", Strings.splitStringByCommaToArray("--help")); - } - - public void testPromptForSetting() throws Exception { - final AtomicInteger counter = new AtomicInteger(); - final AtomicReference promptedSecretValue = new AtomicReference<>(null); - final AtomicReference promptedTextValue = new AtomicReference<>(null); - final Terminal terminal = new MockTerminal() { - @Override - public char[] readSecret(String text) { - counter.incrementAndGet(); - return "changeit".toCharArray(); - } - - @Override - public String readText(String text) { - counter.incrementAndGet(); - return "replaced"; - } - }; - final NamedCommand cmd = new NamedCommand("noop", terminal) { - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) { - promptedSecretValue.set(settings.get("foo.password")); - promptedTextValue.set(settings.get("replace")); - return OK; - } - }; - - System.setProperty("es.foo.password", InternalSettingsPreparer.SECRET_PROMPT_VALUE); - System.setProperty("es.replace", InternalSettingsPreparer.TEXT_PROMPT_VALUE); - try { - new SingleCmdTool("tool", terminal, cmd).execute(); - } finally { - System.clearProperty("es.foo.password"); - System.clearProperty("es.replace"); - } - - assertThat(counter.intValue(), is(2)); - assertThat(promptedSecretValue.get(), is("changeit")); - assertThat(promptedTextValue.get(), is("replaced")); - } - - public void testStopAtNonOptionParsing() throws Exception { - final CliToolConfig.Cmd lenientCommand = cmd("lenient", CliTool.Command.Exit.class).stopAtNonOption(true).build(); - final CliToolConfig.Cmd strictCommand = cmd("strict", CliTool.Command.Exit.class).stopAtNonOption(false).build(); - final CliToolConfig config = CliToolConfig.config("elasticsearch", CliTool.class).cmds(lenientCommand, strictCommand).build(); - - final CaptureOutputTerminal terminal = new CaptureOutputTerminal(); - final CliTool cliTool = new CliTool(config, terminal) { - @Override - protected Command parse(String cmdName, CommandLine cli) throws Exception { - return new NamedCommand(cmdName, terminal) { - @Override - public ExitStatus execute(Settings settings, Environment env) throws Exception { - return OK; - } - }; - } - }; - - // known parameters, no error - assertStatus(cliTool.execute(args("lenient --verbose")), OK); - assertStatus(cliTool.execute(args("lenient -v")), OK); - - // unknown parameters, no error - assertStatus(cliTool.execute(args("lenient --unknown")), OK); - assertStatus(cliTool.execute(args("lenient -u")), OK); - - // unknown parameters, error - assertStatus(cliTool.execute(args("strict --unknown")), USAGE); - assertThat(terminal.getTerminalOutput(), hasItem(containsString("Unrecognized option: --unknown"))); - - terminal.getTerminalOutput().clear(); - assertStatus(cliTool.execute(args("strict -u")), USAGE); - assertThat(terminal.getTerminalOutput(), hasItem(containsString("Unrecognized option: -u"))); - } - - private void assertStatus(CliTool.ExitStatus status, CliTool.ExitStatus expectedStatus) { - assertThat(status, is(expectedStatus)); - } - - private void assertCommandHasBeenExecuted(AtomicReference executed) { - assertThat("Expected command atomic reference counter to be set to true", executed.get(), is(Boolean.TRUE)); - } - - private static class SingleCmdTool extends CliTool { - - private final Command command; - - private SingleCmdTool(String name, Terminal terminal, NamedCommand command) { - super(CliToolConfig.config(name, SingleCmdTool.class) - .cmds(cmd(command.name, command.getClass())) - .build(), terminal); - this.command = command; - } - - @Override - protected Command parse(String cmdName, CommandLine cli) throws Exception { - return command; - } - } - - private static class MultiCmdTool extends CliTool { - - private final Map commands; - - private MultiCmdTool(String name, Terminal terminal, NamedCommand... commands) { - super(CliToolConfig.config(name, MultiCmdTool.class) - .cmds(cmds(commands)) - .build(), terminal); - Map commandByName = new HashMap<>(); - for (int i = 0; i < commands.length; i++) { - commandByName.put(commands[i].name, commands[i]); - } - this.commands = unmodifiableMap(commandByName); - } - - @Override - protected Command parse(String cmdName, CommandLine cli) throws Exception { - return commands.get(cmdName); - } - - private static CliToolConfig.Cmd[] cmds(NamedCommand... commands) { - CliToolConfig.Cmd[] cmds = new CliToolConfig.Cmd[commands.length]; - for (int i = 0; i < commands.length; i++) { - cmds[i] = cmd(commands[i].name, commands[i].getClass()).build(); - } - return cmds; - } - } - - private static abstract class NamedCommand extends CliTool.Command { - - private final String name; - - private NamedCommand(String name, Terminal terminal) { - super(terminal); - this.name = name; - } - } -} diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java index 6ffe4168de1..cedb4f5d878 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java @@ -25,11 +25,9 @@ import java.nio.file.Files; import java.nio.file.Path; import org.apache.lucene.util.LuceneTestCase; -import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.cli.CliTool; +import org.elasticsearch.cli.UserError; import org.elasticsearch.common.cli.CliToolTestCase; import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.cli.UserError; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java index 21a5e0228f6..d96f6bd4e79 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java @@ -19,7 +19,11 @@ package org.elasticsearch.common.cli; -import org.elasticsearch.ExceptionsHelper; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.List; + import org.elasticsearch.common.Strings; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.test.ESTestCase; @@ -27,11 +31,6 @@ import org.elasticsearch.test.StreamsUtils; import org.junit.After; import org.junit.Before; -import java.io.IOException; -import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.List; - import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; From 209da28bb2d63b6e9e517cd996d40e9222224a8c Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 3 Mar 2016 09:37:33 -0800 Subject: [PATCH 03/16] Removed check file command tests, check file command is going away --- .../common/cli/CheckFileCommandTests.java | 329 ------------------ 1 file changed, 329 deletions(-) delete mode 100644 qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CheckFileCommandTests.java diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CheckFileCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CheckFileCommandTests.java deleted file mode 100644 index 45f3df22cd7..00000000000 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CheckFileCommandTests.java +++ /dev/null @@ -1,329 +0,0 @@ -/* - * 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; - -import com.google.common.jimfs.Configuration; -import com.google.common.jimfs.Jimfs; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.set.Sets; -import org.elasticsearch.env.Environment; -import org.elasticsearch.test.ESTestCase; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.FileSystem; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.attribute.GroupPrincipal; -import java.nio.file.attribute.PosixFileAttributeView; -import java.nio.file.attribute.PosixFileAttributes; -import java.nio.file.attribute.PosixFilePermission; -import java.nio.file.attribute.UserPrincipal; -import java.util.Set; - -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; - -/** - * - */ -public class CheckFileCommandTests extends ESTestCase { - - private CliToolTestCase.CaptureOutputTerminal captureOutputTerminal = new CliToolTestCase.CaptureOutputTerminal(); - - private Configuration jimFsConfiguration = Configuration.unix().toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build(); - private Configuration jimFsConfigurationWithoutPermissions = randomBoolean() ? Configuration.unix().toBuilder().setAttributeViews("basic").build() : Configuration.windows(); - - private enum Mode { - CHANGE, KEEP, DISABLED - } - - public void testThatCommandLogsErrorMessageOnFail() throws Exception { - executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.CHANGE)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(containsString("Please ensure that the user account running Elasticsearch has read access to this file"))); - } - - public void testThatCommandLogsNothingWhenPermissionRemains() throws Exception { - executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.KEEP)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsNothingWhenDisabled() throws Exception { - executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsNothingIfFilesystemDoesNotSupportPermissions() throws Exception { - executeCommand(jimFsConfigurationWithoutPermissions, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsOwnerChange() throws Exception { - executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.CHANGE)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(allOf(containsString("Owner of file ["), containsString("] used to be ["), containsString("], but now is [")))); - } - - public void testThatCommandLogsNothingIfOwnerRemainsSame() throws Exception { - executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.KEEP)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsNothingIfOwnerIsDisabled() throws Exception { - executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsNothingIfFileSystemDoesNotSupportOwners() throws Exception { - executeCommand(jimFsConfigurationWithoutPermissions, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsIfGroupChanges() throws Exception { - executeCommand(jimFsConfiguration, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.CHANGE)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(allOf(containsString("Group of file ["), containsString("] used to be ["), containsString("], but now is [")))); - } - - public void testThatCommandLogsNothingIfGroupRemainsSame() throws Exception { - executeCommand(jimFsConfiguration, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.KEEP)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsNothingIfGroupIsDisabled() throws Exception { - executeCommand(jimFsConfiguration, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandLogsNothingIfFileSystemDoesNotSupportGroups() throws Exception { - executeCommand(jimFsConfigurationWithoutPermissions, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED)); - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandDoesNotLogAnythingOnFileCreation() throws Exception { - Configuration configuration = randomBoolean() ? jimFsConfiguration : jimFsConfigurationWithoutPermissions; - - try (FileSystem fs = Jimfs.newFileSystem(configuration)) { - Path path = fs.getPath(randomAsciiOfLength(10)); - Settings settings = Settings.builder() - .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) - .build(); - new CreateFileCommand(captureOutputTerminal, path).execute(settings, new Environment(settings)); - assertThat(Files.exists(path), is(true)); - } - - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - public void testThatCommandWorksIfFileIsDeletedByCommand() throws Exception { - Configuration configuration = randomBoolean() ? jimFsConfiguration : jimFsConfigurationWithoutPermissions; - - try (FileSystem fs = Jimfs.newFileSystem(configuration)) { - Path path = fs.getPath(randomAsciiOfLength(10)); - Files.write(path, "anything".getBytes(StandardCharsets.UTF_8)); - - Settings settings = Settings.builder() - .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) - .build(); - new DeleteFileCommand(captureOutputTerminal, path).execute(settings, new Environment(settings)); - assertThat(Files.exists(path), is(false)); - } - - assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0)); - } - - private void executeCommand(Configuration configuration, AbstractTestCheckFileCommand command) throws Exception { - try (FileSystem fs = Jimfs.newFileSystem(configuration)) { - command.execute(fs); - } - } - - abstract class AbstractTestCheckFileCommand extends CheckFileCommand { - - protected final Mode mode; - protected FileSystem fs; - protected Path[] paths; - final Path baseDir; - - public AbstractTestCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException { - super(terminal); - this.mode = mode; - this.baseDir = baseDir; - } - - public CliTool.ExitStatus execute(FileSystem fs) throws Exception { - this.fs = fs; - this.paths = new Path[] { writePath(fs, "p1", "anything"), writePath(fs, "p2", "anything"), writePath(fs, "p3", "anything") }; - Settings settings = Settings.settingsBuilder() - .put(Environment.PATH_HOME_SETTING.getKey(), baseDir.toString()) - .build(); - return super.execute(Settings.EMPTY, new Environment(settings)); - } - - private Path writePath(FileSystem fs, String name, String content) throws IOException { - Path path = fs.getPath(name); - Files.write(path, content.getBytes(StandardCharsets.UTF_8)); - return path; - } - - @Override - protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) { - return paths; - } - } - - /** - * command that changes permissions from a file if enabled - */ - class PermissionCheckFileCommand extends AbstractTestCheckFileCommand { - - public PermissionCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException { - super(baseDir, terminal, mode); - } - - @Override - public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception { - int randomInt = randomInt(paths.length - 1); - Path randomPath = paths[randomInt]; - switch (mode) { - case CHANGE: - Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8)); - Files.setPosixFilePermissions(randomPath, Sets.newHashSet(PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.OTHERS_EXECUTE, PosixFilePermission.GROUP_EXECUTE)); - break; - case KEEP: - Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8)); - Set posixFilePermissions = Files.getPosixFilePermissions(randomPath); - Files.setPosixFilePermissions(randomPath, posixFilePermissions); - break; - } - return CliTool.ExitStatus.OK; - } - - } - - /** - * command that changes the owner of a file if enabled - */ - class OwnerCheckFileCommand extends AbstractTestCheckFileCommand { - - public OwnerCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException { - super(baseDir, terminal, mode); - } - - @Override - public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception { - int randomInt = randomInt(paths.length - 1); - Path randomPath = paths[randomInt]; - switch (mode) { - case CHANGE: - Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8)); - UserPrincipal randomOwner = fs.getUserPrincipalLookupService().lookupPrincipalByName(randomAsciiOfLength(10)); - Files.setOwner(randomPath, randomOwner); - break; - case KEEP: - Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8)); - UserPrincipal originalOwner = Files.getOwner(randomPath); - Files.setOwner(randomPath, originalOwner); - break; - } - - return CliTool.ExitStatus.OK; - } - } - - /** - * command that changes the group of a file if enabled - */ - class GroupCheckFileCommand extends AbstractTestCheckFileCommand { - - public GroupCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException { - super(baseDir, terminal, mode); - } - - @Override - public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception { - int randomInt = randomInt(paths.length - 1); - Path randomPath = paths[randomInt]; - switch (mode) { - case CHANGE: - Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8)); - GroupPrincipal randomPrincipal = fs.getUserPrincipalLookupService().lookupPrincipalByGroupName(randomAsciiOfLength(10)); - Files.getFileAttributeView(randomPath, PosixFileAttributeView.class).setGroup(randomPrincipal); - break; - case KEEP: - Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8)); - GroupPrincipal groupPrincipal = Files.readAttributes(randomPath, PosixFileAttributes.class).group(); - Files.getFileAttributeView(randomPath, PosixFileAttributeView.class).setGroup(groupPrincipal); - break; - } - - return CliTool.ExitStatus.OK; - } - } - - /** - * A command that creates a non existing file - */ - class CreateFileCommand extends CheckFileCommand { - - private final Path pathToCreate; - - public CreateFileCommand(Terminal terminal, Path pathToCreate) { - super(terminal); - this.pathToCreate = pathToCreate; - } - - @Override - public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception { - Files.write(pathToCreate, "anything".getBytes(StandardCharsets.UTF_8)); - return CliTool.ExitStatus.OK; - } - - @Override - protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception { - return new Path[] { pathToCreate }; - } - } - - /** - * A command that deletes an existing file - */ - class DeleteFileCommand extends CheckFileCommand { - - private final Path pathToDelete; - - public DeleteFileCommand(Terminal terminal, Path pathToDelete) { - super(terminal); - this.pathToDelete = pathToDelete; - } - - @Override - public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception { - Files.delete(pathToDelete); - return CliTool.ExitStatus.OK; - } - - @Override - protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception { - return new Path[] {pathToDelete}; - } - } -} From 8321d7c5c24444b82cba2f2b0468564a525688fb Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 4 Mar 2016 12:11:25 -0800 Subject: [PATCH 04/16] Catch option error during execution too, since OptionSet is passed there --- core/src/main/java/org/elasticsearch/cli/Command.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java index bc44a8eb635..d688347099d 100644 --- a/core/src/main/java/org/elasticsearch/cli/Command.java +++ b/core/src/main/java/org/elasticsearch/cli/Command.java @@ -56,7 +56,7 @@ public abstract class Command { options = parser.parse(args); } catch (OptionException e) { printHelp(terminal); - terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + terminal.println("ERROR: " + e.getMessage()); return ExitCodes.USAGE; } @@ -69,7 +69,7 @@ public abstract class Command { if (options.has(verboseOption)) { // mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it printHelp(terminal); - terminal.println(Terminal.Verbosity.SILENT, "ERROR: Cannot specify -s and -v together"); + terminal.println("ERROR: Cannot specify -s and -v together"); return ExitCodes.USAGE; } terminal.setVerbosity(Terminal.Verbosity.SILENT); @@ -81,6 +81,10 @@ public abstract class Command { try { return execute(terminal, options); + } catch (OptionException e) { + printHelp(terminal); + terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + return ExitCodes.USAGE; } catch (UserError e) { terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); return e.exitCode; From 45b5ab24fec2fae36e0eb205ccd00d8040ed9c7e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 7 Mar 2016 12:42:15 -0800 Subject: [PATCH 05/16] Moved MockTerminal and created a base test case for cli commands. --- .../java/org/elasticsearch/cli/Command.java | 6 +- .../common/cli/CheckFileCommand.java | 138 ------------------ .../common/cli/TerminalTests.java | 7 +- .../logging/LoggingConfigurationTests.java | 2 +- .../InternalSettingsPreparerTests.java | 2 +- .../elasticsearch/plugins/PluginCliTests.java | 4 +- .../bootstrap/BootstrapCliParserTests.java | 10 +- .../plugins/InstallPluginCommandTests.java | 7 +- .../plugins/ListPluginsCommandTests.java | 7 +- .../plugins/RemovePluginCommandTests.java | 4 +- .../elasticsearch/cli/CommandTestCase.java | 48 ++++++ .../{common => }/cli/MockTerminal.java | 10 +- .../common/cli/CliToolTestCase.java | 1 + 13 files changed, 75 insertions(+), 171 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java create mode 100644 test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java rename test/framework/src/main/java/org/elasticsearch/{common => }/cli/MockTerminal.java (92%) diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java index d688347099d..6e57905b5b2 100644 --- a/core/src/main/java/org/elasticsearch/cli/Command.java +++ b/core/src/main/java/org/elasticsearch/cli/Command.java @@ -49,14 +49,14 @@ public abstract class Command { } /** Parses options for this command from args and executes it. */ - public final int main(String[] args, Terminal terminal) throws Exception { + protected final int main(String[] args, Terminal terminal) throws Exception { final OptionSet options; try { options = parser.parse(args); } catch (OptionException e) { printHelp(terminal); - terminal.println("ERROR: " + e.getMessage()); + terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); return ExitCodes.USAGE; } @@ -69,7 +69,7 @@ public abstract class Command { if (options.has(verboseOption)) { // mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it printHelp(terminal); - terminal.println("ERROR: Cannot specify -s and -v together"); + terminal.println(Terminal.Verbosity.SILENT, "ERROR: Cannot specify -s and -v together"); return ExitCodes.USAGE; } terminal.setVerbosity(Terminal.Verbosity.SILENT); diff --git a/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java b/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java deleted file mode 100644 index e2fcbe89df8..00000000000 --- a/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java +++ /dev/null @@ -1,138 +0,0 @@ -/* - * 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; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.attribute.PosixFileAttributeView; -import java.nio.file.attribute.PosixFileAttributes; -import java.nio.file.attribute.PosixFilePermission; -import java.nio.file.attribute.PosixFilePermissions; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; - -/** - * A helper command that checks if configured paths have been changed when running a CLI command. - * It is only executed in case of specified paths by the command and if the paths underlying filesystem - * supports posix permissions. - * - * If this is the case, a warn message is issued whenever an owner, a group or the file permissions is changed by - * the command being executed and not configured back to its prior state, which should be the task of the command - * being executed. - * - */ -public abstract class CheckFileCommand extends CliTool.Command { - - public CheckFileCommand(Terminal terminal) { - super(terminal); - } - - /** - * abstract method, which should implement the same logic as CliTool.Command.execute(), but is wrapped - */ - public abstract CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception; - - /** - * Returns the array of paths, that should be checked if the permissions, user or groups have changed - * before and after execution of the command - * - */ - protected abstract Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception; - - @Override - public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception { - Path[] paths = pathsForPermissionsCheck(settings, env); - - if (paths == null || paths.length == 0) { - return doExecute(settings, env); - } - - Map> permissions = new HashMap<>(paths.length); - Map owners = new HashMap<>(paths.length); - Map groups = new HashMap<>(paths.length); - - if (paths != null && paths.length > 0) { - for (Path path : paths) { - try { - boolean supportsPosixPermissions = Environment.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class); - if (supportsPosixPermissions) { - PosixFileAttributes attributes = Files.readAttributes(path, PosixFileAttributes.class); - permissions.put(path, attributes.permissions()); - owners.put(path, attributes.owner().getName()); - groups.put(path, attributes.group().getName()); - } - } catch (IOException e) { - // silently swallow if not supported, no need to log things - } - } - } - - CliTool.ExitStatus status = doExecute(settings, env); - - // check if permissions differ - for (Map.Entry> entry : permissions.entrySet()) { - if (!Files.exists(entry.getKey())) { - continue; - } - - Set permissionsBeforeWrite = entry.getValue(); - Set permissionsAfterWrite = Files.getPosixFilePermissions(entry.getKey()); - if (!permissionsBeforeWrite.equals(permissionsAfterWrite)) { - terminal.println(Terminal.Verbosity.SILENT, "WARNING: The file permissions of [" + entry.getKey() + "] have changed " - + "from [" + PosixFilePermissions.toString(permissionsBeforeWrite) + "] " - + "to [" + PosixFilePermissions.toString(permissionsAfterWrite) + "]"); - terminal.println(Terminal.Verbosity.SILENT, "Please ensure that the user account running Elasticsearch has read access to this file!"); - } - } - - // check if owner differs - for (Map.Entry entry : owners.entrySet()) { - if (!Files.exists(entry.getKey())) { - continue; - } - - String ownerBeforeWrite = entry.getValue(); - String ownerAfterWrite = Files.getOwner(entry.getKey()).getName(); - if (!ownerAfterWrite.equals(ownerBeforeWrite)) { - terminal.println(Terminal.Verbosity.SILENT, "WARNING: Owner of file [" + entry.getKey() + "] used to be [" + ownerBeforeWrite + "], but now is [" + ownerAfterWrite + "]"); - } - } - - // check if group differs - for (Map.Entry entry : groups.entrySet()) { - if (!Files.exists(entry.getKey())) { - continue; - } - - String groupBeforeWrite = entry.getValue(); - String groupAfterWrite = Files.readAttributes(entry.getKey(), PosixFileAttributes.class).group().getName(); - if (!groupAfterWrite.equals(groupBeforeWrite)) { - terminal.println(Terminal.Verbosity.SILENT, "WARNING: Group of file [" + entry.getKey() + "] used to be [" + groupBeforeWrite + "], but now is [" + groupAfterWrite + "]"); - } - } - - return status; - } -} diff --git a/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java b/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java index deb64e906b4..12fc4cb77e4 100644 --- a/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java +++ b/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java @@ -19,7 +19,10 @@ package org.elasticsearch.common.cli; -public class TerminalTests extends CliToolTestCase { +import org.elasticsearch.cli.MockTerminal; +import org.elasticsearch.test.ESTestCase; + +public class TerminalTests extends ESTestCase { public void testVerbosity() throws Exception { MockTerminal terminal = new MockTerminal(); terminal.setVerbosity(Terminal.Verbosity.SILENT); @@ -48,7 +51,7 @@ public class TerminalTests extends CliToolTestCase { logTerminal.println(verbosity, text); String output = logTerminal.getOutput(); assertTrue(output, output.contains(text)); - logTerminal.resetOutput(); + logTerminal.reset(); } private void assertNotPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception { diff --git a/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java b/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java index 0cca19d33bf..5c812cca0a7 100644 --- a/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java +++ b/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java @@ -27,7 +27,7 @@ import java.util.Arrays; import org.apache.log4j.Appender; import org.apache.log4j.Logger; -import org.elasticsearch.common.cli.MockTerminal; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.node.internal.InternalSettingsPreparer; diff --git a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java index 33876ef61ad..c979b2f4013 100644 --- a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java +++ b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java @@ -24,7 +24,7 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; -import org.elasticsearch.common.cli.MockTerminal; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java index 7a4590f8e25..708cefd91b2 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java @@ -20,10 +20,8 @@ package org.elasticsearch.plugins; import org.elasticsearch.common.cli.CliToolTestCase; -import org.elasticsearch.common.cli.MockTerminal; +import org.elasticsearch.cli.MockTerminal; -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; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java index 12b5ab9eb39..18d6e0ac3c9 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java @@ -25,7 +25,7 @@ import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.cli.CliTool.ExitStatus; import org.elasticsearch.common.cli.CliToolTestCase; import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.cli.MockTerminal; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.monitor.jvm.JvmInfo; import org.junit.After; @@ -89,7 +89,7 @@ public class BootstrapCliParserTests extends CliToolTestCase { assertTrue(output, output.contains(Build.CURRENT.date())); assertTrue(output, output.contains(JvmInfo.jvmInfo().version())); - terminal.resetOutput(); + terminal.reset(); parser = new BootstrapCLIParser(terminal); status = parser.execute(args("start --version")); assertStatus(status, OK_AND_EXIT); @@ -177,7 +177,7 @@ public class BootstrapCliParserTests extends CliToolTestCase { String output = terminal.getOutput(); assertTrue(output, output.contains("Parameter [network.host] needs value")); - terminal.resetOutput(); + terminal.reset(); status = parser.execute(args("start --network.host --foo")); assertStatus(status, USAGE); output = terminal.getOutput(); @@ -194,7 +194,7 @@ public class BootstrapCliParserTests extends CliToolTestCase { assertTrue(output, output.contains("Unrecognized option: --unknown-param")); // single dash in extra params - terminal.resetOutput(); + terminal.reset(); parser = new BootstrapCLIParser(terminal); status = parser.execute(args("start -network.host 127.0.0.1")); assertStatus(status, USAGE); @@ -228,7 +228,7 @@ public class BootstrapCliParserTests extends CliToolTestCase { tuples.add(new Tuple<>("-h", "elasticsearch.help")); for (Tuple tuple : tuples) { - terminal.resetOutput(); + terminal.reset(); BootstrapCLIParser parser = new BootstrapCLIParser(terminal); ExitStatus status = parser.execute(args(tuple.v1())); assertStatus(status, OK_AND_EXIT); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 342b579a175..514090d9869 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -36,7 +36,6 @@ import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; -import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -45,11 +44,7 @@ import java.util.zip.ZipOutputStream; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.Version; -import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.CliToolTestCase; -import org.elasticsearch.common.cli.MockTerminal; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.cli.UserError; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java index aed1696898a..cbdd031dea1 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java @@ -22,15 +22,10 @@ package org.elasticsearch.plugins; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Collections; -import java.util.List; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.CliToolTestCase; -import org.elasticsearch.common.cli.MockTerminal; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java index a678d4f25f6..d9d5661b834 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java @@ -26,9 +26,7 @@ import java.nio.file.Path; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.cli.CliToolTestCase; -import org.elasticsearch.common.cli.MockTerminal; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java new file mode 100644 index 00000000000..3af25509adb --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java @@ -0,0 +1,48 @@ +/* + * 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.cli; + +import joptsimple.OptionSet; +import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.test.ESTestCase; +import org.junit.Before; + +/** + * A base test case for cli tools. + */ +public abstract class CommandTestCase extends ESTestCase { + + protected final MockTerminal terminal = new MockTerminal(); + + @Before + public void resetTerminal() { + terminal.reset(); + terminal.setVerbosity(Terminal.Verbosity.NORMAL); + } + + protected abstract Command newCommand(); + + public String execute(String... args) throws Exception { + Command command = newCommand(); + OptionSet options = command.parser.parse(args); + command.execute(terminal, options); + return terminal.getOutput(); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java similarity index 92% rename from test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java rename to test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java index 3b2903b3fab..bb01369ac50 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.common.cli; +package org.elasticsearch.cli; import java.io.ByteArrayOutputStream; import java.io.OutputStreamWriter; @@ -27,6 +27,8 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; import java.util.Deque; +import org.elasticsearch.common.cli.Terminal; + /** * A terminal for tests which captures all output, and * can be plugged with fake input. @@ -78,8 +80,10 @@ public class MockTerminal extends Terminal { return buffer.toString("UTF-8"); } - /** Wipes the output. */ - public void resetOutput() { + /** Wipes the input and output. */ + public void reset() { buffer.reset(); + textInput.clear(); + secretInput.clear(); } } diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java index 9debf4b8f33..330758223a5 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java @@ -21,6 +21,7 @@ package org.elasticsearch.common.cli; import java.io.IOException; +import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.Strings; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.test.ESTestCase; From e5c852f7678b6c568811f9bcbc397b864df1c15f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 13:39:37 -0800 Subject: [PATCH 06/16] Convert bootstrapcli parser to jopt-simple --- .../elasticsearch/bootstrap/Bootstrap.java | 18 +- .../bootstrap/BootstrapCLIParser.java | 199 +++++------------ .../bootstrap/Elasticsearch.java | 13 +- .../java/org/elasticsearch/cli/Command.java | 63 +++--- .../org/elasticsearch/cli/MultiCommand.java | 4 +- .../org/elasticsearch/cli/TestCommand.java | 41 ---- .../mapper/attachments/StandaloneRunner.java | 189 ---------------- .../bootstrap/BootstrapCliParserTests.java | 206 +++++------------- .../elasticsearch/cli/CommandTestCase.java | 16 +- .../org/elasticsearch/cli/MockTerminal.java | 4 +- 10 files changed, 162 insertions(+), 591 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/cli/TestCommand.java delete mode 100644 plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 215659054d2..73654fdfee5 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -26,7 +26,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.common.PidFile; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.Terminal; import org.elasticsearch.common.inject.CreationException; import org.elasticsearch.common.logging.ESLogger; @@ -218,17 +217,10 @@ final class Bootstrap { * This method is invoked by {@link Elasticsearch#main(String[])} * to startup elasticsearch. */ - static void init(String[] args) throws Throwable { + static void init() throws Throwable { // Set the system property before anything has a chance to trigger its use initLoggerPrefix(); - BootstrapCLIParser bootstrapCLIParser = new BootstrapCLIParser(); - CliTool.ExitStatus status = bootstrapCLIParser.execute(args); - - if (CliTool.ExitStatus.OK != status) { - exit(status.status()); - } - INSTANCE = new Bootstrap(); boolean foreground = !"false".equals(System.getProperty("es.foreground", System.getProperty("es-foreground"))); @@ -307,14 +299,6 @@ final class Bootstrap { System.err.close(); } - @SuppressForbidden(reason = "System#err") - private static void sysError(String line, boolean flush) { - System.err.println(line); - if (flush) { - System.err.flush(); - } - } - private static void checkForCustomConfFile() { String confFileSetting = System.getProperty("es.default.config"); checkUnsetAndMaybeExit(confFileSetting, "es.default.config"); diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java index ec11a773ccc..3567039cd42 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java @@ -19,165 +19,70 @@ package org.elasticsearch.bootstrap; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.Option; +import java.util.Arrays; + +import joptsimple.OptionSet; +import joptsimple.OptionSpec; import org.elasticsearch.Build; -import org.elasticsearch.common.Strings; -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.cli.Command; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.cli.Terminal; import org.elasticsearch.monitor.jvm.JvmInfo; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Locale; -import java.util.Map; -import java.util.Properties; +final class BootstrapCliParser extends Command { -import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; -import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder; + private final OptionSpec versionOption; + private final OptionSpec daemonizeOption; + private final OptionSpec pidfileOption; + private final OptionSpec propertyOption; + private boolean shouldRun = false; -final class BootstrapCLIParser extends CliTool { - - private static final CliToolConfig CONFIG = CliToolConfig.config("elasticsearch", BootstrapCLIParser.class) - .cmds(Start.CMD, Version.CMD) - .build(); - - public BootstrapCLIParser() { - super(CONFIG); - } - - public BootstrapCLIParser(Terminal terminal) { - super(CONFIG, terminal); + BootstrapCliParser() { + super("Starts elasticsearch"); + // TODO: in jopt-simple 5.0, make this mutually exclusive with all other options + versionOption = parser.acceptsAll(Arrays.asList("V", "version"), + "Prints elasticsearch version information and exits"); + daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), + "Starts Elasticsearch in the background"); + // TODO: in jopt-simple 5.0 this option type can be a Path + pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"), + "Creates a pid file in the specified path on start") + .withRequiredArg(); + propertyOption = parser.accepts("E", "Configures an Elasticsearch setting") + .withRequiredArg(); } @Override - protected Command parse(String cmdName, CommandLine cli) throws Exception { - switch (cmdName.toLowerCase(Locale.ROOT)) { - case Start.NAME: - return Start.parse(terminal, cli); - case Version.NAME: - return Version.parse(terminal, cli); - default: - assert false : "should never get here, if the user enters an unknown command, an error message should be shown before parse is called"; - return null; - } - } - - static class Version extends CliTool.Command { - - private static final String NAME = "version"; - - private static final CliToolConfig.Cmd CMD = cmd(NAME, Version.class).build(); - - public static Command parse(Terminal terminal, CommandLine cli) { - return new Version(terminal); - } - - public Version(Terminal terminal) { - super(terminal); - } - - @Override - public ExitStatus execute(Settings settings, Environment env) throws Exception { + protected void execute(Terminal terminal, OptionSet options) throws Exception { + if (options.has(versionOption)) { terminal.println("Version: " + org.elasticsearch.Version.CURRENT - + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date() - + ", JVM: " + JvmInfo.jvmInfo().version()); - return ExitStatus.OK_AND_EXIT; + + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date() + + ", JVM: " + JvmInfo.jvmInfo().version()); + return; } + + // TODO: don't use sysprops for any of these! pass the args through to bootstrap... + if (options.has(daemonizeOption)) { + System.setProperty("es.foreground", "false"); + } + String pidFile = pidfileOption.value(options); + if (Strings.isNullOrEmpty(pidFile) == false) { + System.setProperty("es.pidfile", pidFile); + } + + for (String property : propertyOption.values(options)) { + String[] keyValue = property.split("=", 2); + if (keyValue.length != 2) { + throw new UserError(ExitCodes.USAGE, "Malformed elasticsearch setting, must be of the form key=value"); + } + System.setProperty("es." + keyValue[0], keyValue[1]); + } + shouldRun = true; } - static class Start extends CliTool.Command { - - private static final String NAME = "start"; - - private static final CliToolConfig.Cmd CMD = cmd(NAME, Start.class) - .options( - optionBuilder("d", "daemonize").hasArg(false).required(false), - optionBuilder("p", "pidfile").hasArg(true).required(false), - optionBuilder("V", "version").hasArg(false).required(false), - Option.builder("D").argName("property=value").valueSeparator('=').numberOfArgs(2) - ) - .stopAtNonOption(true) // needed to parse the --foo.bar options, so this parser must be lenient - .build(); - - // 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) throws UserError { - if (cli.hasOption("V")) { - return Version.parse(terminal, cli); - } - - if (cli.hasOption("d")) { - System.setProperty("es.foreground", "false"); - } - - String pidFile = cli.getOptionValue("pidfile"); - if (!Strings.isNullOrEmpty(pidFile)) { - System.setProperty("es.pidfile", pidFile); - } - - if (cli.hasOption("D")) { - Properties properties = cli.getOptionProperties("D"); - for (Map.Entry entry : properties.entrySet()) { - String key = (String) entry.getKey(); - String propertyName = key.startsWith("es.") ? key : "es." + key; - System.setProperty(propertyName, entry.getValue().toString()); - } - } - - // hacky way to extract all the fancy extra args, there is no CLI tool helper for this - Iterator iterator = cli.getArgList().iterator(); - final Map properties = new HashMap<>(); - while (iterator.hasNext()) { - String arg = iterator.next(); - if (!arg.startsWith("--")) { - if (arg.startsWith("-D") || arg.startsWith("-d") || arg.startsWith("-p")) { - throw new UserError(ExitStatus.USAGE.status(), - "Parameter [" + arg + "] starting with \"-D\", \"-d\" or \"-p\" must be before any parameters starting with --" - ); - } else { - throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "]does not start with --"); - } - } - // if there is no = sign, we have to get the next argu - arg = arg.replace("--", ""); - if (arg.contains("=")) { - String[] splitArg = arg.split("=", 2); - String key = splitArg[0]; - String value = splitArg[1]; - properties.put("es." + key, value); - } else { - if (iterator.hasNext()) { - String value = iterator.next(); - if (value.startsWith("--")) { - throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "] needs value"); - } - properties.put("es." + arg, value); - } else { - throw new UserError(ExitStatus.USAGE.status(), "Parameter [" + arg + "] needs value"); - } - } - } - for (Map.Entry entry : properties.entrySet()) { - System.setProperty(entry.getKey(), entry.getValue()); - } - return new Start(terminal); - } - - public Start(Terminal terminal) { - super(terminal); - - } - - @Override - public ExitStatus execute(Settings settings, Environment env) throws Exception { - return ExitStatus.OK; - } + boolean shouldRun() { + return shouldRun; } - } diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 107a955696c..214efe483ca 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -21,6 +21,8 @@ package org.elasticsearch.bootstrap; import java.io.IOException; +import org.elasticsearch.common.cli.Terminal; + /** * This class starts elasticsearch. */ @@ -32,9 +34,16 @@ public final class Elasticsearch { /** * Main entry point for starting elasticsearch */ - public static void main(String[] args) throws StartupError { + public static void main(String[] args) throws Exception { + BootstrapCliParser parser = new BootstrapCliParser(); + parser.main(args, Terminal.DEFAULT); + + if (parser.shouldRun() == false) { + return; + } + try { - Bootstrap.init(args); + Bootstrap.init(); } catch (Throwable t) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java index 6e57905b5b2..f608d0cefb1 100644 --- a/core/src/main/java/org/elasticsearch/cli/Command.java +++ b/core/src/main/java/org/elasticsearch/cli/Command.java @@ -21,6 +21,7 @@ package org.elasticsearch.cli; import java.io.IOException; import java.util.Arrays; +import java.util.List; import joptsimple.OptionException; import joptsimple.OptionParser; @@ -49,38 +50,9 @@ public abstract class Command { } /** Parses options for this command from args and executes it. */ - protected final int main(String[] args, Terminal terminal) throws Exception { - - final OptionSet options; + public final int main(String[] args, Terminal terminal) throws Exception { try { - options = parser.parse(args); - } catch (OptionException e) { - printHelp(terminal); - terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); - return ExitCodes.USAGE; - } - - if (options.has(helpOption)) { - printHelp(terminal); - return ExitCodes.OK; - } - - if (options.has(silentOption)) { - if (options.has(verboseOption)) { - // mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it - printHelp(terminal); - terminal.println(Terminal.Verbosity.SILENT, "ERROR: Cannot specify -s and -v together"); - return ExitCodes.USAGE; - } - terminal.setVerbosity(Terminal.Verbosity.SILENT); - } else if (options.has(verboseOption)) { - terminal.setVerbosity(Terminal.Verbosity.VERBOSE); - } else { - terminal.setVerbosity(Terminal.Verbosity.NORMAL); - } - - try { - return execute(terminal, options); + mainWithoutErrorHandling(args, terminal); } catch (OptionException e) { printHelp(terminal); terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); @@ -89,6 +61,33 @@ public abstract class Command { terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); return e.exitCode; } + return ExitCodes.OK; + } + + /** + * Executes the command, but all errors are thrown. + */ + void mainWithoutErrorHandling(String[] args, Terminal terminal) throws Exception { + final OptionSet options = parser.parse(args); + + if (options.has(helpOption)) { + printHelp(terminal); + return; + } + + if (options.has(silentOption)) { + if (options.has(verboseOption)) { + // mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it + throw new UserError(ExitCodes.USAGE, "Cannot specify -s and -v together"); + } + terminal.setVerbosity(Terminal.Verbosity.SILENT); + } else if (options.has(verboseOption)) { + terminal.setVerbosity(Terminal.Verbosity.VERBOSE); + } else { + terminal.setVerbosity(Terminal.Verbosity.NORMAL); + } + + execute(terminal, options); } /** Prints a help message for the command to the terminal. */ @@ -111,5 +110,5 @@ public abstract class Command { * Executes this command. * * Any runtime user errors (like an input file that does not exist), should throw a {@link UserError}. */ - protected abstract int execute(Terminal terminal, OptionSet options) throws Exception; + protected abstract void execute(Terminal terminal, OptionSet options) throws Exception; } diff --git a/core/src/main/java/org/elasticsearch/cli/MultiCommand.java b/core/src/main/java/org/elasticsearch/cli/MultiCommand.java index 94c403d57d0..5862b6f2311 100644 --- a/core/src/main/java/org/elasticsearch/cli/MultiCommand.java +++ b/core/src/main/java/org/elasticsearch/cli/MultiCommand.java @@ -56,7 +56,7 @@ public class MultiCommand extends Command { } @Override - protected int execute(Terminal terminal, OptionSet options) throws Exception { + protected void execute(Terminal terminal, OptionSet options) throws Exception { if (subcommands.isEmpty()) { throw new IllegalStateException("No subcommands configured"); } @@ -68,6 +68,6 @@ public class MultiCommand extends Command { if (subcommand == null) { throw new UserError(ExitCodes.USAGE, "Unknown command [" + args[0] + "]"); } - return subcommand.main(Arrays.copyOfRange(args, 1, args.length), terminal); + subcommand.mainWithoutErrorHandling(Arrays.copyOfRange(args, 1, args.length), terminal); } } diff --git a/core/src/main/java/org/elasticsearch/cli/TestCommand.java b/core/src/main/java/org/elasticsearch/cli/TestCommand.java deleted file mode 100644 index fe3fa5c6b8c..00000000000 --- a/core/src/main/java/org/elasticsearch/cli/TestCommand.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * 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.cli; - -import joptsimple.OptionSet; -import org.elasticsearch.common.cli.Terminal; - -public class TestCommand extends Command { - - public static void main(String[] args) throws Exception { - exit(new TestCommand().main(args, Terminal.DEFAULT)); - } - - public TestCommand() { - super("some test cli"); - parser.accepts("foo", "some option"); - } - - @Override - protected int execute(Terminal terminal, OptionSet options) throws Exception { - terminal.println("running"); - return ExitCodes.OK; - } -} diff --git a/plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java b/plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java deleted file mode 100644 index 03c6e65047a..00000000000 --- a/plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java +++ /dev/null @@ -1,189 +0,0 @@ -/* - * 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.mapper.attachments; - -import org.apache.commons.cli.CommandLine; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.CliToolConfig; -import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.common.compress.CompressedXContent; -import org.elasticsearch.common.io.PathUtils; -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.env.Environment; -import org.elasticsearch.index.MapperTestUtils; -import org.elasticsearch.index.mapper.DocumentMapper; -import org.elasticsearch.index.mapper.DocumentMapperParser; -import org.elasticsearch.index.mapper.ParseContext; - -import java.io.IOException; -import java.io.InputStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Locale; - -import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; -import static org.elasticsearch.common.cli.CliToolConfig.Builder.option; -import static org.elasticsearch.common.io.Streams.copy; -import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; -import static org.elasticsearch.mapper.attachments.AttachmentUnitTestCase.getIndicesModuleWithRegisteredAttachmentMapper; -import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath; - -/** - * This class provides a simple main class which can be used to test what is extracted from a given binary file. - * You can run it using - * -u file://URL/TO/YOUR/DOC - * --size set extracted size (default to mapper attachment size) - * BASE64 encoded binary - * - * Example: - * StandaloneRunner BASE64Text - * StandaloneRunner -u /tmp/mydoc.pdf - * StandaloneRunner -u /tmp/mydoc.pdf --size 1000000 - */ -@SuppressForbidden(reason = "commandline tool") -public class StandaloneRunner extends CliTool { - - private static final CliToolConfig CONFIG = CliToolConfig.config("tika", StandaloneRunner.class) - .cmds(TikaRunner.CMD) - .build(); - - static { - System.setProperty("es.path.home", "/tmp"); - } - - static class TikaRunner extends Command { - private static final String NAME = "tika"; - private final String url; - private final Integer size; - private final String base64text; - private final DocumentMapper docMapper; - - private static final CliToolConfig.Cmd CMD = cmd(NAME, TikaRunner.class) - .options(option("u", "url").required(false).hasArg(false)) - .options(option("t", "size").required(false).hasArg(false)) - .build(); - - protected TikaRunner(Terminal terminal, String url, Integer size, String base64text) throws IOException { - super(terminal); - this.size = size; - this.url = url; - this.base64text = base64text; - DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(PathUtils.get("."), Settings.EMPTY, getIndicesModuleWithRegisteredAttachmentMapper()).documentMapperParser(); // use CWD b/c it won't be used - - String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/attachment/test/standalone/standalone-mapping.json"); - docMapper = mapperParser.parse("person", new CompressedXContent(mapping)); - } - - @Override - public ExitStatus execute(Settings settings, Environment env) throws Exception { - XContentBuilder builder = jsonBuilder().startObject().field("file").startObject(); - - if (base64text != null) { - // If base64 is provided - builder.field("_content", base64text); - } else { - // A file is provided - byte[] bytes = copyToBytes(PathUtils.get(url)); - builder.field("_content", bytes); - } - - if (size >= 0) { - builder.field("_indexed_chars", size); - } - - BytesReference json = builder.endObject().endObject().bytes(); - - ParseContext.Document doc = docMapper.parse("person", "person", "1", json).rootDoc(); - - terminal.println("## Extracted text"); - terminal.println("--------------------- BEGIN -----------------------"); - terminal.println(doc.get("file.content")); - terminal.println("---------------------- END ------------------------"); - terminal.println("## Metadata"); - printMetadataContent(doc, AttachmentMapper.FieldNames.AUTHOR); - printMetadataContent(doc, AttachmentMapper.FieldNames.CONTENT_LENGTH); - printMetadataContent(doc, AttachmentMapper.FieldNames.CONTENT_TYPE); - printMetadataContent(doc, AttachmentMapper.FieldNames.DATE); - printMetadataContent(doc, AttachmentMapper.FieldNames.KEYWORDS); - printMetadataContent(doc, AttachmentMapper.FieldNames.LANGUAGE); - printMetadataContent(doc, AttachmentMapper.FieldNames.NAME); - printMetadataContent(doc, AttachmentMapper.FieldNames.TITLE); - - return ExitStatus.OK; - } - - private void printMetadataContent(ParseContext.Document doc, String field) { - terminal.println("- " + field + ":" + doc.get(docMapper.mappers().getMapper("file." + field).fieldType().name())); - } - - public static byte[] copyToBytes(Path path) throws IOException { - try (InputStream is = Files.newInputStream(path); - BytesStreamOutput out = new BytesStreamOutput()) { - copy(is, out); - return out.bytes().toBytes(); - } - } - - public static Command parse(Terminal terminal, CommandLine cli) throws IOException { - String url = cli.getOptionValue("u"); - String base64text = null; - String sSize = cli.getOptionValue("size"); - Integer size = sSize != null ? Integer.parseInt(sSize) : -1; - if (url == null && cli.getArgs().length == 0) { - return exitCmd(ExitStatus.USAGE, terminal, "url or BASE64 content should be provided (type -h for help)"); - } - if (url == null) { - if (cli.getArgs().length == 0) { - return exitCmd(ExitStatus.USAGE, terminal, "url or BASE64 content should be provided (type -h for help)"); - } - base64text = cli.getArgs()[0]; - } else { - if (cli.getArgs().length == 1) { - return exitCmd(ExitStatus.USAGE, terminal, "url or BASE64 content should be provided. Not both. (type -h for help)"); - } - } - return new TikaRunner(terminal, url, size, base64text); - } - } - - public StandaloneRunner() { - super(CONFIG); - } - - - public static void main(String[] args) throws Exception { - StandaloneRunner pluginManager = new StandaloneRunner(); - pluginManager.execute(args); - } - - @Override - protected Command parse(String cmdName, CommandLine cli) throws Exception { - switch (cmdName.toLowerCase(Locale.ROOT)) { - case TikaRunner.NAME: return TikaRunner.parse(terminal, cli); - default: - assert false : "can't get here as cmd name is validated before this method is called"; - return exitCmd(ExitStatus.CODE_ERROR); - } - } -} diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java index 18d6e0ac3c9..726d17b0938 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java @@ -19,11 +19,14 @@ package org.elasticsearch.bootstrap; +import joptsimple.OptionException; import org.elasticsearch.Build; import org.elasticsearch.Version; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.CommandTestCase; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.cli.CliTool.ExitStatus; -import org.elasticsearch.common.cli.CliToolTestCase; import org.elasticsearch.cli.UserError; import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.common.collect.Tuple; @@ -46,9 +49,13 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; @SuppressForbidden(reason = "modifies system properties intentionally") -public class BootstrapCliParserTests extends CliToolTestCase { +public class BootstrapCliParserTests extends CommandTestCase { + + @Override + protected Command newCommand() { + return new BootstrapCliParser(); + } - private MockTerminal terminal = new MockTerminal(); private List propertiesToClear = new ArrayList<>(); private Map properties; @@ -66,195 +73,86 @@ public class BootstrapCliParserTests extends CliToolTestCase { assertEquals("properties leaked", properties, new HashMap<>(System.getProperties())); } - public void testThatVersionIsReturned() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - ExitStatus status = parser.execute(args("version")); - assertStatus(status, OK_AND_EXIT); - - String output = terminal.getOutput(); - assertTrue(output, output.contains(Version.CURRENT.toString())); - assertTrue(output, output.contains(Build.CURRENT.shortHash())); - assertTrue(output, output.contains(Build.CURRENT.date())); - assertTrue(output, output.contains(JvmInfo.jvmInfo().version())); + void assertShouldRun(boolean shouldRun) { + BootstrapCliParser parser = (BootstrapCliParser)command; + assertEquals(shouldRun, parser.shouldRun()); } - public void testThatVersionIsReturnedAsStartParameter() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - ExitStatus status = parser.execute(args("start -V")); - assertStatus(status, OK_AND_EXIT); - - String output = terminal.getOutput(); + public void testVersion() throws Exception { + String output = execute("-V"); assertTrue(output, output.contains(Version.CURRENT.toString())); assertTrue(output, output.contains(Build.CURRENT.shortHash())); assertTrue(output, output.contains(Build.CURRENT.date())); assertTrue(output, output.contains(JvmInfo.jvmInfo().version())); + assertShouldRun(false); terminal.reset(); - parser = new BootstrapCLIParser(terminal); - status = parser.execute(args("start --version")); - assertStatus(status, OK_AND_EXIT); - - output = terminal.getOutput(); + output = execute("--version"); assertTrue(output, output.contains(Version.CURRENT.toString())); assertTrue(output, output.contains(Build.CURRENT.shortHash())); assertTrue(output, output.contains(Build.CURRENT.date())); assertTrue(output, output.contains(JvmInfo.jvmInfo().version())); + assertShouldRun(false); } - public void testThatPidFileCanBeConfigured() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); + public void testPidfile() throws Exception { registerProperties("es.pidfile"); - ExitStatus status = parser.execute(args("start --pidfile")); // missing pid file - assertStatus(status, USAGE); + // missing argument + OptionException e = expectThrows(OptionException.class, () -> { + execute("-p"); + }); + assertEquals("Option p/pidfile requires an argument", e.getMessage()); + assertShouldRun(false); // good cases - status = parser.execute(args("start --pidfile /tmp/pid")); - assertStatus(status, OK); + terminal.reset(); + execute("--pidfile", "/tmp/pid"); assertSystemProperty("es.pidfile", "/tmp/pid"); + assertShouldRun(true); System.clearProperty("es.pidfile"); - status = parser.execute(args("start -p /tmp/pid")); - assertStatus(status, OK); + terminal.reset(); + execute("-p", "/tmp/pid"); assertSystemProperty("es.pidfile", "/tmp/pid"); + assertShouldRun(true); } - public void testThatParsingDaemonizeWorks() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); + public void testNoDaemonize() throws Exception { registerProperties("es.foreground"); - ExitStatus status = parser.execute(args("start -d")); - assertStatus(status, OK); - assertThat(System.getProperty("es.foreground"), is("false")); + execute(); + assertSystemProperty("es.foreground", null); + assertShouldRun(true); } - public void testThatNotDaemonizingDoesNotConfigureProperties() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); + public void testDaemonize() throws Exception { registerProperties("es.foreground"); - ExitStatus status = parser.execute(args("start")); - assertStatus(status, OK); - assertThat(System.getProperty("es.foreground"), is(nullValue())); + execute("-d"); + assertSystemProperty("es.foreground", "false"); + assertShouldRun(true); + + System.clearProperty("es.foreground"); + execute("--daemonize"); + assertSystemProperty("es.foreground", "false"); + assertShouldRun(true); } - public void testThatJavaPropertyStyleArgumentsCanBeParsed() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); + public void testConfig() throws Exception { registerProperties("es.foo", "es.spam"); - ExitStatus status = parser.execute(args("start -Dfoo=bar -Dspam=eggs")); - assertStatus(status, OK); + execute("-Efoo=bar", "-Espam=eggs"); assertSystemProperty("es.foo", "bar"); assertSystemProperty("es.spam", "eggs"); + assertShouldRun(true); } - public void testThatJavaPropertyStyleArgumentsWithEsPrefixAreNotPrefixedTwice() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - registerProperties("es.spam", "es.pidfile"); - - ExitStatus status = parser.execute(args("start -Des.pidfile=/path/to/foo/elasticsearch/distribution/zip/target/integ-tests/es.pid -Dspam=eggs")); - assertStatus(status, OK); - assertThat(System.getProperty("es.es.pidfile"), is(nullValue())); - assertSystemProperty("es.pidfile", "/path/to/foo/elasticsearch/distribution/zip/target/integ-tests/es.pid"); - assertSystemProperty("es.spam", "eggs"); - } - - public void testThatUnknownLongOptionsCanBeParsed() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - registerProperties("es.network.host", "es.my.option"); - - ExitStatus status = parser.execute(args("start --network.host 127.0.0.1 --my.option=true")); - assertStatus(status, OK); - assertSystemProperty("es.network.host", "127.0.0.1"); - assertSystemProperty("es.my.option", "true"); - } - - public void testThatUnknownLongOptionsNeedAValue() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - registerProperties("es.network.host"); - - ExitStatus status = parser.execute(args("start --network.host")); - assertStatus(status, USAGE); - String output = terminal.getOutput(); - assertTrue(output, output.contains("Parameter [network.host] needs value")); - - terminal.reset(); - status = parser.execute(args("start --network.host --foo")); - assertStatus(status, USAGE); - output = terminal.getOutput(); - assertTrue(output, output.contains("Parameter [network.host] needs value")); - } - - public void testParsingErrors() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - - // unknown params - ExitStatus status = parser.execute(args("version --unknown-param /tmp/pid")); - assertStatus(status, USAGE); - String output = terminal.getOutput(); - assertTrue(output, output.contains("Unrecognized option: --unknown-param")); - - // single dash in extra params - terminal.reset(); - parser = new BootstrapCLIParser(terminal); - status = parser.execute(args("start -network.host 127.0.0.1")); - assertStatus(status, USAGE); - output = terminal.getOutput(); - assertTrue(output, output.contains("Parameter [-network.host]does not start with --")); - - // never ended parameter - terminal = new MockTerminal(); - parser = new BootstrapCLIParser(terminal); - status = parser.execute(args("start --network.host")); - assertStatus(status, USAGE); - output = terminal.getOutput(); - assertTrue(output, output.contains("Parameter [network.host] needs value")); - - // free floating value - terminal = new MockTerminal(); - parser = new BootstrapCLIParser(terminal); - status = parser.execute(args("start 127.0.0.1")); - assertStatus(status, USAGE); - output = terminal.getOutput(); - assertTrue(output, output.contains("Parameter [127.0.0.1]does not start with --")); - } - - public void testHelpWorks() throws Exception { - List> tuples = new ArrayList<>(); - tuples.add(new Tuple<>("version --help", "elasticsearch-version.help")); - tuples.add(new Tuple<>("version -h", "elasticsearch-version.help")); - tuples.add(new Tuple<>("start --help", "elasticsearch-start.help")); - tuples.add(new Tuple<>("start -h", "elasticsearch-start.help")); - tuples.add(new Tuple<>("--help", "elasticsearch.help")); - tuples.add(new Tuple<>("-h", "elasticsearch.help")); - - for (Tuple tuple : tuples) { - terminal.reset(); - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - ExitStatus status = parser.execute(args(tuple.v1())); - assertStatus(status, OK_AND_EXIT); - assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/bootstrap/" + tuple.v2()); - } - } - - public void testThatSpacesInParametersAreSupported() throws Exception { - // emulates: bin/elasticsearch --node.name "'my node with spaces'" --pidfile "'/tmp/my pid.pid'" - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); - registerProperties("es.pidfile", "es.my.param"); - - ExitStatus status = parser.execute("start", "--pidfile", "foo with space", "--my.param", "my awesome neighbour"); - assertStatus(status, OK); - assertSystemProperty("es.pidfile", "foo with space"); - assertSystemProperty("es.my.param", "my awesome neighbour"); - - } - - public void testThatHelpfulErrorMessageIsGivenWhenParametersAreOutOfOrder() throws Exception { - BootstrapCLIParser parser = new BootstrapCLIParser(terminal); + public void testConfigMalformed() throws Exception { UserError e = expectThrows(UserError.class, () -> { - parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"}); + execute("-Efoo"); }); - assertThat(e.getMessage(), containsString("must be before any parameters starting with --")); - assertNull(System.getProperty("es.foo")); + assertTrue(e.getMessage(), e.getMessage().contains("Malformed elasticsearch setting")); } private void registerProperties(String ... systemProperties) { @@ -265,8 +163,4 @@ public class BootstrapCliParserTests extends CliToolTestCase { String msg = String.format(Locale.ROOT, "Expected property %s to be %s, terminal output was %s", name, expectedValue, terminal.getOutput()); assertThat(msg, System.getProperty(name), is(expectedValue)); } - - private void assertStatus(ExitStatus status, ExitStatus expectedStatus) throws Exception { - assertThat(String.format(Locale.ROOT, "Expected status to be [%s], but was [%s], terminal output was %s", expectedStatus, status, terminal.getOutput()), status, is(expectedStatus)); - } } diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java index 3af25509adb..a9b31b636cc 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java @@ -29,20 +29,30 @@ import org.junit.Before; */ public abstract class CommandTestCase extends ESTestCase { + /** The terminal that execute uses. */ protected final MockTerminal terminal = new MockTerminal(); + /** The last command that was executed. */ + protected Command command; + @Before public void resetTerminal() { terminal.reset(); terminal.setVerbosity(Terminal.Verbosity.NORMAL); } + /** Creates a Command to test execution. */ protected abstract Command newCommand(); + /** + * Runs the command with the given args. + * + * Output can be found in {@link #terminal}. + * The command created can be found in {@link #command}. + */ public String execute(String... args) throws Exception { - Command command = newCommand(); - OptionSet options = command.parser.parse(args); - command.execute(terminal, options); + command = newCommand(); + command.mainWithoutErrorHandling(args, terminal); return terminal.getOutput(); } } diff --git a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java index bb01369ac50..b712b216f9a 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java @@ -47,7 +47,7 @@ public class MockTerminal extends Terminal { @Override public String readText(String prompt) { if (textInput.isEmpty()) { - return null; + throw new IllegalStateException("No text input configured for prompt [" + prompt + "]"); } return textInput.removeFirst(); } @@ -55,7 +55,7 @@ public class MockTerminal extends Terminal { @Override public char[] readSecret(String prompt) { if (secretInput.isEmpty()) { - return null; + throw new IllegalStateException("No secret input configured for prompt [" + prompt + "]"); } return secretInput.removeFirst().toCharArray(); } From 3836f3a73600b7af4751a90d21ac8ef076e9ad7b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 13:40:39 -0800 Subject: [PATCH 07/16] Remove reference to standalonerunner --- .../common/cli/CliToolConfig.java | 53 ------------------- .../plugins/InstallPluginCommand.java | 21 ++++---- .../plugins/ListPluginsCommand.java | 4 +- .../plugins/RemovePluginCommand.java | 3 +- .../elasticsearch/plugins/PluginCliTests.java | 30 ++++++++++- docs/plugins/mapper-attachments.asciidoc | 38 ------------- 6 files changed, 41 insertions(+), 108 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java b/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java index d0ba897b33d..ff752f45ef0 100644 --- a/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java +++ b/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java @@ -33,10 +33,6 @@ import java.util.Map; */ public class CliToolConfig { - public static Builder config(String name, Class toolType) { - return new Builder(name, toolType); - } - private final Class toolType; private final String name; private final Map cmds; @@ -82,55 +78,6 @@ public class CliToolConfig { helpPrinter.print(this, terminal); } - public static class Builder { - - public static Cmd.Builder cmd(String name, Class cmdType) { - return new Cmd.Builder(name, cmdType); - } - - public static OptionBuilder option(String shortName, String longName) { - return new OptionBuilder(shortName, longName); - } - - public static Option.Builder optionBuilder(String shortName, String longName) { - return Option.builder(shortName).argName(longName).longOpt(longName); - } - - public static OptionGroupBuilder optionGroup(boolean required) { - return new OptionGroupBuilder(required); - } - - private final Class toolType; - private final String name; - private Cmd[] cmds; - - private Builder(String name, Class toolType) { - this.name = name; - this.toolType = toolType; - } - - public Builder cmds(Cmd.Builder... cmds) { - this.cmds = new Cmd[cmds.length]; - for (int i = 0; i < cmds.length; i++) { - this.cmds[i] = cmds[i].build(); - this.cmds[i].toolName = name; - } - return this; - } - - public Builder cmds(Cmd... cmds) { - for (int i = 0; i < cmds.length; i++) { - cmds[i].toolName = name; - } - this.cmds = cmds; - return this; - } - - public CliToolConfig build() { - return new CliToolConfig(name, toolType, cmds); - } - } - public static class Cmd { private String toolName; diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index bbe00fddd8c..83273e6c1c4 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -145,7 +145,7 @@ class InstallPluginCommand extends Command { } @Override - protected int execute(Terminal terminal, OptionSet options) throws Exception { + protected void execute(Terminal terminal, OptionSet options) throws Exception { // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args List args = arguments.values(options); if (args.size() != 1) { @@ -154,7 +154,6 @@ class InstallPluginCommand extends Command { String pluginId = args.get(0); boolean isBatch = options.has(batchOption) || System.console() == null; execute(terminal, pluginId, isBatch); - return ExitCodes.OK; } // pkg private for testing @@ -222,14 +221,14 @@ class InstallPluginCommand extends Command { BufferedReader checksumReader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8)); expectedChecksum = checksumReader.readLine(); if (checksumReader.readLine() != null) { - throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "Invalid checksum file at " + checksumUrl); + throw new UserError(ExitCodes.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 UserError(CliTool.ExitStatus.IO_ERROR.status(), "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum); + throw new UserError(ExitCodes.IO_ERROR, "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum); } return zip; @@ -271,7 +270,7 @@ class InstallPluginCommand extends Command { Files.delete(zip); if (hasEsDir == false) { IOUtils.rm(target); - throw new UserError(CliTool.ExitStatus.DATA_ERROR.status(), "`elasticsearch` directory is missing in the plugin zip"); + throw new UserError(ExitCodes.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip"); } return target; } @@ -285,7 +284,7 @@ class InstallPluginCommand extends 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 UserError(CliTool.ExitStatus.USAGE.status(), "plugin '" + info.getName() + "' cannot be installed like this, it is a system module"); + throw new UserError(ExitCodes.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module"); } // check for jar hell before any copying @@ -341,7 +340,7 @@ class InstallPluginCommand extends Command { final Path destination = env.pluginsFile().resolve(info.getName()); if (Files.exists(destination)) { - throw new UserError(CliTool.ExitStatus.USAGE.status(), "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command"); + throw new UserError(ExitCodes.USAGE, "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command"); } Path tmpBinDir = tmpRoot.resolve("bin"); @@ -374,7 +373,7 @@ class InstallPluginCommand extends 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 Exception { if (Files.isDirectory(tmpBinDir) == false) { - throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "bin in plugin " + info.getName() + " is not a directory"); + throw new UserError(ExitCodes.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory"); } Files.createDirectory(destBinDir); @@ -392,7 +391,7 @@ class InstallPluginCommand extends Command { try (DirectoryStream stream = Files.newDirectoryStream(tmpBinDir)) { for (Path srcFile : stream) { if (Files.isDirectory(srcFile)) { - throw new UserError(CliTool.ExitStatus.DATA_ERROR.status(), "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName()); + throw new UserError(ExitCodes.DATA_ERROR, "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName()); } Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile)); @@ -413,7 +412,7 @@ class InstallPluginCommand extends Command { */ private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws Exception { if (Files.isDirectory(tmpConfigDir) == false) { - throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "config in plugin " + info.getName() + " is not a directory"); + throw new UserError(ExitCodes.IO_ERROR, "config in plugin " + info.getName() + " is not a directory"); } // create the plugin's config dir "if necessary" @@ -422,7 +421,7 @@ class InstallPluginCommand extends Command { try (DirectoryStream stream = Files.newDirectoryStream(tmpConfigDir)) { for (Path srcFile : stream) { if (Files.isDirectory(srcFile)) { - throw new UserError(CliTool.ExitStatus.DATA_ERROR.status(), "Directories not allowed in config dir for plugin " + info.getName()); + throw new UserError(ExitCodes.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName()); } Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile)); diff --git a/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java b/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java index 142a18cbde5..276e38f1595 100644 --- a/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java @@ -44,7 +44,7 @@ class ListPluginsCommand extends Command { } @Override - public int execute(Terminal terminal, OptionSet options) throws Exception { + protected void execute(Terminal terminal, OptionSet options) throws Exception { if (Files.exists(env.pluginsFile()) == false) { throw new IOException("Plugins directory missing: " + env.pluginsFile()); } @@ -55,7 +55,5 @@ class ListPluginsCommand extends Command { terminal.println(plugin.getFileName().toString()); } } - - return ExitCodes.OK; } } diff --git a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java index 10a73a0fc9a..f435a16edf0 100644 --- a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java @@ -53,14 +53,13 @@ class RemovePluginCommand extends Command { } @Override - public int execute(Terminal terminal, OptionSet options) throws Exception { + protected void execute(Terminal terminal, OptionSet options) throws Exception { // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args List args = arguments.values(options); if (args.size() != 1) { throw new UserError(ExitCodes.USAGE, "Must supply a single plugin id argument"); } execute(terminal, args.get(0)); - return ExitCodes.OK; } // pkg private for testing diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java index 708cefd91b2..73d97949571 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java @@ -19,13 +19,40 @@ package org.elasticsearch.plugins; +import java.io.IOException; +import java.nio.file.Path; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.CommandTestCase; import org.elasticsearch.common.cli.CliToolTestCase; import org.elasticsearch.cli.MockTerminal; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.junit.Before; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; -public class PluginCliTests extends CliToolTestCase { +public class PluginCliTests extends CommandTestCase { + + // the home dir for each test to use + Path homeDir; + + // settings used to create an Environment for tools + Settings.Builder settingsBuilder; + + @Before + public void setupHome() { + homeDir = createTempDir(); + settingsBuilder = Settings.builder() + .put("path.home", homeDir); + } + + @Override + protected Command newCommand() { + return new PluginCli(new Environment(settingsBuilder.build())); + } + public void testHelpWorks() throws Exception { MockTerminal terminal = new MockTerminal(); /* nocommit @@ -48,4 +75,5 @@ public class PluginCliTests extends CliToolTestCase { assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-list.help"); */ } + } diff --git a/docs/plugins/mapper-attachments.asciidoc b/docs/plugins/mapper-attachments.asciidoc index ed992623a50..0ad452d92e9 100644 --- a/docs/plugins/mapper-attachments.asciidoc +++ b/docs/plugins/mapper-attachments.asciidoc @@ -405,41 +405,3 @@ It gives back: } } -------------------------- - -[[mapper-attachments-standalone]] -==== Stand alone runner - -If you want to run some tests within your IDE, you can use `StandaloneRunner` class. -It accepts arguments: - -* `-u file://URL/TO/YOUR/DOC` -* `--size` set extracted size (default to mapper attachment size) -* `BASE64` encoded binary - -Example: - -[source,sh] --------------------------- -StandaloneRunner BASE64Text -StandaloneRunner -u /tmp/mydoc.pdf -StandaloneRunner -u /tmp/mydoc.pdf --size 1000000 --------------------------- - -It produces something like: - -[source,text] --------------------------- -## Extracted text ---------------------- BEGIN ----------------------- -This is the extracted text ----------------------- END ------------------------ -## Metadata -- author: null -- content_length: null -- content_type: application/pdf -- date: null -- keywords: null -- language: null -- name: null -- title: null --------------------------- From 80198accc11aee7672a309917686cd93cf2a8f86 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 14:13:55 -0800 Subject: [PATCH 08/16] Removed old cli stuff, and add tests for new Command behavior --- .../elasticsearch/bootstrap/Bootstrap.java | 2 +- .../bootstrap/BootstrapCLIParser.java | 2 +- .../bootstrap/Elasticsearch.java | 2 +- .../java/org/elasticsearch/cli/Command.java | 2 - .../org/elasticsearch/cli/MultiCommand.java | 2 - .../{common => }/cli/Terminal.java | 2 +- .../org/elasticsearch/common/cli/CliTool.java | 252 ------------------ .../common/cli/CliToolConfig.java | 249 ----------------- .../elasticsearch/common/cli/HelpPrinter.java | 57 ---- .../common/logging/TerminalAppender.java | 2 +- .../internal/InternalSettingsPreparer.java | 2 +- .../plugins/InstallPluginCommand.java | 5 +- .../plugins/ListPluginsCommand.java | 4 +- .../org/elasticsearch/plugins/PluginCli.java | 2 +- .../elasticsearch/plugins/PluginSecurity.java | 4 +- .../plugins/RemovePluginCommand.java | 9 +- .../org/elasticsearch/cli/CommandTests.java | 123 +++++++++ .../elasticsearch/cli/MultiCommandTests.java | 28 ++ .../{common => }/cli/TerminalTests.java | 3 +- .../bootstrap/BootstrapCliParserTests.java | 31 +-- .../plugins/PluginSecurityTests.java | 2 +- .../elasticsearch/cli/CommandTestCase.java | 2 - .../org/elasticsearch/cli/MockTerminal.java | 2 - 23 files changed, 180 insertions(+), 609 deletions(-) rename core/src/main/java/org/elasticsearch/{common => }/cli/Terminal.java (99%) delete mode 100644 core/src/main/java/org/elasticsearch/common/cli/CliTool.java delete mode 100644 core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java delete mode 100644 core/src/main/java/org/elasticsearch/common/cli/HelpPrinter.java create mode 100644 core/src/test/java/org/elasticsearch/cli/CommandTests.java create mode 100644 core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java rename core/src/test/java/org/elasticsearch/{common => }/cli/TerminalTests.java (96%) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 73654fdfee5..5008229f5f8 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -26,7 +26,7 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.common.PidFile; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.inject.CreationException; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.LogConfigurator; diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java index 3567039cd42..e44e397f67a 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java @@ -28,7 +28,7 @@ import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserError; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.monitor.jvm.JvmInfo; final class BootstrapCliParser extends Command { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 214efe483ca..1d2a0b98232 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -21,7 +21,7 @@ package org.elasticsearch.bootstrap; import java.io.IOException; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; /** * This class starts elasticsearch. diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java index f608d0cefb1..9e6afdd6638 100644 --- a/core/src/main/java/org/elasticsearch/cli/Command.java +++ b/core/src/main/java/org/elasticsearch/cli/Command.java @@ -21,14 +21,12 @@ package org.elasticsearch.cli; import java.io.IOException; import java.util.Arrays; -import java.util.List; import joptsimple.OptionException; import joptsimple.OptionParser; import joptsimple.OptionSet; import joptsimple.OptionSpec; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.cli.Terminal; /** * An action to execute within a cli. diff --git a/core/src/main/java/org/elasticsearch/cli/MultiCommand.java b/core/src/main/java/org/elasticsearch/cli/MultiCommand.java index 5862b6f2311..a9feee0c9bf 100644 --- a/core/src/main/java/org/elasticsearch/cli/MultiCommand.java +++ b/core/src/main/java/org/elasticsearch/cli/MultiCommand.java @@ -19,14 +19,12 @@ package org.elasticsearch.cli; -import java.io.IOException; import java.util.Arrays; import java.util.LinkedHashMap; import java.util.Map; import joptsimple.NonOptionArgumentSpec; import joptsimple.OptionSet; -import org.elasticsearch.common.cli.Terminal; /** * A cli tool which is made up of multiple subcommands. diff --git a/core/src/main/java/org/elasticsearch/common/cli/Terminal.java b/core/src/main/java/org/elasticsearch/cli/Terminal.java similarity index 99% rename from core/src/main/java/org/elasticsearch/common/cli/Terminal.java rename to core/src/main/java/org/elasticsearch/cli/Terminal.java index 6a1c4382e42..00d886aa8ab 100644 --- a/core/src/main/java/org/elasticsearch/common/cli/Terminal.java +++ b/core/src/main/java/org/elasticsearch/cli/Terminal.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.common.cli; +package org.elasticsearch.cli; import java.io.BufferedReader; import java.io.Console; diff --git a/core/src/main/java/org/elasticsearch/common/cli/CliTool.java b/core/src/main/java/org/elasticsearch/common/cli/CliTool.java deleted file mode 100644 index ba2007813d5..00000000000 --- a/core/src/main/java/org/elasticsearch/common/cli/CliTool.java +++ /dev/null @@ -1,252 +0,0 @@ -/* - * 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; - -import org.apache.commons.cli.AlreadySelectedException; -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.cli.UserError; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.elasticsearch.node.internal.InternalSettingsPreparer; - -import java.util.Locale; - -import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS; - -/** - * A base class for command-line interface tool. - * - * Two modes are supported: - * - * - Single command mode. The tool exposes a single command that can potentially accept arguments (eg. CLI options). - * - Multi command mode. The tool support multiple commands, each for different tasks, each potentially accepts arguments. - * - * In a multi-command mode. The first argument must be the command name. For example, the plugin manager - * can be seen as a multi-command tool with two possible commands: install and uninstall - * - * The tool is configured using a {@link CliToolConfig} which encapsulates the tool's commands and their - * potential options. The tool also comes with out of the box simple help support (the -h/--help option is - * automatically handled) where the help text is configured in a dedicated *.help files located in the same package - * as the tool. - */ -public abstract class CliTool { - - // based on sysexits.h - public enum ExitStatus { - OK(0), - OK_AND_EXIT(0), - USAGE(64), /* command line usage error */ - DATA_ERROR(65), /* data format error */ - NO_INPUT(66), /* cannot open input */ - NO_USER(67), /* addressee unknown */ - NO_HOST(68), /* host name unknown */ - UNAVAILABLE(69), /* service unavailable */ - CODE_ERROR(70), /* internal software error */ - CANT_CREATE(73), /* can't create (user) output file */ - IO_ERROR(74), /* input/output error */ - TEMP_FAILURE(75), /* temp failure; user is invited to retry */ - PROTOCOL(76), /* remote error in protocol */ - NOPERM(77), /* permission denied */ - CONFIG(78); /* configuration error */ - - final int status; - - ExitStatus(int status) { - this.status = status; - } - - public int status() { - return status; - } - } - - protected final Terminal terminal; - protected final Environment env; - protected final Settings settings; - - private final CliToolConfig config; - - protected CliTool(CliToolConfig config) { - this(config, Terminal.DEFAULT); - } - - protected CliTool(CliToolConfig config, Terminal terminal) { - if (config.cmds().size() == 0) { - throw new IllegalArgumentException("At least one command must be configured"); - } - this.config = config; - this.terminal = terminal; - env = InternalSettingsPreparer.prepareEnvironment(EMPTY_SETTINGS, terminal); - settings = env.settings(); - } - - 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 - // option will be taken care of on the command level - if (!config.isSingle() && args.length > 0 && (args[0].equals("-h") || args[0].equals("--help"))) { - config.printUsage(terminal); - return ExitStatus.OK_AND_EXIT; - } - - CliToolConfig.Cmd cmd; - if (config.isSingle()) { - cmd = config.single(); - } else { - - if (args.length == 0) { - terminal.println(Terminal.Verbosity.SILENT, "ERROR: command not specified"); - config.printUsage(terminal); - return ExitStatus.USAGE; - } - - String cmdName = args[0]; - cmd = config.cmd(cmdName); - if (cmd == null) { - terminal.println(Terminal.Verbosity.SILENT, "ERROR: unknown command [" + cmdName + "]. Use [-h] option to list available commands"); - return ExitStatus.USAGE; - } - - // we now remove the command name from the args - if (args.length == 1) { - args = new String[0]; - } else { - String[] cmdArgs = new String[args.length - 1]; - System.arraycopy(args, 1, cmdArgs, 0, cmdArgs.length); - args = cmdArgs; - } - } - - try { - return parse(cmd, args).execute(settings, env); - } catch (UserError error) { - terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + error.getMessage()); - return ExitStatus.USAGE; - //return error.exitCode; - } - } - - public Command parse(String cmdName, String[] args) throws Exception { - CliToolConfig.Cmd cmd = config.cmd(cmdName); - return parse(cmd, args); - } - - public Command parse(CliToolConfig.Cmd cmd, String[] args) throws Exception { - CommandLineParser parser = new DefaultParser(); - CommandLine cli = parser.parse(CliToolConfig.OptionsSource.HELP.options(), args, true); - if (cli.hasOption("h")) { - return helpCmd(cmd); - } - 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.status(), e.toString()); - } - - if (cli.hasOption("v")) { - terminal.setVerbosity(Terminal.Verbosity.VERBOSE); - } else if (cli.hasOption("s")) { - terminal.setVerbosity(Terminal.Verbosity.SILENT); - } else { - terminal.setVerbosity(Terminal.Verbosity.NORMAL); - } - return parse(cmd.name(), cli); - } - - protected Command.Help helpCmd(CliToolConfig.Cmd cmd) { - return new Command.Help(cmd, terminal); - } - - protected static Command.Exit exitCmd(ExitStatus status) { - return new Command.Exit(null, status, null); - } - - protected static Command.Exit exitCmd(ExitStatus status, Terminal terminal, String msg, Object... args) { - return new Command.Exit(String.format(Locale.ROOT, msg, args), status, terminal); - } - - protected abstract Command parse(String cmdName, CommandLine cli) throws Exception; - - public static abstract class Command { - - protected final Terminal terminal; - - protected Command(Terminal terminal) { - this.terminal = terminal; - } - - public abstract ExitStatus execute(Settings settings, Environment env) throws Exception; - - public static class Help extends Command { - - private final CliToolConfig.Cmd cmd; - - private Help(CliToolConfig.Cmd cmd, Terminal terminal) { - super(terminal); - this.cmd = cmd; - } - - @Override - public ExitStatus execute(Settings settings, Environment env) throws Exception { - cmd.printUsage(terminal); - return ExitStatus.OK_AND_EXIT; - } - } - - public static class Exit extends Command { - private final String msg; - private final ExitStatus status; - - private Exit(String msg, ExitStatus status, Terminal terminal) { - super(terminal); - this.msg = msg; - this.status = status; - } - - @Override - public ExitStatus execute(Settings settings, Environment env) throws Exception { - if (msg != null) { - if (status != ExitStatus.OK) { - terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + msg); - } else { - terminal.println(msg); - } - } - return status; - } - - public ExitStatus status() { - return status; - } - } - } - - - -} - diff --git a/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java b/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java deleted file mode 100644 index ff752f45ef0..00000000000 --- a/core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java +++ /dev/null @@ -1,249 +0,0 @@ -/* - * 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; - -import org.apache.commons.cli.Option; -import org.apache.commons.cli.OptionGroup; -import org.apache.commons.cli.Options; - -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -/** - * - */ -public class CliToolConfig { - - private final Class toolType; - private final String name; - private final Map cmds; - - private static final HelpPrinter helpPrinter = new HelpPrinter(); - - private CliToolConfig(String name, Class toolType, Cmd[] cmds) { - this.name = name; - this.toolType = toolType; - final Map cmdsMapping = new HashMap<>(); - for (int i = 0; i < cmds.length; i++) { - cmdsMapping.put(cmds[i].name, cmds[i]); - } - this.cmds = Collections.unmodifiableMap(cmdsMapping); - } - - public boolean isSingle() { - return cmds.size() == 1; - } - - public Cmd single() { - assert isSingle() : "Requesting single command on a multi-command tool"; - return cmds.values().iterator().next(); - } - - public Class toolType() { - return toolType; - } - - public String name() { - return name; - } - - public Collection cmds() { - return cmds.values(); - } - - public Cmd cmd(String name) { - return cmds.get(name); - } - - public void printUsage(Terminal terminal) { - helpPrinter.print(this, terminal); - } - - public static class Cmd { - - private String toolName; - private final String name; - private final Class cmdType; - private final Options options; - private final boolean stopAtNonOption; - - private Cmd(String name, Class cmdType, Options options, boolean stopAtNonOption) { - this.name = name; - this.cmdType = cmdType; - this.options = options; - this.stopAtNonOption = stopAtNonOption; - OptionsSource.VERBOSITY.populate(options); - } - - public Class cmdType() { - return cmdType; - } - - public String name() { - return name; - } - - public Options options() { - return options; - } - - public boolean isStopAtNonOption() { - return stopAtNonOption; - } - - public void printUsage(Terminal terminal) { - helpPrinter.print(toolName, this, terminal); - } - - public static class Builder { - - private final String name; - private final Class cmdType; - private Options options = new Options(); - private boolean stopAtNonOption = false; - - private Builder(String name, Class cmdType) { - this.name = name; - this.cmdType = cmdType; - } - - public Builder options(OptionBuilder... optionBuilder) { - for (int i = 0; i < optionBuilder.length; i++) { - options.addOption(optionBuilder[i].build()); - } - return this; - } - - public Builder options(Option.Builder... optionBuilders) { - for (int i = 0; i < optionBuilders.length; i++) { - options.addOption(optionBuilders[i].build()); - } - return this; - } - - public Builder optionGroups(OptionGroupBuilder... optionGroupBuilders) { - for (OptionGroupBuilder builder : optionGroupBuilders) { - options.addOptionGroup(builder.build()); - } - return this; - } - - /** - * @param stopAtNonOption if true an unrecognized argument stops - * the parsing and the remaining arguments are added to the - * args list. If false an unrecognized - * argument triggers a ParseException. - */ - public Builder stopAtNonOption(boolean stopAtNonOption) { - this.stopAtNonOption = stopAtNonOption; - return this; - } - - public Cmd build() { - return new Cmd(name, cmdType, options, stopAtNonOption); - } - } - } - - public static class OptionBuilder { - - private final Option option; - - private OptionBuilder(String shortName, String longName) { - option = new Option(shortName, ""); - option.setLongOpt(longName); - option.setArgName(longName); - } - - public OptionBuilder required(boolean required) { - option.setRequired(required); - return this; - } - - public OptionBuilder hasArg(boolean optional) { - option.setOptionalArg(optional); - option.setArgs(1); - return this; - } - - public Option build() { - return option; - } - } - - public static class OptionGroupBuilder { - - private OptionGroup group; - - private OptionGroupBuilder(boolean required) { - group = new OptionGroup(); - group.setRequired(required); - } - - public OptionGroupBuilder options(OptionBuilder... optionBuilders) { - for (OptionBuilder builder : optionBuilders) { - group.addOption(builder.build()); - } - return this; - } - - public OptionGroup build() { - return group; - } - - } - - static abstract class OptionsSource { - - static final OptionsSource HELP = new OptionsSource() { - - @Override - void populate(Options options) { - options.addOption(new OptionBuilder("h", "help").required(false).build()); - } - }; - - static final OptionsSource VERBOSITY = new OptionsSource() { - @Override - void populate(Options options) { - OptionGroup verbosityGroup = new OptionGroup(); - verbosityGroup.setRequired(false); - verbosityGroup.addOption(new OptionBuilder("s", "silent").required(false).build()); - verbosityGroup.addOption(new OptionBuilder("v", "verbose").required(false).build()); - options.addOptionGroup(verbosityGroup); - } - }; - - private Options options; - - Options options() { - if (options == null) { - options = new Options(); - populate(options); - } - return options; - } - - abstract void populate(Options options); - - } -} diff --git a/core/src/main/java/org/elasticsearch/common/cli/HelpPrinter.java b/core/src/main/java/org/elasticsearch/common/cli/HelpPrinter.java deleted file mode 100644 index ada6cc33a19..00000000000 --- a/core/src/main/java/org/elasticsearch/common/cli/HelpPrinter.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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; - -import org.elasticsearch.common.io.Streams; -import org.elasticsearch.common.util.Callback; - -import java.io.IOException; -import java.io.InputStream; - -/** - * - */ -public class HelpPrinter { - - private static final String HELP_FILE_EXT = ".help"; - - public void print(CliToolConfig config, Terminal terminal) { - print(config.toolType(), config.name(), terminal); - } - - public void print(String toolName, CliToolConfig.Cmd cmd, Terminal terminal) { - print(cmd.cmdType(), toolName + "-" + cmd.name(), terminal); - } - - private static void print(Class clazz, String name, final Terminal terminal) { - terminal.println(Terminal.Verbosity.SILENT, ""); - try (InputStream input = clazz.getResourceAsStream(name + HELP_FILE_EXT)) { - Streams.readAllLines(input, new Callback() { - @Override - public void handle(String line) { - terminal.println(Terminal.Verbosity.SILENT, line); - } - }); - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - terminal.println(Terminal.Verbosity.SILENT, ""); - } -} diff --git a/core/src/main/java/org/elasticsearch/common/logging/TerminalAppender.java b/core/src/main/java/org/elasticsearch/common/logging/TerminalAppender.java index 7031a62a999..e967ad9d79e 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/TerminalAppender.java +++ b/core/src/main/java/org/elasticsearch/common/logging/TerminalAppender.java @@ -22,7 +22,7 @@ package org.elasticsearch.common.logging; import org.apache.log4j.AppenderSkeleton; import org.apache.log4j.spi.LoggingEvent; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; /** * TerminalAppender logs event to Terminal.DEFAULT. It is used for example by the PluginCli. diff --git a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java index faf449586c1..c66cb08dae7 100644 --- a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java @@ -23,7 +23,7 @@ import org.elasticsearch.bootstrap.BootstrapInfo; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.Randomness; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 83273e6c1c4..32c4bf18507 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -27,8 +27,7 @@ import org.elasticsearch.Version; import org.elasticsearch.bootstrap.JarHell; import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserError; import org.elasticsearch.common.hash.MessageDigests; import org.elasticsearch.common.io.FileSystemUtils; @@ -59,7 +58,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; import static java.util.Collections.unmodifiableSet; -import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE; +import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE; import static org.elasticsearch.common.util.set.Sets.newHashSet; /** diff --git a/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java b/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java index 276e38f1595..953e698a4c2 100644 --- a/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java @@ -26,9 +26,7 @@ import java.nio.file.Path; import joptsimple.OptionSet; import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.env.Environment; /** diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginCli.java b/core/src/main/java/org/elasticsearch/plugins/PluginCli.java index 9f2e432a438..323b872044e 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginCli.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginCli.java @@ -22,7 +22,7 @@ package org.elasticsearch.plugins; import org.apache.log4j.BasicConfigurator; import org.apache.log4j.varia.NullAppender; import org.elasticsearch.cli.MultiCommand; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.node.internal.InternalSettingsPreparer; diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java b/core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java index b14bcaf2ff3..f9c3d1826c9 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java @@ -20,8 +20,8 @@ package org.elasticsearch.plugins; import org.apache.lucene.util.IOUtils; -import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.common.cli.Terminal.Verbosity; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.Terminal.Verbosity; import org.elasticsearch.env.Environment; import java.io.IOException; diff --git a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java index f435a16edf0..a3e6c375f83 100644 --- a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java @@ -32,11 +32,10 @@ import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserError; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.cli.CliTool; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.env.Environment; -import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE; +import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE; /** * A command for the plugin cli to remove a plugin from elasticsearch. @@ -68,7 +67,7 @@ class RemovePluginCommand extends Command { Path pluginDir = env.pluginsFile().resolve(pluginName); if (Files.exists(pluginDir) == false) { - throw new UserError(CliTool.ExitStatus.USAGE.status(), "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins."); + throw new UserError(ExitCodes.USAGE, "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins."); } List pluginPaths = new ArrayList<>(); @@ -76,7 +75,7 @@ class RemovePluginCommand extends Command { Path pluginBinDir = env.binFile().resolve(pluginName); if (Files.exists(pluginBinDir)) { if (Files.isDirectory(pluginBinDir) == false) { - throw new UserError(CliTool.ExitStatus.IO_ERROR.status(), "Bin dir for " + pluginName + " is not a directory"); + throw new UserError(ExitCodes.IO_ERROR, "Bin dir for " + pluginName + " is not a directory"); } pluginPaths.add(pluginBinDir); terminal.println(VERBOSE, "Removing: " + pluginBinDir); diff --git a/core/src/test/java/org/elasticsearch/cli/CommandTests.java b/core/src/test/java/org/elasticsearch/cli/CommandTests.java new file mode 100644 index 00000000000..153bd4600b9 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cli/CommandTests.java @@ -0,0 +1,123 @@ +/* + * 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.cli; + +import joptsimple.OptionSet; +import org.elasticsearch.test.ESTestCase; + +public class CommandTests extends ESTestCase { + + static class UserErrorCommand extends Command { + UserErrorCommand() { + super("Throws a user error"); + } + @Override + protected void execute(Terminal terminal, OptionSet options) throws Exception { + throw new UserError(ExitCodes.DATA_ERROR, "Bad input"); + } + } + + static class NoopCommand extends Command { + boolean executed = false; + NoopCommand() { + super("Does nothing"); + } + @Override + protected void execute(Terminal terminal, OptionSet options) throws Exception { + terminal.println("Normal output"); + terminal.println(Terminal.Verbosity.SILENT, "Silent output"); + terminal.println(Terminal.Verbosity.VERBOSE, "Verbose output"); + executed = true; + } + @Override + protected void printAdditionalHelp(Terminal terminal) { + terminal.println("Some extra help"); + } + } + + public void testHelp() throws Exception { + NoopCommand command = new NoopCommand(); + MockTerminal terminal = new MockTerminal(); + String[] args = {"-h"}; + int status = command.main(args, terminal); + String output = terminal.getOutput(); + assertEquals(output, ExitCodes.OK, status); + assertTrue(output, output.contains("Does nothing")); + assertTrue(output, output.contains("Some extra help")); + assertFalse(command.executed); + + command = new NoopCommand(); + String[] args2 = {"--help"}; + status = command.main(args2, terminal); + output = terminal.getOutput(); + assertEquals(output, ExitCodes.OK, status); + assertTrue(output, output.contains("Does nothing")); + assertTrue(output, output.contains("Some extra help")); + assertFalse(command.executed); + } + + public void testVerbositySilentAndVerbose() throws Exception { + MockTerminal terminal = new MockTerminal(); + NoopCommand command = new NoopCommand(); + String[] args = {"-v", "-s"}; + UserError e = expectThrows(UserError.class, () -> { + command.mainWithoutErrorHandling(args, terminal); + }); + assertTrue(e.getMessage(), e.getMessage().contains("Cannot specify -s and -v together")); + } + + public void testSilentVerbosity() throws Exception { + MockTerminal terminal = new MockTerminal(); + NoopCommand command = new NoopCommand(); + String[] args = {"-s"}; + command.main(args, terminal); + String output = terminal.getOutput(); + assertTrue(output, output.contains("Silent output")); + } + + public void testNormalVerbosity() throws Exception { + MockTerminal terminal = new MockTerminal(); + terminal.setVerbosity(Terminal.Verbosity.SILENT); + NoopCommand command = new NoopCommand(); + String[] args = {}; + command.main(args, terminal); + String output = terminal.getOutput(); + assertTrue(output, output.contains("Normal output")); + } + + public void testVerboseVerbosity() throws Exception { + MockTerminal terminal = new MockTerminal(); + NoopCommand command = new NoopCommand(); + String[] args = {"-v"}; + command.main(args, terminal); + String output = terminal.getOutput(); + assertTrue(output, output.contains("Verbose output")); + } + + public void testUserError() throws Exception { + MockTerminal terminal = new MockTerminal(); + UserErrorCommand command = new UserErrorCommand(); + String[] args = {}; + int status = command.main(args, terminal); + String output = terminal.getOutput(); + assertEquals(output, ExitCodes.DATA_ERROR, status); + assertTrue(output, output.contains("ERROR: Bad input")); + } +} diff --git a/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java b/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java new file mode 100644 index 00000000000..cdd7cb7e241 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java @@ -0,0 +1,28 @@ +/* + * 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.cli; + +public class MultiCommandTests extends CommandTestCase { + + @Override + protected Command newCommand() { + return null; + } +} diff --git a/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java b/core/src/test/java/org/elasticsearch/cli/TerminalTests.java similarity index 96% rename from core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java rename to core/src/test/java/org/elasticsearch/cli/TerminalTests.java index 12fc4cb77e4..6673bdbc858 100644 --- a/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java +++ b/core/src/test/java/org/elasticsearch/cli/TerminalTests.java @@ -17,9 +17,8 @@ * under the License. */ -package org.elasticsearch.common.cli; +package org.elasticsearch.cli; -import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.test.ESTestCase; public class TerminalTests extends ESTestCase { diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java index 726d17b0938..2fc08f23a06 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java @@ -19,21 +19,6 @@ package org.elasticsearch.bootstrap; -import joptsimple.OptionException; -import org.elasticsearch.Build; -import org.elasticsearch.Version; -import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.CommandTestCase; -import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.cli.CliTool.ExitStatus; -import org.elasticsearch.cli.UserError; -import org.elasticsearch.cli.MockTerminal; -import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.monitor.jvm.JvmInfo; -import org.junit.After; -import org.junit.Before; - import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -41,12 +26,18 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK; -import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT; -import static org.elasticsearch.common.cli.CliTool.ExitStatus.USAGE; -import static org.hamcrest.Matchers.containsString; +import joptsimple.OptionException; +import org.elasticsearch.Build; +import org.elasticsearch.Version; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.CommandTestCase; +import org.elasticsearch.cli.UserError; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.monitor.jvm.JvmInfo; +import org.junit.After; +import org.junit.Before; + import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; @SuppressForbidden(reason = "modifies system properties intentionally") public class BootstrapCliParserTests extends CommandTestCase { diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java index acc300c6cf5..466f7d05cd1 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java @@ -19,7 +19,7 @@ package org.elasticsearch.plugins; -import org.elasticsearch.common.cli.Terminal; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.test.ESTestCase; import java.nio.file.Path; diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java index a9b31b636cc..e9c6a2eec9c 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java @@ -19,8 +19,6 @@ package org.elasticsearch.cli; -import joptsimple.OptionSet; -import org.elasticsearch.common.cli.Terminal; import org.elasticsearch.test.ESTestCase; import org.junit.Before; diff --git a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java index b712b216f9a..bd8bd493cea 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java @@ -27,8 +27,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; import java.util.Deque; -import org.elasticsearch.common.cli.Terminal; - /** * A terminal for tests which captures all output, and * can be plugged with fake input. From 13424318db53cf1790b6539b6ed0d70491808ec2 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 14:16:39 -0800 Subject: [PATCH 09/16] Remove old help files --- .../bootstrap/elasticsearch-start.help | 28 ------------------- .../bootstrap/elasticsearch-version.help | 16 ----------- .../bootstrap/elasticsearch.help | 22 --------------- 3 files changed, 66 deletions(-) delete mode 100644 core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-start.help delete mode 100644 core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-version.help delete mode 100644 core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch.help diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-start.help b/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-start.help deleted file mode 100644 index 9b27a8dd390..00000000000 --- a/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-start.help +++ /dev/null @@ -1,28 +0,0 @@ -NAME - - start - Start Elasticsearch - -SYNOPSIS - - elasticsearch start - -DESCRIPTION - - This command starts Elasticsearch. You can configure it to run in the foreground, write a pid file - and configure arbitrary options that override file-based configuration. - -OPTIONS - - -h,--help Shows this message - - -p,--pidfile Creates a pid file in the specified path on start - - -d,--daemonize Starts Elasticsearch in the background - - -Dproperty=value Configures an Elasticsearch specific property, like -Dnetwork.host=127.0.0.1 - - --property=value Configures an elasticsearch specific property, like --network.host 127.0.0.1 - --property value - - NOTE: The -d, -p, and -D arguments must appear before any --property arguments. - diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-version.help b/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-version.help deleted file mode 100644 index 00f2a33401c..00000000000 --- a/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-version.help +++ /dev/null @@ -1,16 +0,0 @@ -NAME - - version - Show version information and exit - -SYNOPSIS - - elasticsearch version - -DESCRIPTION - - This command shows Elasticsearch version, timestamp and build information as well as JVM info - -OPTIONS - - -h,--help Shows this message - diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch.help b/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch.help deleted file mode 100644 index 83ee497dc21..00000000000 --- a/core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch.help +++ /dev/null @@ -1,22 +0,0 @@ -NAME - - elasticsearch - Manages elasticsearch - -SYNOPSIS - - elasticsearch - -DESCRIPTION - - Start an elasticsearch node - -COMMANDS - - start Start elasticsearch - - version Show version information and exit - -NOTES - - [*] For usage help on specific commands please type "elasticsearch -h" - From 73ebe36ed001e8202b342a66c1f6358b798ee727 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 17:27:53 -0800 Subject: [PATCH 10/16] More tests --- .../elasticsearch/bootstrap/Bootstrap.java | 26 +++--- .../bootstrap/Elasticsearch.java | 11 +-- .../elasticsearch/cli/MultiCommandTests.java | 79 ++++++++++++++++++- .../elasticsearch/plugins/PluginCliTests.java | 79 ------------------- 4 files changed, 96 insertions(+), 99 deletions(-) delete mode 100644 core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 5008229f5f8..6cd2b4d80fe 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -19,14 +19,22 @@ package org.elasticsearch.bootstrap; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Path; +import java.util.Locale; +import java.util.concurrent.CountDownLatch; + import org.apache.lucene.util.Constants; import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.StringHelper; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.PidFile; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.inject.CreationException; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.LogConfigurator; @@ -39,13 +47,6 @@ import org.elasticsearch.monitor.process.ProcessProbe; import org.elasticsearch.node.Node; import org.elasticsearch.node.internal.InternalSettingsPreparer; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintStream; -import java.nio.file.Path; -import java.util.Locale; -import java.util.concurrent.CountDownLatch; - import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS; /** @@ -217,10 +218,17 @@ final class Bootstrap { * This method is invoked by {@link Elasticsearch#main(String[])} * to startup elasticsearch. */ - static void init() throws Throwable { + static void init(String[] args) throws Throwable { // Set the system property before anything has a chance to trigger its use initLoggerPrefix(); + BootstrapCliParser parser = new BootstrapCliParser(); + int status = parser.main(args, Terminal.DEFAULT); + + if (parser.shouldRun() == false || status != ExitCodes.OK) { + exit(status); + } + INSTANCE = new Bootstrap(); boolean foreground = !"false".equals(System.getProperty("es.foreground", System.getProperty("es-foreground"))); diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 1d2a0b98232..3b95c3f4a6f 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -21,8 +21,6 @@ package org.elasticsearch.bootstrap; import java.io.IOException; -import org.elasticsearch.cli.Terminal; - /** * This class starts elasticsearch. */ @@ -35,15 +33,8 @@ public final class Elasticsearch { * Main entry point for starting elasticsearch */ public static void main(String[] args) throws Exception { - BootstrapCliParser parser = new BootstrapCliParser(); - parser.main(args, Terminal.DEFAULT); - - if (parser.shouldRun() == false) { - return; - } - try { - Bootstrap.init(); + Bootstrap.init(args); } catch (Throwable t) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. diff --git a/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java b/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java index cdd7cb7e241..4f91d378440 100644 --- a/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java +++ b/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java @@ -19,10 +19,87 @@ package org.elasticsearch.cli; +import joptsimple.OptionSet; +import org.junit.Before; + public class MultiCommandTests extends CommandTestCase { + static class DummyMultiCommand extends MultiCommand { + DummyMultiCommand() { + super("A dummy multi command"); + } + } + + static class DummySubCommand extends Command { + DummySubCommand() { + super("A dummy subcommand"); + } + @Override + protected void execute(Terminal terminal, OptionSet options) throws Exception { + terminal.println("Arguments: " + options.nonOptionArguments().toString()); + } + } + + DummyMultiCommand multiCommand; + + @Before + public void setupCommand() { + multiCommand = new DummyMultiCommand(); + } + @Override protected Command newCommand() { - return null; + return multiCommand; + } + + public void testNoCommandsConfigured() throws Exception { + IllegalStateException e = expectThrows(IllegalStateException.class, () -> { + execute(); + }); + assertEquals("No subcommands configured", e.getMessage()); + } + + public void testUnknownCommand() throws Exception { + multiCommand.subcommands.put("something", new DummySubCommand()); + UserError e = expectThrows(UserError.class, () -> { + execute("somethingelse"); + }); + assertEquals(ExitCodes.USAGE, e.exitCode); + assertEquals("Unknown command [somethingelse]", e.getMessage()); + } + + public void testMissingCommand() throws Exception { + multiCommand.subcommands.put("command1", new DummySubCommand()); + UserError e = expectThrows(UserError.class, () -> { + execute(); + }); + assertEquals(ExitCodes.USAGE, e.exitCode); + assertEquals("Missing command", e.getMessage()); + } + + public void testHelp() throws Exception { + multiCommand.subcommands.put("command1", new DummySubCommand()); + multiCommand.subcommands.put("command2", new DummySubCommand()); + execute("-h"); + String output = terminal.getOutput(); + assertTrue(output, output.contains("command1")); + assertTrue(output, output.contains("command2")); + } + + public void testSubcommandHelp() throws Exception { + multiCommand.subcommands.put("command1", new DummySubCommand()); + multiCommand.subcommands.put("command2", new DummySubCommand()); + execute("command2", "-h"); + String output = terminal.getOutput(); + assertFalse(output, output.contains("command1")); + assertTrue(output, output.contains("A dummy subcommand")); + } + + public void testSubcommandArguments() throws Exception { + multiCommand.subcommands.put("command1", new DummySubCommand()); + execute("command1", "foo", "bar"); + String output = terminal.getOutput(); + assertFalse(output, output.contains("command1")); + assertTrue(output, output.contains("Arguments: [foo, bar]")); } } diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java deleted file mode 100644 index 73d97949571..00000000000 --- a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * 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.plugins; - -import java.io.IOException; -import java.nio.file.Path; - -import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.CommandTestCase; -import org.elasticsearch.common.cli.CliToolTestCase; -import org.elasticsearch.cli.MockTerminal; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.junit.Before; - -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.is; - -public class PluginCliTests extends CommandTestCase { - - // the home dir for each test to use - Path homeDir; - - // settings used to create an Environment for tools - Settings.Builder settingsBuilder; - - @Before - public void setupHome() { - homeDir = createTempDir(); - settingsBuilder = Settings.builder() - .put("path.home", homeDir); - } - - @Override - protected Command newCommand() { - return new PluginCli(new Environment(settingsBuilder.build())); - } - - public void testHelpWorks() throws Exception { - MockTerminal terminal = new MockTerminal(); - /* nocommit - assertThat(new PluginCli(terminal).execute(args("--help")), is(OK_AND_EXIT)); - assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin.help"); - - terminal.resetOutput(); - assertThat(new PluginCli(terminal).execute(args("install -h")), is(OK_AND_EXIT)); - assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-install.help"); - for (String plugin : InstallPluginCommand.OFFICIAL_PLUGINS) { - assertThat(terminal.getOutput(), containsString(plugin)); - } - - terminal.resetOutput(); - assertThat(new PluginCli(terminal).execute(args("remove --help")), is(OK_AND_EXIT)); - assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-remove.help"); - - terminal.resetOutput(); - assertThat(new PluginCli(terminal).execute(args("list -h")), is(OK_AND_EXIT)); - assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-list.help"); - */ - } - -} From 6cfdf9f4404a82f9da1f9a75d191184659e117c8 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 17:29:31 -0800 Subject: [PATCH 11/16] Remove old commons-cli dep --- core/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/core/build.gradle b/core/build.gradle index 226158ca094..ab3754e72ff 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -49,7 +49,6 @@ dependencies { compile 'org.elasticsearch:securesm:1.0' // utilities - compile 'commons-cli:commons-cli:1.3.1' // nocommit: remove the old! compile 'net.sf.jopt-simple:jopt-simple:4.9' compile 'com.carrotsearch:hppc:0.7.1' From cb607a8faee195af9737602be2dc01ac2288a397 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 19:18:14 -0800 Subject: [PATCH 12/16] Remove commons-cli sha and add jopt-simple sha --- distribution/licenses/commons-cli-1.3.1.jar.sha1 | 1 - distribution/licenses/jopt-simple-4.9.jar.sha1 | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 distribution/licenses/commons-cli-1.3.1.jar.sha1 create mode 100644 distribution/licenses/jopt-simple-4.9.jar.sha1 diff --git a/distribution/licenses/commons-cli-1.3.1.jar.sha1 b/distribution/licenses/commons-cli-1.3.1.jar.sha1 deleted file mode 100644 index fc366d027f5..00000000000 --- a/distribution/licenses/commons-cli-1.3.1.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -1303efbc4b181e5a58bf2e967dc156a3132b97c0 diff --git a/distribution/licenses/jopt-simple-4.9.jar.sha1 b/distribution/licenses/jopt-simple-4.9.jar.sha1 new file mode 100644 index 00000000000..b86fa62ac20 --- /dev/null +++ b/distribution/licenses/jopt-simple-4.9.jar.sha1 @@ -0,0 +1 @@ +ee9e9eaa0a35360dcfeac129ff4923215fd65904 \ No newline at end of file From 1dafead2ebd00de570863d8c4819f0267fdc656a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 22:55:24 -0800 Subject: [PATCH 13/16] Fix precommit --- .../elasticsearch/bootstrap/BootstrapCLIParser.java | 3 +++ modules/lang-groovy/build.gradle | 9 +++++++++ plugins/repository-hdfs/build.gradle | 10 ++++++++++ 3 files changed, 22 insertions(+) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java index e44e397f67a..f812bda178c 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java @@ -29,6 +29,7 @@ import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserError; import org.elasticsearch.common.Strings; import org.elasticsearch.cli.Terminal; +import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.monitor.jvm.JvmInfo; final class BootstrapCliParser extends Command { @@ -54,6 +55,8 @@ final class BootstrapCliParser extends Command { .withRequiredArg(); } + // TODO: don't use system properties as a way to do this, its horrible... + @SuppressForbidden(reason = "Sets system properties passed as CLI parameters") @Override protected void execute(Terminal terminal, OptionSet options) throws Exception { if (options.has(versionOption)) { diff --git a/modules/lang-groovy/build.gradle b/modules/lang-groovy/build.gradle index 89444a4e926..005a7d4be18 100644 --- a/modules/lang-groovy/build.gradle +++ b/modules/lang-groovy/build.gradle @@ -38,6 +38,15 @@ thirdPartyAudit.excludes = [ // for example we do not need ivy, scripts arent allowed to download code 'com.thoughtworks.xstream.XStream', 'groovyjarjarasm.asm.util.Textifiable', + 'org.apache.commons.cli.CommandLine', + 'org.apache.commons.cli.CommandLineParser', + 'org.apache.commons.cli.GnuParser', + 'org.apache.commons.cli.HelpFormatter', + 'org.apache.commons.cli.Option', + 'org.apache.commons.cli.OptionBuilder', + 'org.apache.commons.cli.Options', + 'org.apache.commons.cli.Parser', + 'org.apache.commons.cli.PosixParser', 'org.apache.ivy.Ivy', 'org.apache.ivy.core.event.IvyListener', 'org.apache.ivy.core.event.download.PrepareDownloadEvent', diff --git a/plugins/repository-hdfs/build.gradle b/plugins/repository-hdfs/build.gradle index 915a85ebdc4..20050bdc31e 100644 --- a/plugins/repository-hdfs/build.gradle +++ b/plugins/repository-hdfs/build.gradle @@ -191,6 +191,16 @@ thirdPartyAudit.excludes = [ 'org.apache.commons.beanutils.DynaClass', 'org.apache.commons.beanutils.DynaProperty', 'org.apache.commons.beanutils.PropertyUtils', + 'org.apache.commons.cli.CommandLine', + 'org.apache.commons.cli.CommandLineParser', + 'org.apache.commons.cli.GnuParser', + 'org.apache.commons.cli.HelpFormatter', + 'org.apache.commons.cli.Option', + 'org.apache.commons.cli.OptionBuilder', + 'org.apache.commons.cli.OptionGroup', + 'org.apache.commons.cli.Options', + 'org.apache.commons.cli.ParseException', + 'org.apache.commons.cli.PosixParser', 'org.apache.commons.compress.archivers.tar.TarArchiveEntry', 'org.apache.commons.compress.archivers.tar.TarArchiveInputStream', 'org.apache.commons.codec.DecoderException', From d822c6558f5983f7b90ae57ca9cd1f341bd72e59 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Mar 2016 23:17:35 -0800 Subject: [PATCH 14/16] Fix file rename to match class name --- .../{BootstrapCLIParser.java => BootstrapCliParser.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename core/src/main/java/org/elasticsearch/bootstrap/{BootstrapCLIParser.java => BootstrapCliParser.java} (100%) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java similarity index 100% rename from core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java rename to core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java From 80ae2b0002eada4c357cf6fad0a174b0946da3da Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 9 Mar 2016 00:10:59 -0800 Subject: [PATCH 15/16] Fix more licenses --- .../bootstrap/BootstrapCliParser.java | 8 +++++-- distribution/licenses/jopt-simple-LICENSE.txt | 24 +++++++++++++++++++ distribution/licenses/jopt-simple-NOTICE.txt | 0 plugins/repository-hdfs/build.gradle | 11 +-------- .../licenses/commons-cli-1.2.jar.sha1 | 1 + .../licenses/commons-cli-LICENSE.txt | 0 .../licenses/commons-cli-NOTICE.txt | 0 7 files changed, 32 insertions(+), 12 deletions(-) create mode 100644 distribution/licenses/jopt-simple-LICENSE.txt create mode 100644 distribution/licenses/jopt-simple-NOTICE.txt create mode 100644 plugins/repository-hdfs/licenses/commons-cli-1.2.jar.sha1 rename {distribution => plugins/repository-hdfs}/licenses/commons-cli-LICENSE.txt (100%) rename {distribution => plugins/repository-hdfs}/licenses/commons-cli-NOTICE.txt (100%) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java index f812bda178c..5c927305f14 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java @@ -51,7 +51,7 @@ final class BootstrapCliParser extends Command { pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"), "Creates a pid file in the specified path on start") .withRequiredArg(); - propertyOption = parser.accepts("E", "Configures an Elasticsearch setting") + propertyOption = parser.accepts("D", "Configures an Elasticsearch setting") .withRequiredArg(); } @@ -80,7 +80,11 @@ final class BootstrapCliParser extends Command { if (keyValue.length != 2) { throw new UserError(ExitCodes.USAGE, "Malformed elasticsearch setting, must be of the form key=value"); } - System.setProperty("es." + keyValue[0], keyValue[1]); + String key = keyValue[0]; + if (key.startsWith("es.") == false) { + key = "es." + key; + } + System.setProperty(key, keyValue[1]); } shouldRun = true; } diff --git a/distribution/licenses/jopt-simple-LICENSE.txt b/distribution/licenses/jopt-simple-LICENSE.txt new file mode 100644 index 00000000000..85f923a9526 --- /dev/null +++ b/distribution/licenses/jopt-simple-LICENSE.txt @@ -0,0 +1,24 @@ +/* + The MIT License + + Copyright (c) 2004-2015 Paul R. Holser, Jr. + + Permission is hereby granted, free of charge, to any person obtaining + a copy of this software and associated documentation files (the + "Software"), to deal in the Software without restriction, including + without limitation the rights to use, copy, modify, merge, publish, + distribute, sublicense, and/or sell copies of the Software, and to + permit persons to whom the Software is furnished to do so, subject to + the following conditions: + + The above copyright notice and this permission notice shall be + included in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +*/ diff --git a/distribution/licenses/jopt-simple-NOTICE.txt b/distribution/licenses/jopt-simple-NOTICE.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/plugins/repository-hdfs/build.gradle b/plugins/repository-hdfs/build.gradle index 20050bdc31e..8fc9e50d7f3 100644 --- a/plugins/repository-hdfs/build.gradle +++ b/plugins/repository-hdfs/build.gradle @@ -45,6 +45,7 @@ dependencies { compile 'com.google.guava:guava:16.0.1' compile 'com.google.protobuf:protobuf-java:2.5.0' compile 'commons-logging:commons-logging:1.1.3' + compile 'commons-cli:commons-cli:1.2' compile 'commons-collections:commons-collections:3.2.2' compile 'commons-configuration:commons-configuration:1.6' compile 'commons-io:commons-io:2.4' @@ -191,16 +192,6 @@ thirdPartyAudit.excludes = [ 'org.apache.commons.beanutils.DynaClass', 'org.apache.commons.beanutils.DynaProperty', 'org.apache.commons.beanutils.PropertyUtils', - 'org.apache.commons.cli.CommandLine', - 'org.apache.commons.cli.CommandLineParser', - 'org.apache.commons.cli.GnuParser', - 'org.apache.commons.cli.HelpFormatter', - 'org.apache.commons.cli.Option', - 'org.apache.commons.cli.OptionBuilder', - 'org.apache.commons.cli.OptionGroup', - 'org.apache.commons.cli.Options', - 'org.apache.commons.cli.ParseException', - 'org.apache.commons.cli.PosixParser', 'org.apache.commons.compress.archivers.tar.TarArchiveEntry', 'org.apache.commons.compress.archivers.tar.TarArchiveInputStream', 'org.apache.commons.codec.DecoderException', diff --git a/plugins/repository-hdfs/licenses/commons-cli-1.2.jar.sha1 b/plugins/repository-hdfs/licenses/commons-cli-1.2.jar.sha1 new file mode 100644 index 00000000000..d38d00127e8 --- /dev/null +++ b/plugins/repository-hdfs/licenses/commons-cli-1.2.jar.sha1 @@ -0,0 +1 @@ +2bf96b7aa8b611c177d329452af1dc933e14501c \ No newline at end of file diff --git a/distribution/licenses/commons-cli-LICENSE.txt b/plugins/repository-hdfs/licenses/commons-cli-LICENSE.txt similarity index 100% rename from distribution/licenses/commons-cli-LICENSE.txt rename to plugins/repository-hdfs/licenses/commons-cli-LICENSE.txt diff --git a/distribution/licenses/commons-cli-NOTICE.txt b/plugins/repository-hdfs/licenses/commons-cli-NOTICE.txt similarity index 100% rename from distribution/licenses/commons-cli-NOTICE.txt rename to plugins/repository-hdfs/licenses/commons-cli-NOTICE.txt From 5bd7da56597391cc484873b413e72fa5757418c7 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 11 Mar 2016 11:46:23 -0800 Subject: [PATCH 16/16] Addressed PR feedback * Fix tests still referring to -E * add comment about missing classes * rename writer constant --- .../main/java/org/elasticsearch/cli/Terminal.java | 14 +++++++------- modules/lang-groovy/build.gradle | 2 ++ .../bootstrap/BootstrapCliParserTests.java | 11 +++++++++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cli/Terminal.java b/core/src/main/java/org/elasticsearch/cli/Terminal.java index 00d886aa8ab..d2dc57263dc 100644 --- a/core/src/main/java/org/elasticsearch/cli/Terminal.java +++ b/core/src/main/java/org/elasticsearch/cli/Terminal.java @@ -89,35 +89,35 @@ public abstract class Terminal { private static class ConsoleTerminal extends Terminal { - private static final Console console = System.console(); + private static final Console CONSOLE = System.console(); ConsoleTerminal() { super(System.lineSeparator()); } static boolean isSupported() { - return console != null; + return CONSOLE != null; } @Override public PrintWriter getWriter() { - return console.writer(); + return CONSOLE.writer(); } @Override public String readText(String prompt) { - return console.readLine("%s", prompt); + return CONSOLE.readLine("%s", prompt); } @Override public char[] readSecret(String prompt) { - return console.readPassword("%s", prompt); + return CONSOLE.readPassword("%s", prompt); } } private static class SystemTerminal extends Terminal { - private static final PrintWriter writer = newWriter(); + private static final PrintWriter WRITER = newWriter(); SystemTerminal() { super(System.lineSeparator()); @@ -130,7 +130,7 @@ public abstract class Terminal { @Override public PrintWriter getWriter() { - return writer; + return WRITER; } @Override diff --git a/modules/lang-groovy/build.gradle b/modules/lang-groovy/build.gradle index 005a7d4be18..2160210ba73 100644 --- a/modules/lang-groovy/build.gradle +++ b/modules/lang-groovy/build.gradle @@ -38,6 +38,8 @@ thirdPartyAudit.excludes = [ // for example we do not need ivy, scripts arent allowed to download code 'com.thoughtworks.xstream.XStream', 'groovyjarjarasm.asm.util.Textifiable', + // commons-cli is referenced by groovy, even though they supposedly + // jarjar it. Since we don't use the cli, we don't need the dep. 'org.apache.commons.cli.CommandLine', 'org.apache.commons.cli.CommandLineParser', 'org.apache.commons.cli.GnuParser', diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java index 2fc08f23a06..fc7504fc97f 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java @@ -133,7 +133,7 @@ public class BootstrapCliParserTests extends CommandTestCase { public void testConfig() throws Exception { registerProperties("es.foo", "es.spam"); - execute("-Efoo=bar", "-Espam=eggs"); + execute("-Dfoo=bar", "-Dspam=eggs"); assertSystemProperty("es.foo", "bar"); assertSystemProperty("es.spam", "eggs"); assertShouldRun(true); @@ -141,11 +141,18 @@ public class BootstrapCliParserTests extends CommandTestCase { public void testConfigMalformed() throws Exception { UserError e = expectThrows(UserError.class, () -> { - execute("-Efoo"); + execute("-Dfoo"); }); assertTrue(e.getMessage(), e.getMessage().contains("Malformed elasticsearch setting")); } + public void testUnknownOption() throws Exception { + OptionException e = expectThrows(OptionException.class, () -> { + execute("--network.host"); + }); + assertTrue(e.getMessage(), e.getMessage().contains("network.host is not a recognized option")); + } + private void registerProperties(String ... systemProperties) { propertiesToClear.addAll(Arrays.asList(systemProperties)); }