Add new parameter resolver + positional params

This commit is contained in:
Eric Bottard
2015-12-16 16:00:30 +01:00
parent c8bfff226f
commit 3e516d3bc6
8 changed files with 305 additions and 19 deletions

View File

@@ -20,7 +20,7 @@ public class DefaultMethodTargetResolver implements MethodTargetResolver {
for (Object bean : commandBeans.values()) {
Class<?> clazz = bean.getClass();
ReflectionUtils.doWithMethods(clazz, method -> {
ShellMapping shellMapping = method.getAnnotation(ShellMapping.class);
ShellMehtod shellMapping = method.getAnnotation(ShellMehtod.class);
String[] keys = shellMapping.value();
if (keys.length == 1 && "".equals(keys[0])) {
keys[0] = method.getName();
@@ -28,7 +28,7 @@ public class DefaultMethodTargetResolver implements MethodTargetResolver {
for (String key : keys) {
methodTargets.put(key, new MethodTarget(method, bean, shellMapping.help()));
}
}, method -> method.getAnnotation(ShellMapping.class) != null);
}, method -> method.getAnnotation(ShellMehtod.class) != null);
}
return methodTargets;
}

View File

@@ -1,18 +1,57 @@
package org.springframework.shell2;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.util.Assert;
import org.springframework.util.ConcurrentReferenceHashMap;
/**
* Created by ericbottard on 09/12/15.
* Default ParameterResolver implementation that supports the following features:<ul>
* <li>named parameters (recognized because they start with some {@link ShellMehtod#prefix()}</li>
* <li>implicit named parameters (from the actual method parameter name)</li>
* <li>positional parameters (in order, for all parameter values that were not resolved via named parameters)</li>
* <li>default values (for all remaining parameters)</li>
* </ul>
*
* <p>Method arguments can consume several words of input at once (driven by {@link ShellOption#arity()}, default 1).
* If several words are consumed, they will be joined together as a comma separated value and passed to the {@link ConversionService}
* (which will typically return a List or array).</p>
*
* <p>Boolean parameters are by default expected to have an arity of 0, allowing invocations in the form {@code rm --force --dir /foo}:
* the presence of {@code --force} passes {@code true} as a parameter value, while its absence passes {@code false}. Both
* the default arity of 0 and the default value of {@code false} can be overridden <i>via</i> {@link ShellOption}
* if needed.</p>
*
* @author Eric Bottard
* @author Florent Biville
*/
class DefaultParameterResolver implements ParameterResolver {
private final ConversionService conversionService;
/**
* A cache from method+input to String representation of actual parameter values.
* Note that the converted result is not cached, to allow dynamic computation to happen at every invocation
* if needed (e.g. if a remote service is involved).
*/
private final Map<CacheKey, Map<Parameter, String>> parameterCache = new ConcurrentReferenceHashMap<>();
public DefaultParameterResolver(ConversionService conversionService) {
this.conversionService = conversionService;
}
@@ -24,8 +63,154 @@ class DefaultParameterResolver implements ParameterResolver {
@Override
public Object resolve(MethodParameter methodParameter, List<String> words) {
TypeDescriptor targetType = new TypeDescriptor(methodParameter);
Object converted = conversionService.convert(words.get(methodParameter.getParameterIndex()), TypeDescriptor.valueOf(String.class), targetType);
return converted;
String prefix = methodParameter.getMethod().getAnnotation(ShellMehtod.class).prefix();
CacheKey cacheKey = new CacheKey(methodParameter.getMethod(), words);
Map<Parameter, String> resolved = parameterCache.computeIfAbsent(cacheKey, (k) -> {
Map<Parameter, String> result = new HashMap<>();
Map<String, String> namedParameters = new HashMap<>();
List<String> positionalValues = new ArrayList<>();
// First, resolve all parameters passed by-name
for (int i = 0; i < words.size(); i++) {
String word = words.get(i);
if (word.startsWith(prefix)) {
String key = word.substring(prefix.length());
Parameter parameter = lookupParameterForKey(methodParameter.getMethod(), key);
int arity = getArity(parameter);
String raw = words.subList(i + 1, i + 1 + arity).stream().collect(Collectors.joining(","));
Assert.isTrue(!namedParameters.containsKey(key), String.format("Option '%s' has already been specified", word));
namedParameters.put(key, raw);
result.put(parameter, raw);
i += arity;
if (arity == 0) {
boolean defaultValue = booleanDefaultValue(parameter);
// Boolean parameter has been specified. Use the opposite of the default value
result.put(parameter, String.valueOf(!defaultValue));
}
} // store for later processing of positional params
else {
positionalValues.add(word);
}
}
// Now have a second pass over params and treat them as positional
int offset = 0;
Parameter[] parameters = methodParameter.getMethod().getParameters();
for (int i = 0, parametersLength = parameters.length; i < parametersLength; i++) {
Parameter parameter = parameters[i];
// Compute the intersection between possible keys for the param and what we've already seen for named params
Collection<String> keys = getKeysForParameter(methodParameter.getMethod(), i);
Collection<String> copy = new HashSet<>(keys);
copy.retainAll(namedParameters.keySet());
if (copy.isEmpty()) { // Was not set via a key (including aliases), must be positional
int arity = getArity(parameter);
if (offset < positionalValues.size() && (offset + arity) <= positionalValues.size()) {
String raw = positionalValues.subList(offset, offset + arity).stream().collect(Collectors.joining(","));
result.put(parameter, raw);
offset += arity;
} // No more input. Try defaultValues
else {
Optional<String> defaultValue = Optional.empty();
ShellOption option = parameter.getAnnotation(ShellOption.class);
if (option != null && !ShellOption.NULL.equals(option.defaultValue())) {
defaultValue = Optional.of(option.defaultValue());
}
String value = defaultValue.orElseThrow(() -> new RuntimeException(String.format("Ran out of input for " + keys)));
result.put(parameter, value);
}
}
else if (copy.size() > 1) {
throw new RuntimeException("Named parameter has been specified multiple times via: " + copy);
}
}
return result;
});
String s = resolved.get(methodParameter.getMethod().getParameters()[methodParameter.getParameterIndex()]);
return conversionService.convert(s, TypeDescriptor.valueOf(String.class), new TypeDescriptor(methodParameter));
}
private boolean booleanDefaultValue(Parameter parameter) {
ShellOption option = parameter.getAnnotation(ShellOption.class);
if (option != null && !ShellOption.NULL.equals(option.defaultValue())) {
return Boolean.parseBoolean(option.defaultValue());
}
return false;
}
/**
* Return the arity of a given parameter. The default arity is 1, except for
* booleans where arity is 0 (can be overridden back to 1 via an annotation)
*/
private int getArity(Parameter parameter) {
ShellOption option = parameter.getAnnotation(ShellOption.class);
int inferred = (parameter.getType() == boolean.class || parameter.getType() == Boolean.class) ? 0 : 1;
return option != null ? option.arity() : inferred;
}
/**
* Return the key(s) the i-th parameter of the command method, resolved either from the {@link ShellOption}
* annotation,
* or from the actual parameter name.
* @throws IllegalArgumentException if parameter names could not be extracted
*/
private Collection<String> getKeysForParameter(Method method, int index) {
Parameter parameter = method.getParameters()[index];
ShellOption option = parameter.getAnnotation(ShellOption.class);
if (option != null && option.value().length > 0) {
return Arrays.asList(option.value());
}
else {
MethodParameter methodParameter = new MethodParameter(method, index);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
String parameterName = methodParameter.getParameterName();
Assert.notNull(parameterName, String.format(
"Could not discover parameter name at index %d for %s, and option key(s) were not specified via %s annotation",
index, method, ShellOption.class.getSimpleName()));
return Collections.singleton(parameterName);
}
}
/**
* Return the method parameter that should be bound to the given key.
*/
private Parameter lookupParameterForKey(Method method, String key) {
Parameter[] parameters = method.getParameters();
for (int i = 0, parametersLength = parameters.length; i < parametersLength; i++) {
Parameter p = parameters[i];
if (getKeysForParameter(method, i).contains(key)) {
return p;
}
}
throw new IllegalStateException(String.format("Could not lookup parameter for '%s' in %s", key, method));
}
private static class CacheKey {
private final Method method;
private final List<String> words;
private CacheKey(Method method, List<String> words) {
this.method = method;
this.words = words;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
CacheKey cacheKey = (CacheKey) o;
return Objects.equals(method, cacheKey.method) &&
Objects.equals(words, cacheKey.words);
}
@Override
public int hashCode() {
return Objects.hash(method, words);
}
}
}

View File

@@ -27,6 +27,7 @@ import org.jline.utils.AttributedStyle;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.convert.ConversionService;
import org.springframework.stereotype.Component;
@@ -117,6 +118,7 @@ public class JLineShell {
Arrays.fill(rawArgs, unresolved);
for (int i = 0; i < parameters.length; i++) {
MethodParameter methodParameter = new MethodParameter(methodTarget.getMethod(), i);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
for (ParameterResolver resolver : parameterResolvers) {
if (resolver.supports(methodParameter)) {
rawArgs[i] = resolver.resolve(methodParameter, words.subList(wordsUsedForCommandKey, words.size()));

View File

@@ -10,10 +10,12 @@ import java.lang.annotation.Target;
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD})
public @interface ShellMapping {
public @interface ShellMehtod {
String[] value() default "";
String help() default "";
String prefix() default "--";
}

View File

@@ -14,5 +14,11 @@ import java.lang.annotation.Target;
@Target(ElementType.PARAMETER)
public @interface ShellOption {
String[] value() default "";
String NULL = "__NULL__";
String[] value() default {};
int arity() default 1;
String defaultValue() default NULL;
}

View File

@@ -0,0 +1,57 @@
package org.springframework.shell2;
import static java.util.Arrays.asList;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
import static org.springframework.util.ReflectionUtils.findMethod;
import java.lang.reflect.Method;
import org.junit.Test;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.convert.support.DefaultConversionService;
/**
* Unit tests for DefaultParameterResolver.
*
* @author Eric Bottard
* @author Florent Biville
*/
public class DefaultParameterResolverTest {
private DefaultParameterResolver resolver = new DefaultParameterResolver(new DefaultConversionService());
@Test
public void testParses() throws Exception {
Method method = findMethod(Remote.class, "zap", boolean.class, String.class, String.class, String.class);
assertThat(resolver.resolve(
makeMethodParameter(method, 0),
asList("--force --name --foo y".split(" "))
), is(true));
assertThat(resolver.resolve(
makeMethodParameter(method, 1),
asList("--force --name --foo y".split(" "))
), is("--foo"));
assertThat(resolver.resolve(
makeMethodParameter(method, 2),
asList("--force --name --foo y".split(" "))
), is("y"));
assertThat(resolver.resolve(
makeMethodParameter(method, 3),
asList("--force --name --foo y".split(" "))
), is("last"));
}
private MethodParameter makeMethodParameter(Method method, int parameterIndex) {
MethodParameter methodParameter = new MethodParameter(method, parameterIndex);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
return methodParameter;
}
}

View File

@@ -0,0 +1,25 @@
package org.springframework.shell2;
/**
* An example commands class.
*
* @author Eric Bottard
* @author Florent Biville
*/
public class Remote {
/**
* A command method that showcases<ul>
* <li>default handling for booleans (force)</li>
* <li>default parameter name discovery (name)</li>
* <li>default value supplying (foo and bar)</li>
* </ul>
*/
@ShellMehtod
public void zap(boolean force,
String name,
@ShellOption(defaultValue="defoolt") String foo,
@ShellOption(defaultValue = "last") String bar) {
}
}

View File

@@ -4,6 +4,8 @@ import static java.util.Arrays.asList;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
import java.lang.reflect.Method;
import org.hamcrest.Matchers;
import org.junit.Rule;
import org.junit.Test;
@@ -13,6 +15,7 @@ import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.shell.converters.BooleanConverter;
import org.springframework.shell.converters.EnumConverter;
@@ -39,16 +42,22 @@ public class LegacyParameterResolverTest {
@Test
public void supportsParameterAnnotatedWithCliOption() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, NAME_OR_ANONYMOUS);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, NAME_OR_ANONYMOUS);
boolean supports = parameterResolver.supports(methodParameter);
assertThat(supports, is(true));
}
private MethodParameter buildMethodParameter(Method method, int index) {
MethodParameter methodParameter = new MethodParameter(method, index);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
return methodParameter;
}
@Test
public void resolvesParameterAnnotatedWithCliOption() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, NAME_OR_ANONYMOUS);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, NAME_OR_ANONYMOUS);
Object result = parameterResolver.resolve(methodParameter, asList("--foo bar --name baz --qix bux".split(" ")));
@@ -57,7 +66,7 @@ public class LegacyParameterResolverTest {
@Test
public void resolvesAnonymousParameterAnnotatedWithCliOption() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, NAME_OR_ANONYMOUS);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, NAME_OR_ANONYMOUS);
Object result = parameterResolver.resolve(methodParameter, asList("--foo bar baz --qix bux".split(" ")));
assertThat(result, is("baz"));
@@ -69,7 +78,7 @@ public class LegacyParameterResolverTest {
@Test
public void usesLegacyConverters() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, TYPE);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, TYPE);
Object result = parameterResolver.resolve(methodParameter, asList("--foo bar --name baz --qix bux --type processor".split(" ")));
@@ -78,7 +87,7 @@ public class LegacyParameterResolverTest {
@Test
public void testUnspecifiedDefaultValue() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, FORCE);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, FORCE);
Object result = parameterResolver.resolve(methodParameter, asList("--foo bar --name baz --qix bux".split(" ")));
@@ -87,7 +96,7 @@ public class LegacyParameterResolverTest {
@Test
public void testSpecifiedDefaultValue() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, FORCE);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, FORCE);
assertThat(parameterResolver.resolve(methodParameter, asList("--force --foo bar --name baz --qix bux".split(" "))), is(true));
assertThat(parameterResolver.resolve(methodParameter, asList("--foo bar --name baz --qix bux --force".split(" "))), is(true));
@@ -95,7 +104,7 @@ public class LegacyParameterResolverTest {
@Test
public void testParameterNotFound() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, COORDINATES);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.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]");
@@ -104,7 +113,7 @@ public class LegacyParameterResolverTest {
@Test
public void testParameterFoundTooManyTimes() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.REGISTER_METHOD, COORDINATES);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.REGISTER_METHOD, COORDINATES);
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Option --coordinates has already been set");
@@ -116,7 +125,7 @@ public class LegacyParameterResolverTest {
@Test
public void testNoConverterFound() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.SUM_METHOD, 0);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.SUM_METHOD, 0);
thrown.expect(IllegalStateException.class);
thrown.expectMessage("No converter found for --v1 from '1' to type int");
@@ -127,7 +136,7 @@ public class LegacyParameterResolverTest {
@Test
public void testNoConverterFoundForUnspecifiedValue() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.SUM_METHOD, 0);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.SUM_METHOD, 0);
thrown.expect(IllegalStateException.class);
thrown.expectMessage("No converter found for --v1 from '38' to type int");
@@ -136,7 +145,7 @@ public class LegacyParameterResolverTest {
@Test
public void testNoConverterFoundForSpecifiedValue() throws Exception {
MethodParameter methodParameter = new MethodParameter(LegacyCommands.SUM_METHOD, 1);
MethodParameter methodParameter = buildMethodParameter(LegacyCommands.SUM_METHOD, 1);
thrown.expect(IllegalStateException.class);
thrown.expectMessage("No converter found for --v2 from '42' to type int");