Make CommandLinePropertySource enumerable

Dave Syer · cbeams · commit 7860af862413 · 2013-05-28T12:57:01.000+02:00
JOpt 4.4 has enumerable options, so this change can be made
if we upgrade. The only awkward thing is that JOpt allows
aliases for options, so we have to pick one to avoid double
counting. This implementation picks the last one in the list
which is the alphebtically last of the long options, if there
are any (e.g. "o1", "option1" returns "option1"). Most of the
time there will only be one or two aliases for each option so
it won't matter.

Issue: SPR-10579
diff --git a/build.gradle b/build.gradle
@@ -205,7 +205,7 @@ project("spring-core") {
 		compile(files(cglibRepackJar))
 		compile("commons-logging:commons-logging:1.1.1")
 		optional("org.aspectj:aspectjweaver:${aspectjVersion}")
-		optional("net.sf.jopt-simple:jopt-simple:3.0")
+		optional("net.sf.jopt-simple:jopt-simple:4.4")
 		optional("log4j:log4j:1.2.17")
 		testCompile("xmlunit:xmlunit:1.3")
 		testCompile("org.codehaus.woodstox:wstx-asl:3.2.7") {
diff --git a/spring-core/src/main/java/org/springframework/core/env/CommandLinePropertySource.java b/spring-core/src/main/java/org/springframework/core/env/CommandLinePropertySource.java
@@ -185,7 +185,7 @@
  * @see SimpleCommandLinePropertySource
  * @see JOptCommandLinePropertySource
  */
-public abstract class CommandLinePropertySource<T> extends PropertySource<T> {
+public abstract class CommandLinePropertySource<T> extends EnumerablePropertySource<T> {
 
 	/** The default name given to {@link CommandLinePropertySource} instances: {@value} */
 	public static final String COMMAND_LINE_PROPERTY_SOURCE_NAME = "commandLineArgs";
@@ -218,7 +218,7 @@ public CommandLinePropertySource(String name, T source) {
 	public void setNonOptionArgsPropertyName(String nonOptionArgsPropertyName) {
 		this.nonOptionArgsPropertyName = nonOptionArgsPropertyName;
 	}
-
+	
 	/**
 	 * Return whether this {@code PropertySource} contains a property with the given name.
 	 * <p>This implementation first checks to see if the name specified is the special
diff --git a/spring-core/src/main/java/org/springframework/core/env/JOptCommandLinePropertySource.java b/spring-core/src/main/java/org/springframework/core/env/JOptCommandLinePropertySource.java
@@ -21,6 +21,7 @@
 import java.util.List;
 
 import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 
 /**
  * {@link CommandLinePropertySource} implementation backed by a JOpt {@link OptionSet}.
@@ -76,7 +77,20 @@ public JOptCommandLinePropertySource(String name, OptionSet options) {
 	protected boolean containsOption(String name) {
 		return this.source.has(name);
 	}
-
+	
+	@Override
+	public String[] getPropertyNames() {
+		List<String> names = new ArrayList<>();
+		for (OptionSpec<?> spec : source.specs()) {
+			List<String> aliases = new ArrayList<>(spec.options());
+			if (!aliases.isEmpty()) {
+				// Only the longest name is used for enumerating
+				names.add(aliases.get(aliases.size()-1));				
+			}
+		}
+		return names.toArray(new String[names.size()]);
+	}
+	
 	@Override
 	public List<String> getOptionValues(String name) {
 		List<?> argValues = this.source.valuesOf(name);
diff --git a/spring-core/src/main/java/org/springframework/core/env/SimpleCommandLinePropertySource.java b/spring-core/src/main/java/org/springframework/core/env/SimpleCommandLinePropertySource.java
@@ -94,6 +94,14 @@ public SimpleCommandLinePropertySource(String... args) {
 	public SimpleCommandLinePropertySource(String name, String[] args) {
 		super(name, new SimpleCommandLineArgsParser().parse(args));
 	}
+	
+	/**
+	 * Get the property names for the option arguments.
+	 */
+	@Override
+	public String[] getPropertyNames() {
+		return source.getOptionNames().toArray(new String[source.getOptionNames().size()]);
+	}
 
 	@Override
 	protected boolean containsOption(String name) {
diff --git a/spring-core/src/test/java/org/springframework/core/env/JOptCommandLinePropertySourceTests.java b/spring-core/src/test/java/org/springframework/core/env/JOptCommandLinePropertySourceTests.java
@@ -117,17 +117,18 @@ public void withDottedOptionName() {
 	@Test
 	public void withDefaultNonOptionArgsNameAndNoNonOptionArgsPresent() {
 		OptionParser parser = new OptionParser();
-		parser.accepts("o1").withRequiredArg();
+		parser.acceptsAll(Arrays.asList("o1","option1")).withRequiredArg();
 		parser.accepts("o2");
 		OptionSet optionSet = parser.parse("--o1=v1", "--o2");
-		PropertySource<?> ps = new JOptCommandLinePropertySource(optionSet);
+		EnumerablePropertySource<?> ps = new JOptCommandLinePropertySource(optionSet);
 
 		assertThat(ps.containsProperty("nonOptionArgs"), is(false));
 		assertThat(ps.containsProperty("o1"), is(true));
 		assertThat(ps.containsProperty("o2"), is(true));
 
 		assertThat(ps.containsProperty("nonOptionArgs"), is(false));
 		assertThat(ps.getProperty("nonOptionArgs"), nullValue());
+		assertThat(ps.getPropertyNames().length, is(2));
 	}
 
 	@Test
diff --git a/spring-core/src/test/java/org/springframework/core/env/SimpleCommandLinePropertySourceTests.java b/spring-core/src/test/java/org/springframework/core/env/SimpleCommandLinePropertySourceTests.java
@@ -16,15 +16,13 @@
 
 package org.springframework.core.env;
 
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.nullValue;
-import static org.junit.Assert.assertThat;
-
 import java.util.List;
 
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
 /**
  * Unit tests for {@link SimpleCommandLinePropertySource}.
  *
@@ -67,14 +65,15 @@ public void withOptionArgsOnly() {
 
 	@Test
 	public void withDefaultNonOptionArgsNameAndNoNonOptionArgsPresent() {
-		PropertySource<?> ps = new SimpleCommandLinePropertySource("--o1=v1", "--o2");
+		EnumerablePropertySource<?> ps = new SimpleCommandLinePropertySource("--o1=v1", "--o2");
 
 		assertThat(ps.containsProperty("nonOptionArgs"), is(false));
 		assertThat(ps.containsProperty("o1"), is(true));
 		assertThat(ps.containsProperty("o2"), is(true));
 
 		assertThat(ps.containsProperty("nonOptionArgs"), is(false));
 		assertThat(ps.getProperty("nonOptionArgs"), nullValue());
+		assertThat(ps.getPropertyNames().length, is(2));
 	}
 
 	@Test
