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. - Backport #667 - Fixes #670
2023-02-18 14:20:41 +00:00
parent 8c7b70b21e
commit e469c148df
3 changed files with 259 additions and 2 deletions
--- 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
@@ -478,9 +478,16 @@ public interface CommandParser {
 				// if it looks like type is a collection just get as list
 				// as conversion will happen later. we just need to know
 				// if user has Set, List, Collection, etc without worrying
-				// about generics.
+				// about generics. handle explicit size 1 so that we're
+				// able to handle string to xxx converter as that what
+				// user expects.
 				else if (type != null && type.asCollection() != ResolvableType.NONE) {
-					value = arguments.stream().collect(Collectors.toList());
+					if (arguments.size() == 1) {
+						value = arguments.get(0);
+					}
+					else {
+						value = arguments.stream().collect(Collectors.toList());
+					}
 				}
 				else {
 					if (!arguments.isEmpty()) {
--- 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
@@ -0,0 +1,193 @@
+/*
+ * 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.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<MyPojo> arg1
+		) {
+			return "Hello " + arg1;
+		}
+
+		@ShellMethod(key = LEGACY_ANNO + "option-conversion-customarray", group = GROUP)
+		public String optionConversionCustomArrayAnnotation(
+			@ShellOption 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<MyPojo> 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<String, MyPojo> stringToMyPojoConverter() {
+			return new StringToMyPojoConverter();
+		}
+
+		@Bean
+		public Converter<String, Set<MyPojo>> 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<String, MyPojo> {
+
+		@Override
+		public MyPojo convert(String from) {
+			return new MyPojo(from);
+		}
+	}
+
+	static class StringToMyPojoSetConverter implements Converter<String, Set<MyPojo>> {
+
+		@Override
+		public Set<MyPojo> convert(String from) {
+			Set<MyPojo> set = new HashSet<>();
+			set.add(new MyPojo(from));
+			return set;
+		}
+	}
+}
--- 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
@@ -0,0 +1,57 @@
+/*
+ * 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.samples.AbstractSampleTests;
+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 })
+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]]");
+	}
+}