From d37d6688d8dfa47f91395047bee1f369dbf39f32 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 23 Jun 2023 11:15:00 +0100 Subject: [PATCH] WebFlux constructs model attribute via DataBinder See gh-26721 --- .../bind/support/WebExchangeDataBinder.java | 42 ++++- .../web/reactive/BindingContext.java | 71 +++---- .../ModelAttributeMethodArgumentResolver.java | 174 +++++++----------- .../InitBinderBindingContextTests.java | 15 +- 4 files changed, 147 insertions(+), 155 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java index 75d0894dbb..8abcd8e286 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * 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. @@ -71,14 +71,30 @@ public class WebExchangeDataBinder extends WebDataBinder { } + /** + * Use a default or single data constructor to create the target by + * binding request parameters, multipart files, or parts to constructor args. + *

After the call, use {@link #getBindingResult()} to check for bind errors. + * If there are none, the target is set, and {@link #bind} can be called for + * further initialization via setters. + * @param exchange the request to bind + * @return a {@code Mono} that completes when the target is created + * @since 6.1 + */ + public Mono construct(ServerWebExchange exchange) { + return getValuesToBind(exchange) + .doOnNext(map -> construct(new MapValueResolver(map))) + .then(); + } + /** * Bind query parameters, form data, or multipart form data to the binder target. * @param exchange the current exchange - * @return a {@code Mono} when binding is complete + * @return a {@code Mono} that completes when binding is complete */ public Mono bind(ServerWebExchange exchange) { return getValuesToBind(exchange) - .doOnNext(values -> doBind(new MutablePropertyValues(values))) + .doOnNext(map -> doBind(new MutablePropertyValues(map))) .then(); } @@ -128,4 +144,22 @@ public class WebExchangeDataBinder extends WebDataBinder { } } -} + + /** + * Resolve values from a map. + */ + private static class MapValueResolver implements ValueResolver { + + private final Map map; + + private MapValueResolver(Map map) { + this.map = map; + } + + @Override + public Object resolveValue(String name, Class type) { + return this.map.get(name); + } + } + +} \ No newline at end of file diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java index 96379aa2a8..f336047019 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java @@ -24,6 +24,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapterRegistry; +import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; import org.springframework.ui.Model; import org.springframework.validation.DataBinder; @@ -92,8 +93,7 @@ public class BindingContext { /** - * Create a {@link WebExchangeDataBinder} to apply data binding and - * validation with on the target, command object. + * Create a binder with a target object. * @param exchange the current exchange * @param target the object to create a data binder for * @param name the name of the target object @@ -101,11 +101,44 @@ public class BindingContext { * @throws ServerErrorException if {@code @InitBinder} method invocation fails */ public WebExchangeDataBinder createDataBinder(ServerWebExchange exchange, @Nullable Object target, String name) { + return createDataBinder(exchange, target, name, null); + } + + /** + * Shortcut method to create a binder without a target object. + * @param exchange the current exchange + * @param name the name of the target object + * @return the created data binder + * @throws ServerErrorException if {@code @InitBinder} method invocation fails + */ + public WebExchangeDataBinder createDataBinder(ServerWebExchange exchange, String name) { + return createDataBinder(exchange, null, name, null); + } + + /** + * Create a binder with a target object and a {@code MethodParameter}. + * If the target is {@code null}, then + * {@link WebExchangeDataBinder#setTargetType targetType} is set. + * @since 6.1 + */ + public WebExchangeDataBinder createDataBinder( + ServerWebExchange exchange, @Nullable Object target, String name, @Nullable MethodParameter parameter) { + WebExchangeDataBinder dataBinder = new ExtendedWebExchangeDataBinder(target, name); + if (target == null && parameter != null) { + dataBinder.setTargetType(ResolvableType.forMethodParameter(parameter)); + } + if (this.initializer != null) { this.initializer.initBinder(dataBinder); } - return initDataBinder(dataBinder, exchange); + dataBinder = initDataBinder(dataBinder, exchange); + + if (this.methodValidationApplicable && parameter != null) { + MethodValidationInitializer.initBinder(dataBinder, parameter); + } + + return dataBinder; } /** @@ -116,36 +149,6 @@ public class BindingContext { return binder; } - /** - * Create a {@link WebExchangeDataBinder} without a target object for type - * conversion of request values to simple types. - * @param exchange the current exchange - * @param name the name of the target object - * @return the created data binder - * @throws ServerErrorException if {@code @InitBinder} method invocation fails - */ - public WebExchangeDataBinder createDataBinder(ServerWebExchange exchange, String name) { - return createDataBinder(exchange, null, name); - } - - /** - * Variant of {@link #createDataBinder(ServerWebExchange, Object, String)} - * with a {@link MethodParameter} for which the {@code DataBinder} is created. - * That may provide more insight to initialize the {@link WebExchangeDataBinder}. - *

By default, if the parameter has {@code @Valid}, Bean Validation is - * excluded, deferring to method validation. - * @since 6.1 - */ - public WebExchangeDataBinder createDataBinder( - ServerWebExchange exchange, @Nullable Object target, String name, MethodParameter parameter) { - - WebExchangeDataBinder dataBinder = createDataBinder(exchange, target, name); - if (this.methodValidationApplicable) { - MethodValidationInitializer.updateBinder(dataBinder, parameter); - } - return dataBinder; - } - /** * Extended variant of {@link WebExchangeDataBinder}, adding path variables. @@ -170,7 +173,7 @@ public class BindingContext { */ private static class MethodValidationInitializer { - public static void updateBinder(DataBinder binder, MethodParameter parameter) { + public static void initBinder(DataBinder binder, MethodParameter parameter) { if (ReactiveAdapterRegistry.getSharedInstance().getAdapter(parameter.getParameterType()) == null) { for (Annotation annotation : parameter.getParameterAnnotations()) { if (annotation.annotationType().getName().equals("jakarta.validation.Valid")) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java index 32c005ddc6..d9987cc9f8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java @@ -17,10 +17,7 @@ package org.springframework.web.reactive.result.method.annotation; import java.lang.annotation.Annotation; -import java.lang.reflect.Constructor; -import java.util.List; import java.util.Map; -import java.util.Optional; import reactor.core.publisher.Mono; import reactor.core.publisher.Sinks; @@ -31,7 +28,6 @@ import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; -import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; import org.springframework.ui.Model; import org.springframework.util.Assert; @@ -100,72 +96,74 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR public Mono resolveArgument( MethodParameter parameter, BindingContext context, ServerWebExchange exchange) { - ResolvableType type = ResolvableType.forMethodParameter(parameter); - Class resolvedType = type.resolve(); - ReactiveAdapter adapter = (resolvedType != null ? getAdapterRegistry().getAdapter(resolvedType) : null); - ResolvableType valueType = (adapter != null ? type.getGeneric() : type); - - Assert.state(adapter == null || !adapter.isMultiValue(), - () -> getClass().getSimpleName() + " does not support multi-value reactive type wrapper: " + - parameter.getGenericParameterType()); + Class resolvedType = parameter.getParameterType(); + ReactiveAdapter adapter = getAdapterRegistry().getAdapter(resolvedType); + Assert.state(adapter == null || !adapter.isMultiValue(), "Multi-value publisher is not supported"); String name = ModelInitializer.getNameForParameter(parameter); - Mono attributeMono = prepareAttributeMono(name, valueType, context, exchange); - // unsafe(): we're intercepting, already serialized Publisher signals + Mono dataBinderMono = initDataBinder( + name, (adapter != null ? parameter.nested() : parameter), context, exchange); + + // unsafe() is OK: source is Reactive Streams Publisher Sinks.One bindingResultSink = Sinks.unsafe().one(); Map model = context.getModel().asMap(); model.put(BindingResult.MODEL_KEY_PREFIX + name, bindingResultSink.asMono()); - return attributeMono.flatMap(attribute -> { - WebExchangeDataBinder binder = context.createDataBinder(exchange, attribute, name, parameter); - return (!bindingDisabled(parameter) ? bindRequestParameters(binder, exchange) : Mono.empty()) - .doOnError(bindingResultSink::tryEmitError) - .doOnSuccess(aVoid -> { - validateIfApplicable(binder, parameter, exchange); - BindingResult bindingResult = binder.getBindingResult(); - model.put(BindingResult.MODEL_KEY_PREFIX + name, bindingResult); - model.put(name, attribute); - // Ignore result: serialized and buffered (should never fail) - bindingResultSink.tryEmitValue(bindingResult); - }) - .then(Mono.fromCallable(() -> { - BindingResult errors = binder.getBindingResult(); - if (adapter != null) { - return adapter.fromPublisher(errors.hasErrors() ? - Mono.error(new WebExchangeBindException(parameter, errors)) : attributeMono); - } - else { - if (errors.hasErrors() && !hasErrorsArgument(parameter)) { - throw new WebExchangeBindException(parameter, errors); - } - return attribute; - } - })); - }); + return dataBinderMono + .flatMap(binder -> { + Object attribute = binder.getTarget(); + Assert.state(attribute != null, "Expected model attribute instance"); + return (!bindingDisabled(parameter) ? bindRequestParameters(binder, exchange) : Mono.empty()) + .doOnError(bindingResultSink::tryEmitError) + .doOnSuccess(aVoid -> { + validateIfApplicable(binder, parameter, exchange); + BindingResult bindingResult = binder.getBindingResult(); + model.put(BindingResult.MODEL_KEY_PREFIX + name, bindingResult); + model.put(name, attribute); + // Ignore result: serialized and buffered (should never fail) + bindingResultSink.tryEmitValue(bindingResult); + }) + .then(Mono.fromCallable(() -> { + BindingResult errors = binder.getBindingResult(); + if (adapter != null) { + Mono mono = (errors.hasErrors() ? + Mono.error(new WebExchangeBindException(parameter, errors)) : + Mono.just(attribute)); + return adapter.fromPublisher(mono); + } + else { + if (errors.hasErrors() && !hasErrorsArgument(parameter)) { + throw new WebExchangeBindException(parameter, errors); + } + return attribute; + } + })); + }); } - private Mono prepareAttributeMono( - String name, ResolvableType type, BindingContext context, ServerWebExchange exchange) { + private Mono initDataBinder( + String name, MethodParameter parameter, BindingContext context, ServerWebExchange exchange) { - Object attribute = context.getModel().asMap().get(name); - - if (attribute == null) { - attribute = removeReactiveAttribute(context.getModel(), name); + Object value = context.getModel().asMap().get(name); + if (value == null) { + value = removeReactiveAttribute(name, context.getModel()); } - - if (attribute == null) { - return createAttribute(name, type.toClass(), context, exchange); + if (value != null) { + ReactiveAdapter adapter = getAdapterRegistry().getAdapter(null, value); + Assert.isTrue(adapter == null || !adapter.isMultiValue(), "Multi-value publisher is not supported"); + return (adapter != null ? Mono.from(adapter.toPublisher(value)) : Mono.just(value)) + .map(attr -> context.createDataBinder(exchange, attr, name, parameter)); + } + else { + WebExchangeDataBinder binder = context.createDataBinder(exchange, null, name, parameter); + return constructAttribute(binder, exchange).thenReturn(binder); } - - ReactiveAdapter adapter = getAdapterRegistry().getAdapter(null, attribute); - Assert.isTrue(adapter == null || !adapter.isMultiValue(), "Model attribute must be single-value publisher"); - return (adapter != null ? Mono.from(adapter.toPublisher(attribute)) : Mono.justOrEmpty(attribute)); } @Nullable - private Object removeReactiveAttribute(Model model, String name) { + private Object removeReactiveAttribute(String name, Model model) { for (Map.Entry entry : model.asMap().entrySet()) { if (entry.getKey().startsWith(name)) { ReactiveAdapter adapter = getAdapterRegistry().getAdapter(null, entry.getValue()); @@ -181,66 +179,24 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR return null; } - private Mono createAttribute( - String attributeName, Class clazz, BindingContext context, ServerWebExchange exchange) { - - Constructor ctor = BeanUtils.getResolvableConstructor(clazz); - return constructAttribute(ctor, attributeName, context, exchange); - } - - private Mono constructAttribute(Constructor ctor, String attributeName, - BindingContext context, ServerWebExchange exchange) { - - if (ctor.getParameterCount() == 0) { - // A single default constructor -> clearly a standard JavaBeans arrangement. - return Mono.just(BeanUtils.instantiateClass(ctor)); - } - - // A single data class constructor -> resolve constructor arguments from request parameters. - WebExchangeDataBinder binder = context.createDataBinder(exchange, null, attributeName); - return getValuesToBind(binder, exchange).map(bindValues -> { - String[] paramNames = BeanUtils.getParameterNames(ctor); - Class[] paramTypes = ctor.getParameterTypes(); - Object[] args = new Object[paramTypes.length]; - String fieldDefaultPrefix = binder.getFieldDefaultPrefix(); - String fieldMarkerPrefix = binder.getFieldMarkerPrefix(); - for (int i = 0; i < paramNames.length; i++) { - String paramName = paramNames[i]; - Class paramType = paramTypes[i]; - Object value = bindValues.get(paramName); - if (value == null) { - if (fieldDefaultPrefix != null) { - value = bindValues.get(fieldDefaultPrefix + paramName); - } - if (value == null && fieldMarkerPrefix != null) { - if (bindValues.get(fieldMarkerPrefix + paramName) != null) { - value = binder.getEmptyValue(paramType); - } - } - } - value = (value instanceof List list ? list.toArray() : value); - MethodParameter methodParam = MethodParameter.forFieldAwareConstructor(ctor, i, paramName); - if (value == null && methodParam.isOptional()) { - args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null); - } - else { - args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam); - } - } - return BeanUtils.instantiateClass(ctor, args); - }); + /** + * Protected method to obtain the values for data binding. + * @deprecated and not called; replaced by built-in support for + * constructor initialization in {@link org.springframework.validation.DataBinder} + */ + @Deprecated(since = "6.1", forRemoval = true) + public Mono> getValuesToBind(WebExchangeDataBinder binder, ServerWebExchange exchange) { + throw new UnsupportedOperationException(); } /** - * Protected method to obtain the values for data binding. By default this - * method delegates to {@link WebExchangeDataBinder#getValuesToBind}. - * @param binder the data binder in use + * Extension point to create the attribute, binding the request to constructor args. + * @param binder the data binder instance to use for the binding * @param exchange the current exchange - * @return a map of bind values - * @since 5.3 + * @since 6.1 */ - public Mono> getValuesToBind(WebExchangeDataBinder binder, ServerWebExchange exchange) { - return binder.getValuesToBind(exchange); + protected Mono constructAttribute(WebExchangeDataBinder binder, ServerWebExchange exchange) { + return binder.construct(exchange); } /** diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java index 96c45cac0d..213ce83326 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java @@ -56,7 +56,7 @@ public class InitBinderBindingContextTests { public void createBinder() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); BindingContext context = createBindingContext("initBinder", WebDataBinder.class); - WebDataBinder dataBinder = context.createDataBinder(exchange, null, null); + WebDataBinder dataBinder = context.createDataBinder(exchange, null); assertThat(dataBinder.getDisallowedFields()).isNotNull(); assertThat(dataBinder.getDisallowedFields()[0]).isEqualTo("id"); @@ -69,7 +69,7 @@ public class InitBinderBindingContextTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); BindingContext context = createBindingContext("initBinder", WebDataBinder.class); - WebDataBinder dataBinder = context.createDataBinder(exchange, null, null); + WebDataBinder dataBinder = context.createDataBinder(exchange, null); assertThat(dataBinder.getConversionService()).isSameAs(conversionService); } @@ -78,7 +78,7 @@ public class InitBinderBindingContextTests { public void createBinderWithAttrName() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); BindingContext context = createBindingContext("initBinderWithAttributeName", WebDataBinder.class); - WebDataBinder dataBinder = context.createDataBinder(exchange, null, "foo"); + WebDataBinder dataBinder = context.createDataBinder(exchange, "foo"); assertThat(dataBinder.getDisallowedFields()).isNotNull(); assertThat(dataBinder.getDisallowedFields()[0]).isEqualTo("id"); @@ -88,7 +88,7 @@ public class InitBinderBindingContextTests { public void createBinderWithAttrNameNoMatch() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); BindingContext context = createBindingContext("initBinderWithAttributeName", WebDataBinder.class); - WebDataBinder dataBinder = context.createDataBinder(exchange, null, "invalidName"); + WebDataBinder dataBinder = context.createDataBinder(exchange, "invalidName"); assertThat(dataBinder.getDisallowedFields()).isNull(); } @@ -97,7 +97,7 @@ public class InitBinderBindingContextTests { public void createBinderNullAttrName() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); BindingContext context = createBindingContext("initBinderWithAttributeName", WebDataBinder.class); - WebDataBinder dataBinder = context.createDataBinder(exchange, null, null); + WebDataBinder dataBinder = context.createDataBinder(exchange, null); assertThat(dataBinder.getDisallowedFields()).isNull(); } @@ -106,8 +106,7 @@ public class InitBinderBindingContextTests { public void returnValueNotExpected() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); BindingContext context = createBindingContext("initBinderReturnValue", WebDataBinder.class); - assertThatIllegalStateException().isThrownBy(() -> - context.createDataBinder(exchange, null, "invalidName")); + assertThatIllegalStateException().isThrownBy(() -> context.createDataBinder(exchange, "invalidName")); } @Test @@ -118,7 +117,7 @@ public class InitBinderBindingContextTests { this.argumentResolvers.add(new RequestParamMethodArgumentResolver(null, adapterRegistry, false)); BindingContext context = createBindingContext("initBinderTypeConversion", WebDataBinder.class, int.class); - WebDataBinder dataBinder = context.createDataBinder(exchange, null, "foo"); + WebDataBinder dataBinder = context.createDataBinder(exchange, "foo"); assertThat(dataBinder.getDisallowedFields()).isNotNull(); assertThat(dataBinder.getDisallowedFields()[0]).isEqualToIgnoringCase("requestParam-22");