Add dynamic command registration #381
Conversation
- 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 spring-projects#379
| @@ -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); | |||
There was a problem hiding this comment.
Should we log warning when a command is overrwrritten? Not sure when this would happen, but it may be helpful to know when/if it does.
| /** | ||
| * Construct a MethodTarget for the unique method named {@literal name} on the given object. Fails with an exception | ||
| * in case of overloaded method. | ||
| */ |
There was a problem hiding this comment.
Do you mind adding doc for the params?
| 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; |
| @@ -158,6 +143,7 @@ public Object evaluate(Input input) { | |||
|
|
|||
| List<String> words = input.words(); | |||
| if (command != null) { | |||
| Map<String, MethodTarget> methodTargets = commandRegistry.listCommands(); | |||
There was a problem hiding this comment.
A slight improvement would be to move this into a methodTargets() method (used in 3 places) and it may also be an easy area of extension to allow getting commands in a slightly different fashion.
| @ShellComponent | ||
| public class RegisterCommands extends AbstractShellComponent { | ||
|
|
||
| private final PojoMethods pojoMethods = new PojoMethods(); |
| import org.springframework.shell.standard.ShellOption; | ||
|
|
||
| @ShellComponent | ||
| public class RegisterCommands extends AbstractShellComponent { |
There was a problem hiding this comment.
This does test the dynamic commands, but is not a test.
I was looking for an IT that does some coarse-grained testing that we could add coverage to. We do not have one - may be a good thing to add.
Do the current unit tests cover the new functionality?
|
@jvalkeal - awesome addition! I added a few comments. |
directly instead of caching commands as registry
is not immutable anymore.