From 1bc26c7e172fe7192cb8160259a7c9f97e217bbd Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Thu, 19 Jan 2023 17:33:18 +0000 Subject: [PATCH] Handle collection types in a parser - Handle any option collection type so that list is generated for values, this then works well when actual type conversions happen. - Backport #630 - Fixes #631 --- .../shell/command/CommandParser.java | 9 ++ .../shell/command/CommandParserTests.java | 33 +++++++ .../shell/samples/e2e/OptionTypeCommands.java | 93 +++++++++++++++++++ 3 files changed, 135 insertions(+) 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 80962a8b..f1de6a5c 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 @@ -18,10 +18,12 @@ package org.springframework.shell.command; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.Deque; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -466,6 +468,13 @@ public interface CommandParser { else if (type != null && type.isArray()) { value = arguments.stream().collect(Collectors.toList()).toArray(); } + // 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. + else if (type != null && type.asCollection() != ResolvableType.NONE) { + value = arguments.stream().collect(Collectors.toList()); + } else { if (!arguments.isEmpty()) { if (arguments.size() == 1) { 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 a4100d77..01614a8b 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 @@ -310,6 +310,39 @@ public class CommandParserTests extends AbstractCommandTests { assertThat(results.results().get(0).value()).isEqualTo(new int[] { 1, 2 }); } + @Test + public void testLongOptionsWithStringArray() { + CommandOption option1 = longOption("arg1", ResolvableType.forType(String[].class)); + List options = Arrays.asList(option1); + String[] args = new String[]{"--arg1", "1", "2"}; + 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(new String[] { "1", "2" }); + } + + @Test + public void testLongOptionsWithPlainList() { + CommandOption option1 = longOption("arg1", ResolvableType.forType(List.class)); + List options = Arrays.asList(option1); + String[] args = new String[]{"--arg1", "1", "2"}; + 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(Arrays.asList("1", "2")); + } + + @Test + public void testLongOptionsWithTypedList() { + CommandOption option1 = longOption("arg1", ResolvableType.forClassWithGenerics(List.class, String.class)); + List options = Arrays.asList(option1); + String[] args = new String[]{"--arg1", "1", "2"}; + 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(Arrays.asList("1", "2")); + } + @Test public void testArityErrors() { CommandOption option1 = CommandOption.of( diff --git a/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionTypeCommands.java b/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionTypeCommands.java index 5a48e404..4cfb5013 100644 --- a/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionTypeCommands.java +++ b/spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionTypeCommands.java @@ -16,6 +16,9 @@ package org.springframework.shell.samples.e2e; import java.io.PrintWriter; +import java.util.Collection; +import java.util.List; +import java.util.Set; import org.springframework.context.annotation.Bean; import org.springframework.shell.command.CommandRegistration; @@ -289,6 +292,96 @@ public class OptionTypeCommands extends BaseE2ECommands { .build(); } + // + // List + // + + @ShellMethod(key = LEGACY_ANNO + "option-type-string-list", group = GROUP) + public String optionTypeStringListAnnotation( + @ShellOption(help = "Desc arg1") List arg1 + ) { + return "Hello " + arg1; + } + + @Bean + public CommandRegistration optionTypeStringListRegistration(CommandRegistration.BuilderSupplier builder) { + return builder.get() + .command(REG, "option-type-string-list") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(List.class) + .required() + .and() + .withTarget() + .function(ctx -> { + List arg1 = ctx.getOptionValue("arg1"); + return "Hello " + arg1; + }) + .and() + .build(); + } + + // + // Set + // + + @ShellMethod(key = LEGACY_ANNO + "option-type-string-set", group = GROUP) + public String optionTypeStringSetAnnotation( + @ShellOption(help = "Desc arg1") Set arg1 + ) { + return "Hello " + arg1; + } + + @Bean + public CommandRegistration optionTypeStringSetRegistration(CommandRegistration.BuilderSupplier builder) { + return builder.get() + .command(REG, "option-type-string-set") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(Set.class) + .required() + .and() + .withTarget() + .function(ctx -> { + Set arg1 = ctx.getOptionValue("arg1"); + return "Hello " + arg1; + }) + .and() + .build(); + } + + // + // Collection + // + + @ShellMethod(key = LEGACY_ANNO + "option-type-string-collection", group = GROUP) + public String optionTypeStringCollectionAnnotation( + @ShellOption(help = "Desc arg1") Collection arg1 + ) { + return "Hello " + arg1; + } + + @Bean + public CommandRegistration optionTypeStringCollectionRegistration(CommandRegistration.BuilderSupplier builder) { + return builder.get() + .command(REG, "option-type-string-collection") + .group(GROUP) + .withOption() + .longNames("arg1") + .type(Collection.class) + .required() + .and() + .withTarget() + .function(ctx -> { + Collection arg1 = ctx.getOptionValue("arg1"); + return "Hello " + arg1; + }) + .and() + .build(); + } + // // Void //