diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java index b7564bcb61..27635ca74b 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java @@ -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 getConvertibleTypes() { - return Collections.singleton(new ConvertiblePair(Object.class, Optional.class)); + Set 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); diff --git a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java index 3d3530fe68..de27ed8207 100644 --- a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java @@ -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(); } diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java index bead54b3eb..4a5298c7ec 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java @@ -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 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 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 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 file, + @RequestPart Map 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 file, + @RequestPart Map 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 file, + @RequestPart Map 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> file, + @RequestPart Map 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 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 json, Model model) { model.addAttribute("json", json); return "redirect:/index"; @@ -139,4 +321,4 @@ public class MultipartControllerTests { } } -} \ No newline at end of file +} diff --git a/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java b/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java index 4362e112cd..f5db46c108 100644 --- a/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java +++ b/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java @@ -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) { diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java index 2d303393a1..eb6513e2f6 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java @@ -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 paramOptional, + @RequestParam("name") Optional paramOptionalArray, + @RequestParam("name") Optional paramOptionalList, @RequestParam("mfile") Optional multipartFileOptional) { }