Skip to content

Commit 8a043ae

Browse files
rstoyanchevrstoyanchev
committed
Replace direct use of Validator and ConversionService
This commit replaces direct use of Validator and ConversionService in the reactive @RequestMapping infrustructure in favor of using the BindingContext. Issue: SPR-14541
1 parent d87aa40 commit 8a043ae

25 files changed

+208
-249
lines changed

‎spring-web-reactive/src/main/java/org/springframework/web/reactive/config/WebReactiveConfiguration.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,18 +252,15 @@ protected void addResourceHandlers(ResourceHandlerRegistry registry) {
252252
@Bean
253253
public RequestMappingHandlerAdapter requestMappingHandlerAdapter() {
254254
RequestMappingHandlerAdapter adapter = createRequestMappingHandlerAdapter();
255+
adapter.setMessageReaders(getMessageReaders());
256+
adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer());
255257

256258
List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>();
257259
addArgumentResolvers(resolvers);
258260
if (!resolvers.isEmpty()) {
259261
adapter.setCustomArgumentResolvers(resolvers);
260262
}
261263

262-
adapter.setMessageReaders(getMessageReaders());
263-
adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer());
264-
adapter.setConversionService(webReactiveConversionService());
265-
adapter.setValidator(webReactiveValidator());
266-
267264
return adapter;
268265
}
269266

‎spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/BindingContext.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@
1717

1818
import reactor.core.publisher.Mono;
1919

20+
import org.springframework.beans.SimpleTypeConverter;
21+
import org.springframework.beans.TypeConverter;
2022
import org.springframework.ui.ModelMap;
2123
import org.springframework.validation.support.BindingAwareModelMap;
2224
import org.springframework.web.bind.WebDataBinder;
2325
import org.springframework.web.bind.WebExchangeDataBinder;
26+
import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
2427
import org.springframework.web.bind.support.WebBindingInitializer;
2528
import org.springframework.web.server.ServerWebExchange;
2629

@@ -37,13 +40,30 @@ public class BindingContext {
3740

3841
private final WebBindingInitializer initializer;
3942

43+
private final TypeConverter typeConverter;
44+
4045

4146
public BindingContext() {
4247
this(null);
4348
}
4449

4550
public BindingContext(WebBindingInitializer initializer) {
4651
this.initializer = initializer;
52+
this.typeConverter = initSimpleTypeConverter(initializer);
53+
}
54+
55+
private static SimpleTypeConverter initSimpleTypeConverter(WebBindingInitializer initializer) {
56+
SimpleTypeConverter converter = new SimpleTypeConverter();
57+
if (initializer instanceof ConfigurableWebBindingInitializer) {
58+
converter.setConversionService(
59+
((ConfigurableWebBindingInitializer) initializer).getConversionService());
60+
}
61+
else if (initializer != null) {
62+
WebDataBinder dataBinder = new WebDataBinder(null);
63+
initializer.initBinder(dataBinder);
64+
converter.setConversionService(dataBinder.getConversionService());
65+
}
66+
return converter;
4767
}
4868

4969

@@ -80,4 +100,15 @@ protected Mono<WebExchangeDataBinder> initBinder(WebExchangeDataBinder dataBinde
80100
return Mono.just(dataBinder);
81101
}
82102

103+
/**
104+
* Return a {@link TypeConverter} for converting plain parameter values.
105+
* This is a shortcut for:
106+
* <pre>
107+
* new WebDataBinder(null).getTypeConverter();
108+
* </pre>
109+
*/
110+
public TypeConverter getTypeConverter() {
111+
return this.typeConverter;
112+
}
113+
83114
}

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

Lines changed: 57 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.lang.annotation.Annotation;
1919
import java.util.Collections;
2020
import java.util.List;
21+
import java.util.Map;
2122
import java.util.function.Function;
2223
import java.util.stream.Collectors;
2324

@@ -36,12 +37,9 @@
3637
import org.springframework.http.server.reactive.ServerHttpRequest;
3738
import org.springframework.http.server.reactive.ServerHttpResponse;
3839
import org.springframework.util.Assert;
39-
import org.springframework.util.ObjectUtils;
40-
import org.springframework.validation.BeanPropertyBindingResult;
41-
import org.springframework.validation.Errors;
42-
import org.springframework.validation.SmartValidator;
4340
import org.springframework.validation.Validator;
4441
import org.springframework.validation.annotation.Validated;
42+
import org.springframework.web.reactive.result.method.BindingContext;
4543
import org.springframework.web.server.ServerWebExchange;
4644
import org.springframework.web.server.ServerWebInputException;
4745
import org.springframework.web.server.UnsupportedMediaTypeStatusException;
@@ -62,8 +60,6 @@ public abstract class AbstractMessageReaderArgumentResolver {
6260

6361
private final List<HttpMessageReader<?>> messageReaders;
6462

65-
private final Validator validator;
66-
6763
private final ReactiveAdapterRegistry adapterRegistry;
6864

6965
private final List<MediaType> supportedMediaTypes;
@@ -72,26 +68,22 @@ public abstract class AbstractMessageReaderArgumentResolver {
7268
/**
7369
* Constructor with {@link HttpMessageReader}'s and a {@link Validator}.
7470
* @param readers readers to convert from the request body
75-
* @param validator validator to validate decoded objects with
7671
*/
77-
protected AbstractMessageReaderArgumentResolver(List<HttpMessageReader<?>> readers, Validator validator) {
78-
79-
this(readers, validator, new ReactiveAdapterRegistry());
72+
protected AbstractMessageReaderArgumentResolver(List<HttpMessageReader<?>> readers) {
73+
this(readers, new ReactiveAdapterRegistry());
8074
}
8175

8276
/**
8377
* Constructor that also accepts a {@link ReactiveAdapterRegistry}.
8478
* @param messageReaders readers to convert from the request body
85-
* @param validator validator to validate decoded objects with
8679
* @param adapterRegistry for adapting to other reactive types from Flux and Mono
8780
*/
8881
protected AbstractMessageReaderArgumentResolver(List<HttpMessageReader<?>> messageReaders,
89-
Validator validator, ReactiveAdapterRegistry adapterRegistry) {
82+
ReactiveAdapterRegistry adapterRegistry) {
9083

9184
Assert.notEmpty(messageReaders, "At least one HttpMessageReader is required.");
9285
Assert.notNull(adapterRegistry, "'adapterRegistry' is required");
9386
this.messageReaders = messageReaders;
94-
this.validator = validator;
9587
this.adapterRegistry = adapterRegistry;
9688
this.supportedMediaTypes = messageReaders.stream()
9789
.flatMap(converter -> converter.getReadableMediaTypes().stream())
@@ -115,7 +107,7 @@ public ReactiveAdapterRegistry getAdapterRegistry() {
115107

116108

117109
protected Mono<Object> readBody(MethodParameter bodyParameter, boolean isBodyRequired,
118-
ServerWebExchange exchange) {
110+
BindingContext bindingContext, ServerWebExchange exchange) {
119111

120112
ResolvableType bodyType = ResolvableType.forMethodParameter(bodyParameter);
121113
ReactiveAdapter adapter = getAdapterRegistry().getAdapterTo(bodyType.resolve());
@@ -135,32 +127,42 @@ protected Mono<Object> readBody(MethodParameter bodyParameter, boolean isBodyReq
135127
for (HttpMessageReader<?> reader : getMessageReaders()) {
136128

137129
if (reader.canRead(elementType, mediaType)) {
138-
130+
Map<String, Object> readHints = Collections.emptyMap();
139131
if (adapter != null && adapter.getDescriptor().isMultiValue()) {
140-
Flux<?> flux = (reader instanceof ServerHttpMessageReader ?
141-
((ServerHttpMessageReader<?>)reader).read(bodyType, elementType,
142-
request, response, Collections.emptyMap()) :
143-
reader.read(elementType, request, Collections.emptyMap())
144-
.onErrorResumeWith(ex -> Flux.error(getReadError(ex, bodyParameter))));
132+
Flux<?> flux;
133+
if (reader instanceof ServerHttpMessageReader) {
134+
ServerHttpMessageReader<?> serverReader = ((ServerHttpMessageReader<?>) reader);
135+
flux = serverReader.read(bodyType, elementType, request, response, readHints);
136+
}
137+
else {
138+
flux = reader.read(elementType, request, readHints);
139+
}
140+
flux = flux.onErrorResumeWith(ex -> Flux.error(wrapReadError(ex, bodyParameter)));
145141
if (checkRequired(adapter, isBodyRequired)) {
146142
flux = flux.switchIfEmpty(Flux.error(getRequiredBodyError(bodyParameter)));
147143
}
148-
if (this.validator != null) {
149-
flux = flux.map(applyValidationIfApplicable(bodyParameter));
144+
Object[] hints = extractValidationHints(bodyParameter);
145+
if (hints != null) {
146+
flux = flux.concatMap(getValidator(hints, bodyParameter, bindingContext, exchange));
150147
}
151148
return Mono.just(adapter.fromPublisher(flux));
152149
}
153150
else {
154-
Mono<?> mono = (reader instanceof ServerHttpMessageReader ?
155-
((ServerHttpMessageReader<?>)reader).readMono(bodyType, elementType,
156-
request, response, Collections.emptyMap()) :
157-
reader.readMono(elementType, request, Collections.emptyMap())
158-
.otherwise(ex -> Mono.error(getReadError(ex, bodyParameter))));
151+
Mono<?> mono;
152+
if (reader instanceof ServerHttpMessageReader) {
153+
ServerHttpMessageReader<?> serverReader = (ServerHttpMessageReader<?>) reader;
154+
mono = serverReader.readMono(bodyType, elementType, request, response, readHints);
155+
}
156+
else {
157+
mono = reader.readMono(elementType, request, readHints);
158+
}
159+
mono = mono.otherwise(ex -> Mono.error(wrapReadError(ex, bodyParameter)));
159160
if (checkRequired(adapter, isBodyRequired)) {
160161
mono = mono.otherwiseIfEmpty(Mono.error(getRequiredBodyError(bodyParameter)));
161162
}
162-
if (this.validator != null) {
163-
mono = mono.map(applyValidationIfApplicable(bodyParameter));
163+
Object[] hints = extractValidationHints(bodyParameter);
164+
if (hints != null) {
165+
mono = mono.then(getValidator(hints, bodyParameter, bindingContext, exchange));
164166
}
165167
if (adapter != null) {
166168
return Mono.just(adapter.fromPublisher(mono));
@@ -175,50 +177,49 @@ protected Mono<Object> readBody(MethodParameter bodyParameter, boolean isBodyReq
175177
return Mono.error(new UnsupportedMediaTypeStatusException(mediaType, this.supportedMediaTypes));
176178
}
177179

178-
protected boolean checkRequired(ReactiveAdapter adapter, boolean isBodyRequired) {
179-
return adapter != null && !adapter.getDescriptor().supportsEmpty() || isBodyRequired;
180+
protected ServerWebInputException wrapReadError(Throwable ex, MethodParameter parameter) {
181+
return new ServerWebInputException("Failed to read HTTP message", parameter, ex);
180182
}
181183

182-
protected ServerWebInputException getReadError(Throwable ex, MethodParameter parameter) {
183-
return new ServerWebInputException("Failed to read HTTP message", parameter, ex);
184+
protected boolean checkRequired(ReactiveAdapter adapter, boolean isBodyRequired) {
185+
return adapter != null && !adapter.getDescriptor().supportsEmpty() || isBodyRequired;
184186
}
185187

186188
protected ServerWebInputException getRequiredBodyError(MethodParameter parameter) {
187189
return new ServerWebInputException("Required request body is missing: " +
188190
parameter.getMethod().toGenericString());
189191
}
190192

191-
protected <T> Function<T, T> applyValidationIfApplicable(MethodParameter methodParam) {
192-
Annotation[] annotations = methodParam.getParameterAnnotations();
193+
/**
194+
* Check if the given MethodParameter requires validation and if so return
195+
* a (possibly empty) Object[] with validation hints. A return value of
196+
* {@code null} indicates that validation is not required.
197+
*/
198+
protected Object[] extractValidationHints(MethodParameter parameter) {
199+
Annotation[] annotations = parameter.getParameterAnnotations();
193200
for (Annotation ann : annotations) {
194201
Validated validAnnot = AnnotationUtils.getAnnotation(ann, Validated.class);
195202
if (validAnnot != null || ann.annotationType().getSimpleName().startsWith("Valid")) {
196203
Object hints = (validAnnot != null ? validAnnot.value() : AnnotationUtils.getValue(ann));
197-
Object[] validHints = (hints instanceof Object[] ? (Object[]) hints : new Object[] {hints});
198-
return element -> {
199-
doValidate(element, validHints, methodParam);
200-
return element;
201-
};
204+
return (hints instanceof Object[] ? (Object[]) hints : new Object[] {hints});
202205
}
203206
}
204-
return element -> element;
207+
return null;
205208
}
206209

207-
/**
208-
* TODO: replace with use of DataBinder
209-
*/
210-
private void doValidate(Object target, Object[] validationHints, MethodParameter methodParam) {
211-
String name = Conventions.getVariableNameForParameter(methodParam);
212-
Errors errors = new BeanPropertyBindingResult(target, name);
213-
if (!ObjectUtils.isEmpty(validationHints) && this.validator instanceof SmartValidator) {
214-
((SmartValidator) this.validator).validate(target, errors, validationHints);
215-
}
216-
else if (this.validator != null) {
217-
this.validator.validate(target, errors);
218-
}
219-
if (errors.hasErrors()) {
220-
throw new ServerWebInputException("Validation failed", methodParam);
221-
}
210+
protected <T> Function<T, Mono<T>> getValidator(Object[] validationHints,
211+
MethodParameter param, BindingContext binding, ServerWebExchange exchange) {
212+
213+
String name = Conventions.getVariableNameForParameter(param);
214+
215+
return target -> binding.createBinder(exchange, target, name)
216+
.map(binder -> {
217+
binder.validate(validationHints);
218+
if (binder.getBindingResult().hasErrors()) {
219+
throw new ServerWebInputException("Validation failed", param);
220+
}
221+
return target;
222+
});
222223
}
223224

224225
}

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

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,13 @@
2222
import reactor.core.publisher.Mono;
2323

2424
import org.springframework.beans.ConversionNotSupportedException;
25-
import org.springframework.beans.SimpleTypeConverter;
25+
import org.springframework.beans.TypeConverter;
2626
import org.springframework.beans.TypeMismatchException;
2727
import org.springframework.beans.factory.config.BeanExpressionContext;
2828
import org.springframework.beans.factory.config.BeanExpressionResolver;
2929
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
3030
import org.springframework.core.MethodParameter;
31-
import org.springframework.core.convert.ConversionService;
3231
import org.springframework.ui.ModelMap;
33-
import org.springframework.util.Assert;
3432
import org.springframework.web.bind.annotation.ValueConstants;
3533
import org.springframework.web.reactive.result.method.BindingContext;
3634
import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver;
@@ -64,22 +62,13 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle
6462

6563
private final Map<MethodParameter, NamedValueInfo> namedValueInfoCache = new ConcurrentHashMap<>(256);
6664

67-
/** Instead of a WebDataBinder for now */
68-
private final SimpleTypeConverter typeConverter;
69-
7065

7166
/**
72-
* @param conversionService for type conversion (to be replaced with WebDataBinder)
7367
* @param beanFactory a bean factory to use for resolving ${...} placeholder
7468
* and #{...} SpEL expressions in default values, or {@code null} if default
7569
* values are not expected to contain expressions
7670
*/
77-
public AbstractNamedValueMethodArgumentResolver(ConversionService conversionService,
78-
ConfigurableBeanFactory beanFactory) {
79-
80-
Assert.notNull(conversionService, "'conversionService' is required.");
81-
this.typeConverter = new SimpleTypeConverter();
82-
this.typeConverter.setConversionService(conversionService);
71+
public AbstractNamedValueMethodArgumentResolver(ConfigurableBeanFactory beanFactory) {
8372
this.configurableBeanFactory = beanFactory;
8473
this.expressionContext = (beanFactory != null ? new BeanExpressionContext(beanFactory, null) : null);
8574
}
@@ -105,11 +94,12 @@ public Mono<Object> resolveArgument(MethodParameter parameter, BindingContext bi
10594
if ("".equals(arg) && namedValueInfo.defaultValue != null) {
10695
arg = resolveStringValue(namedValueInfo.defaultValue);
10796
}
108-
arg = applyConversion(arg, parameter);
97+
arg = applyConversion(arg, parameter, bindingContext);
10998
handleResolvedValue(arg, namedValueInfo.name, parameter, model, exchange);
11099
return arg;
111100
})
112-
.otherwiseIfEmpty(getDefaultValue(namedValueInfo, parameter, model, exchange));
101+
.otherwiseIfEmpty(getDefaultValue(
102+
namedValueInfo, parameter, bindingContext, model, exchange));
113103
}
114104

115105
/**
@@ -179,9 +169,10 @@ private Object resolveStringValue(String value) {
179169
protected abstract Mono<Object> resolveName(String name, MethodParameter parameter,
180170
ServerWebExchange exchange);
181171

182-
private Object applyConversion(Object value, MethodParameter parameter) {
172+
private Object applyConversion(Object value, MethodParameter parameter, BindingContext bindingContext) {
183173
try {
184-
value = this.typeConverter.convertIfNecessary(value, parameter.getParameterType(), parameter);
174+
TypeConverter typeConverter = bindingContext.getTypeConverter();
175+
value = typeConverter.convertIfNecessary(value, parameter.getParameterType(), parameter);
185176
}
186177
catch (ConversionNotSupportedException ex) {
187178
throw new ServerErrorException("Conversion not supported.", parameter, ex);
@@ -193,7 +184,7 @@ private Object applyConversion(Object value, MethodParameter parameter) {
193184
}
194185

195186
private Mono<Object> getDefaultValue(NamedValueInfo namedValueInfo, MethodParameter parameter,
196-
ModelMap model, ServerWebExchange exchange) {
187+
BindingContext bindingContext, ModelMap model, ServerWebExchange exchange) {
197188

198189
Object value = null;
199190
try {
@@ -204,7 +195,7 @@ else if (namedValueInfo.required && !parameter.isOptional()) {
204195
handleMissingValue(namedValueInfo.name, parameter, exchange);
205196
}
206197
value = handleNullValue(namedValueInfo.name, value, parameter.getNestedParameterType());
207-
value = applyConversion(value, parameter);
198+
value = applyConversion(value, parameter, bindingContext);
208199
handleResolvedValue(value, namedValueInfo.name, parameter, model, exchange);
209200
return Mono.justOrEmpty(value);
210201
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
2222
import org.springframework.core.MethodParameter;
23-
import org.springframework.core.convert.ConversionService;
2423
import org.springframework.http.HttpCookie;
2524
import org.springframework.web.bind.annotation.CookieValue;
2625
import org.springframework.web.server.ServerWebExchange;
@@ -44,10 +43,8 @@ public class CookieValueMethodArgumentResolver extends AbstractNamedValueMethodA
4443
* placeholder and #{...} SpEL expressions in default values;
4544
* or {@code null} if default values are not expected to contain expressions
4645
*/
47-
public CookieValueMethodArgumentResolver(ConversionService conversionService,
48-
ConfigurableBeanFactory beanFactory) {
49-
50-
super(conversionService, beanFactory);
46+
public CookieValueMethodArgumentResolver(ConfigurableBeanFactory beanFactory) {
47+
super(beanFactory);
5148
}
5249

5350

0 commit comments

Comments
 (0)