From fe8e6d3d5abe60cc441e4cf178e7eb2a7e01ae03 Mon Sep 17 00:00:00 2001 From: Dmitrii Bocharov Date: Sun, 3 Sep 2023 20:43:26 +0200 Subject: [PATCH] Merge MultipartFileArgumentResolver into RequestPartArgumentResolver Also add test with Optional parameter for a RequestPart argument. See gh-31164 Signed-off-by: Dmitrii Bocharov --- .../invoker/HttpServiceProxyFactory.java | 1 - .../MultipartFileArgumentResolver.java | 63 ------------ .../invoker/RequestPartArgumentResolver.java | 28 +++++- .../MultipartFileArgumentResolverTests.java | 96 ------------------- .../RequestPartArgumentResolverTests.java | 73 +++++++++++++- 5 files changed, 97 insertions(+), 164 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/web/service/invoker/MultipartFileArgumentResolver.java delete mode 100644 spring-web/src/test/java/org/springframework/web/service/invoker/MultipartFileArgumentResolverTests.java diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java index fde248857a..2e18cef483 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/MultipartFileArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/MultipartFileArgumentResolver.java deleted file mode 100644 index 5773ebe124..0000000000 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/MultipartFileArgumentResolver.java +++ /dev/null @@ -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)); - } - -} 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 662d71f497..68709c24b0 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 @@ -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); } diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/MultipartFileArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/MultipartFileArgumentResolverTests.java deleted file mode 100644 index ae53f15893..0000000000 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/MultipartFileArgumentResolverTests.java +++ /dev/null @@ -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> map = (MultiValueMap>) 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> map = (MultiValueMap>) value; - assertThat(map).containsOnlyKeys("anotherPart"); - } - - - @SuppressWarnings("OptionalUsedAsFieldOrParameterType") - private interface MultipartService { - - @PostExchange - void postMultipartFile(MultipartFile file); - - @PostExchange - void postOptionalMultipartFile(Optional file, @RequestPart String anotherPart); - - } - -} 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 228647cd37..d52c54b23a 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 @@ -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 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> map = (MultiValueMap>) 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> map = (MultiValueMap>) 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 part2, - @RequestPart Mono part3); + @RequestPart Mono part3, + @RequestPart Optional optionalPart); + @PostExchange + void postMultipartFile(MultipartFile file); + + @PostExchange + void postRequestPartMultipartFile(@RequestPart(name = "myFile") MultipartFile file); + + @PostExchange + void postRequestPartOptionalMultipartFile(@RequestPart Optional file); + + @PostExchange + void postOptionalMultipartFile(Optional file, @RequestPart String anotherPart); } }