Merge MultipartFileArgumentResolver into RequestPartArgumentResolver

Also add test with Optional parameter for a RequestPart argument.

See gh-31164

Signed-off-by: Dmitrii Bocharov <bdshadow@gmail.com>

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

@@ -274,7 +274,6 @@ public final class HttpServiceProxyFactory {
 			// Specific type
 			resolvers.add(new UrlArgumentResolver());
 			resolvers.add(new HttpMethodArgumentResolver());
-			resolvers.add(new MultipartFileArgumentResolver());
 
 			return resolvers;
 		}

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

@@ -1,63 +0,0 @@
-/*
- * Copyright 2002-2023 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.
- * You may obtain a copy of the License at
- *
- *      https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.web.service.invoker;
-
-import java.util.Optional;
-
-import org.springframework.core.MethodParameter;
-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 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 {
-
-	@Override
-	protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) {
-		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 values) {
-
-		Assert.state(value instanceof MultipartFile, "Expected MultipartFile value");
-		MultipartFile file = (MultipartFile) value;
-
-		HttpHeaders headers = new HttpHeaders();
-		if (file.getOriginalFilename() != null) {
-			headers.setContentDispositionFormData(name, file.getOriginalFilename());
-		}
-		if (file.getContentType() != null) {
-			headers.add(HttpHeaders.CONTENT_TYPE, file.getContentType());
-		}
-
-		values.addRequestPart(name, new HttpEntity<>(file.getResource(), headers));
-	}
-
-}

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

@@ -23,11 +23,13 @@ import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.core.ReactiveAdapter;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.codec.multipart.Part;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.web.bind.annotation.RequestPart;
+import org.springframework.web.multipart.MultipartFile;
 
 /**
  * {@link HttpServiceArgumentResolver} for {@link RequestPart @RequestPart}
@@ -77,8 +79,18 @@ public class RequestPartArgumentResolver extends AbstractNamedValueArgumentResol
 	@Override
 	protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) {
 		RequestPart annot = parameter.getParameterAnnotation(RequestPart.class);
-		return (annot == null ? null :
-				new NamedValueInfo(annot.name(), annot.required(), null, "request part", true));
+		boolean isMultiPartFile = parameter.nestedIfOptional().getNestedParameterType().equals(MultipartFile.class);
+
+		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);
+		}
+		else if (isMultiPartFile) {
+			return new NamedValueInfo("", true, null, "MultipartFile", true);
+		}
+		return null;
 	}
 
 	@Override
@@ -106,6 +118,18 @@ 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;
+		}
 
 		requestValues.addRequestPart(name, value);
 	}

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

@@ -1,96 +0,0 @@
-/*
- * Copyright 2002-2023 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.
- * You may obtain a copy of the License at
- *
- *      https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.web.service.invoker;
-
-import java.util.Optional;
-
-import org.junit.jupiter.api.Test;
-
-import org.springframework.http.HttpEntity;
-import org.springframework.http.HttpHeaders;
-import org.springframework.http.MediaType;
-import org.springframework.util.MultiValueMap;
-import org.springframework.web.bind.annotation.RequestPart;
-import org.springframework.web.multipart.MultipartFile;
-import org.springframework.web.service.annotation.PostExchange;
-import org.springframework.web.testfixture.servlet.MockMultipartFile;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-/**
- * Unit tests for {@link MultipartFileArgumentResolver}.
- * Tests for base class functionality of this resolver can be found in
- * {@link NamedValueArgumentResolverTests}.
- *
- * @author Olga Maciaszek-Sharma
- */
-@SuppressWarnings("unchecked")
-class MultipartFileArgumentResolverTests {
-
-	private final TestExchangeAdapter client = new TestExchangeAdapter();
-
-	private final MultipartService multipartService =
-			HttpServiceProxyFactory.builderFor(this.client).build().createClient(MultipartService.class);
-
-
-	@Test
-	void multipartFile() {
-		String fileName = "testFileName";
-		String originalFileName = "originalTestFileName";
-		MultipartFile testFile = new MockMultipartFile(fileName, originalFileName, "text/plain", "test".getBytes());
-
-		this.multipartService.postMultipartFile(testFile);
-		Object value = this.client.getRequestValues().getBodyValue();
-
-		assertThat(value).isInstanceOf(MultiValueMap.class);
-		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value;
-		assertThat(map).hasSize(1);
-
-		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.multipartService.postOptionalMultipartFile(Optional.empty(), "anotherPart");
-		Object value = client.getRequestValues().getBodyValue();
-
-		assertThat(value).isInstanceOf(MultiValueMap.class);
-		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value;
-		assertThat(map).containsOnlyKeys("anotherPart");
-	}
-
-
-	@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
-	private interface MultipartService {
-
-		@PostExchange
-		void postMultipartFile(MultipartFile file);
-
-		@PostExchange
-		void postOptionalMultipartFile(Optional<MultipartFile> file, @RequestPart String anotherPart);
-
-	}
-
-}

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

@@ -16,14 +16,19 @@
 
 package org.springframework.web.service.invoker;
 
+import java.util.Optional;
+
 import org.junit.jupiter.api.Test;
 import reactor.core.publisher.Mono;
 
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.bind.annotation.RequestPart;
+import org.springframework.web.multipart.MultipartFile;
 import org.springframework.web.service.annotation.PostExchange;
+import org.springframework.web.testfixture.servlet.MockMultipartFile;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -45,6 +50,9 @@ class RequestPartArgumentResolverTests {
 	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());
+
 
 	// Base class functionality should be tested in NamedValueArgumentResolverTests.
 	// Form data vs query params tested in HttpRequestValuesTests.
@@ -54,7 +62,7 @@ class RequestPartArgumentResolverTests {
 		HttpHeaders headers = new HttpHeaders();
 		headers.add("foo", "bar");
 		HttpEntity<String> part2 = new HttpEntity<>("part 2", headers);
-		this.service.postMultipart("part 1", part2, Mono.just("part 3"));
+		this.service.postMultipart("part 1", part2, Mono.just("part 3"), Optional.of("part 4"));
 
 		Object body = this.client.getRequestValues().getBodyValue();
 		assertThat(body).isInstanceOf(MultiValueMap.class);
@@ -64,6 +72,55 @@ class RequestPartArgumentResolverTests {
 		assertThat(map.getFirst("part1").getBody()).isEqualTo("part 1");
 		assertThat(map.getFirst("part2")).isEqualTo(part2);
 		assertThat(((Mono<?>) map.getFirst("part3").getBody()).block()).isEqualTo("part 3");
+		assertThat(map.getFirst("optionalPart").getBody()).isEqualTo("part 4");
+	}
+
+	@Test
+	void multipartFile() {
+		this.service.postMultipartFile(mockMultipartFile);
+		testMultipartFile(mockMultipartFile, "file");
+	}
+
+	@Test
+	void requestPartMultipartFile() {
+		this.service.postRequestPartMultipartFile(mockMultipartFile);
+		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");
+	}
+
+	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);
+
+		HttpEntity<?> entity = map.getFirst(partName);
+		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(partName);
+		assertThat(headers.getContentDisposition().getFilename()).isEqualTo(testFile.getOriginalFilename());
 	}
 
 
@@ -72,8 +129,20 @@ class RequestPartArgumentResolverTests {
 		@PostExchange
 		void postMultipart(
 				@RequestPart String part1, @RequestPart HttpEntity<String> part2,
-				@RequestPart Mono<String> part3);
+				@RequestPart Mono<String> part3,
+				@RequestPart Optional<String> optionalPart);
 
+		@PostExchange
+		void postMultipartFile(MultipartFile file);
+
+		@PostExchange
+		void postRequestPartMultipartFile(@RequestPart(name = "myFile") MultipartFile file);
+
+		@PostExchange
+		void postRequestPartOptionalMultipartFile(@RequestPart Optional<MultipartFile> file);
+
+		@PostExchange
+		void postOptionalMultipartFile(Optional<MultipartFile> file, @RequestPart String anotherPart);
 	}
 
 }