Skip to content

Commit e469c14

Browse files
jvalkealjvalkeal
committed
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
1 parent 8c7b70b commit e469c14

File tree

3 files changed

+259
-2
lines changed

3 files changed

+259
-2
lines changed

‎spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -478,9 +478,16 @@ else if (type != null && type.isArray()) {
478478
// if it looks like type is a collection just get as list
479479
// as conversion will happen later. we just need to know
480480
// if user has Set, List, Collection, etc without worrying
481-
// about generics.
481+
// about generics. handle explicit size 1 so that we're
482+
// able to handle string to xxx converter as that what
483+
// user expects.
482484
else if (type != null && type.asCollection() != ResolvableType.NONE) {
483-
value = arguments.stream().collect(Collectors.toList());
485+
if (arguments.size() == 1) {
486+
value = arguments.get(0);
487+
}
488+
else {
489+
value = arguments.stream().collect(Collectors.toList());
490+
}
484491
}
485492
else {
486493
if (!arguments.isEmpty()) {

‎spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionConversionCommands.java

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.shell.samples.e2e;
17+
18+
import java.util.Arrays;
19+
import java.util.HashSet;
20+
import java.util.Set;
21+
22+
import org.springframework.context.annotation.Bean;
23+
import org.springframework.context.annotation.Configuration;
24+
import org.springframework.core.convert.converter.Converter;
25+
import org.springframework.shell.command.CommandRegistration;
26+
import org.springframework.shell.standard.ShellComponent;
27+
import org.springframework.shell.standard.ShellMethod;
28+
import org.springframework.shell.standard.ShellOption;
29+
import org.springframework.stereotype.Component;
30+
31+
public class OptionConversionCommands {
32+
33+
@ShellComponent
34+
public static class LegacyAnnotation extends BaseE2ECommands {
35+
36+
@ShellMethod(key = LEGACY_ANNO + "option-conversion-integer", group = GROUP)
37+
public String optionConversionIntegerAnnotation(
38+
@ShellOption Integer arg1
39+
) {
40+
return "Hello " + arg1;
41+
}
42+
43+
@ShellMethod(key = LEGACY_ANNO + "option-conversion-custom", group = GROUP)
44+
public String optionConversionCustomAnnotation(
45+
@ShellOption MyPojo arg1
46+
) {
47+
return "Hello " + arg1;
48+
}
49+
50+
@ShellMethod(key = LEGACY_ANNO + "option-conversion-customset", group = GROUP)
51+
public String optionConversionCustomSetAnnotation(
52+
@ShellOption Set<MyPojo> arg1
53+
) {
54+
return "Hello " + arg1;
55+
}
56+
57+
@ShellMethod(key = LEGACY_ANNO + "option-conversion-customarray", group = GROUP)
58+
public String optionConversionCustomArrayAnnotation(
59+
@ShellOption MyPojo[] arg1
60+
) {
61+
return "Hello " + Arrays.asList(arg1);
62+
}
63+
}
64+
65+
@Component
66+
public static class Registration extends BaseE2ECommands {
67+
68+
@Bean
69+
public CommandRegistration optionConversionIntegerRegistration() {
70+
return getBuilder()
71+
.command(REG, "option-conversion-integer")
72+
.group(GROUP)
73+
.withOption()
74+
.longNames("arg1")
75+
.type(Integer.class)
76+
.and()
77+
.withTarget()
78+
.function(ctx -> {
79+
Integer arg1 = ctx.getOptionValue("arg1");
80+
return "Hello " + arg1;
81+
})
82+
.and()
83+
.build();
84+
}
85+
86+
@Bean
87+
public CommandRegistration optionConversionCustomRegistration() {
88+
return getBuilder()
89+
.command(REG, "option-conversion-custom")
90+
.group(GROUP)
91+
.withOption()
92+
.longNames("arg1")
93+
.type(MyPojo.class)
94+
.and()
95+
.withTarget()
96+
.function(ctx -> {
97+
MyPojo arg1 = ctx.getOptionValue("arg1");
98+
return "Hello " + arg1;
99+
})
100+
.and()
101+
.build();
102+
}
103+
104+
@Bean
105+
public CommandRegistration optionConversionCustomSetRegistration() {
106+
return getBuilder()
107+
.command(REG, "option-conversion-customset")
108+
.group(GROUP)
109+
.withOption()
110+
.longNames("arg1")
111+
.type(Set.class)
112+
.and()
113+
.withTarget()
114+
.function(ctx -> {
115+
Set<MyPojo> arg1 = ctx.getOptionValue("arg1");
116+
return "Hello " + arg1;
117+
})
118+
.and()
119+
.build();
120+
}
121+
122+
@Bean
123+
public CommandRegistration optionConversionCustomArrayRegistration() {
124+
return getBuilder()
125+
.command(REG, "option-conversion-customarray")
126+
.group(GROUP)
127+
.withOption()
128+
.longNames("arg1")
129+
.type(MyPojo[].class)
130+
.and()
131+
.withTarget()
132+
.function(ctx -> {
133+
MyPojo[] arg1 = ctx.getOptionValue("arg1");
134+
return "Hello " + Arrays.asList(arg1);
135+
})
136+
.and()
137+
.build();
138+
}
139+
}
140+
141+
@Configuration(proxyBeanMethods = false)
142+
public static class CommonConfiguration {
143+
144+
@Bean
145+
public Converter<String, MyPojo> stringToMyPojoConverter() {
146+
return new StringToMyPojoConverter();
147+
}
148+
149+
@Bean
150+
public Converter<String, Set<MyPojo>> stringToMyPojoSetConverter() {
151+
return new StringToMyPojoSetConverter();
152+
}
153+
}
154+
155+
public static class MyPojo {
156+
private String arg;
157+
158+
public MyPojo(String arg) {
159+
this.arg = arg;
160+
}
161+
162+
public String getArg() {
163+
return arg;
164+
}
165+
166+
public void setArg(String arg) {
167+
this.arg = arg;
168+
}
169+
170+
@Override
171+
public String toString() {
172+
return "MyPojo [arg=" + arg + "]";
173+
}
174+
}
175+
176+
static class StringToMyPojoConverter implements Converter<String, MyPojo> {
177+
178+
@Override
179+
public MyPojo convert(String from) {
180+
return new MyPojo(from);
181+
}
182+
}
183+
184+
static class StringToMyPojoSetConverter implements Converter<String, Set<MyPojo>> {
185+
186+
@Override
187+
public Set<MyPojo> convert(String from) {
188+
Set<MyPojo> set = new HashSet<>();
189+
set.add(new MyPojo(from));
190+
return set;
191+
}
192+
}
193+
}

‎spring-shell-samples/src/test/java/org/springframework/shell/samples/e2e/OptionConversionCommandsTests.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.shell.samples.e2e;
17+
18+
import org.junit.jupiter.params.ParameterizedTest;
19+
20+
import org.springframework.shell.samples.AbstractSampleTests;
21+
import org.springframework.shell.samples.e2e.OptionConversionCommands.CommonConfiguration;
22+
import org.springframework.shell.samples.e2e.OptionConversionCommands.LegacyAnnotation;
23+
import org.springframework.shell.samples.e2e.OptionConversionCommands.Registration;
24+
import org.springframework.shell.test.ShellTestClient.BaseShellSession;
25+
import org.springframework.test.context.ContextConfiguration;
26+
27+
@ContextConfiguration(classes = { LegacyAnnotation.class, Registration.class, CommonConfiguration.class })
28+
class OptionConversionCommandsTests extends AbstractSampleTests {
29+
30+
@ParameterizedTest
31+
@E2ESource(command = "option-conversion-integer --arg1 1")
32+
void optionConversionInteger(String command, boolean interactive) {
33+
BaseShellSession<?> session = createSession(command, interactive);
34+
assertScreenContainsText(session, "Hello 1");
35+
}
36+
37+
@ParameterizedTest
38+
@E2ESource(command = "option-conversion-custom --arg1 hi")
39+
void optionConversionCustom(String command, boolean interactive) {
40+
BaseShellSession<?> session = createSession(command, interactive);
41+
assertScreenContainsText(session, "Hello MyPojo [arg=hi]");
42+
}
43+
44+
@ParameterizedTest
45+
@E2ESource(command = "option-conversion-customset --arg1 hi")
46+
void optionConversionCustomSet(String command, boolean interactive) {
47+
BaseShellSession<?> session = createSession(command, interactive);
48+
assertScreenContainsText(session, "Hello [MyPojo [arg=hi]]");
49+
}
50+
51+
@ParameterizedTest
52+
@E2ESource(command = "option-conversion-customarray --arg1 hi")
53+
void optionConversionCustomArray(String command, boolean interactive) {
54+
BaseShellSession<?> session = createSession(command, interactive);
55+
assertScreenContainsText(session, "Hello [MyPojo [arg=hi]]");
56+
}
57+
}

0 commit comments

Comments
 (0)