Skip to content

Commit ff9daa9

Browse files
rstoyanchevrstoyanchev
committed
Adjust WebFlux behavior for @RequestPart List<T>
List<T> support was added relatively late, incorrectly decoding each part to T which means no way to decode a single part to List<T> and thatis the most common case (vs multipart parts with the same name). This behavior was further misaligned with Spring MVC as well as with the behavior for T[]. Closes gh-22973
1 parent 01fa923 commit ff9daa9

File tree

2 files changed

+92
-82
lines changed

2 files changed

+92
-82
lines changed

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java

+63-52
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.web.reactive.result.method.annotation;
1818

19+
import java.util.Collection;
1920
import java.util.Collections;
2021
import java.util.List;
2122

@@ -33,6 +34,7 @@
3334
import org.springframework.http.server.reactive.ServerHttpRequestDecorator;
3435
import org.springframework.lang.Nullable;
3536
import org.springframework.util.CollectionUtils;
37+
import org.springframework.util.StringUtils;
3638
import org.springframework.web.bind.annotation.RequestPart;
3739
import org.springframework.web.reactive.BindingContext;
3840
import org.springframework.web.server.ServerWebExchange;
@@ -69,77 +71,86 @@ public Mono<Object> resolveArgument(
6971

7072
RequestPart requestPart = parameter.getParameterAnnotation(RequestPart.class);
7173
boolean isRequired = (requestPart == null || requestPart.required());
72-
String name = getPartName(parameter, requestPart);
73-
74-
Flux<Part> parts = exchange.getMultipartData()
75-
.flatMapIterable(map -> {
76-
List<Part> list = map.get(name);
77-
if (CollectionUtils.isEmpty(list)) {
78-
if (isRequired) {
79-
throw getMissingPartException(name, parameter);
80-
}
81-
return Collections.emptyList();
82-
}
83-
return list;
84-
});
74+
Class<?> paramType = parameter.getParameterType();
75+
Flux<Part> partValues = getPartValues(parameter, requestPart, isRequired, exchange);
8576

86-
if (Part.class.isAssignableFrom(parameter.getParameterType())) {
87-
return parts.next().cast(Object.class);
77+
if (Part.class.isAssignableFrom(paramType)) {
78+
return partValues.next().cast(Object.class);
8879
}
8980

90-
if (List.class.isAssignableFrom(parameter.getParameterType())) {
81+
if (Collection.class.isAssignableFrom(paramType) || List.class.isAssignableFrom(paramType)) {
9182
MethodParameter elementType = parameter.nested();
9283
if (Part.class.isAssignableFrom(elementType.getNestedParameterType())) {
93-
return parts.collectList().cast(Object.class);
84+
return partValues.collectList().cast(Object.class);
9485
}
9586
else {
96-
return decodePartValues(parts, elementType, bindingContext, exchange, isRequired)
97-
.collectList().cast(Object.class);
87+
return partValues.next()
88+
.flatMap(part -> decode(part, parameter, bindingContext, exchange, isRequired))
89+
.defaultIfEmpty(Collections.emptyList());
9890
}
9991
}
10092

101-
ReactiveAdapter adapter = getAdapterRegistry().getAdapter(parameter.getParameterType());
102-
if (adapter != null) {
103-
MethodParameter elementType = parameter.nested();
104-
return Mono.just(adapter.fromPublisher(
105-
Part.class.isAssignableFrom(elementType.getNestedParameterType()) ?
106-
parts : decodePartValues(parts, elementType, bindingContext, exchange, isRequired)));
93+
ReactiveAdapter adapter = getAdapterRegistry().getAdapter(paramType);
94+
if (adapter == null) {
95+
return partValues.next().flatMap(part ->
96+
decode(part, parameter, bindingContext, exchange, isRequired));
10797
}
10898

109-
return decodePartValues(parts, parameter, bindingContext, exchange, isRequired)
110-
.next().cast(Object.class);
111-
}
112-
113-
private String getPartName(MethodParameter methodParam, @Nullable RequestPart requestPart) {
114-
String partName = (requestPart != null ? requestPart.name() : "");
115-
if (partName.isEmpty()) {
116-
partName = methodParam.getParameterName();
117-
if (partName == null) {
118-
throw new IllegalArgumentException("Request part name for argument type [" +
119-
methodParam.getNestedParameterType().getName() +
120-
"] not specified, and parameter name information not found in class file either.");
121-
}
99+
MethodParameter elementType = parameter.nested();
100+
if (Part.class.isAssignableFrom(elementType.getNestedParameterType())) {
101+
return Mono.just(adapter.fromPublisher(partValues));
122102
}
123-
return partName;
103+
104+
Flux<?> flux = partValues.flatMap(part -> decode(part, elementType, bindingContext, exchange, isRequired));
105+
return Mono.just(adapter.fromPublisher(flux));
124106
}
125107

126-
private ServerWebInputException getMissingPartException(String name, MethodParameter param) {
127-
String reason = "Required request part '" + name + "' is not present";
128-
return new ServerWebInputException(reason, param);
108+
public Flux<Part> getPartValues(
109+
MethodParameter parameter, @Nullable RequestPart requestPart, boolean isRequired,
110+
ServerWebExchange exchange) {
111+
112+
String name = getPartName(parameter, requestPart);
113+
return exchange.getMultipartData()
114+
.flatMapIterable(map -> {
115+
List<Part> list = map.get(name);
116+
if (CollectionUtils.isEmpty(list)) {
117+
if (isRequired) {
118+
String reason = "Required request part '" + name + "' is not present";
119+
throw new ServerWebInputException(reason, parameter);
120+
}
121+
return Collections.emptyList();
122+
}
123+
return list;
124+
});
129125
}
130126

127+
private String getPartName(MethodParameter methodParam, @Nullable RequestPart requestPart) {
128+
String name = null;
129+
if (requestPart != null) {
130+
name = requestPart.name();
131+
}
132+
if (StringUtils.isEmpty(name)) {
133+
name = methodParam.getParameterName();
134+
}
135+
if (StringUtils.isEmpty(name)) {
136+
throw new IllegalArgumentException("Request part name for argument type [" +
137+
methodParam.getNestedParameterType().getName() +
138+
"] not specified, and parameter name information not found in class file either.");
139+
}
140+
return name;
141+
}
131142

132-
private Flux<?> decodePartValues(Flux<Part> parts, MethodParameter elementType, BindingContext bindingContext,
143+
@SuppressWarnings("unchecked")
144+
private <T> Mono<T> decode(
145+
Part part, MethodParameter elementType, BindingContext bindingContext,
133146
ServerWebExchange exchange, boolean isRequired) {
134147

135-
return parts.flatMap(part -> {
136-
ServerHttpRequest partRequest = new PartServerHttpRequest(exchange.getRequest(), part);
137-
ServerWebExchange partExchange = exchange.mutate().request(partRequest).build();
138-
if (logger.isDebugEnabled()) {
139-
logger.debug(exchange.getLogPrefix() + "Decoding part '" + part.name() + "'");
140-
}
141-
return readBody(elementType, isRequired, bindingContext, partExchange);
142-
});
148+
ServerHttpRequest partRequest = new PartServerHttpRequest(exchange.getRequest(), part);
149+
ServerWebExchange partExchange = exchange.mutate().request(partRequest).build();
150+
if (logger.isDebugEnabled()) {
151+
logger.debug(exchange.getLogPrefix() + "Decoding part '" + part.name() + "'");
152+
}
153+
return (Mono<T>) readBody(elementType, isRequired, bindingContext, partExchange);
143154
}
144155

145156

‎spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java

+29-30
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.web.reactive.result.method.annotation;
1818

1919
import java.time.Duration;
20+
import java.util.Arrays;
2021
import java.util.Collections;
2122
import java.util.List;
2223

@@ -61,17 +62,17 @@
6162
* @author Rossen Stoyanchev
6263
* @author Ilya Lukyanovich
6364
*/
64-
public class RequestPartMethodArgumentResolverTests {
65+
class RequestPartMethodArgumentResolverTests {
6566

6667
private RequestPartMethodArgumentResolver resolver;
6768

68-
private ResolvableMethod testMethod = ResolvableMethod.on(getClass()).named("handle").build();
69+
private final ResolvableMethod testMethod = ResolvableMethod.on(getClass()).named("handle").build();
6970

7071
private MultipartHttpMessageWriter writer;
7172

7273

7374
@BeforeEach
74-
public void setup() throws Exception {
75+
void setup() {
7576
List<HttpMessageReader<?>> readers = ServerCodecConfigurer.create().getReaders();
7677
ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance();
7778
this.resolver = new RequestPartMethodArgumentResolver(readers, registry);
@@ -82,7 +83,7 @@ public void setup() throws Exception {
8283

8384

8485
@Test
85-
public void supportsParameter() {
86+
void supportsParameter() {
8687

8788
MethodParameter param;
8889

@@ -110,7 +111,7 @@ public void supportsParameter() {
110111

111112

112113
@Test
113-
public void person() {
114+
void person() {
114115
MethodParameter param = this.testMethod.annot(requestPart()).arg(Person.class);
115116
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
116117
bodyBuilder.part("name", new Person("Jones"));
@@ -120,19 +121,18 @@ public void person() {
120121
}
121122

122123
@Test
123-
public void listPerson() {
124+
void listPerson() {
124125
MethodParameter param = this.testMethod.annot(requestPart()).arg(List.class, Person.class);
125126
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
126-
bodyBuilder.part("name", new Person("Jones"));
127-
bodyBuilder.part("name", new Person("James"));
127+
bodyBuilder.part("name", Arrays.asList(new Person("Jones"), new Person("James")));
128128
List<Person> actual = resolveArgument(param, bodyBuilder);
129129

130130
assertThat(actual.get(0).getName()).isEqualTo("Jones");
131131
assertThat(actual.get(1).getName()).isEqualTo("James");
132132
}
133133

134134
@Test // gh-23060
135-
public void listPersonNotRequired() {
135+
void listPersonNotRequired() {
136136
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(List.class, Person.class);
137137
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
138138
List<Person> actual = resolveArgument(param, bodyBuilder);
@@ -141,7 +141,7 @@ public void listPersonNotRequired() {
141141
}
142142

143143
@Test
144-
public void monoPerson() {
144+
void monoPerson() {
145145
MethodParameter param = this.testMethod.annot(requestPart()).arg(Mono.class, Person.class);
146146
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
147147
bodyBuilder.part("name", new Person("Jones"));
@@ -151,7 +151,7 @@ public void monoPerson() {
151151
}
152152

153153
@Test // gh-23060
154-
public void monoPersonNotRequired() {
154+
void monoPersonNotRequired() {
155155
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Mono.class, Person.class);
156156
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
157157
Mono<Person> actual = resolveArgument(param, bodyBuilder);
@@ -160,7 +160,7 @@ public void monoPersonNotRequired() {
160160
}
161161

162162
@Test
163-
public void fluxPerson() {
163+
void fluxPerson() {
164164
MethodParameter param = this.testMethod.annot(requestPart()).arg(Flux.class, Person.class);
165165
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
166166
bodyBuilder.part("name", new Person("Jones"));
@@ -173,7 +173,7 @@ public void fluxPerson() {
173173
}
174174

175175
@Test // gh-23060
176-
public void fluxPersonNotRequired() {
176+
void fluxPersonNotRequired() {
177177
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Flux.class, Person.class);
178178
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
179179
Flux<Person> actual = resolveArgument(param, bodyBuilder);
@@ -182,7 +182,7 @@ public void fluxPersonNotRequired() {
182182
}
183183

184184
@Test
185-
public void part() {
185+
void part() {
186186
MethodParameter param = this.testMethod.annot(requestPart()).arg(Part.class);
187187
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
188188
bodyBuilder.part("name", new Person("Jones"));
@@ -193,7 +193,7 @@ public void part() {
193193
}
194194

195195
@Test
196-
public void listPart() {
196+
void listPart() {
197197
MethodParameter param = this.testMethod.annot(requestPart()).arg(List.class, Part.class);
198198
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
199199
bodyBuilder.part("name", new Person("Jones"));
@@ -205,7 +205,7 @@ public void listPart() {
205205
}
206206

207207
@Test // gh-23060
208-
public void listPartNotRequired() {
208+
void listPartNotRequired() {
209209
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(List.class, Part.class);
210210
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
211211
List<Part> actual = resolveArgument(param, bodyBuilder);
@@ -214,7 +214,7 @@ public void listPartNotRequired() {
214214
}
215215

216216
@Test
217-
public void monoPart() {
217+
void monoPart() {
218218
MethodParameter param = this.testMethod.annot(requestPart()).arg(Mono.class, Part.class);
219219
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
220220
bodyBuilder.part("name", new Person("Jones"));
@@ -225,7 +225,7 @@ public void monoPart() {
225225
}
226226

227227
@Test // gh-23060
228-
public void monoPartNotRequired() {
228+
void monoPartNotRequired() {
229229
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Mono.class, Part.class);
230230
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
231231
Mono<Part> actual = resolveArgument(param, bodyBuilder);
@@ -234,7 +234,7 @@ public void monoPartNotRequired() {
234234
}
235235

236236
@Test
237-
public void fluxPart() {
237+
void fluxPart() {
238238
MethodParameter param = this.testMethod.annot(requestPart()).arg(Flux.class, Part.class);
239239
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
240240
bodyBuilder.part("name", new Person("Jones"));
@@ -247,7 +247,7 @@ public void fluxPart() {
247247
}
248248

249249
@Test // gh-23060
250-
public void fluxPartNotRequired() {
250+
void fluxPartNotRequired() {
251251
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Flux.class, Part.class);
252252
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
253253
Flux<Part> actual = resolveArgument(param, bodyBuilder);
@@ -256,7 +256,7 @@ public void fluxPartNotRequired() {
256256
}
257257

258258
@Test
259-
public void personRequired() {
259+
void personRequired() {
260260
MethodParameter param = this.testMethod.annot(requestPart()).arg(Person.class);
261261
ServerWebExchange exchange = createExchange(new MultipartBodyBuilder());
262262
Mono<Object> result = this.resolver.resolveArgument(param, new BindingContext(), exchange);
@@ -265,7 +265,7 @@ public void personRequired() {
265265
}
266266

267267
@Test
268-
public void personNotRequired() {
268+
void personNotRequired() {
269269
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Person.class);
270270
ServerWebExchange exchange = createExchange(new MultipartBodyBuilder());
271271
Mono<Object> result = this.resolver.resolveArgument(param, new BindingContext(), exchange);
@@ -274,7 +274,7 @@ public void personNotRequired() {
274274
}
275275

276276
@Test
277-
public void partRequired() {
277+
void partRequired() {
278278
MethodParameter param = this.testMethod.annot(requestPart()).arg(Part.class);
279279
ServerWebExchange exchange = createExchange(new MultipartBodyBuilder());
280280
Mono<Object> result = this.resolver.resolveArgument(param, new BindingContext(), exchange);
@@ -283,7 +283,7 @@ public void partRequired() {
283283
}
284284

285285
@Test
286-
public void partNotRequired() {
286+
void partNotRequired() {
287287
MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Part.class);
288288
ServerWebExchange exchange = createExchange(new MultipartBodyBuilder());
289289
Mono<Object> result = this.resolver.resolveArgument(param, new BindingContext(), exchange);
@@ -308,16 +308,15 @@ private ServerWebExchange createExchange(MultipartBodyBuilder builder) {
308308
this.writer.write(Mono.just(builder.build()), forClass(MultiValueMap.class),
309309
MediaType.MULTIPART_FORM_DATA, clientRequest, Collections.emptyMap()).block();
310310

311-
MockServerHttpRequest serverRequest = MockServerHttpRequest.post("/")
312-
.contentType(clientRequest.getHeaders().getContentType())
313-
.body(clientRequest.getBody());
311+
MediaType contentType = clientRequest.getHeaders().getContentType();
312+
Flux<DataBuffer> body = clientRequest.getBody();
313+
MockServerHttpRequest serverRequest = MockServerHttpRequest.post("/").contentType(contentType).body(body);
314314

315315
return MockServerWebExchange.from(serverRequest);
316316
}
317317

318318
private String partToUtf8String(Part part) {
319-
DataBuffer buffer = DataBufferUtils.join(part.content()).block();
320-
return buffer.toString(UTF_8);
319+
return DataBufferUtils.join(part.content()).block().toString(UTF_8);
321320
}
322321

323322

@@ -344,7 +343,7 @@ void handle(
344343

345344
private static class Person {
346345

347-
private String name;
346+
private final String name;
348347

349348
@JsonCreator
350349
public Person(@JsonProperty("name") String name) {

0 commit comments

Comments
 (0)