Mercurial > hg > release > thermostat-0.15
changeset 257:9c2e7e00665c
Add global --logLevel option to all commands.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-April/001030.html
PR 928
line wrap: on
line diff
--- a/agent/src/main/java/com/redhat/thermostat/agent/AgentApplication.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/main/java/com/redhat/thermostat/agent/AgentApplication.java Mon Apr 23 12:22:09 2012 +0200 @@ -96,8 +96,6 @@ if (configuration.isDebugConsole()) { LoggingUtils.useDevelConsole(); } - - LoggingUtils.setGlobalLogLevel(configuration.getLogLevel()); final Logger logger = LoggingUtils.getLogger(AgentApplication.class); StorageProvider connProv = new MongoStorageProvider(configuration);
--- a/agent/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Apr 23 12:22:09 2012 +0200 @@ -40,7 +40,6 @@ import java.io.FileInputStream; import java.io.IOException; import java.util.Properties; -import java.util.logging.Level; import com.redhat.thermostat.common.config.ConfigUtils; import com.redhat.thermostat.common.config.InvalidConfigurationException; @@ -52,8 +51,6 @@ AgentStartupConfiguration config = new AgentStartupConfiguration(); File propertyFile = ConfigUtils.getAgentConfigurationFile(); - - config.setLogLevel(Level.FINE); readAndSetProperties(propertyFile, config); return config; @@ -79,12 +76,6 @@ throw new InvalidConfigurationException(AgentProperties.BACKENDS + " property missing"); } - if (properties.containsKey(AgentProperties.LOG_LEVEL.name())) { - String logLevel = properties.getProperty(AgentProperties.LOG_LEVEL.name()); - Level level = getLogLevel(logLevel); - configuration.setLogLevel(level); - } - if (properties.containsKey(AgentProperties.DB_URL.name())) { String db = properties.getProperty(AgentProperties.DB_URL.name()); configuration.setDatabaseURL(db); @@ -97,46 +88,4 @@ } } - public static Level getLogLevel(String logLevel) { - - Level level = Level.FINE; - switch (logLevel.toUpperCase()) { - case "SEVERE": - level = Level.SEVERE; - break; - - case "INFO": - level = Level.INFO; - break; - - case "CONFIG": - level = Level.CONFIG; - break; - - case "FINE": - level = Level.FINE; - break; - - case "FINER": - level = Level.FINER; - break; - - case "FINEST": - level = Level.FINEST; - break; - - case "WARNING": - level = Level.WARNING; - break; - - case "ALL": - level = Level.ALL; - break; - - default: - break; - } - - return level; - } }
--- a/agent/src/main/java/com/redhat/thermostat/agent/config/AgentOptionParser.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/main/java/com/redhat/thermostat/agent/config/AgentOptionParser.java Mon Apr 23 12:22:09 2012 +0200 @@ -38,7 +38,6 @@ import java.util.Arrays; import java.util.Collection; -import java.util.logging.Level; import com.redhat.thermostat.cli.ArgumentSpec; import com.redhat.thermostat.cli.Arguments; @@ -65,12 +64,6 @@ if (args.hasArgument(Args.SAVE_ON_EXIT.option)) { configuration.setPurge(false); } - - if (args.hasArgument(Args.LEVEL.option)) { - String levelString = args.getArgument(Args.LEVEL.option); - Level level = AgentConfigsUtils.getLogLevel(levelString); - configuration.setLogLevel(level); - } configuration.setDebugConsole(args.hasArgument(Args.DEBUG.option)); @@ -94,7 +87,6 @@ private static enum Args { // TODO: localize - LEVEL("logLevel", "log level"), SAVE_ON_EXIT("saveOnExit", "save the data on exit"), DB("dbUrl", "connect to the given url"), DEBUG("debug", "launch with debug console enabled"), @@ -110,10 +102,9 @@ } public static Collection<ArgumentSpec> getAcceptedArguments() { - ArgumentSpec level = new SimpleArgumentSpec(Args.LEVEL.option, Args.LEVEL.description, false, true); ArgumentSpec saveOnExit = new SimpleArgumentSpec(Args.SAVE_ON_EXIT.option, Args.SAVE_ON_EXIT.description); ArgumentSpec db = new SimpleArgumentSpec(Args.DB.option, Args.DB.description, true, true); ArgumentSpec debug = new SimpleArgumentSpec(Args.DEBUG.option, Args.DEBUG.description); - return Arrays.asList(level, saveOnExit, db, debug); + return Arrays.asList(saveOnExit, db, debug); } }
--- a/agent/src/main/java/com/redhat/thermostat/agent/config/AgentProperties.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/main/java/com/redhat/thermostat/agent/config/AgentProperties.java Mon Apr 23 12:22:09 2012 +0200 @@ -40,7 +40,6 @@ // backend list, comma separated BACKENDS, - LOG_LEVEL, DEBUG_CONSOLE, DB_URL, SAVE_ON_EXIT
--- a/agent/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Apr 23 12:22:09 2012 +0200 @@ -42,7 +42,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Properties; -import java.util.logging.Level; import com.redhat.thermostat.backend.BackendID; import com.redhat.thermostat.backend.BackendsProperties; @@ -54,7 +53,6 @@ private List<BackendID> backends; - private Level logLevel; private boolean debugConsole; private boolean purge; @@ -71,14 +69,6 @@ return url; } - public Level getLogLevel() { - return this.logLevel; - } - - void setLogLevel(Level level) { - this.logLevel = level; - } - void parseBackends(String[] backendsList) throws InvalidConfigurationException { backends.clear();
--- a/agent/src/test/java/com/redhat/thermostat/TestUtils.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/test/java/com/redhat/thermostat/TestUtils.java Mon Apr 23 12:22:09 2012 +0200 @@ -82,7 +82,6 @@ props.setProperty(AgentProperties.BACKENDS.name(), "system"); props.setProperty(AgentProperties.SAVE_ON_EXIT.name(), "false"); - props.setProperty(AgentProperties.LOG_LEVEL.name(), "WARNING"); props.store(new FileOutputStream(tmpConfigs), "thermostat agent test properties");
--- a/agent/src/test/java/com/redhat/thermostat/agent/AgentApplicationTest.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/test/java/com/redhat/thermostat/agent/AgentApplicationTest.java Mon Apr 23 12:22:09 2012 +0200 @@ -87,10 +87,9 @@ public void testAcceptedArguments() { Collection<ArgumentSpec> args = agent.getAcceptedArguments(); assertNotNull(args); - assertEquals(4, args.size()); + assertEquals(3, args.size()); assertTrue(args.contains(new SimpleArgumentSpec("saveOnExit", "save the data on exit"))); assertTrue(args.contains(new SimpleArgumentSpec("debug", "launch with debug console enabled"))); assertTrue(args.contains(new SimpleArgumentSpec("dbUrl", "connect to the given url", true, true))); - assertTrue(args.contains(new SimpleArgumentSpec("logLevel", "log level", false, true))); } }
--- a/agent/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Apr 23 12:22:09 2012 +0200 @@ -38,7 +38,6 @@ import java.io.IOException; import java.util.List; -import java.util.logging.Level; import org.junit.Assert; import org.junit.Before; @@ -63,7 +62,5 @@ // the test property only define the system backend so far Assert.assertEquals(1, backends.size()); Assert.assertEquals("system", backends.get(0).getSimpleName()); - - Assert.assertEquals(Level.WARNING, config.getLogLevel()); } }
--- a/agent/src/test/java/com/redhat/thermostat/agent/config/AgentOptionParserTest.java Mon Apr 23 10:52:52 2012 +0200 +++ b/agent/src/test/java/com/redhat/thermostat/agent/config/AgentOptionParserTest.java Mon Apr 23 12:22:09 2012 +0200 @@ -38,7 +38,6 @@ import java.io.File; import java.io.IOException; -import java.util.logging.Level; import junit.framework.Assert; @@ -68,7 +67,6 @@ public void testConfigs1() throws IOException, InvalidConfigurationException { SimpleArguments args = new SimpleArguments(); - args.addArgument("logLevel", "ALL"); args.addArgument("dbUrl", "testURL"); args.addArgument("debug", "--debug"); @@ -77,7 +75,6 @@ parser.parse(); Assert.assertEquals("testURL", configs.getDBConnectionString()); - Assert.assertEquals(Level.ALL, configs.getLogLevel()); Assert.assertTrue(configs.isDebugConsole()); Assert.assertTrue(configs.purge()); } @@ -86,7 +83,6 @@ public void testConfigs2() throws IOException, InvalidConfigurationException { SimpleArguments args = new SimpleArguments(); - args.addArgument("logLevel", "FINE"); args.addArgument("dbUrl", "testURL2"); args.addArgument("saveOnExit", "--saveOnExit"); @@ -95,7 +91,6 @@ parser.parse(); Assert.assertEquals("testURL2", configs.getDBConnectionString()); - Assert.assertEquals(Level.FINE, configs.getLogLevel()); Assert.assertFalse(configs.isDebugConsole()); Assert.assertFalse(configs.purge()); }
--- a/common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java Mon Apr 23 10:52:52 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java Mon Apr 23 12:22:09 2012 +0200 @@ -69,6 +69,7 @@ } } + private Options convertToCommonsCLIOptions(Collection<ArgumentSpec> args) { Options options = new Options(); for (ArgumentSpec spec : args) { @@ -88,7 +89,9 @@ HelpFormatter helpFormatter = new HelpFormatter(); helpFormatter.setOptPrefix("--"); PrintWriter pw = new PrintWriter(ctx.getConsole().getOutput()); - Options options = convertToCommonsCLIOptions(cmd.getAcceptedArguments()); + CommonCommandOptions commonOpts = new CommonCommandOptions(); + Collection<ArgumentSpec> acceptedOptions = commonOpts.getAcceptedOptionsFor(cmd); + Options options = convertToCommonsCLIOptions(acceptedOptions); helpFormatter.printHelp(pw, 80, cmd.getName(), cmd.getUsage(), options, 2, 4, null, true); pw.flush(); }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/src/main/java/com/redhat/thermostat/cli/CommonCommandOptions.java Mon Apr 23 12:22:09 2012 +0200 @@ -0,0 +1,69 @@ +/* + * 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.cli; + +import java.util.ArrayList; +import java.util.Collection; + +class CommonCommandOptions { + + static final String DB_URL_ARG = "dbUrl"; + private static final String DB_URL_DESC = "the URL of the storage to connect to"; + + static final String LOG_LEVEL_ARG = "logLevel"; + private static final String LOG_LEVEL_DESC = "log level"; + + Collection<ArgumentSpec> getAcceptedOptionsFor(Command cmd) { + + Collection<ArgumentSpec> acceptedArguments = cmd.getAcceptedArguments(); + acceptedArguments = new ArrayList<>(acceptedArguments); + addDbUrlOptionForStorageCommand(cmd, acceptedArguments); + addLogLevelOption(acceptedArguments); + return acceptedArguments; + } + + private void addDbUrlOptionForStorageCommand(Command cmd, Collection<ArgumentSpec> acceptedArguments) { + if (cmd.isStorageRequired()) { + acceptedArguments.add(new SimpleArgumentSpec(DB_URL_ARG, DB_URL_DESC, true, true)); + } + } + + private void addLogLevelOption(Collection<ArgumentSpec> acceptedArguments) { + acceptedArguments.add(new SimpleArgumentSpec(LOG_LEVEL_ARG, LOG_LEVEL_DESC, false, true)); + } + +}
--- a/common/src/main/java/com/redhat/thermostat/cli/Launcher.java Mon Apr 23 10:52:52 2012 +0200 +++ b/common/src/main/java/com/redhat/thermostat/cli/Launcher.java Mon Apr 23 12:22:09 2012 +0200 @@ -36,10 +36,10 @@ package com.redhat.thermostat.cli; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.ServiceLoader; +import java.util.logging.Level; import com.redhat.thermostat.common.config.InvalidConfigurationException; import com.redhat.thermostat.common.storage.ConnectionException; @@ -47,10 +47,6 @@ 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) { @@ -108,12 +104,30 @@ private void parseArgsAndRunCommand(String cmdName, String[] cmdArgs) throws CommandException { Command cmd = getCommand(cmdName); - Collection<ArgumentSpec> acceptedArguments = getAcceptedCommandArguments(cmd); - Arguments args = parseCommandArguments(cmdArgs, acceptedArguments); + CommonCommandOptions commonOpts = new CommonCommandOptions(); + Collection<ArgumentSpec> acceptedOptions = commonOpts.getAcceptedOptionsFor(cmd); + Arguments args = parseCommandArguments(cmdArgs, acceptedOptions); + setupLogLevel(args); CommandContext ctx = setupCommandContext(cmd, args); cmd.run(ctx); } + private void setupLogLevel(Arguments args) { + if (args.hasArgument(CommonCommandOptions.LOG_LEVEL_ARG)) { + String levelOption = args.getArgument(CommonCommandOptions.LOG_LEVEL_ARG); + setLogLevel(levelOption); + } + } + + private void setLogLevel(String levelOption) { + try { + Level level = Level.parse(levelOption); + LoggingUtils.setGlobalLogLevel(level); + } catch (IllegalArgumentException ex) { + // Ignore this, use default loglevel. + } + } + private Command getCommand(String cmdName) { CommandContextFactory cmdCtxFactory = CommandContextFactory.getInstance(); @@ -122,20 +136,6 @@ 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 { @@ -150,7 +150,7 @@ CommandContextFactory cmdCtxFactory = CommandContextFactory.getInstance(); CommandContext ctx = cmdCtxFactory.createContext(args); if (cmd.isStorageRequired()) { - String dbUrl = ctx.getArguments().getArgument(DB_URL_ARG); + String dbUrl = ctx.getArguments().getArgument(CommonCommandOptions.DB_URL_ARG); try { ctx.getAppContextSetup().setupAppContext(dbUrl); } catch (ConnectionException ex) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/src/test/java/com/redhat/thermostat/cli/CommonCommandOptionsTest.java Mon Apr 23 12:22:09 2012 +0200 @@ -0,0 +1,67 @@ +/* + * 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.cli; + +import static org.junit.Assert.*; + +import java.util.Collection; + +import org.junit.Test; + +public class CommonCommandOptionsTest { + + @Test + public void verifyStorageCommandAddsDbUrlOption() { + TestCommand cmd = new TestCommand("test1"); + cmd.setStorageRequired(true); + + CommonCommandOptions commonOpts = new CommonCommandOptions(); + Collection<ArgumentSpec> cmdOpts = commonOpts.getAcceptedOptionsFor(cmd); + + assertTrue(cmdOpts.contains(new SimpleArgumentSpec("dbUrl", "the URL of the storage to connect to", true, true))); + } + + @Test + public void verifyLogLevelOption() { + TestCommand cmd = new TestCommand("test1"); + + CommonCommandOptions commonOpts = new CommonCommandOptions(); + Collection<ArgumentSpec> cmdOpts = commonOpts.getAcceptedOptionsFor(cmd); + + assertTrue(cmdOpts.contains(new SimpleArgumentSpec("logLevel", "log level", false, true))); + } +}
--- a/common/src/test/java/com/redhat/thermostat/cli/HelpCommandTest.java Mon Apr 23 10:52:52 2012 +0200 +++ b/common/src/test/java/com/redhat/thermostat/cli/HelpCommandTest.java Mon Apr 23 12:22:09 2012 +0200 @@ -116,7 +116,24 @@ cmd.run(ctxFactory.createContext(args)); String actual = ctxFactory.getOutput(); - assertEquals("usage: test1\ntest usage command 1\n\n", actual); + assertEquals("usage: test1 [-logLevel <arg>]\ntest usage command 1\n --logLevel <arg> log level\n", actual); + } + + @Test + public void verifyHelpKnownStorageCmdPrintsCommandUsageWithDbUrl() { + TestCommand cmd1 = new TestCommand("test1"); + String usage = "test usage command 1"; + cmd1.setUsage(usage); + cmd1.setStorageRequired(true); + ctxFactory.getCommandRegistry().registerCommands(Arrays.asList(cmd1)); + + HelpCommand cmd = new HelpCommand(); + Arguments args = mock(Arguments.class); + when(args.getNonOptionArguments()).thenReturn(Arrays.asList("test1")); + cmd.run(ctxFactory.createContext(args)); + + String actual = ctxFactory.getOutput(); + assertEquals("usage: test1 -dbUrl <arg> [-logLevel <arg>]\ntest usage command 1\n --dbUrl <arg> the URL of the storage to connect to\n --logLevel <arg> log level\n", actual); } @Test
--- a/common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java Mon Apr 23 10:52:52 2012 +0200 +++ b/common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java Mon Apr 23 12:22:09 2012 +0200 @@ -41,6 +41,8 @@ import static org.mockito.Mockito.verify; import java.util.Arrays; +import java.util.logging.Level; +import java.util.logging.Logger; import org.junit.After; import org.junit.Before; @@ -142,6 +144,13 @@ } @Test + public void verifySetLogLevel() { + runAndVerifyCommand(new String[] {"test1", "--logLevel", "WARNING", "--arg1", "Hello", "--arg2", "World"}, "Hello, World"); + Logger globalLogger = Logger.getLogger("com.redhat.thermostat"); + assertEquals(Level.WARNING, globalLogger.getLevel()); + } + + @Test public void testMainBadCommand1() { String expected = "list of commands:\n\n" + " help show help for a given command or help overview\n"