Polishing in MultipartFileArgumentResolver
Closes gh-30728
This commit is contained in:
		
							parent
							
								
									e69a1d22f9
								
							
						
					
					
						commit
						40bf923d7d
					
				|  | @ -448,6 +448,10 @@ method parameters: | ||||||
|   Object (entity to be encoded, e.g. as JSON), `HttpEntity` (part content and headers), |   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. |   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` | | `@CookieValue` | ||||||
| | Add a cookie or multiple cookies. The argument may be a `Map<String, ?>` or | | 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 |   `MultiValueMap<String, ?>` with multiple cookies, a `Collection<?>` of values, or an | ||||||
|  |  | ||||||
|  | @ -19,7 +19,6 @@ package org.springframework.web.service.invoker; | ||||||
| import java.util.Optional; | import java.util.Optional; | ||||||
| 
 | 
 | ||||||
| import org.springframework.core.MethodParameter; | import org.springframework.core.MethodParameter; | ||||||
| import org.springframework.core.io.Resource; |  | ||||||
| import org.springframework.http.HttpEntity; | import org.springframework.http.HttpEntity; | ||||||
| import org.springframework.http.HttpHeaders; | import org.springframework.http.HttpHeaders; | ||||||
| import org.springframework.util.Assert; | import org.springframework.util.Assert; | ||||||
|  | @ -27,36 +26,29 @@ import org.springframework.web.multipart.MultipartFile; | ||||||
| 
 | 
 | ||||||
| /** | /** | ||||||
|  * {@link HttpServiceArgumentResolver} for arguments of type {@link MultipartFile}. |  * {@link HttpServiceArgumentResolver} for arguments of type {@link MultipartFile}. | ||||||
|  * The arguments should not be annotated. To allow for non-required arguments, |  * The argument is recognized by type, and does not need to be annotated. To make | ||||||
|  * the {@link MultipartFile} parameters can also be wrapped with {@link Optional}. |  * it optional, declare the parameter with an {@link Optional} wrapper. | ||||||
|  * |  * | ||||||
|  * @author Olga Maciaszek-Sharma |  * @author Olga Maciaszek-Sharma | ||||||
|  |  * @author Rossen Stoyanchev | ||||||
|  * @since 6.1 |  * @since 6.1 | ||||||
|  */ |  */ | ||||||
| public class MultipartFileArgumentResolver extends AbstractNamedValueArgumentResolver { | public class MultipartFileArgumentResolver extends AbstractNamedValueArgumentResolver { | ||||||
| 
 | 
 | ||||||
| 	private static final String MULTIPART_FILE_LABEL = "multipart file"; |  | ||||||
| 
 |  | ||||||
| 	@Override | 	@Override | ||||||
| 	protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) { | 	protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) { | ||||||
| 		if (!parameter.nestedIfOptional().getNestedParameterType().equals(MultipartFile.class)) { | 		Class<?> type = parameter.nestedIfOptional().getNestedParameterType(); | ||||||
| 			return null; | 		return (type.equals(MultipartFile.class) ? | ||||||
| 		} | 				new NamedValueInfo("", true, null, "MultipartFile", true) : null); | ||||||
| 		return new NamedValueInfo("", true, null, MULTIPART_FILE_LABEL, true); |  | ||||||
| 
 |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	@Override | 	@Override | ||||||
| 	protected void addRequestValue(String name, Object value, MethodParameter parameter, | 	protected void addRequestValue( | ||||||
| 			HttpRequestValues.Builder requestValues) { | 			String name, Object value, MethodParameter parameter, HttpRequestValues.Builder values) { | ||||||
| 		Assert.state(value instanceof MultipartFile, |  | ||||||
| 				"The value has to be of type 'MultipartFile'"); |  | ||||||
| 
 | 
 | ||||||
|  | 		Assert.state(value instanceof MultipartFile, "Expected MultipartFile value"); | ||||||
| 		MultipartFile file = (MultipartFile) value; | 		MultipartFile file = (MultipartFile) value; | ||||||
| 		requestValues.addRequestPart(name, toHttpEntity(name, file)); |  | ||||||
| 	} |  | ||||||
| 
 | 
 | ||||||
| 	private HttpEntity<Resource> toHttpEntity(String name, MultipartFile file) { |  | ||||||
| 		HttpHeaders headers = new HttpHeaders(); | 		HttpHeaders headers = new HttpHeaders(); | ||||||
| 		if (file.getOriginalFilename() != null) { | 		if (file.getOriginalFilename() != null) { | ||||||
| 			headers.setContentDispositionFormData(name, file.getOriginalFilename()); | 			headers.setContentDispositionFormData(name, file.getOriginalFilename()); | ||||||
|  | @ -64,7 +56,8 @@ public class MultipartFileArgumentResolver extends AbstractNamedValueArgumentRes | ||||||
| 		if (file.getContentType() != null) { | 		if (file.getContentType() != null) { | ||||||
| 			headers.add(HttpHeaders.CONTENT_TYPE, file.getContentType()); | 			headers.add(HttpHeaders.CONTENT_TYPE, file.getContentType()); | ||||||
| 		} | 		} | ||||||
| 		return new HttpEntity<>(file.getResource(), headers); | 
 | ||||||
|  | 		values.addRequestPart(name, new HttpEntity<>(file.getResource(), headers)); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -21,7 +21,6 @@ import java.util.Optional; | ||||||
| import org.junit.jupiter.api.BeforeEach; | import org.junit.jupiter.api.BeforeEach; | ||||||
| import org.junit.jupiter.api.Test; | import org.junit.jupiter.api.Test; | ||||||
| 
 | 
 | ||||||
| import org.springframework.http.ContentDisposition; |  | ||||||
| import org.springframework.http.HttpEntity; | import org.springframework.http.HttpEntity; | ||||||
| import org.springframework.http.HttpHeaders; | import org.springframework.http.HttpHeaders; | ||||||
| import org.springframework.http.MediaType; | import org.springframework.http.MediaType; | ||||||
|  | @ -35,7 +34,8 @@ import static org.assertj.core.api.Assertions.assertThat; | ||||||
| 
 | 
 | ||||||
| /** | /** | ||||||
|  * Unit tests for {@link MultipartFileArgumentResolver}. |  * 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 |  * @author Olga Maciaszek-Sharma | ||||||
|  */ |  */ | ||||||
|  | @ -46,48 +46,49 @@ class MultipartFileArgumentResolverTests { | ||||||
| 
 | 
 | ||||||
| 	private TestClient client; | 	private TestClient client; | ||||||
| 
 | 
 | ||||||
|  | 
 | ||||||
| 	@BeforeEach | 	@BeforeEach | ||||||
| 	void setUp() { | 	void setUp() { | ||||||
| 		HttpServiceProxyFactory proxyFactory = HttpServiceProxyFactory.builder(this.clientAdapter).build(); | 		HttpServiceProxyFactory factory = HttpServiceProxyFactory.builder(this.clientAdapter).build(); | ||||||
| 		this.client = proxyFactory.createClient(TestClient.class); | 		this.client = factory.createClient(TestClient.class); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 
 | ||||||
| 	@Test | 	@Test | ||||||
| 	void multipartFile() { | 	void multipartFile() { | ||||||
| 		String fileName = "testFileName"; | 		String fileName = "testFileName"; | ||||||
| 		String originalFileName = "originalTestFileName"; | 		String originalFileName = "originalTestFileName"; | ||||||
| 		MultipartFile testFile = new MockMultipartFile(fileName, originalFileName, | 		MultipartFile testFile = new MockMultipartFile(fileName, originalFileName, "text/plain", "test".getBytes()); | ||||||
| 				MediaType.APPLICATION_JSON_VALUE, "test".getBytes()); | 
 | ||||||
| 		this.client.postMultipartFile(testFile); | 		this.client.postMultipartFile(testFile); | ||||||
|  | 		Object value = this.clientAdapter.getRequestValues().getBodyValue(); | ||||||
| 
 | 
 | ||||||
| 		Object body = clientAdapter.getRequestValues().getBodyValue(); | 		assertThat(value).isInstanceOf(MultiValueMap.class); | ||||||
|  | 		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value; | ||||||
|  | 		assertThat(map).hasSize(1); | ||||||
| 
 | 
 | ||||||
| 		assertThat(body).isInstanceOf(MultiValueMap.class); | 		HttpEntity<?> entity = map.getFirst("file"); | ||||||
| 		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) body; | 		assertThat(entity).isNotNull(); | ||||||
| 		assertThat(map.size()).isEqualTo(1); | 		assertThat(entity.getBody()).isEqualTo(testFile.getResource()); | ||||||
| 		assertThat(map.getFirst("file")).isNotNull(); | 
 | ||||||
| 		HttpEntity<?> fileEntity = map.getFirst("file"); | 		HttpHeaders headers = entity.getHeaders(); | ||||||
| 		assertThat(fileEntity.getBody()).isEqualTo(testFile.getResource()); | 		assertThat(headers.getContentType()).isEqualTo(MediaType.TEXT_PLAIN); | ||||||
| 		HttpHeaders headers = fileEntity.getHeaders(); | 		assertThat(headers.getContentDisposition().getType()).isEqualTo("form-data"); | ||||||
| 		assertThat(headers.getContentType()).isEqualTo(MediaType.APPLICATION_JSON); | 		assertThat(headers.getContentDisposition().getName()).isEqualTo("file"); | ||||||
| 		ContentDisposition contentDisposition = headers.getContentDisposition(); | 		assertThat(headers.getContentDisposition().getFilename()).isEqualTo(originalFileName); | ||||||
| 		assertThat(contentDisposition.getType()).isEqualTo("form-data"); |  | ||||||
| 		assertThat(contentDisposition.getName()).isEqualTo("file"); |  | ||||||
| 		assertThat(contentDisposition.getFilename()).isEqualTo(originalFileName); |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	@Test | 	@Test | ||||||
| 	void optionalMultipartFile() { | 	void optionalMultipartFile() { | ||||||
| 		this.client.postOptionalMultipartFile(Optional.empty(), "anotherPart"); | 		this.client.postOptionalMultipartFile(Optional.empty(), "anotherPart"); | ||||||
|  | 		Object value = clientAdapter.getRequestValues().getBodyValue(); | ||||||
| 
 | 
 | ||||||
| 		Object body = clientAdapter.getRequestValues().getBodyValue(); | 		assertThat(value).isInstanceOf(MultiValueMap.class); | ||||||
| 
 | 		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) value; | ||||||
| 		assertThat(body).isInstanceOf(MultiValueMap.class); | 		assertThat(map).containsOnlyKeys("anotherPart"); | ||||||
| 		MultiValueMap<String, HttpEntity<?>> map = (MultiValueMap<String, HttpEntity<?>>) body; |  | ||||||
| 		assertThat(map.size()).isEqualTo(1); |  | ||||||
| 		assertThat(map.getFirst("anotherPart")).isNotNull(); |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 
 | ||||||
| 	@SuppressWarnings("OptionalUsedAsFieldOrParameterType") | 	@SuppressWarnings("OptionalUsedAsFieldOrParameterType") | ||||||
| 	private interface TestClient { | 	private interface TestClient { | ||||||
| 
 | 
 | ||||||
|  | @ -98,4 +99,5 @@ class MultipartFileArgumentResolverTests { | ||||||
| 		void postOptionalMultipartFile(Optional<MultipartFile> file, @RequestPart String anotherPart); | 		void postOptionalMultipartFile(Optional<MultipartFile> file, @RequestPart String anotherPart); | ||||||
| 
 | 
 | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -190,7 +190,7 @@ public class WebClientHttpServiceProxyTests { | ||||||
| 		@PostExchange(contentType = "application/x-www-form-urlencoded") | 		@PostExchange(contentType = "application/x-www-form-urlencoded") | ||||||
| 		void postForm(@RequestParam MultiValueMap<String, String> params); | 		void postForm(@RequestParam MultiValueMap<String, String> params); | ||||||
| 
 | 
 | ||||||
| 		@PostExchange(contentType = MediaType.MULTIPART_FORM_DATA_VALUE) | 		@PostExchange | ||||||
| 		void postMultipart(MultipartFile file, @RequestPart String anotherPart); | 		void postMultipart(MultipartFile file, @RequestPart String anotherPart); | ||||||
| 
 | 
 | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue