From 3d3d37294f0873a7042f1ba5658619cd0ab39dcd Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sat, 18 Feb 2023 14:19:56 +0000 Subject: [PATCH] Use correct type with set - When target is set and only one option argument is given, we should not convert to list as user expects string to xxx Converter to work. - This is how it used to work and previous changes caused regression. - Bug is actually in an old parser and new parser works fine. - Fixes #667 --- .../samples/e2e/OptionConversionCommands.java | 231 ++++++++++++++++++ .../e2e/OptionConversionCommandsTests.java | 60 +++++ 2 files changed, 291 insertions(+) create mode 100644 spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionConversionCommands.java create mode 100644 spring-shell-samples/src/test/java/org/springframework/shell/samples/e2e/OptionConversionCommandsTests.java diff --git a/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionConversionCommands.java b/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionConversionCommands.java new file mode 100644 index 00000000..fc7868c0 --- /dev/null +++ b/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionConversionCommands.java @@ -0,0 +1,231 @@ +/* + * Copyright 2023 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.e2e; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.convert.converter.Converter; +import org.springframework.shell.command.CommandRegistration; +import org.springframework.shell.command.annotation.Command; +import org.springframework.shell.command.annotation.Option; +import org.springframework.shell.standard.ShellComponent; +import org.springframework.shell.standard.ShellMethod; +import org.springframework.shell.standard.ShellOption; +import org.springframework.stereotype.Component; + +public class OptionConversionCommands { + + @ShellComponent + public static class LegacyAnnotation extends BaseE2ECommands { + + @ShellMethod(key = LEGACY_ANNO + "option-conversion-integer", group = GROUP) + public String optionConversionIntegerAnnotation( + @ShellOption Integer arg1 + ) { + return "Hello " + arg1; + } + + @ShellMethod(key = LEGACY_ANNO + "option-conversion-custom", group = GROUP) + public String optionConversionCustomAnnotation( + @ShellOption MyPojo arg1 + ) { + return "Hello " + arg1; + } + + @ShellMethod(key = LEGACY_ANNO + "option-conversion-customset", group = GROUP) + public String optionConversionCustomSetAnnotation( + @ShellOption Set arg1 + ) { + return "Hello " + arg1; + } + + @ShellMethod(key = LEGACY_ANNO + "option-conversion-customarray", group = GROUP) + public String optionConversionCustomArrayAnnotation( + @ShellOption MyPojo[] arg1 + ) { + return "Hello " + Arrays.asList(arg1); + } + } + + @Command(command = BaseE2ECommands.ANNO, group = BaseE2ECommands.GROUP) + public static class Annotation extends BaseE2ECommands { + + @Command(command = "option-conversion-integer") + public String optionConversionIntegerAnnotation( + @Option(longNames = "arg1") + Integer arg1 + ) { + return "Hello " + arg1; + } + + @Command(command = "option-conversion-custom") + public String optionConversionCustomAnnotation( + @Option(longNames = "arg1") + MyPojo arg1 + ) { + return "Hello " + arg1; + } + + @Command(command = "option-conversion-customset") + public String optionConversionCustomSetAnnotation( + @Option(longNames = "arg1") + Set arg1 + ) { + return "Hello " + arg1; + } + + @Command(command = "option-conversion-customarray") + public String optionConversionCustomArrayAnnotation( + @Option(longNames = "arg1") + MyPojo[] arg1 + ) { + return "Hello " + Arrays.asList(arg1); + } + } + + @Component + public static class Registration extends BaseE2ECommands { + + @Bean + public CommandRegistration optionConversionIntegerRegistration() { + return getBuilder() + .command(REG, "option-conversion-integer") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(Integer.class) + .and() + .withTarget() + .function(ctx -> { + Integer arg1 = ctx.getOptionValue("arg1"); + return "Hello " + arg1; + }) + .and() + .build(); + } + + @Bean + public CommandRegistration optionConversionCustomRegistration() { + return getBuilder() + .command(REG, "option-conversion-custom") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(MyPojo.class) + .and() + .withTarget() + .function(ctx -> { + MyPojo arg1 = ctx.getOptionValue("arg1"); + return "Hello " + arg1; + }) + .and() + .build(); + } + + @Bean + public CommandRegistration optionConversionCustomSetRegistration() { + return getBuilder() + .command(REG, "option-conversion-customset") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(Set.class) + .and() + .withTarget() + .function(ctx -> { + Set arg1 = ctx.getOptionValue("arg1"); + return "Hello " + arg1; + }) + .and() + .build(); + } + + @Bean + public CommandRegistration optionConversionCustomArrayRegistration() { + return getBuilder() + .command(REG, "option-conversion-customarray") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(MyPojo[].class) + .and() + .withTarget() + .function(ctx -> { + MyPojo[] arg1 = ctx.getOptionValue("arg1"); + return "Hello " + Arrays.asList(arg1); + }) + .and() + .build(); + } + } + + @Configuration(proxyBeanMethods = false) + public static class CommonConfiguration { + + @Bean + public Converter stringToMyPojoConverter() { + return new StringToMyPojoConverter(); + } + + @Bean + public Converter> stringToMyPojoSetConverter() { + return new StringToMyPojoSetConverter(); + } + } + + public static class MyPojo { + private String arg; + + public MyPojo(String arg) { + this.arg = arg; + } + + public String getArg() { + return arg; + } + + public void setArg(String arg) { + this.arg = arg; + } + + @Override + public String toString() { + return "MyPojo [arg=" + arg + "]"; + } + } + + static class StringToMyPojoConverter implements Converter { + + @Override + public MyPojo convert(String from) { + return new MyPojo(from); + } + } + + static class StringToMyPojoSetConverter implements Converter> { + + @Override + public Set convert(String from) { + Set set = new HashSet<>(); + set.add(new MyPojo(from)); + return set; + } + } +} diff --git a/spring-shell-samples/src/test/java/org/springframework/shell/samples/e2e/OptionConversionCommandsTests.java b/spring-shell-samples/src/test/java/org/springframework/shell/samples/e2e/OptionConversionCommandsTests.java new file mode 100644 index 00000000..2fcdd828 --- /dev/null +++ b/spring-shell-samples/src/test/java/org/springframework/shell/samples/e2e/OptionConversionCommandsTests.java @@ -0,0 +1,60 @@ +/* + * Copyright 2023 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.e2e; + +import org.junit.jupiter.params.ParameterizedTest; + +import org.springframework.shell.command.annotation.EnableCommand; +import org.springframework.shell.samples.AbstractSampleTests; +import org.springframework.shell.samples.e2e.OptionConversionCommands.Annotation; +import org.springframework.shell.samples.e2e.OptionConversionCommands.CommonConfiguration; +import org.springframework.shell.samples.e2e.OptionConversionCommands.LegacyAnnotation; +import org.springframework.shell.samples.e2e.OptionConversionCommands.Registration; +import org.springframework.shell.test.ShellTestClient.BaseShellSession; +import org.springframework.test.context.ContextConfiguration; + +@ContextConfiguration(classes = { LegacyAnnotation.class, Registration.class, CommonConfiguration.class }) +@EnableCommand(Annotation.class) +class OptionConversionCommandsTests extends AbstractSampleTests { + + @ParameterizedTest + @E2ESource(command = "option-conversion-integer --arg1 1") + void optionConversionInteger(String command, boolean interactive) { + BaseShellSession session = createSession(command, interactive); + assertScreenContainsText(session, "Hello 1"); + } + + @ParameterizedTest + @E2ESource(command = "option-conversion-custom --arg1 hi") + void optionConversionCustom(String command, boolean interactive) { + BaseShellSession session = createSession(command, interactive); + assertScreenContainsText(session, "Hello MyPojo [arg=hi]"); + } + + @ParameterizedTest + @E2ESource(command = "option-conversion-customset --arg1 hi") + void optionConversionCustomSet(String command, boolean interactive) { + BaseShellSession session = createSession(command, interactive); + assertScreenContainsText(session, "Hello [MyPojo [arg=hi]]"); + } + + @ParameterizedTest + @E2ESource(command = "option-conversion-customarray --arg1 hi") + void optionConversionCustomArray(String command, boolean interactive) { + BaseShellSession session = createSession(command, interactive); + assertScreenContainsText(session, "Hello [MyPojo [arg=hi]]"); + } +}