changeset 1101:97e66ed2e4ae

Add localize() variant and remove cause-only CommandException constructor reviewed-by: omajid review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-May/006596.html
author Jon VanAlten <jon.vanalten@redhat.com>
date Mon, 13 May 2013 16:26:16 -0600
parents c469675494d7
children 8757e35030f2
files agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ShellCommand.java client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties common/core/src/main/java/com/redhat/thermostat/common/cli/CommandException.java common/core/src/main/java/com/redhat/thermostat/common/locale/LocaleResources.java common/core/src/main/java/com/redhat/thermostat/common/locale/Translate.java common/core/src/main/java/com/redhat/thermostat/common/utils/HostPortsParser.java common/core/src/main/resources/com/redhat/thermostat/common/locale/strings.properties common/core/src/test/java/com/redhat/thermostat/common/cli/CommandExceptionTest.java common/core/src/test/java/com/redhat/thermostat/common/locale/TranslateTest.java common/core/src/test/java/com/redhat/thermostat/common/utils/HostPortsParserTest.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandLineArgumentParseException.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandLineArgumentsParser.java launcher/src/main/resources/com/redhat/thermostat/launcher/internal/strings.properties web/cmd/src/main/java/com/redhat/thermostat/web/cmd/LocaleResources.java web/cmd/src/main/java/com/redhat/thermostat/web/cmd/WebServiceCommand.java web/cmd/src/main/resources/com/redhat/thermostat/web/cmd/strings.properties
diffstat 19 files changed, 107 insertions(+), 70 deletions(-) [+]
line wrap: on
line diff
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Thu May 02 10:58:20 2013 -0600
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Mon May 13 16:26:16 2013 -0600
@@ -184,15 +184,11 @@
 
     @Override
     public void run(CommandContext ctx) throws CommandException {
-        try {
-            configuration = configurationCreator.create();
+        configuration = configurationCreator.create();
 
-            parseArguments(ctx.getArguments());
-            if (!parser.isHelp()) {
-                runAgent(ctx);
-            }
-        } catch (InvalidConfigurationException ex) {
-            throw new CommandException(ex);
+        parseArguments(ctx.getArguments());
+        if (!parser.isHelp()) {
+            runAgent(ctx);
         }
     }
     
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java	Thu May 02 10:58:20 2013 -0600
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java	Mon May 13 16:26:16 2013 -0600
@@ -83,12 +83,7 @@
     
     @Override
     public void run(CommandContext ctx) throws CommandException {
-
-        try {
-            parseArgsAndRun(ctx);
-        } catch (InvalidConfigurationException e) {
-            throw new CommandException(e);
-        }
+        parseArgsAndRun(ctx);
     }
 
     private void parseArgsAndRun(CommandContext ctx)
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java	Thu May 02 10:58:20 2013 -0600
+++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java	Mon May 13 16:26:16 2013 -0600
@@ -58,6 +58,8 @@
     COMMAND_DISCONNECT_NOT_CONNECTED,
     COMMAND_DISCONNECT_ERROR,
 
+    COMMAND_SHELL_IO_EXCEPTION,
+
     VM_INFO_PROCESS_ID,
     VM_INFO_START_TIME,
     VM_INFO_STOP_TIME,
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ShellCommand.java	Thu May 02 10:58:20 2013 -0600
+++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ShellCommand.java	Mon May 13 16:26:16 2013 -0600
@@ -108,7 +108,7 @@
             ctx.getConsole().getOutput().println(version.getVersionInfo());
             shellMainLoop(ctx, history, term);
         } catch (IOException ex) {
-            throw new CommandException(ex);
+            throw new CommandException(t.localize(LocaleResources.COMMAND_SHELL_IO_EXCEPTION), ex);
         } finally {
             closeTerminal(term);
             if (history != null) {
@@ -125,7 +125,7 @@
         try {
             term.restore();
         } catch (Exception e) {
-            throw new CommandException(e);
+            logger.log(Level.WARNING, "Error restoring terminal state.", e);
         }
     }
 
--- a/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties	Thu May 02 10:58:20 2013 -0600
+++ b/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties	Mon May 13 16:26:16 2013 -0600
@@ -16,6 +16,8 @@
 COMMAND_DISCONNECT_NOT_CONNECTED = Not connected to storage. You may use the connect command for establishing connections.
 COMMAND_DISCONNECT_ERROR = Failed to disconnect from database.
 
+COMMAND_SHELL_IO_EXCEPTION = IOException caught during Thermostat shell session.
+
 VM_INFO_PROCESS_ID = Process ID:
 VM_INFO_START_TIME = Start time:
 VM_INFO_STOP_TIME = Stop time:
--- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandException.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandException.java	Mon May 13 16:26:16 2013 -0600
@@ -53,11 +53,6 @@
         localizedMessage = message;
     }
 
-    public CommandException(Throwable cause) {
-        super(cause);
-        localizedMessage = new LocalizedString(cause.getLocalizedMessage());
-    }
-
     public CommandException(LocalizedString message, Throwable cause) {
         super(message.getContents(), cause);
         localizedMessage = message;
--- a/common/core/src/main/java/com/redhat/thermostat/common/locale/LocaleResources.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/main/java/com/redhat/thermostat/common/locale/LocaleResources.java	Mon May 13 16:26:16 2013 -0600
@@ -52,6 +52,8 @@
     LOGGING_PROPERTIES_ISSUE,
 
     ENV_NO_HOME,
+
+    INVALID_IPPORT,
     ;
 
     public static final String RESOURCE_BUNDLE =
--- a/common/core/src/main/java/com/redhat/thermostat/common/locale/Translate.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/main/java/com/redhat/thermostat/common/locale/Translate.java	Mon May 13 16:26:16 2013 -0600
@@ -59,5 +59,36 @@
     public LocalizedString localize(T toTranslate, String... params) {
         return new LocalizedString(MessageFormat.format(localize(toTranslate).getContents(), (Object[]) params));
     }
+
+    /**
+     * Creates a {@link LocalizedString} that can contain one list of unknown length
+     * that gets inserted as a single String, with separator as specified, into
+     * the regular translate parameters at a given position.
+     * @param toTranslate The translation key
+     * @param specialParams The list to be inserted into the parameters
+     * @param separator The separator to use between each element in the list
+     * @param position The position within the regular parameters at which to insert the list-string (zero indexed)
+     * @param params The parameters to apply to the translation
+     * @return The {@link LocalizedString}
+     */
+    public LocalizedString localize(T toTranslate, String[] specialParams, String separator, int position, String... params) {
+        StringBuilder builder = new StringBuilder(specialParams[0]);
+        for (int i = 1; i < specialParams.length; i++) {
+            builder.append(separator).append(specialParams[i]);
+        }
+        String replacedList = builder.toString();
+
+
+        
+        String[] newParams = new String[params.length + 1];
+        for (int i = 0; i < position; i++) {
+            newParams[i] = params[i];
+        }
+        newParams[position] = replacedList;
+        for (int i = position + 1, j = position; j < params.length; i++, j++) {
+            newParams[i] = params[j];
+        }
+        return new LocalizedString(MessageFormat.format(localize(toTranslate).getContents(), (Object[]) newParams));
+    }
 }
 
--- a/common/core/src/main/java/com/redhat/thermostat/common/utils/HostPortsParser.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/main/java/com/redhat/thermostat/common/utils/HostPortsParser.java	Mon May 13 16:26:16 2013 -0600
@@ -39,6 +39,10 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import com.redhat.thermostat.common.config.InvalidConfigurationException;
+import com.redhat.thermostat.common.locale.LocaleResources;
+import com.redhat.thermostat.common.locale.Translate;
+
 /**
  * Parses Host/Port pairs from a raw string.
  * 
@@ -68,14 +72,15 @@
 
     private final String rawString;
     private List<HostPortPair> ipPorts;
-    private final IllegalArgumentException formatException; 
+    private final InvalidConfigurationException formatException; 
+    private final Translate<LocaleResources> t = LocaleResources.createLocalizer();
     
     public HostPortsParser(String parseString) {
         this.rawString = parseString;
-        this.formatException = new IllegalArgumentException("Invalid format of IP/port argument " + rawString);
+        this.formatException = new InvalidConfigurationException(t.localize(LocaleResources.INVALID_IPPORT, rawString));
     }
     
-    public void parse() throws IllegalArgumentException {
+    public void parse() throws InvalidConfigurationException {
         ipPorts = new ArrayList<>();
         for (String ipPortPair: rawString.split(",")) {
             // if we have a '[' in the ip:port pair string we likely have an IPv6
--- a/common/core/src/main/resources/com/redhat/thermostat/common/locale/strings.properties	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/main/resources/com/redhat/thermostat/common/locale/strings.properties	Mon May 13 16:26:16 2013 -0600
@@ -14,3 +14,5 @@
 LOGGING_PROPERTIES_ISSUE = Could not read logging.properties
 
 ENV_NO_HOME = THERMOSTAT_HOME not defined...
+
+INVALID_IPPORT = Invalid format of IP/port argument {0}
--- a/common/core/src/test/java/com/redhat/thermostat/common/cli/CommandExceptionTest.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/test/java/com/redhat/thermostat/common/cli/CommandExceptionTest.java	Mon May 13 16:26:16 2013 -0600
@@ -52,13 +52,6 @@
     }
 
     @Test
-    public void testCauseConstructor() {
-        Exception cause = new Exception("test fluff");
-        CommandException ce = new CommandException(cause);
-        verifyMessageAndCause(ce, "java.lang.Exception: test fluff", cause);
-    }
-
-    @Test
     public void testCombinedConstructor() {
         Exception cause = new Exception("test fluff");
         CommandException ce = new CommandException(new LocalizedString("test"), cause);
--- a/common/core/src/test/java/com/redhat/thermostat/common/locale/TranslateTest.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/test/java/com/redhat/thermostat/common/locale/TranslateTest.java	Mon May 13 16:26:16 2013 -0600
@@ -46,8 +46,8 @@
 public class TranslateTest {
 
     enum TestStrings {
-        SIMPLE_STRING,
-        STRING_WITH_PARAMETER,
+        THE_STRING,
+        ;
     }
 
     // Mockito can't mock the final method getMessage() which is what Translate
@@ -68,21 +68,36 @@
 
     @Test
     public void testLocalizeWithoutArguments() {
-        ResourceBundle resources = new LocalizedResourceBundle(TestStrings.SIMPLE_STRING.name(), "Localized String");
+        Translate<TestStrings> translate = getTranslator(TestStrings.THE_STRING.name(), "Localized String");
 
-        Translate<TestStrings> translate = new Translate<>(resources, TestStrings.class);
-
-        assertEquals("Localized String", translate.localize(TestStrings.SIMPLE_STRING).getContents());
+        assertEquals("Localized String", translate.localize(TestStrings.THE_STRING).getContents());
     }
 
     @Test
     public void testLocalizeWithArguments() {
-        ResourceBundle resources = new LocalizedResourceBundle(TestStrings.STRING_WITH_PARAMETER.name(), "Parameter: {0}");
+        Translate<TestStrings> translate = getTranslator(TestStrings.THE_STRING.name(), "Parameter: {0}");
+
+        assertEquals("Parameter: FOO", translate.localize(TestStrings.THE_STRING, "FOO").getContents());
+    }
+
+    @Test
+    public void testLocalizeWithSeveralArguments() {
+        Translate<TestStrings> translate = getTranslator(TestStrings.THE_STRING.name(), "Parameter1: {0}  Parameter2: {1}  Parameter3: {2}");
+
+        assertEquals("Parameter1: ONE  Parameter2: TWO  Parameter3: THREE", translate.localize(TestStrings.THE_STRING, "ONE", "TWO", "THREE").getContents());
+    }
 
-        Translate<TestStrings> translate = new Translate<>(resources, TestStrings.class);
+    @Test
+    public void testLocalizeWithSpecialList() {
+        Translate<TestStrings> translate = getTranslator(TestStrings.THE_STRING.name(), "Parameter1: {0}  ParameterList: {1}  Parameter2: {2}");
 
-        assertEquals("Parameter: FOO", translate.localize(TestStrings.STRING_WITH_PARAMETER, "FOO").getContents());
+        assertEquals("Parameter1: ONE  ParameterList: FOO, BAR, BAZ  Parameter2: TWO", translate.localize(TestStrings.THE_STRING, new String[]{"FOO", "BAR", "BAZ"}, ", ", 1, "ONE", "TWO").getContents());
+       
+    }
 
+    private Translate<TestStrings> getTranslator(String key, String localizedString) {
+        ResourceBundle resources = new LocalizedResourceBundle(key, localizedString);
+        return new Translate<>(resources, TestStrings.class);
     }
 }
 
--- a/common/core/src/test/java/com/redhat/thermostat/common/utils/HostPortsParserTest.java	Thu May 02 10:58:20 2013 -0600
+++ b/common/core/src/test/java/com/redhat/thermostat/common/utils/HostPortsParserTest.java	Mon May 13 16:26:16 2013 -0600
@@ -42,6 +42,7 @@
 
 import org.junit.Test;
 
+import com.redhat.thermostat.common.config.InvalidConfigurationException;
 import com.redhat.thermostat.common.utils.HostPortPair;
 import com.redhat.thermostat.common.utils.HostPortsParser;
 
@@ -100,19 +101,19 @@
         int exptns = 0;
         try {
             parser.parse();
-        } catch (IllegalArgumentException e) {
+        } catch (InvalidConfigurationException e) {
             exptns++;
         }
         parser = new HostPortsParser("blah,test");
         try {
             parser.parse();
-        } catch (IllegalArgumentException e) {
+        } catch (InvalidConfigurationException e) {
             exptns++;
         }
         parser = new HostPortsParser("127.0.0.1:80,127.0.0.2:bad");
         try {
             parser.parse();
-        } catch (IllegalArgumentException e) {
+        } catch (InvalidConfigurationException e) {
             exptns++;
         }
         assertEquals(expectedExcptns, exptns);
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandLineArgumentParseException.java	Thu May 02 10:58:20 2013 -0600
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandLineArgumentParseException.java	Mon May 13 16:26:16 2013 -0600
@@ -45,10 +45,6 @@
         super(message);
     }
 
-    public CommandLineArgumentParseException(Throwable cause) {
-        super(cause);
-    }
-
     public CommandLineArgumentParseException(LocalizedString message, Throwable cause) {
         super(message, cause);
     }
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandLineArgumentsParser.java	Thu May 02 10:58:20 2013 -0600
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandLineArgumentsParser.java	Mon May 13 16:26:16 2013 -0600
@@ -82,26 +82,26 @@
     private LocalizedString createMissingOptionsMessage(MissingOptionException mae) {
         @SuppressWarnings("unchecked")
         List<String> missingOptions = mae.getMissingOptions();
-        StringBuilder msg = new StringBuilder();
-        if (missingOptions.size() == 1) {
-            msg.append(tr.localize(LocaleResources.MISSING_OPTION).getContents());
-        } else {
-            msg.append(tr.localize(LocaleResources.MISSING_OPTIONS).getContents());
-        }
+        String[] presentableMissingOptions = new String[missingOptions.size()];
+        int optIndex = 0;
         for (Iterator<String> i = missingOptions.iterator(); i.hasNext();) {
+            StringBuilder missingOptionBuilder = new StringBuilder();
             String missingOption = i.next();
             if (missingOption.length() > 1) {
-                msg.append("--");
+                missingOptionBuilder.append("--");
             } else {
-                msg.append("-");
+                missingOptionBuilder.append("-");
             }
-            msg.append(missingOption);
-            if (i.hasNext()) {
-                msg.append(", ");
-            }
+            missingOptionBuilder.append(missingOption);
+
+            presentableMissingOptions[optIndex] = missingOptionBuilder.toString();
+            optIndex++;
         }
-        // Workaround to create this (complex) localized string.
-        return new LocalizedString(msg.toString());
+        if (missingOptions.size() == 1) {
+            return tr.localize(LocaleResources.MISSING_OPTION, presentableMissingOptions, ", ", 0, new String[]{});
+        } else {
+            return tr.localize(LocaleResources.MISSING_OPTIONS, presentableMissingOptions, ", ", 0, new String[]{});
+        }
     }
 }
 
--- a/launcher/src/main/resources/com/redhat/thermostat/launcher/internal/strings.properties	Thu May 02 10:58:20 2013 -0600
+++ b/launcher/src/main/resources/com/redhat/thermostat/launcher/internal/strings.properties	Mon May 13 16:26:16 2013 -0600
@@ -12,8 +12,8 @@
  for <logLevel> in decreasing severity are: SEVERE, WARNING, INFO, CONFIG, FINE, \
  FINER, FINEST and OFF
 
-MISSING_OPTION = Missing required option: 
-MISSING_OPTIONS = Missing required options: 
+MISSING_OPTION = Missing required option: {0}
+MISSING_OPTIONS = Missing required options: {0}
 PARSE_EXCEPTION_MESSAGE = Could not parse options: {0}
 
 LAUNCHER_USER_AUTH_PROMPT_ERROR = Error while prompting for username and password.
--- a/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/LocaleResources.java	Thu May 02 10:58:20 2013 -0600
+++ b/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/LocaleResources.java	Mon May 13 16:26:16 2013 -0600
@@ -43,6 +43,8 @@
     NEED_URL,
     NEED_IP,
     INVALID_PORT,
+    ERROR_STARTING_SERVICE,
+    ERROR_STOPPING_SERVICE,
     ;
 
     static final String RESOURCE_BUNDLE = "com.redhat.thermostat.web.cmd.strings";
--- a/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/WebServiceCommand.java	Thu May 02 10:58:20 2013 -0600
+++ b/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/WebServiceCommand.java	Mon May 13 16:26:16 2013 -0600
@@ -42,12 +42,14 @@
 import com.redhat.thermostat.common.cli.CommandContext;
 import com.redhat.thermostat.common.cli.CommandException;
 import com.redhat.thermostat.common.cli.AbstractCommand;
+import com.redhat.thermostat.common.locale.Translate;
 import com.redhat.thermostat.common.utils.HostPortPair;
 import com.redhat.thermostat.common.utils.HostPortsParser;
 
 public class WebServiceCommand extends AbstractCommand {
 
     private WebServiceLauncher serviceLauncher;
+    private final Translate<LocaleResources> t = LocaleResources.createLocalizer();
     
     public WebServiceCommand() {
         this.serviceLauncher = new WebServiceLauncher();
@@ -76,11 +78,11 @@
                 serviceLauncher.stop();
             } catch (Exception ex) {
                 ex.printStackTrace(ctx.getConsole().getError());
-                throw new CommandException(ex);
+                throw new CommandException(t.localize(LocaleResources.ERROR_STOPPING_SERVICE), ex);
             }
         } catch (Exception ex) {
             ex.printStackTrace(ctx.getConsole().getError());
-            throw new CommandException(ex);
+            throw new CommandException(t.localize(LocaleResources.ERROR_STARTING_SERVICE), ex);
         }
     }
 
@@ -96,11 +98,7 @@
 
     private List<HostPortPair> parseIPsPorts(String rawIpsPorts) throws CommandException {
         HostPortsParser parser = new HostPortsParser(rawIpsPorts);
-        try {
-           parser.parse(); 
-        } catch (IllegalArgumentException e) {
-            throw new CommandException(e);
-        }
+        parser.parse();
         return parser.getHostsPorts();
     }
 
--- a/web/cmd/src/main/resources/com/redhat/thermostat/web/cmd/strings.properties	Thu May 02 10:58:20 2013 -0600
+++ b/web/cmd/src/main/resources/com/redhat/thermostat/web/cmd/strings.properties	Mon May 13 16:26:16 2013 -0600
@@ -1,3 +1,5 @@
 NEED_URL = Storage URL must be set
 NEED_IP = IP adresses to bind to must be set
 INVALID_PORT = Invalid port number {0}
+ERROR_STARTING_SERVICE = Error starting web storage service
+ERROR_STOPPING_SERVICE = Error stopping web storage service