root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Consistent conversion of Optional array/list arrangements 

Issue: SPR-15918
Issue: SPR-15919
Issue: SPR-15676
This commit is contained in:
Juergen Hoeller 2017-09-20 18:28:49 +02:00
parent ea01c4113a
commit 15c82afc1c
5 changed files with 302 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java
View File

@ -16,7 +16,9 @@
package org.springframework.core.convert.support;
import java.util.Collections;
import java.lang.reflect.Array;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.Optional;
import java.util.Set;
@ -46,7 +48,11 @@ final class ObjectToOptionalConverter implements ConditionalGenericConverter {
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return Collections.singleton(new ConvertiblePair(Object.class, Optional.class));
Set<ConvertiblePair> convertibleTypes = new LinkedHashSet<>(4);
convertibleTypes.add(new ConvertiblePair(Collection.class, Optional.class));
convertibleTypes.add(new ConvertiblePair(Object[].class, Optional.class));
convertibleTypes.add(new ConvertiblePair(Object.class, Optional.class));
return convertibleTypes;
}
@Override
@ -69,7 +75,11 @@ final class ObjectToOptionalConverter implements ConditionalGenericConverter {
}
else if (targetType.getResolvableType().hasGenerics()) {
Object target = this.conversionService.convert(source, sourceType, new GenericTypeDescriptor(targetType));
return Optional.ofNullable(target);
if (target == null || (target.getClass().isArray() && Array.getLength(target) == 0) ||
(target instanceof Collection && ((Collection) target).isEmpty())) {
return Optional.empty();
}
return Optional.of(target);
}
else {
return Optional.of(source);

6
spring-core/src/main/java/org/springframework/util/ObjectUtils.java
View File

@ -139,12 +139,12 @@ public abstract class ObjectUtils {
if (obj instanceof Optional) {
return !((Optional) obj).isPresent();
}
if (obj.getClass().isArray()) {
return Array.getLength(obj) == 0;
}
if (obj instanceof CharSequence) {
return ((CharSequence) obj).length() == 0;
}
if (obj.getClass().isArray()) {
return Array.getLength(obj) == 0;
}
if (obj instanceof Collection) {
return ((Collection) obj).isEmpty();
}

204
spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java
View File

@ -19,7 +19,9 @@ package org.springframework.test.web.servlet.samples.standalone;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
@ -28,6 +30,7 @@ import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.Part;
import org.junit.Assert;
import org.junit.Test;
import org.springframework.http.MediaType;
@ -49,12 +52,12 @@ import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
/**
* @author Rossen Stoyanchev
* @since 5.0
* @author Juergen Hoeller
*/
public class MultipartControllerTests {
@Test
public void multipartRequest() throws Exception {
public void multipartRequestWithSingleFile() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
MockMultipartFile filePart = new MockMultipartFile("file", "orig", null, fileContent);
@ -62,12 +65,127 @@ public class MultipartControllerTests {
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/test-multipartfile").file(filePart).file(jsonPart))
.perform(multipart("/multipartfile").file(filePart).file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithFileArray() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/multipartfilearray").file(filePart1).file(filePart2).file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithFileList() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/multipartfilelist").file(filePart1).file(filePart2).file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithOptionalFile() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
MockMultipartFile filePart = new MockMultipartFile("file", "orig", null, fileContent);
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/optionalfile").file(filePart).file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithOptionalFileNotPresent() throws Exception {
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/optionalfile").file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attributeDoesNotExist("fileContent"))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithOptionalFileArray() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/optionalfilearray").file(filePart1).file(filePart2).file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithOptionalFileArrayNotPresent() throws Exception {
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/optionalfilearray").file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attributeDoesNotExist("fileContent"))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithOptionalFileList() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
MockMultipartFile filePart1 = new MockMultipartFile("file", "orig", null, fileContent);
MockMultipartFile filePart2 = new MockMultipartFile("file", "orig", null, fileContent);
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/optionalfilelist").file(filePart1).file(filePart2).file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithOptionalFileListNotPresent() throws Exception {
byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
MockMultipartFile jsonPart = new MockMultipartFile("json", "json", "application/json", json);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/optionalfilelist").file(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attributeDoesNotExist("fileContent"))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
}
@Test
public void multipartRequestWithServletParts() throws Exception {
byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
@ -78,7 +196,7 @@ public class MultipartControllerTests {
jsonPart.getHeaders().setContentType(MediaType.APPLICATION_JSON);
standaloneSetup(new MultipartController()).build()
.perform(multipart("/test-multipartfile").part(filePart).part(jsonPart))
.perform(multipart("/multipartfile").part(filePart).part(jsonPart))
.andExpect(status().isFound())
.andExpect(model().attribute("fileContent", fileContent))
.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
@ -93,34 +211,98 @@ public class MultipartControllerTests {
MockMvc mockMvc = standaloneSetup(new MultipartController()).addFilter(filter).build();
Map<String, String> jsonMap = Collections.singletonMap("name", "yeeeah");
mockMvc.perform(multipart("/test-json").file(jsonPart)).andExpect(model().attribute("json", jsonMap));
mockMvc.perform(multipart("/json").file(jsonPart)).andExpect(model().attribute("json", jsonMap));
}
@Controller
private static class MultipartController {
@RequestMapping(value = "/test-multipartfile", method = RequestMethod.POST)
@RequestMapping(value = "/multipartfile", method = RequestMethod.POST)
public String processMultipartFile(@RequestParam MultipartFile file,
@RequestPart Map<String, String> json, Model model) throws IOException {
model.addAttribute("jsonContent", json);
model.addAttribute("fileContent", file.getBytes());
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/test-part", method = RequestMethod.POST)
@RequestMapping(value = "/multipartfilearray", method = RequestMethod.POST)
public String processMultipartFileArray(@RequestParam MultipartFile[] file,
@RequestPart Map<String, String> json, Model model) throws IOException {
byte[] content = file[0].getBytes();
Assert.assertArrayEquals(content, file[1].getBytes());
model.addAttribute("fileContent", content);
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/multipartfilelist", method = RequestMethod.POST)
public String processMultipartFileList(@RequestParam List<MultipartFile> file,
@RequestPart Map<String, String> json, Model model) throws IOException {
byte[] content = file.get(0).getBytes();
Assert.assertArrayEquals(content, file.get(1).getBytes());
model.addAttribute("fileContent", content);
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/optionalfile", method = RequestMethod.POST)
public String processOptionalFile(@RequestParam Optional<MultipartFile> file,
@RequestPart Map<String, String> json, Model model) throws IOException {
if (file.isPresent()) {
model.addAttribute("fileContent", file.get().getBytes());
}
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/optionalfilearray", method = RequestMethod.POST)
public String processOptionalFileArray(@RequestParam Optional<MultipartFile[]> file,
@RequestPart Map<String, String> json, Model model) throws IOException {
if (file.isPresent()) {
byte[] content = file.get()[0].getBytes();
Assert.assertArrayEquals(content, file.get()[1].getBytes());
model.addAttribute("fileContent", content);
}
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/optionalfilelist", method = RequestMethod.POST)
public String processOptionalFileList(@RequestParam Optional<List<MultipartFile>> file,
@RequestPart Map<String, String> json, Model model) throws IOException {
if (file.isPresent()) {
byte[] content = file.get().get(0).getBytes();
Assert.assertArrayEquals(content, file.get().get(1).getBytes());
model.addAttribute("fileContent", content);
}
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/part", method = RequestMethod.POST)
public String processPart(@RequestParam Part part,
@RequestPart Map<String, String> json, Model model) throws IOException {
model.addAttribute("jsonContent", json);
model.addAttribute("fileContent", part.getInputStream());
model.addAttribute("jsonContent", json);
return "redirect:/index";
}
@RequestMapping(value = "/test-json", method = RequestMethod.POST)
@RequestMapping(value = "/json", method = RequestMethod.POST)
public String processMultipart(@RequestPart Map<String, String> json, Model model) {
model.addAttribute("json", json);
return "redirect:/index";
@ -139,4 +321,4 @@ public class MultipartControllerTests {
}
}
}
}

7
spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java
View File

@ -54,7 +54,7 @@ import org.springframework.util.ObjectUtils;
import org.springframework.util.ReflectionUtils;
import org.springframework.web.bind.annotation.ValueConstants;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.*;
/**
* Convenience class to resolve method parameters from hints.
@ -238,9 +238,8 @@ public class ResolvableMethod {
}
private static ResolvableType toResolvableType(Class<?> type, Class<?>... generics) {
return ObjectUtils.isEmpty(generics) ?
ResolvableType.forClass(type) :
ResolvableType.forClassWithGenerics(type, generics);
return (ObjectUtils.isEmpty(generics) ? ResolvableType.forClass(type) :
ResolvableType.forClassWithGenerics(type, generics));
}
private static ResolvableType toResolvableType(Class<?> type, ResolvableType generic, ResolvableType... generics) {

101
spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java
View File

@ -48,16 +48,9 @@ import org.springframework.web.multipart.MultipartException;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.support.MissingServletRequestPartException;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.mock;
import static org.springframework.web.method.MvcAnnotationPredicates.requestParam;
import static org.springframework.web.method.MvcAnnotationPredicates.requestPart;
import static org.junit.Assert.*;
import static org.mockito.BDDMockito.*;
import static org.springframework.web.method.MvcAnnotationPredicates.*;
/**
* Test fixture with {@link org.springframework.web.method.annotation.RequestParamMethodArgumentResolver}.
@ -76,13 +69,15 @@ public class RequestParamMethodArgumentResolverTests {
private ResolvableMethod testMethod = ResolvableMethod.on(getClass()).named("handle").build();
@Before
public void setUp() throws Exception {
public void setup() throws Exception {
resolver = new RequestParamMethodArgumentResolver(null, true);
request = new MockHttpServletRequest();
webRequest = new ServletWebRequest(request, new MockHttpServletResponse());
}
@Test
public void supportsParameter() {
resolver = new RequestParamMethodArgumentResolver(null, true);
@ -470,6 +465,88 @@ public class RequestParamMethodArgumentResolverTests {
assertEquals(123, ((Optional) result).get());
}
@Test
@SuppressWarnings("rawtypes")
public void missingOptionalParamValue() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(new DefaultConversionService());
WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer);
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Optional.class, Integer.class);
Object result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.empty(), result);
result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.class, result.getClass());
assertFalse(((Optional) result).isPresent());
}
@Test
@SuppressWarnings("rawtypes")
public void resolveOptionalParamArray() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(new DefaultConversionService());
WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer);
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Optional.class, Integer[].class);
Object result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.empty(), result);
this.request.addParameter("name", "123", "456");
result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.class, result.getClass());
assertArrayEquals(new Integer[] {123, 456}, (Integer[]) ((Optional) result).get());
}
@Test
@SuppressWarnings("rawtypes")
public void missingOptionalParamArray() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(new DefaultConversionService());
WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer);
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Optional.class, Integer[].class);
Object result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.empty(), result);
result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.class, result.getClass());
assertFalse(((Optional) result).isPresent());
}
@Test
@SuppressWarnings("rawtypes")
public void resolveOptionalParamList() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(new DefaultConversionService());
WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer);
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Optional.class, List.class);
Object result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.empty(), result);
this.request.addParameter("name", "123", "456");
result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.class, result.getClass());
assertEquals(Arrays.asList("123", "456"), ((Optional) result).get());
}
@Test
@SuppressWarnings("rawtypes")
public void missingOptionalParamList() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(new DefaultConversionService());
WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer);
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(Optional.class, List.class);
Object result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.empty(), result);
result = resolver.resolveArgument(param, null, webRequest, binderFactory);
assertEquals(Optional.class, result.getClass());
assertFalse(((Optional) result).isPresent());
}
@Test
public void resolveOptionalMultipartFile() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
@ -536,6 +613,8 @@ public class RequestParamMethodArgumentResolverTests {
@RequestParam("name") String paramRequired,
@RequestParam(name = "name", required = false) String paramNotRequired,
@RequestParam("name") Optional<Integer> paramOptional,
@RequestParam("name") Optional<Integer[]> paramOptionalArray,
@RequestParam("name") Optional<List> paramOptionalList,
@RequestParam("mfile") Optional<MultipartFile> multipartFileOptional) {
}