From 74972fb75139d46fa6873717728eca15e49c0558 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 11 Jul 2023 16:32:44 +0100 Subject: [PATCH] Add updateModel to BindingContext The method includes logic that is currently in ViewResolutionResultHandler but fits well in BindingContext and also includes the call to saveModel method from the InitBinderBindingContext subclass, which was called too early until now from RequestMappingHandlerAdapter before the model has been fully updated. This mirrors a similar method in ModelFactory on the Spring MVC side which also combines those two tasks. Closes gh-30821 --- .../web/reactive/BindingContext.java | 47 +++++++++++++++++-- .../annotation/InitBinderBindingContext.java | 19 ++++---- .../RequestMappingHandlerAdapter.java | 9 ++-- .../view/ViewResolutionResultHandler.java | 28 +---------- .../InitBinderBindingContextTests.java | 3 +- .../annotation/ModelInitializerTests.java | 9 ++-- 6 files changed, 68 insertions(+), 47 deletions(-) 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 803bb7d905..9906303d1b 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 @@ -17,16 +17,19 @@ package org.springframework.web.reactive; import java.lang.annotation.Annotation; +import java.util.Collection; import java.util.Collections; import java.util.Map; import reactor.core.publisher.Mono; +import org.springframework.beans.BeanUtils; 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.BindingResult; import org.springframework.validation.DataBinder; import org.springframework.validation.support.BindingAwareConcurrentModel; import org.springframework.web.bind.support.WebBindingInitializer; @@ -57,20 +60,30 @@ public class BindingContext { private boolean methodValidationApplicable; + private final ReactiveAdapterRegistry reactiveAdapterRegistry; + /** - * Create a new {@code BindingContext}. + * Create an instance without an initializer. */ public BindingContext() { this(null); } /** - * Create a new {@code BindingContext} with the given initializer. - * @param initializer the binding initializer to apply (may be {@code null}) + * Create an instance with the given initializer, which may be {@code null}. */ public BindingContext(@Nullable WebBindingInitializer initializer) { + this(initializer, ReactiveAdapterRegistry.getSharedInstance()); + } + + /** + * Create an instance with the given initializer and {@code ReactiveAdapterRegistry}. + * @since 6.1 + */ + public BindingContext(@Nullable WebBindingInitializer initializer, ReactiveAdapterRegistry registry) { this.initializer = initializer; + this.reactiveAdapterRegistry = new ReactiveAdapterRegistry(); } @@ -151,6 +164,34 @@ public class BindingContext { return binder; } + /** + * Invoked before rendering to add {@link BindingResult} attributes where + * necessary, and also to promote model attributes listed as + * {@code @SessionAttributes} to the session. + * @param exchange the current exchange + * @since 6.1 + */ + public void updateModel(ServerWebExchange exchange) { + Map model = getModel().asMap(); + for (Map.Entry entry : model.entrySet()) { + String name = entry.getKey(); + Object value = entry.getValue(); + if (isBindingCandidate(name, value)) { + if (!model.containsKey(BindingResult.MODEL_KEY_PREFIX + name)) { + WebExchangeDataBinder binder = createDataBinder(exchange, value, name); + model.put(BindingResult.MODEL_KEY_PREFIX + name, binder.getBindingResult()); + } + } + } + } + + private boolean isBindingCandidate(String name, @Nullable Object value) { + return (!name.startsWith(BindingResult.MODEL_KEY_PREFIX) && value != null && + !value.getClass().isArray() && !(value instanceof Collection) && !(value instanceof Map) && + this.reactiveAdapterRegistry.getAdapter(null, value) == null && + !BeanUtils.isSimpleValueType(value.getClass())); + } + /** * Extended variant of {@link WebExchangeDataBinder}, adding path variables. diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContext.java index 18b2b7ab49..8fa00f33d5 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContext.java @@ -18,6 +18,7 @@ package org.springframework.web.reactive.result.method.annotation; import java.util.List; +import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -52,11 +53,11 @@ class InitBinderBindingContext extends BindingContext { InitBinderBindingContext( @Nullable WebBindingInitializer initializer, List binderMethods, - boolean methodValidationApplicable) { + boolean methodValidationApplicable, ReactiveAdapterRegistry registry) { - super(initializer); + super(initializer, registry); this.binderMethods = binderMethods; - this.binderMethodContext = new BindingContext(initializer); + this.binderMethodContext = new BindingContext(initializer, registry); setMethodValidationApplicable(methodValidationApplicable); } @@ -101,8 +102,8 @@ class InitBinderBindingContext extends BindingContext { } /** - * Provide the context required to apply {@link #saveModel()} after the - * controller method has been invoked. + * Provide the context required to promote model attributes listed as + * {@code @SessionAttributes} to the session during {@link #updateModel}. */ public void setSessionContext(SessionAttributesHandler attributesHandler, WebSession session) { this.saveModelOperation = () -> { @@ -115,14 +116,12 @@ class InitBinderBindingContext extends BindingContext { }; } - /** - * Save model attributes in the session based on a type-level declarations - * in an {@code @SessionAttributes} annotation. - */ - public void saveModel() { + @Override + public void updateModel(ServerWebExchange exchange) { if (this.saveModelOperation != null) { this.saveModelOperation.run(); } + super.updateModel(exchange); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java index 53579c9fdd..5d35eff509 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java @@ -186,12 +186,16 @@ public class RequestMappingHandlerAdapter @Override public Mono handle(ServerWebExchange exchange, Object handler) { + + Assert.state(this.methodResolver != null && + this.modelInitializer != null && this.reactiveAdapterRegistry != null, "Not initialized"); + HandlerMethod handlerMethod = (HandlerMethod) handler; - Assert.state(this.methodResolver != null && this.modelInitializer != null, "Not initialized"); InitBinderBindingContext bindingContext = new InitBinderBindingContext( this.webBindingInitializer, this.methodResolver.getInitBinderMethods(handlerMethod), - this.methodResolver.hasMethodValidator() && handlerMethod.shouldValidateArguments()); + this.methodResolver.hasMethodValidator() && handlerMethod.shouldValidateArguments(), + this.reactiveAdapterRegistry); InvocableHandlerMethod invocableMethod = this.methodResolver.getRequestMappingMethod(handlerMethod); @@ -202,7 +206,6 @@ public class RequestMappingHandlerAdapter .initModel(handlerMethod, bindingContext, exchange) .then(Mono.defer(() -> invocableMethod.invoke(exchange, bindingContext))) .doOnNext(result -> result.setExceptionHandler(exceptionHandler)) - .doOnNext(result -> bindingContext.saveModel()) .onErrorResume(ex -> exceptionHandler.handleError(exchange, ex)); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index f6389c5bc1..4fb2346424 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.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. @@ -17,7 +17,6 @@ package org.springframework.web.reactive.result.view; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -41,9 +40,7 @@ import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.ui.Model; import org.springframework.util.StringUtils; -import org.springframework.validation.BindingResult; import org.springframework.web.bind.annotation.ModelAttribute; -import org.springframework.web.bind.support.WebExchangeDataBinder; import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.HandlerResultHandler; @@ -243,7 +240,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp viewsMono = resolveViews(getDefaultViewName(exchange), locale); } BindingContext bindingContext = result.getBindingContext(); - updateBindingResult(bindingContext, exchange); + bindingContext.updateModel(exchange); return viewsMono.flatMap(views -> render(views, model.asMap(), bindingContext, exchange)); }); } @@ -289,27 +286,6 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp .orElseGet(() -> Conventions.getVariableNameForParameter(returnType)); } - private void updateBindingResult(BindingContext context, ServerWebExchange exchange) { - Map model = context.getModel().asMap(); - for (Map.Entry entry : model.entrySet()) { - String name = entry.getKey(); - Object value = entry.getValue(); - if (isBindingCandidate(name, value)) { - if (!model.containsKey(BindingResult.MODEL_KEY_PREFIX + name)) { - WebExchangeDataBinder binder = context.createDataBinder(exchange, value, name); - model.put(BindingResult.MODEL_KEY_PREFIX + name, binder.getBindingResult()); - } - } - } - } - - private boolean isBindingCandidate(String name, @Nullable Object value) { - return (!name.startsWith(BindingResult.MODEL_KEY_PREFIX) && value != null && - !value.getClass().isArray() && !(value instanceof Collection) && !(value instanceof Map) && - getAdapterRegistry().getAdapter(null, value) == null && - !BeanUtils.isSimpleValueType(value.getClass())); - } - private Mono render(List views, Map model, BindingContext bindingContext, ServerWebExchange 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 213ce83326..00b6ba38c3 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 @@ -133,7 +133,8 @@ public class InitBinderBindingContextTests { handlerMethod.setParameterNameDiscoverer(new DefaultParameterNameDiscoverer()); return new InitBinderBindingContext( - this.bindingInitializer, Collections.singletonList(handlerMethod), false); + this.bindingInitializer, Collections.singletonList(handlerMethod), false, + ReactiveAdapterRegistry.getSharedInstance()); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java index 54ca957494..d9f20e9ed9 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java @@ -144,7 +144,7 @@ public class ModelInitializerTests { assertThat(session).isNotNull(); assertThat(session.getAttributes()).isEmpty(); - context.saveModel(); + context.updateModel(this.exchange); assertThat(session.getAttributes()).hasSize(1); assertThat(((TestBean) session.getRequiredAttribute("bean")).getName()).isEqualTo("Bean"); } @@ -164,7 +164,7 @@ public class ModelInitializerTests { HandlerMethod handlerMethod = new HandlerMethod(controller, method); this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); - context.saveModel(); + context.updateModel(this.exchange); assertThat(session.getAttributes()).hasSize(1); assertThat(((TestBean) session.getRequiredAttribute("bean")).getName()).isEqualTo("Session Bean"); } @@ -197,7 +197,7 @@ public class ModelInitializerTests { this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); context.getSessionStatus().setComplete(); - context.saveModel(); + context.updateModel(this.exchange); assertThat(session.getAttributes()).isEmpty(); } @@ -211,7 +211,8 @@ public class ModelInitializerTests { .toList(); WebBindingInitializer bindingInitializer = new ConfigurableWebBindingInitializer(); - return new InitBinderBindingContext(bindingInitializer, binderMethods, false); + return new InitBinderBindingContext( + bindingInitializer, binderMethods, false, ReactiveAdapterRegistry.getSharedInstance()); }