Add dynamic command registration

- Extend CommandRegistry for add/remove methods.
- For rest of shell classes move to use registry
  directly instead of caching commands as registry
  is not immutable anymore.
- Add new sample
- Fixes #379

Author: jvalkeal

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 the original author or authors.
+ * Copyright 2015-2022 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.
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.springframework.shell;
 
 import java.util.Map;
@@ -23,13 +22,27 @@
  * discover available commands.
  *
  * @author Eric Bottard
+ * @author Janne Valkealahti
  */
 public interface CommandRegistry {
 
-
 	/**
 	 * Return the mapping from command trigger keywords to implementation.
 	 */
-	public Map<String, MethodTarget> listCommands();
+	Map<String, MethodTarget> listCommands();
 
+	/**
+	 * Register a new command.
+	 *
+	 * @param name the command name
+	 * @param target the method target
+	 */
+	void addCommand(String name, MethodTarget target);
+
+	/**
+	 * Deregister a command.
+	 *
+	 * @param name the command name
+	 */
+	void removeCommand(String name);
 }

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

@@ -58,6 +58,16 @@ else if (mim == InteractionMode.NONINTERACTIVE) {
 				.collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue()));
 	}
 
+	@Override
+	public void addCommand(String name, MethodTarget target) {
+		commands.put(name, target);
+	}
+
+	@Override
+	public void removeCommand(String name) {
+		commands.remove(name);
+	}
+
 	public void register(String name, MethodTarget target) {
 		MethodTarget previous = commands.get(name);
 		if (previous != null) {

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

@@ -68,6 +68,14 @@ public MethodTarget(Method method, Object bean, Help help, Supplier<Availability
 		this.interactionMode = interactionMode;
 	}
 
+	/**
+	 * Construct a MethodTarget for the unique method named {@literal name} on the given object. Fails with an exception
+	 * in case of overloaded method.
+	 */
+	public static MethodTarget of(String name, Object bean, String description, String group) {
+		return of(name, bean, new Help(description, group));
+	}
+
 	/**
 	 * Construct a MethodTarget for the unique method named {@literal name} on the given object. Fails with an exception
 	 * in case of overloaded method.

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

@@ -22,21 +22,18 @@
 import java.nio.channels.ClosedByInterruptException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import javax.annotation.PostConstruct;
 import javax.validation.ConstraintViolation;
 import javax.validation.Validator;
 import javax.validation.ValidatorFactory;
 
 import org.jline.utils.Signals;
 
 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;
@@ -66,15 +63,10 @@ public class Shell {
 	 */
 	public static final Object NO_INPUT = new Object();
 
-	@Autowired
-	protected ApplicationContext applicationContext;
-
-	private CommandRegistry commandRegistry;
+	private final CommandRegistry commandRegistry;
 
 	private Validator validator = Utils.defaultValidator();
 
-	protected Map<String, MethodTarget> methodTargets = new HashMap<>();
-
 	protected List<ParameterResolver> parameterResolvers;
 
 	/**
@@ -93,13 +85,6 @@ public void setValidatorFactory(ValidatorFactory validatorFactory) {
 		this.validator = validatorFactory.getValidator();
 	}
 
-	@PostConstruct
-	public void gatherMethodTargets() throws Exception {
-		methodTargets = commandRegistry.listCommands();
-		methodTargets.values()
-				.forEach(this::validateParameterResolvers);
-	}
-
 	@Autowired
 	public void setParameterResolvers(List<ParameterResolver> resolvers) {
 		this.parameterResolvers = new ArrayList<>(resolvers);
@@ -158,6 +143,7 @@ public Object evaluate(Input input) {
 
 		List<String> words = input.words();
 		if (command != null) {
+			Map<String, MethodTarget> methodTargets = commandRegistry.listCommands();
 			MethodTarget methodTarget = methodTargets.get(command);
 			Availability availability = methodTarget.getAvailability();
 			if (availability.isAvailable()) {
@@ -240,6 +226,7 @@ public List<CompletionProposal> complete(CompletionContext context) {
 		if (best != null) {
 			CompletionContext argsContext = context.drop(best.split(" ").length);
 			// Try to complete arguments
+			Map<String, MethodTarget> methodTargets = commandRegistry.listCommands();
 			MethodTarget methodTarget = methodTargets.get(best);
 			Method method = methodTarget.getMethod();
 
@@ -260,6 +247,7 @@ private List<CompletionProposal> commandsStartingWith(String prefix) {
 		// Workaround for https://github.com/spring-projects/spring-shell/issues/150
 		// (sadly, this ties this class to JLine somehow)
 		int lastWordStart = prefix.lastIndexOf(' ') + 1;
+		Map<String, MethodTarget> methodTargets = commandRegistry.listCommands();
 		return methodTargets.entrySet().stream()
 				.filter(e -> e.getKey().startsWith(prefix))
 				.map(e -> toCommandProposal(e.getKey().substring(lastWordStart), e.getValue()))
@@ -313,30 +301,16 @@ private Object[] resolveArgs(Method method, List<String> wordsForArgs) {
 		return args;
 	}
 
-	/**
-	 * Verifies that we have at least one {@link ParameterResolver} that supports each of the
-	 * {@link MethodParameter}s in the method.
-	 */
-	private void validateParameterResolvers(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.
 	 *
 	 * @return a valid command name, or {@literal null} if none matched
 	 */
 	private String findLongestCommand(String prefix) {
+		Map<String, MethodTarget> methodTargets = commandRegistry.listCommands();
 		String result = methodTargets.keySet().stream()
 				.filter(command -> prefix.equals(command) || prefix.startsWith(command + " "))
 				.reduce("", (c1, c2) -> c1.length() > c2.length() ? c1 : c2);
 		return "".equals(result) ? null : result;
 	}
-
 }

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

@@ -19,7 +19,9 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -29,15 +31,11 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
-import org.springframework.context.ApplicationContext;
-
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 /**
@@ -54,6 +52,9 @@ public class ShellTest {
 	@Mock
 	ResultHandlerService resultHandlerService;
 
+	@Mock
+	CommandRegistry commandRegistry;
+
 	@Mock
 	private ParameterResolver parameterResolver;
 
@@ -77,7 +78,8 @@ public void commandMatch() throws IOException {
 		when(parameterResolver.resolve(any(), any())).thenReturn(valueResult);
 		doThrow(new Exit()).when(resultHandlerService).handle(any());
 
-		shell.methodTargets = Collections.singletonMap("hello world", MethodTarget.of("helloWorld", this, new Command.Help("Say hello")));
+		when(commandRegistry.listCommands()).thenReturn(Collections.singletonMap("hello world",
+				MethodTarget.of("helloWorld", this, new Command.Help("Say hello"))));
 
 		try {
 			shell.run(inputProvider);
@@ -95,7 +97,8 @@ public void commandNotFound() throws IOException {
 		when(inputProvider.readInput()).thenReturn(() -> "hello world how are you doing ?");
 		doThrow(new Exit()).when(resultHandlerService).handle(isA(CommandNotFound.class));
 
-		shell.methodTargets = Collections.singletonMap("bonjour", MethodTarget.of("helloWorld", this, new Command.Help("Say hello")));
+		when(commandRegistry.listCommands()).thenReturn(Collections.singletonMap("bonjour",
+				MethodTarget.of("helloWorld", this, new Command.Help("Say hello"))));
 
 		try {
 			shell.run(inputProvider);
@@ -112,7 +115,8 @@ public void commandNotFoundPrefix() throws IOException {
 		when(inputProvider.readInput()).thenReturn(() -> "helloworld how are you doing ?");
 		doThrow(new Exit()).when(resultHandlerService).handle(isA(CommandNotFound.class));
 
-		shell.methodTargets = Collections.singletonMap("hello", MethodTarget.of("helloWorld", this, new Command.Help("Say hello")));
+		when(commandRegistry.listCommands()).thenReturn(
+				Collections.singletonMap("hello", MethodTarget.of("helloWorld", this, new Command.Help("Say hello"))));
 
 		try {
 			shell.run(inputProvider);
@@ -131,7 +135,8 @@ public void noCommand() throws IOException {
 		when(parameterResolver.resolve(any(), any())).thenReturn(valueResult);
 		doThrow(new Exit()).when(resultHandlerService).handle(any());
 
-		shell.methodTargets = Collections.singletonMap("hello world", MethodTarget.of("helloWorld", this, new Command.Help("Say hello")));
+		when(commandRegistry.listCommands()).thenReturn(Collections.singletonMap("hello world",
+				MethodTarget.of("helloWorld", this, new Command.Help("Say hello"))));
 
 		try {
 			shell.run(inputProvider);
@@ -149,7 +154,8 @@ public void commandThrowingAnException() throws IOException {
 		when(inputProvider.readInput()).thenReturn(() -> "fail");
 		doThrow(new Exit()).when(resultHandlerService).handle(isA(SomeException.class));
 
-		shell.methodTargets = Collections.singletonMap("fail", MethodTarget.of("failing", this, new Command.Help("Will throw an exception")));
+		when(commandRegistry.listCommands()).thenReturn(Collections.singletonMap("fail",
+				MethodTarget.of("failing", this, new Command.Help("Will throw an exception"))));
 
 		try {
 			shell.run(inputProvider);
@@ -169,36 +175,19 @@ public void comments() throws IOException {
 		shell.run(inputProvider);
 	}
 
-	// no need to test as we're moving away from postconstruct
-	// @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, new Command.Help("hellow world")));
-				}));
-
-		assertThatThrownBy(() -> {
-			shell.gatherMethodTargets();
-		}).isInstanceOf(ParameterResolverMissingException.class);
-	}
-
-	// @Test
+	@Test
 	public void commandNameCompletion() throws Exception {
-		shell.applicationContext = mock(ApplicationContext.class);
+		Map<String, MethodTarget> methodTargets = new HashMap<>();
+		methodTargets.put("hello world", MethodTarget.of("helloWorld", this, new Command.Help("hellow world")));
+		methodTargets.put("another command", MethodTarget.of("helloWorld", this, new Command.Help("another command")));
 		when(parameterResolver.supports(any())).thenReturn(true);
-		when(shell.applicationContext.getBeansOfType(MethodTargetRegistrar.class))
-				.thenReturn(Collections.singletonMap("foo", r -> {
-					r.register("hello world", MethodTarget.of("helloWorld", this, new Command.Help("hellow world")));
-					r.register("another command", MethodTarget.of("helloWorld", this, new Command.Help("another command")));
-				}));
-		shell.gatherMethodTargets();
+		when(commandRegistry.listCommands()).thenReturn(methodTargets);
 
 		// Invoke at very start
 		List<String> proposals = shell.complete(new CompletionContext(Arrays.asList(""), 0, "".length()))
 				.stream().map(CompletionProposal::value).collect(Collectors.toList());
-		assertThat(proposals).containsExactly("another command", "hello world");
+		assertThat(proposals).containsExactlyInAnyOrder("another command", "hello world");
+		// assertThat(proposals).containsExactly("another command", "hello world");
 
 		// Invoke in middle of first word
 		proposals = shell.complete(new CompletionContext(Arrays.asList("hel"), 0, "hel".length()))

spring-shell-samples/src/main/java/org/springframework/shell/samples/standard/RegisterCommands.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2022 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
+ *
+ *      https://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.samples.standard;
+
+import org.springframework.shell.MethodTarget;
+import org.springframework.shell.standard.AbstractShellComponent;
+import org.springframework.shell.standard.ShellComponent;
+import org.springframework.shell.standard.ShellMethod;
+import org.springframework.shell.standard.ShellOption;
+
+@ShellComponent
+public class RegisterCommands extends AbstractShellComponent {
+
+	private final PojoMethods pojoMethods = new PojoMethods();
+
+    @ShellMethod(key = "register add", value = "Register commands", group = "Register Commands")
+    public String register() {
+		MethodTarget target1 = MethodTarget.of("dynamic1", pojoMethods, "Dynamic1 command", "Register Commands");
+		MethodTarget target2 = MethodTarget.of("dynamic2", pojoMethods, "Dynamic2 command", "Register Commands");
+		MethodTarget target3 = MethodTarget.of("dynamic3", pojoMethods, "Dynamic3 command", "Register Commands");
+		getCommandRegistry().addCommand("register dynamic1", target1);
+		getCommandRegistry().addCommand("register dynamic2", target2);
+		getCommandRegistry().addCommand("register dynamic3", target3);
+		return "Registered commands dynamic1, dynamic2, dynamic3";
+    }
+
+    @ShellMethod(key = "register remove", value = "Deregister commands", group = "Register Commands")
+    public String deregister() {
+		getCommandRegistry().removeCommand("register dynamic1");
+		getCommandRegistry().removeCommand("register dynamic2");
+		getCommandRegistry().removeCommand("register dynamic3");
+		return "Deregistered commands dynamic1, dynamic2, dynamic3";
+    }
+
+	public static class PojoMethods {
+
+		@ShellMethod
+		public String dynamic1() {
+			return "dynamic1";
+		}
+
+		@ShellMethod
+		public String dynamic2(String arg1) {
+			return "dynamic2" + arg1;
+		}
+
+		@ShellMethod
+		public String dynamic3(@ShellOption(defaultValue = ShellOption.NULL) String arg1) {
+			return "dynamic3" + arg1;
+		}
+	}
+}