Spring/spring-shell
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Changed return value of ParameterResolver::resolve to use a ValueResult that contains metadata about the value that was resolved

Browse Source 
Added new ValueResultAsserts to facilitate testing of ParameterResolvers
This commit is contained in:
camilojc
2017-06-22 00:06:28 +01:00
committed by Eric Bottard
parent bf10f679ec
commit 5fd1f716d9
15 changed files with 444 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
spring-shell2-shell1-adapter/pom.xml
View File

@@ -27,6 +27,11 @@
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.shell</groupId>
<artifactId>spring-shell2-core-tests</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>

65
spring-shell2-shell1-adapter/src/main/java/org/springframework/shell2/legacy/LegacyParameterResolver.java
View File

@@ -19,6 +19,7 @@ package org.springframework.shell2.legacy;
import java.lang.reflect.Parameter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
@@ -36,6 +37,7 @@ import org.springframework.shell2.CompletionContext;
import org.springframework.shell2.CompletionProposal;
import org.springframework.shell2.ParameterDescription;
import org.springframework.shell2.ParameterResolver;
import org.springframework.shell2.ValueResult;
import org.springframework.stereotype.Component;
import org.springframework.util.Assert;
@@ -64,21 +66,23 @@ public class LegacyParameterResolver implements ParameterResolver {
}
@Override
public Object resolve(MethodParameter methodParameter, List<String> words) {
public ValueResult resolve(MethodParameter methodParameter, List<String> words) {
CliOption cliOption = methodParameter.getParameterAnnotation(CliOption.class);
Optional<Converter<?>> converter = converters.stream()
.filter(c -> c.supports(methodParameter.getParameterType(), cliOption.optionContext()))
.findFirst();
Map<String, String> values = parseOptions(words);
Map<String, Object> seenValues = convertValues(values, methodParameter, converter);
Map<String, ParseResult> values = parseOptions(words);
Map<String, ValueResult> seenValues = convertValues(values, methodParameter, converter);
switch (seenValues.size()) {
case 0:
if (!cliOption.mandatory()) {
String value = cliOption.unspecifiedDefaultValue();
return converter
Object resolvedValue = converter
.orElseThrow(noConverterFound(cliOption.key()[0], value, methodParameter.getParameterType()))
.convertFromText(value, methodParameter.getParameterType(), cliOption.optionContext());
return new ValueResult(methodParameter, resolvedValue);
}
else {
throw new IllegalArgumentException("Could not find parameter values for " + prettifyKeys(Arrays.asList(cliOption.key())) + " in " + words);
@@ -117,38 +121,49 @@ public class LegacyParameterResolver implements ParameterResolver {
return null;
}
private Map<String, String> parseOptions(List<String> words) {
Map<String, String> values = new HashMap<>();
private Map<String, ParseResult> parseOptions(List<String> words) {
Map<String, ParseResult> values = new HashMap<>();
for (int i = 0; i < words.size(); i++) {
int from = i;
String word = words.get(i);
if (word.startsWith("--")) {
String key = word.substring("--".length());
if (word.startsWith(CLI_PREFIX)) {
String key = word.substring(CLI_PREFIX.length());
// If next word doesn't exist or starts with '--', this is an unary option. Store null
String value = i < words.size() - 1 && !words.get(i + 1).startsWith("--") ? words.get(++i) : null;
Assert.isTrue(!values.containsKey(key), String.format("Option --%s has already been set", key));
values.put(key, value);
String value = i < words.size() - 1 && !words.get(i + 1).startsWith(CLI_PREFIX) ? words.get(++i) : null;
Assert.isTrue(!values.containsKey(key), String.format("Option %s%s has already been set", CLI_PREFIX, key));
values.put(key, new ParseResult(value, from));
} // Must be the 'anonymous' option
else {
Assert.isTrue(!values.containsKey(""), "Anonymous option has already been set");
values.put("", word);
values.put("", new ParseResult(word, from));
}
}
return values;
}
private Map<String, Object> convertValues(Map<String, String> values, MethodParameter methodParameter, Optional<Converter<?>> converter) {
Map<String, Object> seenValues = new HashMap<>();
private Map<String, ValueResult> convertValues(Map<String, ParseResult> values, MethodParameter methodParameter, Optional<Converter<?>> converter) {
Map<String, ValueResult> seenValues = new HashMap<>();
CliOption option = methodParameter.getParameterAnnotation(CliOption.class);
for (String key : option.key()) {
if (values.containsKey(key)) {
String value = values.get(key);
ParseResult parseResult = values.get(key);
String value = parseResult.value;
if (value == null && !CLI_OPTION_NULL.equals(option.specifiedDefaultValue())) {
value = option.specifiedDefaultValue();
}
Class<?> parameterType = methodParameter.getParameterType();
seenValues.put(key, converter
Object resolvedValue = converter
.orElseThrow(noConverterFound(key, value, parameterType))
.convertFromText(value, parameterType, option.optionContext()));
.convertFromText(value, parameterType, option.optionContext());
int from = parseResult.from;
int to = key.isEmpty() || parseResult.value == null ? from : from + 1;
BitSet wordsUsed = new BitSet();
wordsUsed.set(from, to + 1);
BitSet wordsUsedForValues = new BitSet();
if (parseResult.value != null) {
wordsUsedForValues.set(to);
}
seenValues.put(key, new ValueResult(methodParameter, resolvedValue, wordsUsed, wordsUsedForValues));
}
}
return seenValues;
@@ -158,11 +173,23 @@ public class LegacyParameterResolver implements ParameterResolver {
* Return the list of possible keys for an option, suitable for displaying in an error message.
*/
private String prettifyKeys(Collection<String> keys) {
return keys.stream().map(s -> "".equals(s) ? "<anonymous>" : "--" + s).collect(Collectors.joining(", ", "[", "]"));
return keys.stream().map(s -> "".equals(s) ? "<anonymous>" : CLI_PREFIX + s).collect(Collectors.joining(", ", "[", "]"));
}
private Supplier<IllegalStateException> noConverterFound(String key, String value, Class<?> parameterType) {
return () -> new IllegalStateException("No converter found for --" + key + " from '" + value + "' to type " + parameterType);
return () -> new IllegalStateException("No converter found for " + CLI_PREFIX + key + " from '" + value + "' to type " + parameterType);
}
private static class ParseResult {
private final String value;
private final Integer from;
public ParseResult(String value, Integer from) {
this.value = value;
this.from = from;
}
}
}

156
spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyParameterResolverTest.java
View File

@@ -18,13 +18,13 @@ package org.springframework.shell2.legacy;
import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.shell2.ValueResultAsserts.assertThat;
import static org.springframework.shell2.legacy.LegacyCommands.REGISTER_METHOD;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -37,6 +37,7 @@ import org.springframework.shell.core.annotation.CliOption;
import org.springframework.shell2.ParameterDescription;
import org.springframework.shell2.ParameterResolver;
import org.springframework.shell2.Utils;
import org.springframework.shell2.ValueResult;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -68,47 +69,46 @@ public class LegacyParameterResolverTest {
public void resolvesParameterAnnotatedWithCliOption() throws Exception {
MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, NAME_OR_ANONYMOUS);
Object result = resolve(methodParameter, "--foo bar --name baz --qix bux");
assertThat(result).isEqualTo("baz");
assertThat(resolve(methodParameter, "--foo bar --name baz --qix bux")).hasValue("baz").usesWords(2, 3)
.usesWordsForValue(3);
}
@Test
public void resolvesAnonymousParameterAnnotatedWithCliOption() throws Exception {
MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, NAME_OR_ANONYMOUS);
Object result = resolve(methodParameter, "--foo bar baz --qix bux");
assertThat(result).isEqualTo("baz");
assertThat(resolve(methodParameter, "--foo bar baz --qix bux")).hasValue("baz").usesWords(2)
.usesWordsForValue(2);
// As first param
result = resolve(methodParameter, "baz --foo bar --qix bux");
assertThat(result).isEqualTo("baz");
assertThat(resolve(methodParameter, "baz --foo bar --qix bux")).hasValue("baz").usesWords(0)
.usesWordsForValue(0);
}
@Test
public void usesLegacyConverters() throws Exception {
MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, TYPE);
Object result = resolve(methodParameter, "--foo bar --name baz --qix bux --type processor");
assertThat(result).isSameAs(ArtifactType.processor);
assertThat(resolve(methodParameter, "--foo bar --name baz --qix bux --type processor"))
.hasValue(ArtifactType.processor).usesWords(6, 7).usesWordsForValue(7);
}
@Test
public void testUnspecifiedDefaultValue() throws Exception {
MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, FORCE);
Object result = resolve(methodParameter, "--foo bar --name baz --qix bux");
assertThat(result).isEqualTo(false);
assertThat(resolve(methodParameter, "--foo bar --name baz --qix bux"))
.hasValue(false).notUsesWords().notUsesWordsForValue();
}
@Test
public void testSpecifiedDefaultValue() throws Exception {
MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, FORCE);
assertThat(resolve(methodParameter, "--force --foo bar --name baz --qix bux")).isEqualTo(true);
assertThat(resolve(methodParameter, "--foo bar --name baz --qix bux --force")).isEqualTo(true);
assertThat(resolve(methodParameter, "--force --foo bar --name baz --qix bux"))
.hasValue(true).usesWords(0).notUsesWordsForValue();
assertThat(resolve(methodParameter, "--foo bar --name baz --qix bux --force"))
.hasValue(true).usesWords(6).notUsesWordsForValue();
}
@Test
@@ -116,7 +116,8 @@ public class LegacyParameterResolverTest {
MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, COORDINATES);
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Could not find parameter values for [--coordinates, --coords] in [--force, --foo, bar, --name, baz, --qix, bux]");
thrown.expectMessage(
"Could not find parameter values for [--coordinates, --coords] in [--force, --foo, bar, --name, baz, --qix, bux]");
resolve(methodParameter, "--force --foo bar --name baz --qix bux");
}
@@ -155,101 +156,102 @@ public class LegacyParameterResolverTest {
thrown.expectMessage("No converter found for --v2 from '42' to type int");
resolve(methodParameter, "--v1 1 --v2");
}
@Test
public void testDescribeBothDefaultsNotDeclared() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.REGISTER_METHOD, 1);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--type");
assertThat(description.formal()).isEqualTo(Utils.unCamelify(ArtifactType.class.getSimpleName()));
assertThat(description.defaultValue().isPresent()).isFalse();
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeBothDefaultsDeclared() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 1);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--option");
assertThat(description.formal()).isEqualTo(boolean.class.getName());
assertThat(description.defaultValue().get()).isEqualTo("false");
assertThat(description.defaultValueWhenFlag().get()).isEqualTo("true");
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeOnlySpecifiedDefaultDeclared() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 1);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--v2");
assertThat(description.formal()).isEqualTo(int.class.getName());
assertThat(description.defaultValue().get()).isEqualTo("null");
assertThat(description.defaultValueWhenFlag().get()).isEqualTo("42");
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeOnlyUnspecifiedDefaultDeclared() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 0);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--v1");
assertThat(description.formal()).isEqualTo(int.class.getName());
assertThat(description.defaultValue().get()).isEqualTo("38");
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeDefaultKey() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.LEGACY_ECHO_METHOD, 0);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).isEmpty();
assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName()));
assertThat(description.defaultValue().isPresent()).isFalse();
assertThat(description.mandatoryKey()).isFalse();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeNonMandatoryNoDefaults() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 0);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--key");
assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName()));
assertThat(description.defaultValue().get()).isEqualTo("null");
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
private Object resolve(MethodParameter methodParameter, String command) {
return parameterResolver.resolve(methodParameter, asList(command.split(" ")));
@Test
public void testDescribeOnlyUnspecifiedDefaultDeclared() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 0);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--v1");
assertThat(description.formal()).isEqualTo(int.class.getName());
assertThat(description.defaultValue().get()).isEqualTo("38");
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeDefaultKey() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.LEGACY_ECHO_METHOD, 0);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).isEmpty();
assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName()));
assertThat(description.defaultValue().isPresent()).isFalse();
assertThat(description.mandatoryKey()).isFalse();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
@Test
public void testDescribeNonMandatoryNoDefaults() {
MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 0);
ParameterDescription description = parameterResolver.describe(methodParameter).findFirst().get();
assertThat(description.keys()).containsExactly("--key");
assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName()));
assertThat(description.defaultValue().get()).isEqualTo("null");
assertThat(description.mandatoryKey()).isTrue();
String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
assertThat(description.help()).isEqualTo(expectedHelp);
}
private ValueResult resolve(MethodParameter methodParameter, String command) {
ValueResult result = parameterResolver.resolve(methodParameter, asList(command.split(" ")));
return result;
}
@Configuration