Skip to content

Commit 15c82af

Browse files
jhoellerjhoeller
committed
Consistent conversion of Optional array/list arrangements
Issue: SPR-15918 Issue: SPR-15919 Issue: SPR-15676
1 parent ea01c41 commit 15c82af

File tree

5 files changed

+302
-32
lines changed

5 files changed

+302
-32
lines changed

‎spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616

1717
package org.springframework.core.convert.support;
1818

19-
import java.util.Collections;
19+
import java.lang.reflect.Array;
20+
import java.util.Collection;
21+
import java.util.LinkedHashSet;
2022
import java.util.Optional;
2123
import java.util.Set;
2224

@@ -46,7 +48,11 @@ public ObjectToOptionalConverter(ConversionService conversionService) {
4648

4749
@Override
4850
public Set<ConvertiblePair> getConvertibleTypes() {
49-
return Collections.singleton(new ConvertiblePair(Object.class, Optional.class));
51+
Set<ConvertiblePair> convertibleTypes = new LinkedHashSet<>(4);
52+
convertibleTypes.add(new ConvertiblePair(Collection.class, Optional.class));
53+
convertibleTypes.add(new ConvertiblePair(Object[].class, Optional.class));
54+
convertibleTypes.add(new ConvertiblePair(Object.class, Optional.class));
55+
return convertibleTypes;
5056
}
5157

5258
@Override
@@ -69,7 +75,11 @@ else if (source instanceof Optional) {
6975
}
7076
else if (targetType.getResolvableType().hasGenerics()) {
7177
Object target = this.conversionService.convert(source, sourceType, new GenericTypeDescriptor(targetType));
72-
return Optional.ofNullable(target);
78+
if (target == null || (target.getClass().isArray() && Array.getLength(target) == 0) ||
79+
(target instanceof Collection && ((Collection) target).isEmpty())) {
80+
return Optional.empty();
81+
}
82+
return Optional.of(target);
7383
}
7484
else {
7585
return Optional.of(source);

‎spring-core/src/main/java/org/springframework/util/ObjectUtils.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,12 @@ public static boolean isEmpty(@Nullable Object obj) {
139139
if (obj instanceof Optional) {
140140
return !((Optional) obj).isPresent();
141141
}
142-
if (obj.getClass().isArray()) {
143-
return Array.getLength(obj) == 0;
144-
}
145142
if (obj instanceof CharSequence) {
146143
return ((CharSequence) obj).length() == 0;
147144
}
145+
if (obj.getClass().isArray()) {
146+
return Array.getLength(obj) == 0;
147+
}
148148
if (obj instanceof Collection) {
149149
return ((Collection) obj).isEmpty();
150150
}

‎spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java

Lines changed: 193 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import java.io.IOException;
2020
import java.nio.charset.StandardCharsets;
2121
import java.util.Collections;
22+
import java.util.List;
2223
import java.util.Map;
24+
import java.util.Optional;
2325
import javax.servlet.Filter;
2426
import javax.servlet.FilterChain;
2527
import javax.servlet.ServletException;
@@ -28,6 +30,7 @@
2830
import javax.servlet.http.HttpServletResponse;
2931
import javax.servlet.http.Part;
3032

33+
import org.junit.Assert;
3134
import org.junit.Test;
3235

3336
import org.springframework.http.MediaType;
@@ -49,25 +52,140 @@
4952

5053
/**
5154
* @author Rossen Stoyanchev
52-
* @since 5.0
55+
* @author Juergen Hoeller
5356
*/
5457
public class MultipartControllerTests {
5558

5659
@Test
57-
public void multipartRequest() throws Exception {
60+
public void multipartRequestWithSingleFile() throws Exception {
5861
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
5962
MockMultipartFile filePart = new MockMultipartFile("file", "orig", null, fileContent);
6063

6164
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
6265
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
6366

6467
standaloneSetup(new MultipartController()).build()
65-
.perform(multipart("/test-multipartfile").file(filePart).file(jsonPart))
68+
.perform(multipart("/multipartfile").file(filePart).file(jsonPart))
6669
.andExpect(status().isFound())
6770
.andExpect(model().attribute("fileContent", fileContent))
6871
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
6972
}
7073

74+
@Test
75+
public void multipartRequestWithFileArray() throws Exception {
76+
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
77+
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
78+
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
79+
80+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
81+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
82+
83+
standaloneSetup(new MultipartController()).build()
84+
.perform(multipart("/multipartfilearray").file(filePart1).file(filePart2).file(jsonPart))
85+
.andExpect(status().isFound())
86+
.andExpect(model().attribute("fileContent", fileContent))
87+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
88+
}
89+
90+
@Test
91+
public void multipartRequestWithFileList() throws Exception {
92+
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
93+
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
94+
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
95+
96+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
97+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
98+
99+
standaloneSetup(new MultipartController()).build()
100+
.perform(multipart("/multipartfilelist").file(filePart1).file(filePart2).file(jsonPart))
101+
.andExpect(status().isFound())
102+
.andExpect(model().attribute("fileContent", fileContent))
103+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
104+
}
105+
106+
@Test
107+
public void multipartRequestWithOptionalFile() throws Exception {
108+
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
109+
MockMultipartFile filePart = new MockMultipartFile("file", "orig", null, fileContent);
110+
111+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
112+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
113+
114+
standaloneSetup(new MultipartController()).build()
115+
.perform(multipart("/optionalfile").file(filePart).file(jsonPart))
116+
.andExpect(status().isFound())
117+
.andExpect(model().attribute("fileContent", fileContent))
118+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
119+
}
120+
121+
@Test
122+
public void multipartRequestWithOptionalFileNotPresent() throws Exception {
123+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
124+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
125+
126+
standaloneSetup(new MultipartController()).build()
127+
.perform(multipart("/optionalfile").file(jsonPart))
128+
.andExpect(status().isFound())
129+
.andExpect(model().attributeDoesNotExist("fileContent"))
130+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
131+
}
132+
133+
@Test
134+
public void multipartRequestWithOptionalFileArray() throws Exception {
135+
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
136+
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
137+
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
138+
139+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
140+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
141+
142+
standaloneSetup(new MultipartController()).build()
143+
.perform(multipart("/optionalfilearray").file(filePart1).file(filePart2).file(jsonPart))
144+
.andExpect(status().isFound())
145+
.andExpect(model().attribute("fileContent", fileContent))
146+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
147+
}
148+
149+
@Test
150+
public void multipartRequestWithOptionalFileArrayNotPresent() throws Exception {
151+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
152+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
153+
154+
standaloneSetup(new MultipartController()).build()
155+
.perform(multipart("/optionalfilearray").file(jsonPart))
156+
.andExpect(status().isFound())
157+
.andExpect(model().attributeDoesNotExist("fileContent"))
158+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
159+
}
160+
161+
@Test
162+
public void multipartRequestWithOptionalFileList() throws Exception {
163+
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
164+
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
165+
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
166+
167+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
168+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
169+
170+
standaloneSetup(new MultipartController()).build()
171+
.perform(multipart("/optionalfilelist").file(filePart1).file(filePart2).file(jsonPart))
172+
.andExpect(status().isFound())
173+
.andExpect(model().attribute("fileContent", fileContent))
174+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
175+
}
176+
177+
@Test
178+
public void multipartRequestWithOptionalFileListNotPresent() throws Exception {
179+
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
180+
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
181+
182+
standaloneSetup(new MultipartController()).build()
183+
.perform(multipart("/optionalfilelist").file(jsonPart))
184+
.andExpect(status().isFound())
185+
.andExpect(model().attributeDoesNotExist("fileContent"))
186+
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
187+
}
188+
71189
@Test
72190
public void multipartRequestWithServletParts() throws Exception {
73191
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
@@ -78,7 +196,7 @@ public void multipartRequestWithServletParts() throws Exception {
78196
jsonPart.getHeaders().setContentType(MediaType.APPLICATION_JSON);
79197

80198
standaloneSetup(new MultipartController()).build()
81-
.perform(multipart("/test-multipartfile").part(filePart).part(jsonPart))
199+
.perform(multipart("/multipartfile").part(filePart).part(jsonPart))
82200
.andExpect(status().isFound())
83201
.andExpect(model().attribute("fileContent", fileContent))
84202
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
@@ -93,34 +211,98 @@ public void multipartRequestWrapped() throws Exception {
93211
MockMvc mockMvc = standaloneSetup(new MultipartController()).addFilter(filter).build();
94212

95213
Map<String, String> jsonMap = Collections.singletonMap("name", "yeeeah");
96-
mockMvc.perform(multipart("/test-json").file(jsonPart)).andExpect(model().attribute("json", jsonMap));
214+
mockMvc.perform(multipart("/json").file(jsonPart)).andExpect(model().attribute("json", jsonMap));
97215
}
98216

99217

100218
@Controller
101219
private static class MultipartController {
102220

103-
@RequestMapping(value = "/test-multipartfile", method = RequestMethod.POST)
221+
@RequestMapping(value = "/multipartfile", method = RequestMethod.POST)
104222
public String processMultipartFile(@RequestParam MultipartFile file,
105223
@RequestPart Map<String, String> json, Model model) throws IOException {
106224

107-
model.addAttribute("jsonContent", json);
108225
model.addAttribute("fileContent", file.getBytes());
226+
model.addAttribute("jsonContent", json);
109227

110228
return "redirect:/index";
111229
}
112230

113-
@RequestMapping(value = "/test-part", method = RequestMethod.POST)
114-
public String processPart(@RequestParam Part part,
231+
@RequestMapping(value = "/multipartfilearray", method = RequestMethod.POST)
232+
public String processMultipartFileArray(@RequestParam MultipartFile[] file,
115233
@RequestPart Map<String, String> json, Model model) throws IOException {
116234

235+
byte[] content = file[0].getBytes();
236+
Assert.assertArrayEquals(content, file[1].getBytes());
237+
model.addAttribute("fileContent", content);
117238
model.addAttribute("jsonContent", json);
239+
240+
return "redirect:/index";
241+
}
242+
243+
@RequestMapping(value = "/multipartfilelist", method = RequestMethod.POST)
244+
public String processMultipartFileList(@RequestParam List<MultipartFile> file,
245+
@RequestPart Map<String, String> json, Model model) throws IOException {
246+
247+
byte[] content = file.get(0).getBytes();
248+
Assert.assertArrayEquals(content, file.get(1).getBytes());
249+
model.addAttribute("fileContent", content);
250+
model.addAttribute("jsonContent", json);
251+
252+
return "redirect:/index";
253+
}
254+
255+
@RequestMapping(value = "/optionalfile", method = RequestMethod.POST)
256+
public String processOptionalFile(@RequestParam Optional<MultipartFile> file,
257+
@RequestPart Map<String, String> json, Model model) throws IOException {
258+
259+
if (file.isPresent()) {
260+
model.addAttribute("fileContent", file.get().getBytes());
261+
}
262+
model.addAttribute("jsonContent", json);
263+
264+
return "redirect:/index";
265+
}
266+
267+
@RequestMapping(value = "/optionalfilearray", method = RequestMethod.POST)
268+
public String processOptionalFileArray(@RequestParam Optional<MultipartFile[]> file,
269+
@RequestPart Map<String, String> json, Model model) throws IOException {
270+
271+
if (file.isPresent()) {
272+
byte[] content = file.get()[0].getBytes();
273+
Assert.assertArrayEquals(content, file.get()[1].getBytes());
274+
model.addAttribute("fileContent", content);
275+
}
276+
model.addAttribute("jsonContent", json);
277+
278+
return "redirect:/index";
279+
}
280+
281+
@RequestMapping(value = "/optionalfilelist", method = RequestMethod.POST)
282+
public String processOptionalFileList(@RequestParam Optional<List<MultipartFile>> file,
283+
@RequestPart Map<String, String> json, Model model) throws IOException {
284+
285+
if (file.isPresent()) {
286+
byte[] content = file.get().get(0).getBytes();
287+
Assert.assertArrayEquals(content, file.get().get(1).getBytes());
288+
model.addAttribute("fileContent", content);
289+
}
290+
model.addAttribute("jsonContent", json);
291+
292+
return "redirect:/index";
293+
}
294+
295+
@RequestMapping(value = "/part", method = RequestMethod.POST)
296+
public String processPart(@RequestParam Part part,
297+
@RequestPart Map<String, String> json, Model model) throws IOException {
298+
118299
model.addAttribute("fileContent", part.getInputStream());
300+
model.addAttribute("jsonContent", json);
119301

120302
return "redirect:/index";
121303
}
122304

123-
@RequestMapping(value = "/test-json", method = RequestMethod.POST)
305+
@RequestMapping(value = "/json", method = RequestMethod.POST)
124306
public String processMultipart(@RequestPart Map<String, String> json, Model model) {
125307
model.addAttribute("json", json);
126308
return "redirect:/index";
@@ -139,4 +321,4 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
139321
}
140322
}
141323

142-
}
324+
}

‎spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
import org.springframework.util.ReflectionUtils;
5555
import org.springframework.web.bind.annotation.ValueConstants;
5656

57-
import static java.util.stream.Collectors.joining;
57+
import static java.util.stream.Collectors.*;
5858

5959
/**
6060
* Convenience class to resolve method parameters from hints.
@@ -238,9 +238,8 @@ private String formatAnnotation(Annotation annotation) {
238238
}
239239

240240
private static ResolvableType toResolvableType(Class<?> type, Class<?>... generics) {
241-
return ObjectUtils.isEmpty(generics) ?
242-
ResolvableType.forClass(type) :
243-
ResolvableType.forClassWithGenerics(type, generics);
241+
return (ObjectUtils.isEmpty(generics) ? ResolvableType.forClass(type) :
242+
ResolvableType.forClassWithGenerics(type, generics));
244243
}
245244

246245
private static ResolvableType toResolvableType(Class<?> type, ResolvableType generic, ResolvableType... generics) {

0 commit comments

Comments
 (0)