From da4430e27ef13a42a47c954386a3a1f66e4f6cd2 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 16 May 2019 20:41:37 -0400 Subject: [PATCH] BindingResult for resolved async attributes ViewResolutionResultHandler no longer adds BindingResult objects for async model attributes. Instead AbstractView adds them later when those attributes are resolved to concrete values. Closes gh-22933 --- .../reactive/result/view/AbstractView.java | 56 ++++++++++++++----- .../web/reactive/result/view/View.java | 14 ++++- .../view/ViewResolutionResultHandler.java | 46 +++++++++------ .../result/view/AbstractViewTests.java | 51 +++++++++-------- 4 files changed, 113 insertions(+), 54 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java index 5335adb725..a4d0801e5f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java @@ -19,6 +19,7 @@ package org.springframework.web.reactive.result.view; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -29,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanNameAware; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -37,6 +39,8 @@ import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.validation.BindingResult; +import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebExchange; /** @@ -54,9 +58,6 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo /** Logger that is available to subclasses. */ protected final Log logger = LogFactory.getLog(getClass()); - private static final Object NO_VALUE = new Object(); - - private final ReactiveAdapterRegistry adapterRegistry; private final List mediaTypes = new ArrayList<>(4); @@ -219,18 +220,23 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo if (model != null) { attributes.putAll(model); } - return resolveAsyncAttributes(attributes).thenReturn(attributes); + //noinspection deprecation + return resolveAsyncAttributes(attributes) + .then(resolveAsyncAttributes(attributes, exchange)) + .doOnTerminate(() -> exchange.getAttributes().remove(BINDING_CONTEXT_ATTRIBUTE)) + .thenReturn(attributes); } /** * Use the configured {@link ReactiveAdapterRegistry} to adapt asynchronous - * model attributes to {@code Mono} or {@code Mono>} and resolve - * them to actual values via {@link Mono#zip(Mono, Mono)}, so that when - * the returned result {@code Mono} completes, the model has its asynchronous - * attributes replaced with synchronous values. + * attributes to {@code Mono} or {@code Mono>} and then wait to + * resolve them into actual values. When the returned {@code Mono} + * completes, the asynchronous attributes in the model would have been + * replaced with their corresponding resolved values. * @return result {@code Mono} that completes when the model is ready + * @since 5.1.8 */ - protected Mono resolveAsyncAttributes(Map model) { + protected Mono resolveAsyncAttributes(Map model, ServerWebExchange exchange) { List> asyncAttributes = null; for (Map.Entry entry : model.entrySet()) { Object value = entry.getValue(); @@ -247,10 +253,7 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo asyncAttributes.add( Flux.from(adapter.toPublisher(value)) .collectList() - .doOnSuccess(result -> { - result = result != null ? result : Collections.emptyList(); - model.put(name, result); - })); + .doOnSuccess(result -> model.put(name, result))); } else { asyncAttributes.add( @@ -258,6 +261,7 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo .doOnSuccess(result -> { if (result != null) { model.put(name, result); + addBindingResult(name, result, model, exchange); } else { model.remove(name); @@ -269,6 +273,32 @@ public abstract class AbstractView implements View, BeanNameAware, ApplicationCo return asyncAttributes != null ? Mono.when(asyncAttributes) : Mono.empty(); } + private void addBindingResult(String name, Object value, Map model, ServerWebExchange exchange) { + BindingContext context = exchange.getAttribute(BINDING_CONTEXT_ATTRIBUTE); + if (context == null || value.getClass().isArray() || value instanceof Collection || + value instanceof Map || BeanUtils.isSimpleValueType(value.getClass())) { + return; + } + BindingResult result = context.createDataBinder(exchange, value, name).getBindingResult(); + model.put(BindingResult.MODEL_KEY_PREFIX + name, result); + } + + /** + * Use the configured {@link ReactiveAdapterRegistry} to adapt asynchronous + * attributes to {@code Mono} or {@code Mono>} and then wait to + * resolve them into actual values. When the returned {@code Mono} + * completes, the asynchronous attributes in the model would have been + * replaced with their corresponding resolved values. + * @return result {@code Mono} that completes when the model is ready + * @deprecated as of 5.1.8 this method is still invoked but it is a no-op. + * Please, use {@link #resolveAsyncAttributes(Map, ServerWebExchange)} + * instead. It is invoked after this one and does the actual work. + */ + @Deprecated + protected Mono resolveAsyncAttributes(Map model) { + return Mono.empty(); + } + /** * Create a RequestContext to expose under the specified attribute name. *

The default implementation creates a standard RequestContext instance diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/View.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/View.java index 3b0e23e2f7..4df2b19424 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/View.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/View.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -44,6 +44,18 @@ import org.springframework.web.server.ServerWebExchange; */ public interface View { + /** + * The name of the exchange attribute that contains the + * {@link org.springframework.web.reactive.BindingContext BindingContext} + * for the request which can be used to create + * {@link org.springframework.validation.BindingResult BindingResult} + * instances for objects in to the model. + *

Note: This attribute is not required and may not be present. + * @since 5.1.8 + */ + String BINDING_CONTEXT_ATTRIBUTE = View.class.getName() + ".bindingContext"; + + /** * Return the list of media types this View supports, or an empty list. */ 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 859c78a4e6..691fac712b 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-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -245,10 +245,9 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport model.addAttribute(name, returnValue); viewsMono = resolveViews(getDefaultViewName(exchange), locale); } - - updateBindingContext(result.getBindingContext(), exchange); - - return viewsMono.flatMap(views -> render(views, model.asMap(), exchange)); + BindingContext bindingContext = result.getBindingContext(); + updateBindingResult(bindingContext, exchange); + return viewsMono.flatMap(views -> render(views, model.asMap(), bindingContext, exchange)); }); } @@ -288,39 +287,42 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport .orElseGet(() -> Conventions.getVariableNameForParameter(returnType)); } - private void updateBindingContext(BindingContext context, ServerWebExchange exchange) { + private void updateBindingResult(BindingContext context, ServerWebExchange exchange) { Map model = context.getModel().asMap(); - model.keySet().stream() - .filter(name -> isBindingCandidate(name, model.get(name))) - .filter(name -> !model.containsKey(BindingResult.MODEL_KEY_PREFIX + name)) - .forEach(name -> { - WebExchangeDataBinder binder = context.createDataBinder(exchange, model.get(name), name); + 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) && !BeanUtils.isSimpleValueType(value.getClass())); + !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, - ServerWebExchange exchange) { + BindingContext bindingContext, ServerWebExchange exchange) { for (View view : views) { if (view.isRedirectView()) { - return view.render(model, null, exchange); + return renderWith(view, model, null, exchange, bindingContext); } } - List mediaTypes = getMediaTypes(views); MediaType bestMediaType = selectMediaType(exchange, () -> mediaTypes); if (bestMediaType != null) { for (View view : views) { for (MediaType mediaType : view.getSupportedMediaTypes()) { if (mediaType.isCompatibleWith(bestMediaType)) { - return view.render(model, mediaType, exchange); + return renderWith(view, model, mediaType, exchange, bindingContext); } } } @@ -328,6 +330,14 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport throw new NotAcceptableStatusException(mediaTypes); } + private Mono renderWith(View view, Map model, + @Nullable MediaType mediaType, ServerWebExchange exchange, BindingContext bindingContext) { + + exchange.getAttributes().put(View.BINDING_CONTEXT_ATTRIBUTE, bindingContext); + return view.render(model, mediaType, exchange) + .doOnTerminate(() -> exchange.getAttributes().remove(View.BINDING_CONTEXT_ATTRIBUTE)); + } + private List getMediaTypes(List views) { return views.stream() .flatMap(view -> view.getSupportedMediaTypes().stream()) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/AbstractViewTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/AbstractViewTests.java index 0a9c239a7c..b3cfa1395b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/AbstractViewTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/AbstractViewTests.java @@ -17,8 +17,8 @@ package org.springframework.web.reactive.result.view; import java.time.Duration; +import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import io.reactivex.Observable; @@ -29,14 +29,15 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.tests.sample.beans.TestBean; +import org.springframework.validation.BindingResult; +import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebExchange; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.*; /** * Unit tests for {@link AbstractView}. @@ -54,25 +55,31 @@ public class AbstractViewTests { TestBean testBean1 = new TestBean("Bean1"); TestBean testBean2 = new TestBean("Bean2"); - Map attributes = new HashMap<>(); - attributes.put("attr1", Mono.just(testBean1).delayElement(Duration.ofMillis(10))); - attributes.put("attr2", Flux.just(testBean1, testBean2).delayElements(Duration.ofMillis(10))); - attributes.put("attr3", Single.just(testBean2)); - attributes.put("attr4", Observable.just(testBean1, testBean2)); - attributes.put("attr5", Mono.empty()); + + Map inMap = new HashMap<>(); + inMap.put("attr1", Mono.just(testBean1).delayElement(Duration.ofMillis(10))); + inMap.put("attr2", Flux.just(testBean1, testBean2).delayElements(Duration.ofMillis(10))); + inMap.put("attr3", Single.just(testBean2)); + inMap.put("attr4", Observable.just(testBean1, testBean2)); + inMap.put("attr5", Mono.empty()); + + this.exchange.getAttributes().put(View.BINDING_CONTEXT_ATTRIBUTE, new BindingContext()); TestView view = new TestView(); - StepVerifier.create( - view.render(attributes, null, this.exchange)).verifyComplete(); - - Map actual = view.attributes; - assertEquals(testBean1, actual.get("attr1")); - assertArrayEquals(new TestBean[] { testBean1, testBean2 }, - ((List) actual.get("attr2")).toArray()); - assertEquals(testBean2, actual.get("attr3")); - assertArrayEquals(new TestBean[] { testBean1, testBean2 }, - ((List) actual.get("attr4")).toArray()); - assertNull(actual.get("attr5")); + StepVerifier.create(view.render(inMap, null, this.exchange)).verifyComplete(); + + Map outMap = view.attributes; + assertEquals(testBean1, outMap.get("attr1")); + assertEquals(Arrays.asList(testBean1, testBean2), outMap.get("attr2")); + assertEquals(testBean2, outMap.get("attr3")); + assertEquals(Arrays.asList(testBean1, testBean2), outMap.get("attr4")); + assertNull(outMap.get("attr5")); + + assertNotNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr1")); + assertNotNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr3")); + assertNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr2")); + assertNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr4")); + assertNull(outMap.get(BindingResult.MODEL_KEY_PREFIX + "attr5")); } private static class TestView extends AbstractView { @@ -81,7 +88,7 @@ public class AbstractViewTests { @Override protected Mono renderInternal(Map renderAttributes, - MediaType contentType, ServerWebExchange exchange) { + @Nullable MediaType contentType, ServerWebExchange exchange) { this.attributes = renderAttributes; return Mono.empty();