Mercurial > hg > release > thermostat-0.15
changeset 255:2dce17087aaf
Improved error handling in case of missing DB.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-April/000996.html
line wrap: on
line diff
--- a/common/src/main/java/com/redhat/thermostat/cli/Command.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/cli/Command.java Fri Apr 20 18:43:24 2012 +0200 @@ -50,4 +50,6 @@ String getUsage(); Collection<ArgumentSpec> getAcceptedArguments(); + + boolean isStorageRequired(); }
--- a/common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java Fri Apr 20 18:43:24 2012 +0200 @@ -65,7 +65,7 @@ commandLine = parser.parse(options, args); return new CommandLineArguments(commandLine); } catch (ParseException e) { - throw new CommandLineArgumentParseException(e); + throw new CommandLineArgumentParseException(e.getMessage(), e); } }
--- a/common/src/main/java/com/redhat/thermostat/cli/HelpCommand.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/cli/HelpCommand.java Fri Apr 20 18:43:24 2012 +0200 @@ -111,4 +111,9 @@ return Collections.emptyList(); } + @Override + public boolean isStorageRequired() { + return false; + } + }
--- a/common/src/main/java/com/redhat/thermostat/cli/Launcher.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/cli/Launcher.java Fri Apr 20 18:43:24 2012 +0200 @@ -36,11 +36,19 @@ package com.redhat.thermostat.cli; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.ServiceLoader; +import com.redhat.thermostat.common.storage.ConnectionException; + public class Launcher { + private static final String DB_URL_ARG = "dbUrl"; + + private static final String DB_URL_DESC = "the URL of the storage to connect to"; + private String[] args; public void run(String[] args) { @@ -73,24 +81,67 @@ } private void runCommand(String cmdName, String[] cmdArgs) { - CommandContextFactory cmdCtxFactory = CommandContextFactory.getInstance(); try { - parseArgsAndRunCommand(cmdName, cmdArgs, cmdCtxFactory); + parseArgsAndRunCommand(cmdName, cmdArgs); } catch (CommandException e) { + CommandContextFactory cmdCtxFactory = CommandContextFactory.getInstance(); cmdCtxFactory.getConsole().getError().println(e.getMessage()); } } - private void parseArgsAndRunCommand(String cmdName, String[] cmdArgs, CommandContextFactory cmdCtxFactory) - throws CommandLineArgumentParseException, CommandException { + private void parseArgsAndRunCommand(String cmdName, String[] cmdArgs) throws CommandException { + Command cmd = getCommand(cmdName); + Collection<ArgumentSpec> acceptedArguments = getAcceptedCommandArguments(cmd); + Arguments args = parseCommandArguments(cmdArgs, acceptedArguments); + CommandContext ctx = setupCommandContext(cmd, args); + cmd.run(ctx); + } + + private Command getCommand(String cmdName) { + + CommandContextFactory cmdCtxFactory = CommandContextFactory.getInstance(); CommandRegistry registry = cmdCtxFactory.getCommandRegistry(); Command cmd = registry.getCommand(cmdName); + return cmd; + } + + private Collection<ArgumentSpec> getAcceptedCommandArguments(Command cmd) { + + Collection<ArgumentSpec> acceptedArguments = cmd.getAcceptedArguments(); + if (cmd.isStorageRequired()) { + acceptedArguments = new ArrayList<>(acceptedArguments); + acceptedArguments.add(createDbUrlArgumentSpec()); + } + return acceptedArguments; + } + + private ArgumentSpec createDbUrlArgumentSpec() { + return new SimpleArgumentSpec(DB_URL_ARG, DB_URL_DESC, true, true); + } + + private Arguments parseCommandArguments(String[] cmdArgs, Collection<ArgumentSpec> acceptedArguments) + throws CommandLineArgumentParseException { + CommandLineArgumentsParser cliArgsParser = new CommandLineArgumentsParser(); - cliArgsParser.addArguments(cmd.getAcceptedArguments()); + cliArgsParser.addArguments(acceptedArguments); Arguments args = cliArgsParser.parse(cmdArgs); + return args; + } + + private CommandContext setupCommandContext(Command cmd, Arguments args) throws CommandException { + + CommandContextFactory cmdCtxFactory = CommandContextFactory.getInstance(); CommandContext ctx = cmdCtxFactory.createContext(args); - cmd.run(ctx); + if (cmd.isStorageRequired()) { + String dbUrl = ctx.getArguments().getArgument(DB_URL_ARG); + try { + ctx.getAppContextSetup().setupAppContext(dbUrl); + } catch (ConnectionException ex) { + throw new CommandException("Could not connect to: " + dbUrl, ex); + } + } + return ctx; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/src/main/java/com/redhat/thermostat/common/storage/ConnectionException.java Fri Apr 20 18:43:24 2012 +0200 @@ -0,0 +1,58 @@ +/* + * Copyright 2012 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.common.storage; + +public class ConnectionException extends RuntimeException { + + public ConnectionException() { + super(); + } + + public ConnectionException(String message) { + super(message); + } + + public ConnectionException(Throwable cause) { + super(cause); + } + + public ConnectionException(String message, Throwable cause) { + super(message, cause); + } + + +}
--- a/common/src/main/java/com/redhat/thermostat/common/storage/MongoConnection.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/common/storage/MongoConnection.java Fri Apr 20 18:43:24 2012 +0200 @@ -36,9 +36,8 @@ package com.redhat.thermostat.common.storage; +import java.io.IOException; import java.net.UnknownHostException; -import java.util.logging.Level; -import java.util.logging.Logger; import com.mongodb.DB; import com.mongodb.Mongo; @@ -46,11 +45,9 @@ import com.mongodb.MongoURI; import com.redhat.thermostat.common.NotImplementedException; import com.redhat.thermostat.common.config.StartupConfiguration; -import com.redhat.thermostat.common.utils.LoggingUtils; class MongoConnection extends Connection { - private static final Logger logger = LoggingUtils.getLogger(MongoConnection.class); private Mongo m = null; private DB db = null; private StartupConfiguration conf; @@ -65,12 +62,11 @@ createConnection(); /* the mongo java driver does not ensure this connection is actually working */ testConnection(); - } catch (MongoException | UnknownHostException | + } catch (IOException | MongoException | NotImplementedException | IllegalArgumentException e) { - logger.log(Level.WARNING, "Connection failed.", e); fireChanged(ConnectionStatus.FAILED_TO_CONNECT); - return; + throw new ConnectionException(e.getMessage(), e); } fireChanged(ConnectionStatus.CONNECTED); connected = true;
--- a/common/src/main/java/com/redhat/thermostat/tools/BasicCommand.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/tools/BasicCommand.java Fri Apr 20 18:43:24 2012 +0200 @@ -46,6 +46,7 @@ public abstract class BasicCommand implements Command { private ActionNotifier<ApplicationState> notifier; + private boolean storageRequired; public BasicCommand() { this.notifier = new ActionNotifier<>(this); @@ -56,4 +57,13 @@ } public abstract StartupConfiguration getConfiguration(); + + @Override + public boolean isStorageRequired() { + return storageRequired; + } + + protected void setStorageRequired(boolean storageRequired) { + this.storageRequired = storageRequired; + } }
--- a/common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java Fri Apr 20 18:43:24 2012 +0200 @@ -37,6 +37,8 @@ package com.redhat.thermostat.cli; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import java.util.Arrays; @@ -67,12 +69,13 @@ } private TestCommandContextFactory ctxFactory; + private AppContextSetup appContextSetup; @Before public void setUp() { CLITestEnvironment.setUp(); - ctxFactory = new TestCommandContextFactory(); + setupCommandContextFactory(); CommandContextFactory.setInstance(ctxFactory); TestCommand cmd1 = new TestCommand("test1", new TestCmd1()); @@ -93,10 +96,27 @@ arg4.setUsingAdditionalArgument(true); cmd2.addArguments(arg3, arg4); cmd2.setDescription("description 2"); - ctxFactory.getCommandRegistry().registerCommands(Arrays.asList(cmd1, cmd2, new HelpCommand())); + + TestCommand cmd3 = new TestCommand("test3"); + cmd3.setStorageRequired(true); + cmd3.setDescription("description 3"); + + ctxFactory.getCommandRegistry().registerCommands(Arrays.asList(cmd1, cmd2, cmd3, new HelpCommand())); } + private void setupCommandContextFactory() { + appContextSetup = mock(AppContextSetup.class); + ctxFactory = new TestCommandContextFactory() { + @Override + protected AppContextSetup getAppContextSetup() { + return appContextSetup; + } + }; + CommandContextFactory.setInstance(ctxFactory); + } + + @After public void tearDown() { CLITestEnvironment.tearDown(); @@ -116,7 +136,8 @@ String expected = "list of commands:\n\n" + " help show help for a given command or help overview\n" + " test1 description 1\n" - + " test2 description 2\n"; + + " test2 description 2\n" + + " test3 description 3\n"; runAndVerifyCommand(new String[0], expected); } @@ -125,7 +146,8 @@ String expected = "list of commands:\n\n" + " help show help for a given command or help overview\n" + " test1 description 1\n" - + " test2 description 2\n"; + + " test2 description 2\n" + + " test3 description 3\n"; runAndVerifyCommand(new String[] {"--help"}, expected); } @@ -134,7 +156,8 @@ String expected = "list of commands:\n\n" + " help show help for a given command or help overview\n" + " test1 description 1\n" - + " test2 description 2\n"; + + " test2 description 2\n" + + " test3 description 3\n"; runAndVerifyCommand(new String[] {"-help"}, expected); } @@ -143,7 +166,8 @@ String expected = "list of commands:\n\n" + " help show help for a given command or help overview\n" + " test1 description 1\n" - + " test2 description 2\n"; + + " test2 description 2\n" + + " test3 description 3\n"; runAndVerifyCommand(new String[] {"foobarbaz"}, expected); } @@ -152,7 +176,8 @@ String expected = "list of commands:\n\n" + " help show help for a given command or help overview\n" + " test1 description 1\n" - + " test2 description 2\n"; + + " test2 description 2\n" + + " test3 description 3\n"; runAndVerifyCommand(new String[] {"foo", "--bar", "baz"}, expected); } @@ -176,4 +201,16 @@ new Launcher().run(args); assertEquals(expected, ctxFactory.getOutput()); } + + @Test + public void verifyStorageCommandSetsUpDAOFactory() { + new Launcher().run(new String[] { "test3" , "--dbUrl", "mongo://fluff:12345" }); + verify(appContextSetup).setupAppContext("mongo://fluff:12345"); + } + + @Test + public void verifyStorageCommandRequiresDbUrl() { + new Launcher().run(new String[] { "test3" }); + assertEquals("Missing required option: dbUrl\n", ctxFactory.getError()); + } }
--- a/common/src/test/java/com/redhat/thermostat/cli/TestCommand.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/test/java/com/redhat/thermostat/cli/TestCommand.java Fri Apr 20 18:43:24 2012 +0200 @@ -48,6 +48,7 @@ private Handle handle; private String description; private String usage; + private boolean storageRequired; private List<ArgumentSpec> arguments = new LinkedList<ArgumentSpec>(); @@ -66,7 +67,9 @@ @Override public void run(CommandContext ctx) throws CommandException { - handle.run(ctx); + if (handle != null) { + handle.run(ctx); + } } @Override @@ -100,4 +103,12 @@ void addArguments(ArgumentSpec... arguments) { this.arguments.addAll(Arrays.asList(arguments)); } + + public boolean isStorageRequired() { + return storageRequired; + } + + void setStorageRequired(boolean storageRequired) { + this.storageRequired = storageRequired; + } }
--- a/common/src/test/java/com/redhat/thermostat/common/storage/MongoConnectionTest.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/test/java/com/redhat/thermostat/common/storage/MongoConnectionTest.java Fri Apr 20 18:43:24 2012 +0200 @@ -36,7 +36,13 @@ package com.redhat.thermostat.common.storage; -import java.net.UnknownHostException; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; import org.junit.After; import org.junit.Before; @@ -55,11 +61,6 @@ import com.redhat.thermostat.common.storage.Connection.ConnectionListener; import com.redhat.thermostat.common.storage.Connection.ConnectionStatus; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - @PrepareForTest(MongoConnection.class) @RunWith(PowerMockRunner.class) public class MongoConnectionTest { @@ -95,18 +96,29 @@ } @Test - public void testConnectUnknownHostException() throws Exception { - PowerMockito.whenNew(Mongo.class).withParameterTypes(MongoURI.class).withArguments(any(MongoURI.class)).thenThrow(new UnknownHostException()); - conn.connect(); - + public void testConnectIOException() throws Exception { + PowerMockito.whenNew(Mongo.class).withParameterTypes(MongoURI.class).withArguments(any(MongoURI.class)).thenThrow(new IOException()); + boolean exceptionThrown = false; + try { + conn.connect(); + } catch (ConnectionException ex) { + exceptionThrown = true; + } verify(listener).changed(ConnectionStatus.FAILED_TO_CONNECT); + assertTrue(exceptionThrown); } @Test public void testConnectMongoException() throws Exception { PowerMockito.whenNew(Mongo.class).withParameterTypes(MongoURI.class).withArguments(any(MongoURI.class)).thenThrow(new MongoException("fluff")); - conn.connect(); + boolean exceptionThrown = false; + try { + conn.connect(); + } catch (ConnectionException ex) { + exceptionThrown = true; + } verify(listener).changed(ConnectionStatus.FAILED_TO_CONNECT); + assertTrue(exceptionThrown); } }
--- a/common/src/test/java/com/redhat/thermostat/tools/BasicCommandTest.java Fri Apr 20 18:39:24 2012 +0200 +++ b/common/src/test/java/com/redhat/thermostat/tools/BasicCommandTest.java Fri Apr 20 18:43:24 2012 +0200 @@ -86,6 +86,11 @@ public Collection<ArgumentSpec> getAcceptedArguments() { return null; } + + @Override + public boolean isStorageRequired() { + return false; + } }; }
--- a/tools/src/main/java/com/redhat/thermostat/tools/cli/ListVMsCommand.java Fri Apr 20 18:39:24 2012 +0200 +++ b/tools/src/main/java/com/redhat/thermostat/tools/cli/ListVMsCommand.java Fri Apr 20 18:43:24 2012 +0200 @@ -38,6 +38,7 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import com.redhat.thermostat.cli.ArgumentSpec; import com.redhat.thermostat.cli.Command; @@ -60,17 +61,9 @@ private static final String USAGE = DESCRIPTION; - private static final String DB_URL_ARG = "dbUrl"; - - private static final String DB_URL_DESC = "the URL of the storage to connect to"; - @Override public void run(CommandContext ctx) throws CommandException { - String dbUrl = ctx.getArguments().getArgument(DB_URL_ARG); - - ctx.getAppContextSetup().setupAppContext(dbUrl); - DAOFactory daoFactory = ApplicationContext.getInstance().getDAOFactory(); HostInfoDAO hostsDAO = daoFactory.getHostInfoDAO(); Collection<HostRef> hosts = hostsDAO.getHosts(); @@ -102,8 +95,12 @@ @Override public Collection<ArgumentSpec> getAcceptedArguments() { - ArgumentSpec dbUrl = new SimpleArgumentSpec(DB_URL_ARG, DB_URL_DESC, true, true); - return Arrays.asList(dbUrl); + return Collections.emptyList(); + } + + @Override + public boolean isStorageRequired() { + return true; } }
--- a/tools/src/test/java/com/redhat/thermostat/tools/cli/ListVMsCommandTest.java Fri Apr 20 18:39:24 2012 +0200 +++ b/tools/src/test/java/com/redhat/thermostat/tools/cli/ListVMsCommandTest.java Fri Apr 20 18:43:24 2012 +0200 @@ -117,19 +117,6 @@ } @Test - public void testRunCreatesConnectionFromArgumentURL() throws CommandException { - - SimpleArguments args = new SimpleArguments(); - args.addArgument("dbUrl", "mongo://fluff:12345"); - CommandContext ctx = cmdCtxFactory.createContext(args); - - cmd.run(ctx); - - verify(appContextSetup).setupAppContext("mongo://fluff:12345"); - - } - - @Test public void verifyOutputFormatOneLine() throws CommandException { HostRef host1 = new HostRef("123", "h1"); @@ -191,7 +178,6 @@ public void testAcceptedArguments() { Collection<ArgumentSpec> args = cmd.getAcceptedArguments(); assertNotNull(args); - assertEquals(1, args.size()); - assertTrue(args.contains(new SimpleArgumentSpec("dbUrl", "the URL of the storage to connect to", true, true))); + assertTrue(args.isEmpty()); } }