From dcdf2d00b8de5d9b4d3ae91ce6290e9af45c55e1 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 28 May 2013 10:25:01 +0100 Subject: [PATCH] [bs-135] Simplify Command interface a bit more --- .../bootstrap/cli/Command.java | 11 ++--- .../bootstrap/cli/SpringBootstrapCli.java | 9 ++-- .../cli/command/AbstractCommand.java | 6 +-- .../bootstrap/cli/command/OptionHandler.java | 13 ++++-- .../cli/command/OptionParsingCommand.java | 7 +-- .../bootstrap/cli/command/ScriptCommand.java | 46 +++++++++++++++---- .../cli/SpringBootstrapCliTests.java | 4 +- .../cli/command/ScriptCommandTests.java | 26 +++++++---- 8 files changed, 78 insertions(+), 44 deletions(-) diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/Command.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/Command.java index 22d9334f26..2a26cef245 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/Command.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/Command.java @@ -16,8 +16,6 @@ package org.springframework.bootstrap.cli; -import java.io.IOException; -import java.io.PrintStream; /** * A single command that can be run from the CLI. @@ -40,17 +38,16 @@ public interface Command { /** * Returns usage help for the command. This should be a simple one-line string - * describing basic usage. eg. '[options] <file>'. Do not include the name of + * describing basic usage. e.g. '[options] <file>'. Do not include the name of * the command in this string. */ String getUsageHelp(); /** - * Prints help for the command. - * @param out the output writer to display help - * @throws IOException + * Gets full help text for the command, e.g. a longer description and one line per + * option. */ - void printHelp(PrintStream out) throws IOException; + String getHelp(); /** * Run the command. diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/SpringBootstrapCli.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/SpringBootstrapCli.java index 099a0014d4..3adcd49940 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/SpringBootstrapCli.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/SpringBootstrapCli.java @@ -16,8 +16,6 @@ package org.springframework.bootstrap.cli; -import java.io.IOException; -import java.io.PrintStream; import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; @@ -196,7 +194,9 @@ public class SpringBootstrapCli { + " " + command.getUsageHelp()); System.out.println(); } - command.printHelp(System.out); + if (command.getHelp() != null) { + System.out.println(command.getHelp()); + } return; } } @@ -219,7 +219,8 @@ public class SpringBootstrapCli { } @Override - public void printHelp(PrintStream out) throws IOException { + public String getHelp() { + return null; } } diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/AbstractCommand.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/AbstractCommand.java index f68259d901..a81241c826 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/AbstractCommand.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/AbstractCommand.java @@ -16,9 +16,6 @@ package org.springframework.bootstrap.cli.command; -import java.io.IOException; -import java.io.PrintStream; - import org.springframework.bootstrap.cli.Command; /** @@ -59,7 +56,8 @@ public abstract class AbstractCommand implements Command { } @Override - public void printHelp(PrintStream out) throws IOException { + public String getHelp() { + return null; } } diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionHandler.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionHandler.java index ccab872681..a745a027be 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionHandler.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionHandler.java @@ -15,8 +15,9 @@ */ package org.springframework.bootstrap.cli.command; +import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.PrintStream; +import java.io.OutputStream; import java.util.Collection; import joptsimple.OptionParser; @@ -56,8 +57,14 @@ public abstract class OptionHandler { protected abstract void run(OptionSet options) throws Exception; - public void printHelp(PrintStream out) throws IOException { - getParser().printHelpOn(out); + public String getHelp() { + OutputStream out = new ByteArrayOutputStream(); + try { + getParser().printHelpOn(out); + } catch (IOException e) { + return "Help not available"; + } + return out.toString(); } } diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionParsingCommand.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionParsingCommand.java index 57cb381bf4..c612eaf770 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionParsingCommand.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/OptionParsingCommand.java @@ -16,9 +16,6 @@ package org.springframework.bootstrap.cli.command; -import java.io.IOException; -import java.io.PrintStream; - import org.springframework.bootstrap.cli.Command; /** @@ -37,8 +34,8 @@ public abstract class OptionParsingCommand extends AbstractCommand { } @Override - public void printHelp(PrintStream out) throws IOException { - this.handler.printHelp(out); + public String getHelp() { + return this.handler.getHelp(); } @Override diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/ScriptCommand.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/ScriptCommand.java index 26653026ad..b500bde218 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/ScriptCommand.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/command/ScriptCommand.java @@ -19,7 +19,6 @@ import groovy.lang.Script; import java.io.File; import java.io.IOException; -import java.io.PrintStream; import java.net.URL; import org.apache.ivy.util.FileUtil; @@ -29,12 +28,12 @@ import org.springframework.bootstrap.cli.compiler.GroovyCompiler; import org.springframework.bootstrap.cli.compiler.GroovyCompilerConfiguration; /** - * {@link Command} to run a script. + * {@link Command} to run a Groovy script. * * @author Dave Syer * */ -public class ScriptCommand extends AbstractCommand { +public class ScriptCommand implements Command { private static String[] DEFAULT_PATHS = new String[] { "${SPRING_HOME}/ext", "${SPRING_HOME}/bin" }; @@ -45,20 +44,44 @@ public class ScriptCommand extends AbstractCommand { private Object main; - public ScriptCommand(String name, String description) { - super(name, description); + private String name; + + public ScriptCommand(String script) { + this.name = script; + } + + @Override + public String getName() { + if (getMain() instanceof Command) { + return ((Command) getMain()).getName(); + } + return this.name; + } + + @Override + public String getDescription() { + if (getMain() instanceof Command) { + return ((Command) getMain()).getDescription(); + } + return this.name; } @Override - public void printHelp(PrintStream out) throws IOException { + public String getHelp() { if (getMain() instanceof OptionHandler) { - ((OptionHandler) getMain()).printHelp(out); + return ((OptionHandler) getMain()).getHelp(); } + if (getMain() instanceof Command) { + return ((Command) getMain()).getHelp(); + } + return null; } @Override public void run(String... args) throws Exception { - if (getMain() instanceof OptionHandler) { + if (getMain() instanceof Command) { + ((Command) getMain()).run(args); + } else if (getMain() instanceof OptionHandler) { ((OptionHandler) getMain()).run(args); } else if (this.main instanceof Runnable) { ((Runnable) this.main).run(); @@ -80,6 +103,9 @@ public class ScriptCommand extends AbstractCommand { @Override public String getUsageHelp() { + if (getMain() instanceof Command) { + return ((Command) getMain()).getDescription(); + } return "[options] "; } @@ -88,7 +114,7 @@ public class ScriptCommand extends AbstractCommand { try { this.main = getMainClass().newInstance(); } catch (Exception e) { - throw new IllegalStateException("Cannot create main class: " + getName(), + throw new IllegalStateException("Cannot create main class: " + this.name, e); } } @@ -98,7 +124,7 @@ public class ScriptCommand extends AbstractCommand { private void compile() { GroovyCompiler compiler = new GroovyCompiler(new ScriptConfiguration()); compiler.addCompilationCustomizers(new ScriptCompilationCustomizer()); - File source = locateSource(getName()); + File source = locateSource(this.name); Class[] classes; try { classes = compiler.compile(source); diff --git a/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/SpringBootstrapCliTests.java b/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/SpringBootstrapCliTests.java index 0b9e82e2f8..fe82f69ffc 100644 --- a/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/SpringBootstrapCliTests.java +++ b/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/SpringBootstrapCliTests.java @@ -1,6 +1,5 @@ package org.springframework.bootstrap.cli; -import java.io.PrintStream; import java.util.Arrays; import java.util.EnumSet; import java.util.Set; @@ -18,7 +17,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.willThrow; -import static org.mockito.Matchers.anyObject; import static org.mockito.Mockito.verify; /** @@ -141,7 +139,7 @@ public class SpringBootstrapCliTests { @Test public void help() throws Exception { this.cli.run("help", "command"); - verify(this.regularCommand).printHelp((PrintStream) anyObject()); + verify(this.regularCommand).getHelp(); } @Test diff --git a/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/command/ScriptCommandTests.java b/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/command/ScriptCommandTests.java index 9ed2b5d983..11ce5ebd30 100644 --- a/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/command/ScriptCommandTests.java +++ b/spring-bootstrap-cli/src/test/java/org/springframework/bootstrap/cli/command/ScriptCommandTests.java @@ -17,9 +17,6 @@ package org.springframework.bootstrap.cli.command; import groovy.lang.Script; -import java.io.ByteArrayOutputStream; -import java.io.PrintStream; - import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -33,21 +30,34 @@ public class ScriptCommandTests { public static boolean executed = false; + @Test(expected = IllegalStateException.class) + public void testMissing() throws Exception { + ScriptCommand command = new ScriptCommand("missing"); + command.run("World"); + } + @Test public void testScript() throws Exception { - ScriptCommand command = new ScriptCommand("script", "Run a test command"); + ScriptCommand command = new ScriptCommand("script"); command.run("World"); assertEquals("World", ((String[]) ((Script) command.getMain()).getProperty("args"))[0]); } + @Test + public void testCommand() throws Exception { + ScriptCommand command = new ScriptCommand("command"); + assertEquals("My script command", command.getUsageHelp()); + command.run("World"); + assertTrue(executed); + } + @Test public void testOptions() throws Exception { - ScriptCommand command = new ScriptCommand("test", "Run a test command"); + ScriptCommand command = new ScriptCommand("test"); command.run("World", "--foo"); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - ((OptionHandler) command.getMain()).printHelp(new PrintStream(out)); - assertTrue("Wrong output: " + out, out.toString().contains("--foo")); + String out = ((OptionHandler) command.getMain()).getHelp(); + assertTrue("Wrong output: " + out, out.contains("--foo")); assertTrue(executed); }