Spring/spring-shell
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Replace parser string joining with list

Browse Source 
- Issue in #622 is that its command type is `String` and internally
  some incoming arguments(it's List) were converted to String by
  joining with space. This caused one case with help command to get
  conversion via spring's ConversionService(CollectionToStringConverter)
  which joins by commas. That was we saw in failed example.
- Remove needed joins in CommandParser and let it just pass List which
  then works better with ConversionService.
- This then needs a `command` option type change from String to String[]
  which it really is as you should be able to give whole command as
  an argument.
- Backport #622
- Fixes #623
This commit is contained in:
Janne Valkealahti
2023-01-16 17:47:21 +00:00
parent 7c91782841
commit b853112e0b
4 changed files with 29 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java
View File

@@ -261,7 +261,7 @@ public interface CommandParser {
}
if (pop != null && pop.option == null) {
if (!pop.args.isEmpty()) {
oargs.add(pop.args.stream().collect(Collectors.joining(" ")));
oargs.addAll(pop.args);
}
}
}
@@ -370,11 +370,7 @@ public interface CommandParser {
.filter(co -> co.getDefaultValue() != null)
.forEach(co -> {
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());
}
}
value = convertOptionType(co, value);
results.add(ParserResult.of(co, Collections.emptyList(), value, null));
});
return ParserResults.of(results);
@@ -478,7 +474,7 @@ public interface CommandParser {
else {
if (arityMax > 0) {
int limit = Math.min(arguments.size(), arityMax);
value = arguments.stream().limit(limit).collect(Collectors.joining(" "));
value = arguments.stream().limit(limit).collect(Collectors.toList());
unmapped.addAll(arguments.subList(limit, arguments.size()));
}
else {

18
spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java
View File

@@ -94,9 +94,9 @@ public class CommandParserTests extends AbstractCommandTests {
CommandParserResults results = parser.parse(options, args);
assertThat(results.results()).hasSize(2);
assertThat(results.results().get(0).option()).isSameAs(option1);
assertThat(results.results().get(0).value()).isEqualTo("foo1 foo2");
assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("foo1", "foo2"));
assertThat(results.results().get(1).option()).isSameAs(option2);
assertThat(results.results().get(1).value()).isEqualTo("bar1 bar2");
assertThat(results.results().get(1).value()).isEqualTo(Arrays.asList("bar1", "bar2"));
assertThat(results.positional()).isEmpty();
}
@@ -215,6 +215,18 @@ public class CommandParserTests extends AbstractCommandTests {
assertThat(results.positional()).containsExactly("value", "foo");
}
@Test
public void testMappedFromArgToString() {
CommandOption option1 = longOption("arg1", ResolvableType.forType(String.class), false, 0, 1, 2);
List<CommandOption> options = Arrays.asList(option1);
String[] args = new String[]{"--arg1", "value", "foo"};
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("value,foo");
assertThat(results.positional()).isEmpty();
}
@Test
public void testShortOptionsCombined() {
CommandOption optionA = shortOption('a');
@@ -381,7 +393,7 @@ public class CommandParserTests extends AbstractCommandTests {
assertThat(results.results()).hasSize(2);
assertThat(results.results().get(0).option()).isSameAs(option1);
assertThat(results.results().get(1).option()).isSameAs(option2);
assertThat(results.results().get(0).value()).isEqualTo("1");
assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("1"));
// no type so we get raw list
assertThat(results.results().get(1).value()).isEqualTo(Arrays.asList("2"));
}