Polishing in MultipartFileArgumentResolver

Closes gh-30728

framework-docs/modules/ROOT/pages/integration/rest-clients.adoc

@@ -448,6 +448,10 @@ method parameters:
   Object (entity to be encoded, e.g. as JSON), `HttpEntity` (part content and headers),
   a Spring `Part`, or Reactive Streams `Publisher` of any of the above.
 
+| `MultipartFile`
+| Add a request part from a `MultipartFile`, typically used in a Spring MVC controller
+  where it represents an uploaded file.
+
 | `@CookieValue`
 | Add a cookie or multiple cookies. The argument may be a `Map<String, ?>` or
   `MultiValueMap<String, ?>` with multiple cookies, a `Collection<?>` of values, or an

spring-web/src/main/java/org/springframework/web/service/invoker/MultipartFileArgumentResolver.java

@@ -19,52 +19,45 @@
 import java.util.Optional;
 
 import org.springframework.core.MethodParameter;
-import org.springframework.core.io.Resource;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
 import org.springframework.util.Assert;
 import org.springframework.web.multipart.MultipartFile;
 
 /**
  * {@link HttpServiceArgumentResolver} for arguments of type {@link MultipartFile}.
- * The arguments should not be annotated. To allow for non-required arguments,
- * the {@link MultipartFile} parameters can also be wrapped with {@link Optional}.
+ * The argument is recognized by type, and does not need to be annotated. To make
+ * it optional, declare the parameter with an {@link Optional} wrapper.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Rossen Stoyanchev
  * @since 6.1
  */
 public class MultipartFileArgumentResolver extends AbstractNamedValueArgumentResolver {
 
-	private static final String MULTIPART_FILE_LABEL = "multipart file";
-
 	@Override
 	protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) {
-		if (!parameter.nestedIfOptional().getNestedParameterType().equals(MultipartFile.class)) {
-			return null;
-		}
-		return new NamedValueInfo("", true, null, MULTIPART_FILE_LABEL, true);
-
+		Class<?> type = parameter.nestedIfOptional().getNestedParameterType();
+		return (type.equals(MultipartFile.class) ?
+				new NamedValueInfo("", true, null, "MultipartFile", true) : null);
 	}
 
 	@Override
-	protected void addRequestValue(String name, Object value, MethodParameter parameter,
-			HttpRequestValues.Builder requestValues) {
-		Assert.state(value instanceof MultipartFile,
-				"The value has to be of type 'MultipartFile'");
+	protected void addRequestValue(
+			String name, Object value, MethodParameter parameter, HttpRequestValues.Builder values) {
 
+		Assert.state(value instanceof MultipartFile, "Expected MultipartFile value");
 		MultipartFile file = (MultipartFile) value;
-		requestValues.addRequestPart(name, toHttpEntity(name, file));
-	}
 
-	private HttpEntity<Resource> toHttpEntity(String name, MultipartFile file) {
 		HttpHeaders headers = new HttpHeaders();
 		if (file.getOriginalFilename() != null) {
 			headers.setContentDispositionFormData(name, file.getOriginalFilename());
 		}
 		if (file.getContentType() != null) {
 			headers.add(HttpHeaders.CONTENT_TYPE, file.getContentType());
 		}
-		return new HttpEntity<>(file.getResource(), headers);
+
+		values.addRequestPart(name, new HttpEntity<>(file.getResource(), headers));
 	}
 
 }

spring-web/src/test/java/org/springframework/web/service/invoker/MultipartFileArgumentResolverTests.java

@@ -21,7 +21,6 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.http.ContentDisposition;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
@@ -35,7 +34,8 @@
 
 /**
  * Unit tests for {@link MultipartFileArgumentResolver}.
- * Tests for base class functionality of this resolver can be found in {@link NamedValueArgumentResolverTests}.
+ * Tests for base class functionality of this resolver can be found in
+ * {@link NamedValueArgumentResolverTests}.
  *
  * @author Olga Maciaszek-Sharma
  */
@@ -46,48 +46,49 @@ class MultipartFileArgumentResolverTests {
 
 	private TestClient client;
 
+
 	@BeforeEach
 	void setUp() {
-		HttpServiceProxyFactory proxyFactory = HttpServiceProxyFactory.builder(this.clientAdapter).build();
-		this.client = proxyFactory.createClient(TestClient.class);
+		HttpServiceProxyFactory factory = HttpServiceProxyFactory.builder(this.clientAdapter).build();
+		this.client = factory.createClient(TestClient.class);
 	}
 
+
 	@Test
 	void multipartFile() {
 		String fileName = "testFileName";
 		String originalFileName = "originalTestFileName";
-		MultipartFile testFile = new MockMultipartFile(fileName, originalFileName,
-				MediaType.APPLICATION_JSON_VALUE, "test".getBytes());
+		MultipartFile testFile = new MockMultipartFile(fileName, originalFileName, "text/plain", "test".getBytes());
+
 		this.client.postMultipartFile(testFile);
+		Object value = this.clientAdapter.getRequestValues().getBodyValue();
+
+		assertThat(value).isInstanceOf(MultiValueMap.class);
+		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value;
+		assertThat(map).hasSize(1);
 
-		Object body = clientAdapter.getRequestValues().getBodyValue();
-
-		assertThat(body).isInstanceOf(MultiValueMap.class);
-		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) body;
-		assertThat(map.size()).isEqualTo(1);
-		assertThat(map.getFirst("file")).isNotNull();
-		HttpEntity<?> fileEntity = map.getFirst("file");
-		assertThat(fileEntity.getBody()).isEqualTo(testFile.getResource());
-		HttpHeaders headers = fileEntity.getHeaders();
-		assertThat(headers.getContentType()).isEqualTo(MediaType.APPLICATION_JSON);
-		ContentDisposition contentDisposition = headers.getContentDisposition();
-		assertThat(contentDisposition.getType()).isEqualTo("form-data");
-		assertThat(contentDisposition.getName()).isEqualTo("file");
-		assertThat(contentDisposition.getFilename()).isEqualTo(originalFileName);
+		HttpEntity<?> entity = map.getFirst("file");
+		assertThat(entity).isNotNull();
+		assertThat(entity.getBody()).isEqualTo(testFile.getResource());
+
+		HttpHeaders headers = entity.getHeaders();
+		assertThat(headers.getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
+		assertThat(headers.getContentDisposition().getType()).isEqualTo("form-data");
+		assertThat(headers.getContentDisposition().getName()).isEqualTo("file");
+		assertThat(headers.getContentDisposition().getFilename()).isEqualTo(originalFileName);
 	}
 
 	@Test
 	void optionalMultipartFile() {
 		this.client.postOptionalMultipartFile(Optional.empty(), "anotherPart");
+		Object value = clientAdapter.getRequestValues().getBodyValue();
 
-		Object body = clientAdapter.getRequestValues().getBodyValue();
-
-		assertThat(body).isInstanceOf(MultiValueMap.class);
-		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) body;
-		assertThat(map.size()).isEqualTo(1);
-		assertThat(map.getFirst("anotherPart")).isNotNull();
+		assertThat(value).isInstanceOf(MultiValueMap.class);
+		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value;
+		assertThat(map).containsOnlyKeys("anotherPart");
 	}
 
+
 	@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
 	private interface TestClient {
 
@@ -98,4 +99,5 @@ private interface TestClient {
 		void postOptionalMultipartFile(Optional<MultipartFile> file, @RequestPart String anotherPart);
 
 	}
+
 }

spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientHttpServiceProxyTests.java

@@ -190,7 +190,7 @@ private interface TestHttpService {
 		@PostExchange(contentType = "application/x-www-form-urlencoded")
 		void postForm(@RequestParam MultiValueMap<String, String> params);
 
-		@PostExchange(contentType = MediaType.MULTIPART_FORM_DATA_VALUE)
+		@PostExchange
 		void postMultipart(MultipartFile file, @RequestPart String anotherPart);
 
 	}