Convert bootstrapcli parser to jopt-simple

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");

core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java

@@ -19,165 +19,70 @@
 
 package org.elasticsearch.bootstrap;
 
-import org.apache.commons.cli.CommandLine;
+import java.util.Arrays;
-import org.apache.commons.cli.Option;
+
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 import org.elasticsearch.Build;
-import org.elasticsearch.common.Strings;
+import org.elasticsearch.cli.Command;
-import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.cli.ExitCodes;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolConfig;
-import org.elasticsearch.common.cli.Terminal;
 import org.elasticsearch.cli.UserError;
-import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.Strings;
-import org.elasticsearch.env.Environment;
+import org.elasticsearch.common.cli.Terminal;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 
-import java.util.HashMap;
+final class BootstrapCliParser extends Command {
-import java.util.Iterator;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Properties;
 
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
+    private final OptionSpec<Void> versionOption;
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder;
+    private final OptionSpec<Void> daemonizeOption;
+    private final OptionSpec<String> pidfileOption;
+    private final OptionSpec<String> propertyOption;
+    private boolean shouldRun = false;
 
-final class BootstrapCLIParser extends CliTool {
+    BootstrapCliParser() {
-
+        super("Starts elasticsearch");
-    private static final CliToolConfig CONFIG = CliToolConfig.config("elasticsearch", BootstrapCLIParser.class)
+        // TODO: in jopt-simple 5.0, make this mutually exclusive with all other options
-            .cmds(Start.CMD, Version.CMD)
+        versionOption = parser.acceptsAll(Arrays.asList("V", "version"),
-            .build();
+            "Prints elasticsearch version information and exits");
-
+        daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"),
-    public BootstrapCLIParser() {
+            "Starts Elasticsearch in the background");
-        super(CONFIG);
+        // 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")
-    public BootstrapCLIParser(Terminal terminal) {
+            .withRequiredArg();
-        super(CONFIG, terminal);
+        propertyOption = parser.accepts("E", "Configures an Elasticsearch setting")
+            .withRequiredArg();
     }
 
     @Override
-    protected Command parse(String cmdName, CommandLine cli) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
-        switch (cmdName.toLowerCase(Locale.ROOT)) {
+        if (options.has(versionOption)) {
-            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 {
             terminal.println("Version: " + org.elasticsearch.Version.CURRENT
-                    + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
+                + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
-                    + ", JVM: " + JvmInfo.jvmInfo().version());
+                + ", JVM: " + JvmInfo.jvmInfo().version());
-            return ExitStatus.OK_AND_EXIT;
+            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 {
+    boolean shouldRun() {
-
+        return shouldRun;
-        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<Object, Object> 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<String> iterator = cli.getArgList().iterator();
-            final Map<String, String> 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<String, String> 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;
-        }
     }
-
 }

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.

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 {
+    public final int main(String[] args, Terminal terminal) throws Exception {
-
-        final OptionSet options;
         try {
-            options = parser.parse(args);
+            mainWithoutErrorHandling(args, terminal);
-        } 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 (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;
 }

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

core/src/main/java/org/elasticsearch/cli/TestCommand.java

@@ -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;
-    }
-}

plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java

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

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<String> propertiesToClear = new ArrayList<>();
     private Map<Object, Object> properties;
 
@@ -66,195 +73,86 @@ public class BootstrapCliParserTests extends CliToolTestCase {
         assertEquals("properties leaked", properties, new HashMap<>(System.getProperties()));
     }
 
-    public void testThatVersionIsReturned() throws Exception {
+    void assertShouldRun(boolean shouldRun) {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
+        BootstrapCliParser parser = (BootstrapCliParser)command;
-        ExitStatus status = parser.execute(args("version"));
+        assertEquals(shouldRun, parser.shouldRun());
-        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()));
     }
 
-    public void testThatVersionIsReturnedAsStartParameter() throws Exception {
+    public void testVersion() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
+        String output = execute("-V");
-        ExitStatus status = parser.execute(args("start -V"));
-        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()));
+        assertShouldRun(false);
 
         terminal.reset();
-        parser = new BootstrapCLIParser(terminal);
+        output = execute("--version");
-        status = parser.execute(args("start --version"));
-        assertStatus(status, OK_AND_EXIT);
-
-        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()));
+        assertShouldRun(false);
     }
 
-    public void testThatPidFileCanBeConfigured() throws Exception {
+    public void testPidfile() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
         registerProperties("es.pidfile");
 
-        ExitStatus status = parser.execute(args("start --pidfile")); // missing pid file
+        // missing argument
-        assertStatus(status, USAGE);
+        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"));
+        terminal.reset();
-        assertStatus(status, OK);
+        execute("--pidfile", "/tmp/pid");
         assertSystemProperty("es.pidfile", "/tmp/pid");
+        assertShouldRun(true);
 
         System.clearProperty("es.pidfile");
-        status = parser.execute(args("start -p /tmp/pid"));
+        terminal.reset();
-        assertStatus(status, OK);
+        execute("-p", "/tmp/pid");
         assertSystemProperty("es.pidfile", "/tmp/pid");
+        assertShouldRun(true);
     }
 
-    public void testThatParsingDaemonizeWorks() throws Exception {
+    public void testNoDaemonize() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
         registerProperties("es.foreground");
 
-        ExitStatus status = parser.execute(args("start -d"));
+        execute();
-        assertStatus(status, OK);
+        assertSystemProperty("es.foreground", null);
-        assertThat(System.getProperty("es.foreground"), is("false"));
+        assertShouldRun(true);
     }
 
-    public void testThatNotDaemonizingDoesNotConfigureProperties() throws Exception {
+    public void testDaemonize() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
         registerProperties("es.foreground");
 
-        ExitStatus status = parser.execute(args("start"));
+        execute("-d");
-        assertStatus(status, OK);
+        assertSystemProperty("es.foreground", "false");
-        assertThat(System.getProperty("es.foreground"), is(nullValue()));
+        assertShouldRun(true);
+
+        System.clearProperty("es.foreground");
+        execute("--daemonize");
+        assertSystemProperty("es.foreground", "false");
+        assertShouldRun(true);
     }
 
-    public void testThatJavaPropertyStyleArgumentsCanBeParsed() throws Exception {
+    public void testConfig() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
         registerProperties("es.foo", "es.spam");
 
-        ExitStatus status = parser.execute(args("start -Dfoo=bar -Dspam=eggs"));
+        execute("-Efoo=bar", "-Espam=eggs");
-        assertStatus(status, OK);
         assertSystemProperty("es.foo", "bar");
         assertSystemProperty("es.spam", "eggs");
+        assertShouldRun(true);
     }
 
-    public void testThatJavaPropertyStyleArgumentsWithEsPrefixAreNotPrefixedTwice() throws Exception {
+    public void testConfigMalformed() 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<Tuple<String, String>> 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<String, String> 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);
         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 --"));
+        assertTrue(e.getMessage(), e.getMessage().contains("Malformed elasticsearch setting"));
-        assertNull(System.getProperty("es.foo"));
     }
 
     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));
-    }
 }

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();
+        command = newCommand();
-        OptionSet options = command.parser.parse(args);
+        command.mainWithoutErrorHandling(args, terminal);
-        command.execute(terminal, options);
         return terminal.getOutput();
     }
 }

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