From 38a9c2ceebaf5d36ff5bf23418b37f251ea8f002 Mon Sep 17 00:00:00 2001
From: rstoyanchev <rstoyanchev@vmware.com>
Date: Mon, 11 Sep 2023 11:26:35 +0100
Subject: [PATCH] Polishing contribution

See gh-31164
---
 .../invoker/RequestPartArgumentResolver.java  | 35 ++++++++++---------
 .../RequestPartArgumentResolverTests.java     | 25 +++++--------
 2 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/RequestPartArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/RequestPartArgumentResolver.java
index 68709c24b0b..a35900e540e 100644
--- a/spring-web/src/main/java/org/springframework/web/service/invoker/RequestPartArgumentResolver.java
+++ b/spring-web/src/main/java/org/springframework/web/service/invoker/RequestPartArgumentResolver.java
@@ -39,6 +39,7 @@ import org.springframework.web.multipart.MultipartFile;
  * <ul>
  * <li>String -- form field
  * <li>{@link org.springframework.core.io.Resource Resource} -- file part
+ * <li>{@link MultipartFile} -- uploaded file
  * <li>Object -- content to be encoded (e.g. to JSON)
  * <li>{@link HttpEntity} -- part content and headers although generally it's
  * easier to add headers through the returned builder
@@ -80,16 +81,15 @@ public class RequestPartArgumentResolver extends AbstractNamedValueArgumentResol
 	protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) {
 		RequestPart annot = parameter.getParameterAnnotation(RequestPart.class);
 		boolean isMultiPartFile = parameter.nestedIfOptional().getNestedParameterType().equals(MultipartFile.class);
+		String label = (isMultiPartFile ? "MultipartFile" : "request part");
 
-		if (annot != null && isMultiPartFile) {
-			return new NamedValueInfo(annot.name(), annot.required(), null, "MultipartFile", true);
-		}
-		else if (annot != null) {
-			return new NamedValueInfo(annot.name(), annot.required(), null, "request part", true);
+		if (annot != null) {
+			return new NamedValueInfo(annot.name(), annot.required(), null, label, true);
 		}
 		else if (isMultiPartFile) {
-			return new NamedValueInfo("", true, null, "MultipartFile", true);
+			return new NamedValueInfo("", true, null, label, true);
 		}
+
 		return null;
 	}
 
@@ -118,17 +118,9 @@ public class RequestPartArgumentResolver extends AbstractNamedValueArgumentResol
 				return;
 			}
 		}
-		if (value instanceof 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());
-			}
 
-			requestValues.addRequestPart(name, new HttpEntity<>(file.getResource(), headers));
-			return;
+		if (value instanceof MultipartFile multipartFile) {
+			value = toHttpEntity(name, multipartFile);
 		}
 
 		requestValues.addRequestPart(name, value);
@@ -138,4 +130,15 @@ public class RequestPartArgumentResolver extends AbstractNamedValueArgumentResol
 		return ParameterizedTypeReference.forType(nestedParam.getNestedGenericParameterType());
 	}
 
+	private static Object toHttpEntity(String name, MultipartFile multipartFile) {
+		HttpHeaders headers = new HttpHeaders();
+		if (multipartFile.getOriginalFilename() != null) {
+			headers.setContentDispositionFormData(name, multipartFile.getOriginalFilename());
+		}
+		if (multipartFile.getContentType() != null) {
+			headers.add(HttpHeaders.CONTENT_TYPE, multipartFile.getContentType());
+		}
+		return new HttpEntity<>(multipartFile.getResource(), headers);
+	}
+
 }
diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/RequestPartArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/RequestPartArgumentResolverTests.java
index d52c54b23a3..5d4e5ea56ff 100644
--- a/spring-web/src/test/java/org/springframework/web/service/invoker/RequestPartArgumentResolverTests.java
+++ b/spring-web/src/test/java/org/springframework/web/service/invoker/RequestPartArgumentResolverTests.java
@@ -45,13 +45,13 @@ import static org.assertj.core.api.Assertions.assertThat;
  */
 class RequestPartArgumentResolverTests {
 
+	private static final MockMultipartFile mockMultipartFile =
+			new MockMultipartFile("testFileName", "originalTestFileName", "text/plain", "test".getBytes());
+
+
 	private final TestReactorExchangeAdapter client = new TestReactorExchangeAdapter();
 
-	private final Service service =
-			HttpServiceProxyFactory.builderFor(this.client).build().createClient(Service.class);
-
-	private static final MockMultipartFile mockMultipartFile = new MockMultipartFile(
-			"testFileName", "originalTestFileName", "text/plain", "test".getBytes());
+	private final Service service = HttpServiceProxyFactory.builderFor(this.client).build().createClient(Service.class);
 
 
 	// Base class functionality should be tested in NamedValueArgumentResolverTests.
@@ -87,27 +87,23 @@ class RequestPartArgumentResolverTests {
 		testMultipartFile(mockMultipartFile, "myFile");
 	}
 
-	@Test
-	void requestPartOptionalMultipartFile() {
-		this.service.postRequestPartOptionalMultipartFile(Optional.of(mockMultipartFile));
-		testMultipartFile(mockMultipartFile, "file");
-	}
-
 	@Test
 	void optionalMultipartFile() {
 		this.service.postOptionalMultipartFile(Optional.empty(), "anotherPart");
 		Object value = client.getRequestValues().getBodyValue();
 
 		assertThat(value).isInstanceOf(MultiValueMap.class);
+
 		@SuppressWarnings("unchecked")
 		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value;
-		assertThat(map).containsOnlyKeys("anotherPart");
+		assertThat(map).hasSize(1).containsKey("anotherPart");
 	}
 
 	private void testMultipartFile(MultipartFile testFile, String partName) {
 		Object value = this.client.getRequestValues().getBodyValue();
 
 		assertThat(value).isInstanceOf(MultiValueMap.class);
+
 		@SuppressWarnings("unchecked")
 		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value;
 		assertThat(map).hasSize(1);
@@ -139,10 +135,7 @@ class RequestPartArgumentResolverTests {
 		void postRequestPartMultipartFile(@RequestPart(name = "myFile") MultipartFile file);
 
 		@PostExchange
-		void postRequestPartOptionalMultipartFile(@RequestPart Optional<MultipartFile> file);
-
-		@PostExchange
-		void postOptionalMultipartFile(Optional<MultipartFile> file, @RequestPart String anotherPart);
+		void postOptionalMultipartFile(@RequestPart Optional<MultipartFile> file, @RequestPart String anotherPart);
 	}
 
 }