From 57f6cff7c652628260eae3596ac7f23fea44da7b Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Wed, 18 May 2022 08:00:49 +0100 Subject: [PATCH] Convert default value - Change parser to user conversion service to attempt conversion from a default value to an expected type. - Make more use of shellConversionService bean so that we use the one which can be customized. --- .../boot/SpringShellAutoConfiguration.java | 10 +++-- .../java/org/springframework/shell/Shell.java | 9 ++++- .../shell/command/CommandExecution.java | 22 ++++++----- .../shell/command/CommandParser.java | 37 ++++++++++++++----- .../invocation/InvocableShellMethod.java | 14 ++++++- .../shell/command/CommandExecutionTests.java | 6 ++- .../shell/command/CommandParserTests.java | 19 +++++++++- 7 files changed, 91 insertions(+), 26 deletions(-) diff --git a/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/SpringShellAutoConfiguration.java b/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/SpringShellAutoConfiguration.java index 11751bdd..de591e9a 100644 --- a/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/SpringShellAutoConfiguration.java +++ b/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/SpringShellAutoConfiguration.java @@ -20,6 +20,7 @@ import java.util.Set; import org.jline.terminal.Terminal; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.convert.ApplicationConversionService; import org.springframework.context.ApplicationContext; @@ -44,7 +45,7 @@ import org.springframework.shell.result.ResultHandlerConfig; public class SpringShellAutoConfiguration { @Bean - @ConditionalOnMissingBean(ConversionService.class) + @ConditionalOnMissingBean(value = ConversionService.class, name = { "shellConversionService" }) public ConversionService shellConversionService(ApplicationContext applicationContext) { FormattingConversionService service = new FormattingConversionService(); DefaultConversionService.addDefaultConverters(service); @@ -63,7 +64,10 @@ public class SpringShellAutoConfiguration { } @Bean - public Shell shell(ResultHandlerService resultHandlerService, CommandCatalog commandRegistry, Terminal terminal) { - return new Shell(resultHandlerService, commandRegistry, terminal); + public Shell shell(ResultHandlerService resultHandlerService, CommandCatalog commandRegistry, Terminal terminal, + @Qualifier("shellConversionService") ConversionService shellConversionService) { + Shell shell = new Shell(resultHandlerService, commandRegistry, terminal); + shell.setConversionService(shellConversionService); + return shell; } } 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 15e18cb0..244b5acc 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 @@ -33,6 +33,8 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.annotation.AnnotationAwareOrderComparator; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.shell.command.CommandCatalog; import org.springframework.shell.command.CommandExecution; import org.springframework.shell.command.CommandExecution.CommandExecutionException; @@ -61,6 +63,7 @@ public class Shell { private final CommandCatalog commandRegistry; protected List completionResolvers = new ArrayList<>(); private CommandExecutionHandlerMethodArgumentResolvers argumentResolvers; + private ConversionService conversionService = new DefaultConversionService(); /** * Marker object to distinguish unresolved arguments from {@code null}, which is a valid @@ -87,6 +90,10 @@ public class Shell { this.argumentResolvers = argumentResolvers; } + public void setConversionService(ConversionService shellConversionService) { + this.conversionService = shellConversionService; + } + @Autowired(required = false) public void setValidatorFactory(ValidatorFactory validatorFactory) { this.validator = validatorFactory.getValidator(); @@ -159,7 +166,7 @@ public class Shell { Object sh = Signals.register("INT", () -> commandThread.interrupt()); try { CommandExecution execution = CommandExecution - .of(argumentResolvers != null ? argumentResolvers.getResolvers() : null, validator, terminal); + .of(argumentResolvers != null ? argumentResolvers.getResolvers() : null, validator, terminal, conversionService); return execution.evaluate(commandRegistration.get(), wordsForArgs.toArray(new String[0])); } catch (UndeclaredThrowableException e) { diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandExecution.java b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandExecution.java index 5ddd1c44..25349ba2 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandExecution.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandExecution.java @@ -26,7 +26,6 @@ import org.jline.terminal.Terminal; import org.springframework.core.MethodParameter; import org.springframework.core.annotation.Order; import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.messaging.Message; import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver; import org.springframework.messaging.support.MessageBuilder; @@ -60,7 +59,7 @@ public interface CommandExecution { * @return default command execution */ public static CommandExecution of(List resolvers) { - return new DefaultCommandExecution(resolvers, null, null); + return new DefaultCommandExecution(resolvers, null, null, null); } /** @@ -69,11 +68,12 @@ public interface CommandExecution { * @param resolvers the handler method argument resolvers * @param validator the validator * @param terminal the terminal + * @param conversionService the conversion services * @return default command execution */ public static CommandExecution of(List resolvers, Validator validator, - Terminal terminal) { - return new DefaultCommandExecution(resolvers, validator, terminal); + Terminal terminal, ConversionService conversionService) { + return new DefaultCommandExecution(resolvers, validator, terminal, conversionService); } /** @@ -84,18 +84,20 @@ public interface CommandExecution { private List resolvers; private Validator validator; private Terminal terminal; + private ConversionService conversionService; public DefaultCommandExecution(List resolvers, Validator validator, - Terminal terminal) { + Terminal terminal, ConversionService conversionService) { this.resolvers = resolvers; this.validator = validator; this.terminal = terminal; + this.conversionService = conversionService; } public Object evaluate(CommandRegistration registration, String[] args) { List options = registration.getOptions(); - CommandParser parser = CommandParser.of(); + CommandParser parser = CommandParser.of(conversionService); CommandParserResults results = parser.parse(options, args); if (!results.errors().isEmpty()) { @@ -135,13 +137,14 @@ public interface CommandExecution { messageBuilder.setHeader(CommandContextMethodArgumentResolver.HEADER_COMMAND_CONTEXT, ctx); InvocableShellMethod invocableShellMethod = new InvocableShellMethod(targetInfo.getBean(), targetInfo.getMethod()); + invocableShellMethod.setConversionService(conversionService); invocableShellMethod.setValidator(validator); ShellMethodArgumentResolverComposite argumentResolvers = new ShellMethodArgumentResolverComposite(); if (resolvers != null) { argumentResolvers.addResolvers(resolvers); } if (!paramValues.isEmpty()) { - argumentResolvers.addResolver(new ParamNameHandlerMethodArgumentResolver(paramValues)); + argumentResolvers.addResolver(new ParamNameHandlerMethodArgumentResolver(paramValues, conversionService)); } invocableShellMethod.setMessageMethodArgumentResolvers(argumentResolvers); @@ -160,10 +163,11 @@ public interface CommandExecution { static class ParamNameHandlerMethodArgumentResolver implements HandlerMethodArgumentResolver { private final Map paramValues = new HashMap<>(); - ConversionService conversionService = new DefaultConversionService(); + private final ConversionService conversionService; - ParamNameHandlerMethodArgumentResolver(Map paramValues) { + ParamNameHandlerMethodArgumentResolver(Map paramValues, ConversionService conversionService) { this.paramValues.putAll(paramValues); + this.conversionService = conversionService; } @Override diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java index 710207ea..a7ece482 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java @@ -26,6 +26,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.springframework.core.ResolvableType; +import org.springframework.core.convert.ConversionService; import org.springframework.shell.Utils; import org.springframework.util.StringUtils; @@ -127,7 +128,17 @@ public interface CommandParser { * @return instance of a default command parser */ static CommandParser of() { - return new DefaultCommandParser(); + return of(null); + } + + /** + * Gets an instance of a default command parser. + * + * @param conversionService the conversion service + * @return instance of a default command parser + */ + static CommandParser of(ConversionService conversionService) { + return new DefaultCommandParser(conversionService); } /** @@ -190,6 +201,12 @@ public interface CommandParser { */ static class DefaultCommandParser implements CommandParser { + private final ConversionService conversionService; + + DefaultCommandParser(ConversionService conversionService) { + this.conversionService = conversionService; + } + @Override public CommandParserResults parse(List options, String[] args) { List requiredOptions = options.stream() @@ -293,7 +310,7 @@ public interface CommandParser { * Parser works on a results from a lexer. It looks for given options * and builds parsing results. */ - private static class Parser { + private class Parser { ParserResults visit(List> lexerResults, List options) { List results = lexerResults.stream() .flatMap(lr -> { @@ -327,7 +344,13 @@ public interface CommandParser { defaultValueOptionsToCheck.stream() .filter(co -> co.getDefaultValue() != null) .forEach(co -> { - results.add(ParserResult.of(co, Collections.emptyList(), co.getDefaultValue(), null)); + Object value = co.getDefaultValue(); + if (conversionService != null && co.getType() != null) { + if (conversionService.canConvert(co.getDefaultValue().getClass(), co.getType().getRawClass())) { + value = conversionService.convert(co.getDefaultValue(), co.getType().getRawClass()); + } + } + results.add(ParserResult.of(co, Collections.emptyList(), value, null)); }); return ParserResults.of(results); } @@ -425,10 +448,10 @@ public interface CommandParser { } } - return ConvertArgumentsHolder.of(value, unmapped); + return new ConvertArgumentsHolder(value, unmapped); } - private static class ConvertArgumentsHolder { + private class ConvertArgumentsHolder { Object value; final List unmapped = new ArrayList<>(); @@ -438,10 +461,6 @@ public interface CommandParser { this.unmapped.addAll(unmapped); } } - - static ConvertArgumentsHolder of(Object value, List unmapped) { - return new ConvertArgumentsHolder(value, unmapped); - } } } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/invocation/InvocableShellMethod.java b/spring-shell-core/src/main/java/org/springframework/shell/command/invocation/InvocableShellMethod.java index 7c0aaca5..fad1096e 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/invocation/InvocableShellMethod.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/invocation/InvocableShellMethod.java @@ -93,6 +93,8 @@ public class InvocableShellMethod { private Validator validator; + private ConversionService conversionService = new DefaultConversionService(); + /** * Create an instance from a bean instance and a method. */ @@ -146,6 +148,17 @@ public class InvocableShellMethod { this.parameters = initMethodParameters(); } + /** + * Sets a conversion service + * + * @param conversionService the conversion service + */ + public void setConversionService(ConversionService conversionService) { + if (conversionService != null) { + this.conversionService = conversionService; + } + } + /** * Copy constructor for use in subclasses. */ @@ -226,7 +239,6 @@ public class InvocableShellMethod { *

The resulting array will be passed into {@link #doInvoke}. */ protected Object[] getMethodArgumentValues(Message message, Object... providedArgs) throws Exception { - ConversionService conversionService = new DefaultConversionService(); MethodParameter[] parameters = getMethodParameters(); if (ObjectUtils.isEmpty(parameters)) { return EMPTY_ARGS; diff --git a/spring-shell-core/src/test/java/org/springframework/shell/command/CommandExecutionTests.java b/spring-shell-core/src/test/java/org/springframework/shell/command/CommandExecutionTests.java index da43ecfc..c26d80c7 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/command/CommandExecutionTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/command/CommandExecutionTests.java @@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver; import org.springframework.shell.command.CommandExecution.CommandParserExceptionsException; @@ -37,10 +38,11 @@ public class CommandExecutionTests extends AbstractCommandTests { @BeforeEach public void setupCommandExecutionTests() { + ConversionService conversionService = new DefaultConversionService(); List resolvers = new ArrayList<>(); - resolvers.add(new ArgumentHeaderMethodArgumentResolver(new DefaultConversionService(), null)); + resolvers.add(new ArgumentHeaderMethodArgumentResolver(conversionService, null)); resolvers.add(new CommandContextMethodArgumentResolver()); - execution = CommandExecution.of(resolvers); + execution = CommandExecution.of(resolvers, null, null, conversionService); } @Test diff --git a/spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java b/spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java index a15685ec..f15afaac 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java @@ -23,6 +23,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.core.ResolvableType; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.shell.command.CommandParser.CommandParserResults; import static org.assertj.core.api.Assertions.assertThat; @@ -33,7 +35,8 @@ public class CommandParserTests extends AbstractCommandTests { @BeforeEach public void setupCommandParserTests() { - parser = CommandParser.of(); + ConversionService conversionService = new DefaultConversionService(); + parser = CommandParser.of(conversionService); } @Test @@ -320,6 +323,20 @@ public class CommandParserTests extends AbstractCommandTests { assertThat(results.results().get(1).value()).isEqualTo("2"); } + @Test + public void testBooleanWithDefault() { + ResolvableType type = ResolvableType.forType(boolean.class); + CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false, + "true", null, null, null); + + List options = Arrays.asList(option1); + String[] args = new String[]{}; + CommandParserResults results = parser.parse(options, args); + assertThat(results.results()).hasSize(1); + assertThat(results.results().get(0).option()).isSameAs(option1); + assertThat(results.results().get(0).value()).isEqualTo(true); + } + private static CommandOption longOption(String name) { return longOption(name, null); }