Polishing contribution

Closes gh-27830
spring-projects · Apr 28, 2022 · f0d149b · f0d149b
1 parent caaf83b
commit f0d149b
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 97 deletions.
diff --git a/...ava/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java b/...ava/org/springframework/test/web/servlet/samples/standalone/MultipartControllerTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,15 +32,16 @@
 import javax.servlet.http.Part;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
-import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockMultipartFile;
 import org.springframework.mock.web.MockPart;
 import org.springframework.stereotype.Controller;
 import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.request.MockMultipartHttpServletRequestBuilder;
 import org.springframework.ui.Model;
 import org.springframework.util.StreamUtils;
-import org.springframework.validation.BindException;
 import org.springframework.validation.BindingResult;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
@@ -63,16 +64,20 @@
  */
 public class MultipartControllerTests {
 
-	@Test
-	public void multipartRequestWithSingleFile() throws Exception {
+	@ParameterizedTest
+	@ValueSource(strings = {"/multipartfile", "/part"})
+	public void multipartRequestWithSingleFileOrPart(String url) 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);
 
+		MockMultipartHttpServletRequestBuilder requestBuilder = (url.endsWith("file") ?
+				multipart(url).file(new MockMultipartFile("file", "orig", null, fileContent)) :
+				multipart(url).part(new MockPart("part", "orig", fileContent)));
+
 		standaloneSetup(new MultipartController()).build()
-				.perform(multipart("/multipartfile").file(filePart).file(jsonPart))
+				.perform(requestBuilder.file(jsonPart))
 				.andExpect(status().isFound())
 				.andExpect(model().attribute("fileContent", fileContent))
 				.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
@@ -229,65 +234,16 @@ public void multipartRequestWithOptionalFileListNotPresent() throws Exception {
 	}
 
 	@Test
-	public void multipartRequestWithParts_resolvesMultipartFileArguments() throws Exception {
-		byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
-		MockPart filePart = new MockPart("file", "orig", fileContent);
-
-		byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
-		MockPart jsonPart = new MockPart("json", json);
-		jsonPart.getHeaders().setContentType(MediaType.APPLICATION_JSON);
-
-		standaloneSetup(new MultipartController()).build()
-				.perform(multipart("/multipartfile").part(filePart).part(jsonPart))
-				.andExpect(status().isFound())
-				.andExpect(model().attribute("fileContent", fileContent))
-				.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
-	}
-
-	@Test
-	public void multipartRequestWithParts_resolvesPartArguments() throws Exception {
+	public void multipartRequestWithDataBindingToFile() throws Exception {
 		byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
 		MockPart filePart = new MockPart("file", "orig", fileContent);
 
-		byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
-		MockPart jsonPart = new MockPart("json", json);
-		jsonPart.getHeaders().setContentType(MediaType.APPLICATION_JSON);
-
 		standaloneSetup(new MultipartController()).build()
-				.perform(multipart("/part").part(filePart).part(jsonPart))
-				.andExpect(status().isFound())
-				.andExpect(model().attribute("fileContent", fileContent))
-				.andExpect(model().attribute("jsonContent", Collections.singletonMap("name", "yeeeah")));
-	}
-
-	@Test
-	public void multipartRequestWithParts_resolvesMultipartFileProperties() throws Exception {
-		byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
-		MockPart filePart = new MockPart("file", "orig", fileContent);
-
-		standaloneSetup(new MultipartController()).build()
-				.perform(multipart("/multipartfileproperty").part(filePart))
+				.perform(multipart("/multipartfilebinding").part(filePart))
 				.andExpect(status().isFound())
 				.andExpect(model().attribute("fileContent", fileContent));
 	}
 
-	@Test
-	public void multipartRequestWithParts_cannotResolvePartProperties() throws Exception {
-		byte[] fileContent = "bar".getBytes(StandardCharsets.UTF_8);
-		MockPart filePart = new MockPart("file", "orig", fileContent);
-
-		Exception exception = standaloneSetup(new MultipartController()).build()
-				.perform(multipart("/partproperty").part(filePart))
-				.andExpect(status().is4xxClientError())
-				.andReturn()
-				.getResolvedException();
-
-		assertThat(exception).isNotNull();
-		assertThat(exception).isInstanceOf(BindException.class);
-		assertThat(((BindException) exception).getFieldError("file"))
-				.as("MultipartRequest would not bind Part properties.").isNotNull();
-	}
-
 	@Test  // SPR-13317
 	public void multipartRequestWrapped() throws Exception {
 		byte[] json = "{\"name\":\"yeeeah\"}".getBytes(StandardCharsets.UTF_8);
@@ -391,11 +347,11 @@ public String processOptionalFileList(@RequestParam Optional<List<MultipartFile>
 		}
 
 		@RequestMapping(value = "/part", method = RequestMethod.POST)
-		public String processPart(@RequestPart Part file,
+		public String processPart(@RequestPart Part part,
 				@RequestPart Map<String, String> json, Model model) throws IOException {
 
-			if (file != null) {
-				byte[] content = StreamUtils.copyToByteArray(file.getInputStream());
+			if (part != null) {
+				byte[] content = StreamUtils.copyToByteArray(part.getInputStream());
 				model.addAttribute("fileContent", content);
 			}
 			model.addAttribute("jsonContent", json);
@@ -409,9 +365,9 @@ public String processMultipart(@RequestPart Map<String, String> json, Model mode
 			return "redirect:/index";
 		}
 
-		@RequestMapping(value = "/multipartfileproperty", method = RequestMethod.POST)
-		public String processMultipartFileBean(MultipartFileBean multipartFileBean, Model model, BindingResult bindingResult)
-				throws IOException {
+		@RequestMapping(value = "/multipartfilebinding", method = RequestMethod.POST)
+		public String processMultipartFileBean(
+				MultipartFileBean multipartFileBean, Model model, BindingResult bindingResult) throws IOException {
 
 			if (!bindingResult.hasErrors()) {
 				MultipartFile file = multipartFileBean.getFile();
@@ -421,20 +377,6 @@ public String processMultipartFileBean(MultipartFileBean multipartFileBean, Mode
 			}
 			return "redirect:/index";
 		}
-
-		@RequestMapping(value = "/partproperty", method = RequestMethod.POST)
-		public String processPartBean(PartBean partBean, Model model, BindingResult bindingResult)
-				throws IOException {
-
-			if (!bindingResult.hasErrors()) {
-				Part file = partBean.getFile();
-				if (file != null) {
-					byte[] content = StreamUtils.copyToByteArray(file.getInputStream());
-					model.addAttribute("fileContent", content);
-				}
-			}
-			return "redirect:/index";
-		}
 	}
 
 	private static class MultipartFileBean {
@@ -450,18 +392,6 @@ public void setFile(MultipartFile file) {
 		}
 	}
 
-	private static class PartBean {
-
-		private Part file;
-
-		public Part getFile() {
-			return file;
-		}
-
-		public void setFile(Part file) {
-			this.file = file;
-		}
-	}
 
 	private static class RequestWrappingFilter extends OncePerRequestFilter {
 

diff --git a/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java
@@ -104,9 +104,8 @@ public ServletRequestDataBinder(@Nullable Object target, String objectName) {
 	 * HTTP parameters: i.e. "uploadedFile" to an "uploadedFile" bean property,
 	 * invoking a "setUploadedFile" setter method.
 	 * <p>The type of the target property for a multipart file can be MultipartFile,
-	 * Part, byte[], or String. The Part binding is only supported when the request
-	 * is not a MultipartRequest. The latter two receive the contents of the uploaded file;
-	 * all metadata like original file name, content type, etc are lost in those cases.
+	 * byte[], or String. Servlet Part binding is also supported when the
+	 * request has not been parsed to MultipartRequest via MultipartResolver.
 	 * @param request the request with parameters to bind (can be multipart)
 	 * @see org.springframework.web.multipart.MultipartHttpServletRequest
 	 * @see org.springframework.web.multipart.MultipartRequest

diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebRequestDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebRequestDataBinder.java
@@ -107,10 +107,9 @@ public WebRequestDataBinder(@Nullable Object target, String objectName) {
 	 * <p>Multipart files are bound via their parameter name, just like normal
 	 * HTTP parameters: i.e. "uploadedFile" to an "uploadedFile" bean property,
 	 * invoking a "setUploadedFile" setter method.
-	 * <p>The type of the target property for a multipart file can be Part, MultipartFile,
-	 * byte[], or String. The Part binding is only supported when the request
-	 * is not a MultipartRequest. The latter two receive the contents of the uploaded file;
-	 * all metadata like original file name, content type, etc are lost in those cases.
+	 * <p>The type of the target property for a multipart file can be MultipartFile,
+	 * byte[], or String. Servlet Part binding is also supported when the
+	 * request has not been parsed to MultipartRequest via MultipartResolver.
 	 * @param request the request with parameters to bind (can be multipart)
 	 * @see org.springframework.web.multipart.MultipartRequest
 	 * @see org.springframework.web.multipart.MultipartFile