From 32f77c1917dc44be6a7b04dddf217e480e1aafcc Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Wed, 6 Jul 2022 09:02:06 +0100 Subject: [PATCH] Rework completion interfaces - Use same interface type in a generic interactive completions in a method level and option value level. - Change CompletionResolver to have same function signature as with options and use CompletionContext to keep relevant information. - Fixes #449 --- .../ParameterResolverAutoConfiguration.java | 19 +++++++++++++++++-- .../shell/CompletionContext.java | 10 ++++++++++ .../java/org/springframework/shell/Shell.java | 5 +++-- .../shell/command/CommandOption.java | 16 ++++++---------- .../shell/command/CommandRegistration.java | 11 +++++------ .../shell/completion/CompletionResolver.java | 14 +++----------- ...egistrationOptionsCompletionResolver.java} | 13 ++++++++----- .../org/springframework/shell/ShellTests.java | 9 ++------- .../StandardMethodTargetRegistrar.java | 7 +++---- 9 files changed, 57 insertions(+), 47 deletions(-) rename spring-shell-core/src/main/java/org/springframework/shell/completion/{DefaultCompletionResolver.java => RegistrationOptionsCompletionResolver.java} (77%) diff --git a/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/ParameterResolverAutoConfiguration.java b/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/ParameterResolverAutoConfiguration.java index a8b47709..01a1eb90 100644 --- a/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/ParameterResolverAutoConfiguration.java +++ b/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/ParameterResolverAutoConfiguration.java @@ -1,3 +1,18 @@ +/* + * 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.boot; import java.util.ArrayList; @@ -11,7 +26,7 @@ import org.springframework.shell.command.ArgumentHeaderMethodArgumentResolver; import org.springframework.shell.command.CommandContextMethodArgumentResolver; import org.springframework.shell.command.CommandExecution.CommandExecutionHandlerMethodArgumentResolvers; import org.springframework.shell.completion.CompletionResolver; -import org.springframework.shell.completion.DefaultCompletionResolver; +import org.springframework.shell.completion.RegistrationOptionsCompletionResolver; import org.springframework.shell.config.ShellConversionServiceSupplier; import org.springframework.shell.standard.ShellOptionMethodArgumentResolver; @@ -20,7 +35,7 @@ public class ParameterResolverAutoConfiguration { @Bean public CompletionResolver defaultCompletionResolver() { - return new DefaultCompletionResolver(); + return new RegistrationOptionsCompletionResolver(); } @Bean diff --git a/spring-shell-core/src/main/java/org/springframework/shell/CompletionContext.java b/spring-shell-core/src/main/java/org/springframework/shell/CompletionContext.java index 1906db32..5a42c123 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/CompletionContext.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/CompletionContext.java @@ -105,7 +105,17 @@ public class CompletionContext { position, commandRegistration, commandOption); } + /** + * Return a copy of this context with given command option. + */ public CompletionContext commandOption(CommandOption commandOption) { return new CompletionContext(words, wordIndex, position, commandRegistration, commandOption); } + + /** + * Return a copy of this context with given command registration. + */ + public CompletionContext commandRegistration(CommandRegistration commandRegistration) { + return new CompletionContext(words, wordIndex, position, commandRegistration, commandOption); + } } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/Shell.java b/spring-shell-core/src/main/java/org/springframework/shell/Shell.java index eda63b82..22769824 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/Shell.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/Shell.java @@ -277,11 +277,12 @@ public class Shell { String best = findLongestCommand(prefix); if (best != null) { - CompletionContext argsContext = context.drop(best.split(" ").length); + context = context.drop(best.split(" ").length); CommandRegistration registration = commandRegistry.getRegistrations().get(best); + CompletionContext argsContext = context.commandRegistration(registration); for (CompletionResolver resolver : completionResolvers) { - List resolved = resolver.resolve(registration, argsContext); + List resolved = resolver.apply(argsContext); candidates.addAll(resolved); } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandOption.java b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandOption.java index df240f4b..c9773389 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandOption.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandOption.java @@ -15,12 +15,8 @@ */ package org.springframework.shell.command; -import java.util.List; -import java.util.function.Function; - import org.springframework.core.ResolvableType; -import org.springframework.shell.CompletionContext; -import org.springframework.shell.CompletionProposal; +import org.springframework.shell.completion.CompletionResolver; /** * Interface representing an option in a command. @@ -104,7 +100,7 @@ public interface CommandOption { * * @return the completion function */ - Function> getCompletion(); + CompletionResolver getCompletion(); /** * Gets an instance of a default {@link CommandOption}. @@ -150,7 +146,7 @@ public interface CommandOption { */ public static CommandOption of(String[] longNames, Character[] shortNames, String description, ResolvableType type, boolean required, String defaultValue, Integer position, Integer arityMin, - Integer arityMax, String label, Function> completion) { + Integer arityMax, String label, CompletionResolver completion) { return new DefaultCommandOption(longNames, shortNames, description, type, required, defaultValue, position, arityMin, arityMax, label, completion); } @@ -170,12 +166,12 @@ public interface CommandOption { private int arityMin; private int arityMax; private String label; - private Function> completion; + private CompletionResolver completion; public DefaultCommandOption(String[] longNames, Character[] shortNames, String description, ResolvableType type, boolean required, String defaultValue, Integer position, Integer arityMin, Integer arityMax, String label, - Function> completion) { + CompletionResolver completion) { this.longNames = longNames != null ? longNames : new String[0]; this.shortNames = shortNames != null ? shortNames : new Character[0]; this.description = description; @@ -240,7 +236,7 @@ public interface CommandOption { } @Override - public Function> getCompletion() { + public CompletionResolver getCompletion() { return completion; } } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java index 099ffbd1..5660ad44 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java @@ -29,8 +29,7 @@ import java.util.stream.Stream; import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; import org.springframework.shell.Availability; -import org.springframework.shell.CompletionContext; -import org.springframework.shell.CompletionProposal; +import org.springframework.shell.completion.CompletionResolver; import org.springframework.shell.context.InteractionMode; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -216,7 +215,7 @@ public interface CommandRegistration { * @param completion the completion function * @return option spec for chaining */ - OptionSpec completion(Function> completion); + OptionSpec completion(CompletionResolver completion); /** * Return a builder for chaining. @@ -538,7 +537,7 @@ public interface CommandRegistration { private Integer arityMin; private Integer arityMax; private String label; - private Function> completion; + private CompletionResolver completion; DefaultOptionSpec(BaseBuilder builder) { this.builder = builder; @@ -638,7 +637,7 @@ public interface CommandRegistration { } @Override - public OptionSpec completion(Function> completion) { + public OptionSpec completion(CompletionResolver completion) { this.completion = completion; return this; } @@ -688,7 +687,7 @@ public interface CommandRegistration { return label; } - public Function> getCompletion() { + public CompletionResolver getCompletion() { return completion; } } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/completion/CompletionResolver.java b/spring-shell-core/src/main/java/org/springframework/shell/completion/CompletionResolver.java index 1ac4053a..6ddbec56 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/completion/CompletionResolver.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/completion/CompletionResolver.java @@ -16,24 +16,16 @@ package org.springframework.shell.completion; import java.util.List; +import java.util.function.Function; import org.springframework.shell.CompletionContext; import org.springframework.shell.CompletionProposal; -import org.springframework.shell.command.CommandRegistration; /** * Interface resolving completion proposals. * * @author Janne Valkealahti */ -public interface CompletionResolver { - - /** - * Resolve completions. - * - * @param registration the command registration - * @param context the completion context - * @return list of resolved completions - */ - List resolve(CommandRegistration registration, CompletionContext context); +@FunctionalInterface +public interface CompletionResolver extends Function> { } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/completion/DefaultCompletionResolver.java b/spring-shell-core/src/main/java/org/springframework/shell/completion/RegistrationOptionsCompletionResolver.java similarity index 77% rename from spring-shell-core/src/main/java/org/springframework/shell/completion/DefaultCompletionResolver.java rename to spring-shell-core/src/main/java/org/springframework/shell/completion/RegistrationOptionsCompletionResolver.java index 9e691a8c..b69b05b2 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/completion/DefaultCompletionResolver.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/completion/RegistrationOptionsCompletionResolver.java @@ -16,28 +16,31 @@ package org.springframework.shell.completion; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.stream.Stream; import org.springframework.shell.CompletionContext; import org.springframework.shell.CompletionProposal; -import org.springframework.shell.command.CommandRegistration; /** * Default implementation of a {@link CompletionResolver}. * * @author Janne Valkealahti */ -public class DefaultCompletionResolver implements CompletionResolver { +public class RegistrationOptionsCompletionResolver implements CompletionResolver { @Override - public List resolve(CommandRegistration registration, CompletionContext context) { + public List apply(CompletionContext context) { + if (context.getCommandRegistration() == null) { + return Collections.emptyList(); + } List candidates = new ArrayList<>(); - registration.getOptions().stream() + context.getCommandRegistration().getOptions().stream() .flatMap(o -> Stream.of(o.getLongNames())) .map(ln -> new CompletionProposal("--" + ln)) .forEach(candidates::add); - registration.getOptions().stream() + context.getCommandRegistration().getOptions().stream() .flatMap(o -> Stream.of(o.getShortNames())) .map(ln -> new CompletionProposal("-" + ln)) .forEach(candidates::add); diff --git a/spring-shell-core/src/test/java/org/springframework/shell/ShellTests.java b/spring-shell-core/src/test/java/org/springframework/shell/ShellTests.java index 1b40cf27..184e905c 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/ShellTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/ShellTests.java @@ -30,7 +30,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.shell.command.CommandCatalog; import org.springframework.shell.command.CommandRegistration; -import org.springframework.shell.completion.CompletionResolver; +import org.springframework.shell.completion.RegistrationOptionsCompletionResolver; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; @@ -56,9 +56,6 @@ public class ShellTests { @Mock CommandCatalog commandRegistry; - @Mock - private CompletionResolver completionResolver; - @InjectMocks private Shell shell; @@ -66,7 +63,7 @@ public class ShellTests { @BeforeEach public void setUp() { - shell.setCompletionResolvers(Arrays.asList(completionResolver)); + shell.setCompletionResolvers(Arrays.asList(new RegistrationOptionsCompletionResolver())); } @Test @@ -272,7 +269,6 @@ public class ShellTests { @Test public void completionArgWithMethod() throws Exception { - when(completionResolver.resolve(any(), any())).thenReturn(Arrays.asList(new CompletionProposal("--arg1"))); CommandRegistration registration1 = CommandRegistration.builder() .command("hello world") .withTarget() @@ -294,7 +290,6 @@ public class ShellTests { @Test public void completionArgWithFunction() throws Exception { - when(completionResolver.resolve(any(), any())).thenReturn(Arrays.asList(new CompletionProposal("--arg1"))); CommandRegistration registration1 = CommandRegistration.builder() .command("hello world") .withTarget() diff --git a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java index ff8f04f8..08599402 100644 --- a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java +++ b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java @@ -23,7 +23,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -37,7 +36,6 @@ import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.messaging.handler.invocation.InvocableHandlerMethod; import org.springframework.shell.Availability; -import org.springframework.shell.CompletionContext; import org.springframework.shell.CompletionProposal; import org.springframework.shell.MethodTargetRegistrar; import org.springframework.shell.Utils; @@ -45,6 +43,7 @@ import org.springframework.shell.command.CommandCatalog; import org.springframework.shell.command.CommandRegistration; import org.springframework.shell.command.CommandRegistration.Builder; import org.springframework.shell.command.CommandRegistration.OptionSpec; +import org.springframework.shell.completion.CompletionResolver; import org.springframework.shell.standard.ShellOption.NoValueProvider; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -149,12 +148,12 @@ public class StandardMethodTargetRegistrar implements MethodTargetRegistrar, App optionSpec.required(); } if (!ClassUtils.isAssignable(NoValueProvider.class, so.valueProvider())) { - Function> completionFunction = ctx -> { + CompletionResolver completionResolver = ctx -> { ValueProvider valueProviderBean = this.applicationContext.getBean(so.valueProvider()); List complete = valueProviderBean.complete(ctx); return complete; }; - optionSpec.completion(completionFunction); + optionSpec.completion(completionResolver); } } }