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(); }