modules/core/plugin-api/src/main/java/org/rhq/core/pluginapi/util/JavaCommandLine.java | 47 +++++-----
modules/core/plugin-api/src/test/java/org/rhq/core/pluginapi/util/JavaCommandLineTest.java | 16 ++-
2 files changed, 40 insertions(+), 23 deletions(-)
New commits:
commit bdb7941db69414f7f0f104d2f88d623e4da68754
Author: Ian Springer <ian.springer(a)redhat.com>
Date: Wed Apr 25 16:03:12 2012 -0400
fix a bug in JavaCommandLine.getClassOption() where a bogus warning was logged whenever the requested option was not present on the command line; fix a bug in JavaCommandLine.toString() where Array.toString() was called on several non-arrays; add tests for these changes
diff --git a/modules/core/plugin-api/src/main/java/org/rhq/core/pluginapi/util/JavaCommandLine.java b/modules/core/plugin-api/src/main/java/org/rhq/core/pluginapi/util/JavaCommandLine.java
index 4763be1..1c9e08c 100644
--- a/modules/core/plugin-api/src/main/java/org/rhq/core/pluginapi/util/JavaCommandLine.java
+++ b/modules/core/plugin-api/src/main/java/org/rhq/core/pluginapi/util/JavaCommandLine.java
@@ -107,11 +107,11 @@ public class JavaCommandLine {
public JavaCommandLine(String[] args, boolean includeSystemPropertiesFromClassArguments,
Set<OptionValueDelimiter> shortClassOptionValueDelims, Set<OptionValueDelimiter> longClassOptionValueDelims) {
if (args == null) {
- throw new IllegalArgumentException("'commandLine' parameter is null.");
+ throw new IllegalArgumentException("'args' parameter is null.");
}
if (args.length == 0) {
- throw new IllegalArgumentException("'commandLine' parameter is an empty array.");
+ throw new IllegalArgumentException("'args' parameter is an empty array.");
}
this.includeSystemPropertiesFromClassArguments = includeSystemPropertiesFromClassArguments;
@@ -130,15 +130,17 @@ public class JavaCommandLine {
throw new IllegalArgumentException("'longClassOptionValueDelims' parameter is an empty set.");
}
- if (log.isDebugEnabled()) {
- log.debug("Parsing " + toString());
- }
+ // Wrap as list and store as field for use by getArguments() and toString().
+ this.arguments = Arrays.asList(args);
parseCommandLine(args);
}
private void parseCommandLine(String[] args) {
- this.arguments = Arrays.asList(args);
+ if (log.isDebugEnabled()) {
+ log.debug("Parsing " + this + "...");
+ }
+
this.javaExecutable = new File(args[0]);
this.classPath = new ArrayList<String>();
this.systemProperties = new LinkedHashMap<String, String>();
@@ -340,10 +342,12 @@ public class JavaCommandLine {
*/
@Nullable
public String getClassOption(CommandLineOption option) {
- String optionValue;
- if (option.getLongName() != null && this.longClassOptionNameToOptionValueMap.containsKey(option.getLongName())) {
+ String optionValue = null;
+ // Note, we never store null values in either of the option value maps.
+
+ if ((option.getLongName() != null) && this.longClassOptionNameToOptionValueMap.containsKey(option.getLongName())) {
optionValue = this.longClassOptionNameToOptionValueMap.get(option.getLongName());
- if (optionValue != null && !option.isExpectsValue()) {
+ if (!optionValue.isEmpty() && !option.isExpectsValue()) {
// TODO: Store the delims used for each of the options in another set of maps, so we can handle
// things differently here depending on what delim was used.
if (this.longClassOptionValueDelims.equals(EnumSet.of(OptionValueDelimiter.EQUALS_SIGN))) {
@@ -353,9 +357,11 @@ public class JavaCommandLine {
optionValue = "";
}
}
- } else if (option.getShortName() != null && this.shortClassOptionNameToOptionValueMap.containsKey(option.getShortName())) {
+ }
+
+ if ((optionValue == null) && (option.getShortName() != null) && this.shortClassOptionNameToOptionValueMap.containsKey(option.getShortName())) {
optionValue = this.shortClassOptionNameToOptionValueMap.get(option.getShortName());
- if (optionValue != null && !option.isExpectsValue()) {
+ if (!optionValue.isEmpty() && !option.isExpectsValue()) {
// TODO: Store the delims used for each of the options in another set of maps, so we can handle
// things differently here depending on what delim was used.
if (this.shortClassOptionValueDelims.equals(EnumSet.of(OptionValueDelimiter.EQUALS_SIGN))) {
@@ -365,12 +371,11 @@ public class JavaCommandLine {
optionValue = "";
}
}
- } else {
- optionValue = null;
- if (option.isExpectsValue()) {
- log.warn("Class option [" + option + "] expects a value, but no value was specified on command line ["
- + this + "].");
- }
+ }
+
+ if (optionValue != null && optionValue.isEmpty() && option.isExpectsValue()) {
+ log.warn("Class option [" + option + "] expects a value, but no value was specified on command line ["
+ + this + "].");
}
return optionValue;
@@ -383,10 +388,10 @@ public class JavaCommandLine {
@Override
public String toString() {
- return "JavaCommandLine [arguments=" + Arrays.asList(arguments) //
- + ", includeSystemPropertiesFromClassArguments=" + includeSystemPropertiesFromClassArguments //
- + ", shortClassOptionFormat=" + Arrays.asList(shortClassOptionValueDelims) //
- + ", longClassOptionFormat=" + Arrays.asList(longClassOptionValueDelims) + "]";
+ return "JavaCommandLine[arguments=" + this.arguments //
+ + ", includeSystemPropertiesFromClassArguments=" + this.includeSystemPropertiesFromClassArguments //
+ + ", shortClassOptionFormat=" + this.shortClassOptionValueDelims //
+ + ", longClassOptionFormat=" + this.longClassOptionValueDelims + "]";
}
}
diff --git a/modules/core/plugin-api/src/test/java/org/rhq/core/pluginapi/util/JavaCommandLineTest.java b/modules/core/plugin-api/src/test/java/org/rhq/core/pluginapi/util/JavaCommandLineTest.java
index e6dc4ae..a8bab91 100644
--- a/modules/core/plugin-api/src/test/java/org/rhq/core/pluginapi/util/JavaCommandLineTest.java
+++ b/modules/core/plugin-api/src/test/java/org/rhq/core/pluginapi/util/JavaCommandLineTest.java
@@ -122,10 +122,15 @@ public class JavaCommandLineTest {
Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("x", null, true)), "blah");
Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption(null, "novaluelong", false)), "");
Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption(null, "novaluelong", true)), "");
- Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption(null, "long", false)), "longval");
- Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption(null, "long", true)), "longval");
+ Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("l", "long", false)), "longval");
+ Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("l", "long", true)), "longval");
Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("n", null, false)), "");
Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("n", null, true)), "");
+
+ javaCommandLine = new JavaCommandLine("java", "-Dshape=circle", "-Xmx100M", "-Dcolor=blue",
+ "-ea", "-cp", cp, "org.example.Main", "-x", "blah", "--novaluelong", "-l", "longval", "-n");
+ Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("l", "long", false)), "");
+ Assert.assertEquals(javaCommandLine.getClassOption(new CommandLineOption("l", "long", true)), "longval");
}
public void testGetAbsentClassOption() throws Exception {
@@ -148,4 +153,11 @@ public class JavaCommandLineTest {
Assert.assertNull(javaCommandLine.getClassOption(new CommandLineOption("n", null, false)));
}
+ public void testToString() throws Exception {
+ String cp = "a.jar" + File.pathSeparator + "b.jar" + File.pathSeparator + "c.jar";
+ JavaCommandLine javaCommandLine = new JavaCommandLine("java", "-Dshape=circle", "-Xmx100M", "-Dcolor=blue",
+ "-ea", "-cp", cp, "org.example.Main", "-x", "blah", "--novaluelong", "--long=longval", "-n");
+ Assert.assertNotNull(javaCommandLine.toString());
+ }
+
}