Order ParameterResolvers. Validate there is at least one per parameter

spring-shell-core/src/main/java/org/springframework/shell/ParameterResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 the original author or authors.
+ * Copyright 2015-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,10 +20,19 @@ import java.util.List;
 import java.util.stream.Stream;
 
 import org.springframework.core.MethodParameter;
+import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 
 /**
  * Implementations of this interface are responsible, once the command has been identified, of transforming the textual
  * input to an actual parameter object.
+ * 
+ * <p>
+ * An order can also be specified in case more than one {@link ParameterResolver} supports a {@link MethodParameter}.
+ * See {@link AnnotationAwareOrderComparator} for details..
+ * </p>
+ * 
+ * @author Eric Bottard
+ * @author Camilo Gonzalez
  */
 public interface ParameterResolver {
 

spring-shell-core/src/main/java/org/springframework/shell/ParameterResolverMissingException.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.shell;
+
+import org.springframework.core.MethodParameter;
+
+/**
+ * Thrown when no {@link ParameterResolver} is found for a parameter during a {@link ParameterResolver#resolve}
+ * operation.
+ * 
+ * @author Camilo Gonzalez
+ */
+public class ParameterResolverMissingException extends RuntimeException {
+	public ParameterResolverMissingException(MethodParameter parameter) {
+		super(String.format("No parameter resolver found for parameter with index %d (named '%s') of %s ", parameter.getParameterIndex(), parameter.getParameterName(), parameter.getMethod()));
+	}
+}

spring-shell-core/src/main/java/org/springframework/shell/Shell.java

@@ -18,7 +18,6 @@ package org.springframework.shell;
 
 import java.io.IOException;
 import java.lang.reflect.Method;
-import java.lang.reflect.Parameter;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -35,6 +34,7 @@ import javax.validation.executable.ExecutableValidator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
 import org.springframework.core.MethodParameter;
+import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.util.ReflectionUtils;
 
 /**
@@ -58,8 +58,7 @@ public class Shell implements CommandRegistry {
 
 	protected Map<String, MethodTarget> methodTargets = new HashMap<>();
 
-	@Autowired
-	protected List<ParameterResolver> parameterResolvers = new ArrayList<>();
+	protected List<ParameterResolver> parameterResolvers;
 
 	/**
 	 * Marker object to distinguish unresolved arguments from {@code null}, which is a valid value.
@@ -83,6 +82,14 @@ public class Shell implements CommandRegistry {
 			resolver.register(registry);
 		}
 		methodTargets = registry.listCommands();
+		methodTargets.values()
+				.forEach(this::validateParameters);
+	}
+	
+	@Autowired
+	public void setParameterResolvers(List<ParameterResolver> resolvers) {
+		this.parameterResolvers = new ArrayList<>(resolvers);
+		AnnotationAwareOrderComparator.sort(parameterResolvers);
 	}
 
 	/**
@@ -141,7 +148,7 @@ public class Shell implements CommandRegistry {
 		}
 		resultHandler.handleResult(result);
 	}
-
+	
 	/**
 	 * Return true if the parsed input ends up being empty (<em>e.g.</em> hitting ENTER on an empty line or blank space).
 	 *
@@ -185,10 +192,16 @@ public class Shell implements CommandRegistry {
 			// Try to complete arguments
 			MethodTarget methodTarget = methodTargets.get(best);
 			Method method = methodTarget.getMethod();
-			Arrays.stream(method.getParameters())
-				.map(Utils::createMethodParameter)
-				.flatMap(mp -> findResolver(mp).complete(mp, argsContext).stream())
-				.forEach(candidates::add);
+			
+			List<MethodParameter> parameters = Utils.createMethodParameters(method).collect(Collectors.toList());
+			for (ParameterResolver resolver : parameterResolvers) {
+				for (int index = 0; index < parameters.size(); index++) {
+					MethodParameter parameter = parameters.get(index);
+					if (resolver.supports(parameter)) {
+						resolver.complete(parameter, argsContext).stream().forEach(candidates::add);
+					}
+				}
+			}
 		}
 		return candidates;
 	}
@@ -234,24 +247,34 @@ public class Shell implements CommandRegistry {
 	 * resolved
 	 */
 	private Object[] resolveArgs(Method method, List<String> wordsForArgs) {
-		Parameter[] parameters = method.getParameters();
-		Object[] args = new Object[parameters.length];
+		List<MethodParameter> parameters = Utils.createMethodParameters(method).collect(Collectors.toList());
+		Object[] args = new Object[parameters.size()];
 		Arrays.fill(args, UNRESOLVED);
-		for (int i = 0; i < parameters.length; i++) {
-			MethodParameter methodParameter = Utils.createMethodParameter(method, i);
-			args[i] = findResolver(methodParameter).resolve(methodParameter, wordsForArgs).resolvedValue();
+		for (ParameterResolver resolver : parameterResolvers) {
+			for (int argIndex = 0; argIndex < args.length; argIndex++) {
+				MethodParameter parameter = parameters.get(argIndex);
+				if (args[argIndex] == UNRESOLVED && resolver.supports(parameter)) {
+					args[argIndex] = resolver.resolve(parameter, wordsForArgs).resolvedValue();
+				}
+			}
 		}
 		return args;
 	}
-
-	private ParameterResolver findResolver(MethodParameter parameter) {
-		return parameterResolvers.stream()
-			.filter(resolver -> resolver.supports(parameter))
-			.findFirst()
-			.orElseThrow(() -> new RuntimeException("resolver not found"));
+	
+	/**
+	 * Verifies that we have at least one {@link ParameterResolver} that supports each of
+	 * the {@link MethodParameter}s in the method.
+	 */
+	private void validateParameters(MethodTarget methodTarget) {
+		Utils.createMethodParameters(methodTarget.getMethod())
+				.forEach(parameter -> {
+					parameterResolvers.stream()
+							.filter(resolver -> resolver.supports(parameter))
+							.findFirst()
+							.orElseThrow(() -> new ParameterResolverMissingException(parameter));
+				});
 	}
-
-
+	
 	/**
 	 * Returns the longest command that can be matched as first word(s) in the given buffer.
 	 *

spring-shell-core/src/test/java/org/springframework/shell/ShellTest.java

@@ -24,14 +24,17 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
+import org.springframework.context.ApplicationContext;
 
 import static org.assertj.core.api.Assertions.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 /**
@@ -44,6 +47,9 @@ public class ShellTest {
 	@Rule
 	public MockitoRule mockitoRule = MockitoJUnit.rule();
 
+	@Rule
+	public ExpectedException thrown= ExpectedException.none();
+
 	@Mock
 	private InputProvider inputProvider;
 
@@ -152,6 +158,19 @@ public class ShellTest {
 
 	}
 
+	@Test
+	public void parametersSupported() throws Exception {
+		when(parameterResolver.supports(any())).thenReturn(false);
+		shell.applicationContext = mock(ApplicationContext.class);
+		when(shell.applicationContext.getBeansOfType(MethodTargetRegistrar.class))
+				.thenReturn(Collections.singletonMap("foo", r -> {
+					r.register("hw", MethodTarget.of("helloWorld", this, "hellow world"));
+				}));
+
+		thrown.expect(ParameterResolverMissingException.class);
+		shell.gatherMethodTargets();
+	}
+
 	private void helloWorld(String a) {
 		invoked = true;
 	}

spring-shell-jcommander-adapter/src/main/java/org/springframework/shell/jcommander/JCommanderParameterResolver.java

@@ -34,7 +34,6 @@ import org.springframework.shell.CompletionProposal;
 import org.springframework.shell.ParameterDescription;
 import org.springframework.shell.ParameterResolver;
 import org.springframework.shell.ValueResult;
-import org.springframework.stereotype.Component;
 import org.springframework.util.ReflectionUtils;
 
 import com.beust.jcommander.DynamicParameter;