From d8b150e83d1a5d617ae73d1a65d1f9364dff6d9b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 1 Mar 2017 12:12:14 -0500 Subject: [PATCH] Refactor BindingContext initialization In preparation for SPR-15132. Turn the BindingContextFactory into ModelInitializer (both package private) since creating BindingContext itself is quite simple. The ModelInitializer also has only one field and no need to be aware of fields the RequestMappingHandlerAdapter. --- .../annotation/BindingContextFactory.java | 170 ------------------ .../method/annotation/ModelInitializer.java | 127 +++++++++++++ .../RequestMappingHandlerAdapter.java | 52 ++++-- ...yTests.java => ModelInitializerTests.java} | 59 ++++-- 4 files changed, 208 insertions(+), 200 deletions(-) delete mode 100644 spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactory.java create mode 100644 spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java rename spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/{BindingContextFactoryTests.java => ModelInitializerTests.java} (67%) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactory.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactory.java deleted file mode 100644 index 4e6f677fe04..00000000000 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactory.java +++ /dev/null @@ -1,170 +0,0 @@ -/* - * Copyright 2002-2016 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.reactive.result.method.annotation; - -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import reactor.core.publisher.Mono; - -import org.springframework.core.MethodParameter; -import org.springframework.core.ReactiveAdapter; -import org.springframework.core.ReactiveAdapterRegistry; -import org.springframework.core.ResolvableType; -import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.util.ClassUtils; -import org.springframework.util.StringUtils; -import org.springframework.web.bind.annotation.ModelAttribute; -import org.springframework.web.bind.support.WebBindingInitializer; -import org.springframework.web.method.HandlerMethod; -import org.springframework.web.reactive.BindingContext; -import org.springframework.web.reactive.HandlerResult; -import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver; -import org.springframework.web.reactive.result.method.InvocableHandlerMethod; -import org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentResolver; -import org.springframework.web.reactive.result.method.SyncInvocableHandlerMethod; -import org.springframework.web.server.ServerWebExchange; - -/** - * A helper class for {@link RequestMappingHandlerAdapter} that assists with - * creating a {@code BindingContext} and initialize it, and its model, through - * {@code @InitBinder} and {@code @ModelAttribute} methods. - * - * @author Rossen Stoyanchev - * @since 5.0 - */ -class BindingContextFactory { - - private final RequestMappingHandlerAdapter adapter; - - - public BindingContextFactory(RequestMappingHandlerAdapter adapter) { - this.adapter = adapter; - } - - - public RequestMappingHandlerAdapter getAdapter() { - return this.adapter; - } - - private WebBindingInitializer getBindingInitializer() { - return getAdapter().getWebBindingInitializer(); - } - - private List getInitBinderArgumentResolvers() { - return getAdapter().getInitBinderArgumentResolvers(); - } - - private List getArgumentResolvers() { - return getAdapter().getArgumentResolvers(); - } - - private ReactiveAdapterRegistry getAdapterRegistry() { - return getAdapter().getReactiveAdapterRegistry(); - } - - private Stream getInitBinderMethods(HandlerMethod handlerMethod) { - return getAdapter().getInitBinderMethods(handlerMethod.getBeanType()).stream(); - } - - private Stream getModelAttributeMethods(HandlerMethod handlerMethod) { - return getAdapter().getModelAttributeMethods(handlerMethod.getBeanType()).stream(); - } - - - /** - * Create and initialize a BindingContext for the current request. - * @param handlerMethod the request handling method - * @param exchange the current exchange - * @return Mono with the BindingContext instance - */ - public Mono createBindingContext(HandlerMethod handlerMethod, - ServerWebExchange exchange) { - - List invocableMethods = getInitBinderMethods(handlerMethod) - .map(method -> { - Object bean = handlerMethod.getBean(); - SyncInvocableHandlerMethod invocable = new SyncInvocableHandlerMethod(bean, method); - invocable.setSyncArgumentResolvers(getInitBinderArgumentResolvers()); - return invocable; - }) - .collect(Collectors.toList()); - - BindingContext bindingContext = - new InitBinderBindingContext(getBindingInitializer(), invocableMethods); - - return initModel(handlerMethod, bindingContext, exchange).then(Mono.just(bindingContext)); - } - - @SuppressWarnings("Convert2MethodRef") - private Mono initModel(HandlerMethod handlerMethod, BindingContext context, - ServerWebExchange exchange) { - - List> resultMonos = getModelAttributeMethods(handlerMethod) - .map(method -> { - Object bean = handlerMethod.getBean(); - InvocableHandlerMethod invocable = new InvocableHandlerMethod(bean, method); - invocable.setArgumentResolvers(getArgumentResolvers()); - return invocable; - }) - .map(invocable -> invocable.invoke(exchange, context)) - .collect(Collectors.toList()); - - return Mono - .when(resultMonos, resultArr -> processModelMethodMonos(resultArr, context)) - .then(voidMonos -> Mono.when(voidMonos)); - } - - private List> processModelMethodMonos(Object[] resultArr, BindingContext context) { - return Arrays.stream(resultArr) - .map(result -> processModelMethodResult((HandlerResult) result, context)) - .collect(Collectors.toList()); - } - - private Mono processModelMethodResult(HandlerResult result, BindingContext context) { - Object value = result.getReturnValue().orElse(null); - if (value == null) { - return Mono.empty(); - } - - ResolvableType type = result.getReturnType(); - ReactiveAdapter adapter = getAdapterRegistry().getAdapter(type.getRawClass(), value); - Class valueType = (adapter != null ? type.resolveGeneric(0) : type.resolve()); - - if (Void.class.equals(valueType) || void.class.equals(valueType)) { - return (adapter != null ? Mono.from(adapter.toPublisher(value)) : Mono.empty()); - } - - String name = getAttributeName(valueType, result.getReturnTypeSource()); - context.getModel().asMap().putIfAbsent(name, value); - return Mono.empty(); - } - - private String getAttributeName(Class valueType, MethodParameter parameter) { - Method method = parameter.getMethod(); - ModelAttribute annot = AnnotatedElementUtils.findMergedAnnotation(method, ModelAttribute.class); - if (annot != null && StringUtils.hasText(annot.value())) { - return annot.value(); - } - // TODO: Conventions does not deal with async wrappers - return ClassUtils.getShortNameAsProperty(valueType); - } - -} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java new file mode 100644 index 00000000000..caf8780cd6e --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java @@ -0,0 +1,127 @@ +/* + * Copyright 2002-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.web.reactive.result.method.annotation; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import reactor.core.publisher.Mono; + +import org.springframework.core.MethodParameter; +import org.springframework.core.ReactiveAdapter; +import org.springframework.core.ReactiveAdapterRegistry; +import org.springframework.core.ResolvableType; +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.util.ClassUtils; +import org.springframework.util.StringUtils; +import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.reactive.BindingContext; +import org.springframework.web.reactive.HandlerResult; +import org.springframework.web.reactive.result.method.InvocableHandlerMethod; +import org.springframework.web.server.ServerWebExchange; + + +/** + * Helper class to assist {@link RequestMappingHandlerAdapter} with + * initialization of the default model through {@code @ModelAttribute} methods. + * + * @author Rossen Stoyanchev + * @since 5.0 + */ +class ModelInitializer { + + private final ReactiveAdapterRegistry adapterRegistry; + + + public ModelInitializer(ReactiveAdapterRegistry adapterRegistry) { + this.adapterRegistry = adapterRegistry; + } + + + private ReactiveAdapterRegistry getAdapterRegistry() { + return this.adapterRegistry; + } + + + /** + * Initialize the default model in the given {@code BindingContext} through + * the {@code @ModelAttribute} methods and indicate when complete. + * + *

This will wait for {@code @ModelAttribute} methods that return + * {@code Mono} since those may be adding attributes asynchronously. + * However if methods return async attributes, those will be added to the + * model as-is and without waiting for them to be resolved. + * + * @param bindingContext the BindingContext with the default model + * @param attributeMethods the {@code @ModelAttribute} methods + * @param exchange the current exchange + * + * @return a {@code Mono} for when the model is populated. + */ + @SuppressWarnings("Convert2MethodRef") + public Mono initModel(BindingContext bindingContext, + List attributeMethods, ServerWebExchange exchange) { + + List> resultList = new ArrayList<>(); + attributeMethods.forEach(invocable -> resultList.add(invocable.invoke(exchange, bindingContext))); + + return Mono.when(resultList, objectArray -> { + return Arrays.stream(objectArray) + .map(object -> (HandlerResult) object) + .map(handlerResult -> handleResult(handlerResult, bindingContext)) + .collect(Collectors.toList()); + }).then(completionList -> Mono.when(completionList)); + } + + private Mono handleResult(HandlerResult handlerResult, BindingContext bindingContext) { + + return handlerResult.getReturnValue() + .map(value -> { + ResolvableType type = handlerResult.getReturnType(); + ReactiveAdapter adapter = getAdapterRegistry().getAdapter(type.getRawClass(), value); + + Class attributeType; + if (adapter != null) { + attributeType = adapter.isNoValue() ? Void.class : type.resolveGeneric(0); + if (attributeType.equals(Void.class)) { + return Mono.from(adapter.toPublisher(value)); + } + } + else { + attributeType = type.resolve(); + } + + String name = getAttributeName(attributeType, handlerResult.getReturnTypeSource()); + bindingContext.getModel().asMap().putIfAbsent(name, value); + return Mono.empty(); + }) + .orElse(Mono.empty()); + } + + private String getAttributeName(Class valueType, MethodParameter parameter) { + Method method = parameter.getMethod(); + ModelAttribute annot = AnnotatedElementUtils.findMergedAnnotation(method, ModelAttribute.class); + if (annot != null && StringUtils.hasText(annot.value())) { + return annot.value(); + } + // TODO: Conventions does not deal with async wrappers + return ClassUtils.getShortNameAsProperty(valueType); + } + +} 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 495534807a0..28f8c984bbc 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 @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -54,6 +55,7 @@ import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver; import org.springframework.web.reactive.result.method.InvocableHandlerMethod; import org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentResolver; +import org.springframework.web.reactive.result.method.SyncInvocableHandlerMethod; import org.springframework.web.server.ServerWebExchange; /** @@ -71,7 +73,7 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory private WebBindingInitializer webBindingInitializer; - private ReactiveAdapterRegistry reactiveAdapters = new ReactiveAdapterRegistry(); + private ReactiveAdapterRegistry reactiveAdapterRegistry = new ReactiveAdapterRegistry(); private List customArgumentResolvers; @@ -84,7 +86,7 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory private ConfigurableBeanFactory beanFactory; - private final BindingContextFactory bindingContextFactory = new BindingContextFactory(this); + private ModelInitializer modelInitializer; private final Map, Set> initBinderCache = new ConcurrentHashMap<>(64); @@ -133,11 +135,11 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory } public void setReactiveAdapterRegistry(ReactiveAdapterRegistry registry) { - this.reactiveAdapters = registry; + this.reactiveAdapterRegistry = registry; } public ReactiveAdapterRegistry getReactiveAdapterRegistry() { - return this.reactiveAdapters; + return this.reactiveAdapterRegistry; } /** @@ -226,6 +228,7 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory if (this.initBinderArgumentResolvers == null) { this.initBinderArgumentResolvers = getDefaultInitBinderArgumentResolvers(); } + this.modelInitializer = new ModelInitializer(getReactiveAdapterRegistry()); } protected List getDefaultArgumentResolvers() { @@ -305,10 +308,13 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory InvocableHandlerMethod invocable = new InvocableHandlerMethod(handlerMethod); invocable.setArgumentResolvers(getArgumentResolvers()); - Mono bindingContextMono = - this.bindingContextFactory.createBindingContext(handlerMethod, exchange); + BindingContext bindingContext = new InitBinderBindingContext( + getWebBindingInitializer(), getInitBinderMethods(handlerMethod)); - return bindingContextMono.then(bindingContext -> + Mono modelCompletion = this.modelInitializer.initModel( + bindingContext, getAttributeMethods(handlerMethod), exchange); + + return modelCompletion.then(() -> invocable.invoke(exchange, bindingContext) .doOnNext(result -> result.setExceptionHandler( ex -> handleException(ex, handlerMethod, bindingContext, exchange))) @@ -316,14 +322,38 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory ex, handlerMethod, bindingContext, exchange))); } - Set getInitBinderMethods(Class handlerType) { - return this.initBinderCache.computeIfAbsent(handlerType, aClass -> + private List getInitBinderMethods(HandlerMethod handlerMethod) { + + Class handlerType = handlerMethod.getBeanType(); + + Set methods = this.initBinderCache.computeIfAbsent(handlerType, aClass -> MethodIntrospector.selectMethods(handlerType, INIT_BINDER_METHODS)); + + return methods.stream() + .map(method -> { + Object bean = handlerMethod.getBean(); + SyncInvocableHandlerMethod invocable = new SyncInvocableHandlerMethod(bean, method); + invocable.setSyncArgumentResolvers(getInitBinderArgumentResolvers()); + return invocable; + }) + .collect(Collectors.toList()); } - Set getModelAttributeMethods(Class handlerType) { - return this.modelAttributeCache.computeIfAbsent(handlerType, aClass -> + private List getAttributeMethods(HandlerMethod handlerMethod) { + + Class handlerType = handlerMethod.getBeanType(); + + Set methods = this.modelAttributeCache.computeIfAbsent(handlerType, aClass -> MethodIntrospector.selectMethods(handlerType, MODEL_ATTRIBUTE_METHODS)); + + return methods.stream() + .map(method -> { + Object bean = handlerMethod.getBean(); + InvocableHandlerMethod invocable = new InvocableHandlerMethod(bean, method); + invocable.setArgumentResolvers(getArgumentResolvers()); + return invocable; + }) + .collect(Collectors.toList()); } private Mono handleException(Throwable ex, HandlerMethod handlerMethod, diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactoryTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java similarity index 67% rename from spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactoryTests.java rename to spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java index f09c2cdc78f..556658d5b35 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactoryTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java @@ -17,14 +17,17 @@ package org.springframework.web.reactive.result.method.annotation; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; import rx.Single; -import org.springframework.context.support.StaticApplicationContext; +import org.springframework.core.MethodIntrospector; +import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.ui.Model; @@ -34,35 +37,35 @@ import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.InitBinder; import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; +import org.springframework.web.bind.support.WebBindingInitializer; import org.springframework.web.bind.support.WebExchangeDataBinder; -import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.BindingContext; -import org.springframework.web.reactive.config.WebFluxConfigurationSupport; -import org.springframework.web.reactive.result.ResolvableMethod; +import org.springframework.web.reactive.result.method.InvocableHandlerMethod; +import org.springframework.web.reactive.result.method.SyncInvocableHandlerMethod; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; +import static org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter.INIT_BINDER_METHODS; +import static org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter.MODEL_ATTRIBUTE_METHODS; /** - * Unit tests for {@link BindingContextFactory}. + * Unit tests for {@link ModelInitializer}. * @author Rossen Stoyanchev */ -public class BindingContextFactoryTests { +public class ModelInitializerTests { - private BindingContextFactory contextFactory; + private ModelInitializer modelInitializer; private ServerWebExchange exchange; @Before public void setup() throws Exception { - WebFluxConfigurationSupport configurationSupport = new WebFluxConfigurationSupport(); - configurationSupport.setApplicationContext(new StaticApplicationContext()); - RequestMappingHandlerAdapter adapter = configurationSupport.requestMappingHandlerAdapter(); - adapter.afterPropertiesSet(); - this.contextFactory = new BindingContextFactory(adapter); + + this.modelInitializer = new ModelInitializer(new ReactiveAdapterRegistry()); MockServerHttpRequest request = MockServerHttpRequest.get("/path").build(); MockServerHttpResponse response = new MockServerHttpResponse(); @@ -74,15 +77,15 @@ public class BindingContextFactoryTests { @Test public void basic() throws Exception { Validator validator = mock(Validator.class); - TestController controller = new TestController(validator); + Object controller = new TestController(validator); + + List binderMethods = getBinderMethods(controller); + List attributeMethods = getAttributeMethods(controller); - HandlerMethod handlerMethod = ResolvableMethod.on(controller) - .annotated(RequestMapping.class) - .resolveHandlerMethod(); + WebBindingInitializer bindingInitializer = new ConfigurableWebBindingInitializer(); + BindingContext bindingContext = new InitBinderBindingContext(bindingInitializer, binderMethods); - BindingContext bindingContext = - this.contextFactory.createBindingContext(handlerMethod, this.exchange) - .blockMillis(5000); + this.modelInitializer.initModel(bindingContext, attributeMethods, this.exchange).blockMillis(5000); WebExchangeDataBinder binder = bindingContext.createDataBinder(this.exchange, "name"); assertEquals(Collections.singletonList(validator), binder.getValidators()); @@ -106,6 +109,24 @@ public class BindingContextFactoryTests { assertEquals("Void Mono Method Bean", ((TestBean) value).getName()); } + private List getBinderMethods(Object controller) { + return MethodIntrospector + .selectMethods(controller.getClass(), INIT_BINDER_METHODS).stream() + .map(method -> new SyncInvocableHandlerMethod(controller, method)) + .collect(Collectors.toList()); + } + + private List getAttributeMethods(Object controller) { + return MethodIntrospector + .selectMethods(controller.getClass(), MODEL_ATTRIBUTE_METHODS).stream() + .map(method -> { + InvocableHandlerMethod invocable = new InvocableHandlerMethod(controller, method); + invocable.setArgumentResolvers(Collections.singletonList(new ModelArgumentResolver())); + return invocable; + }) + .collect(Collectors.toList()); + } + @SuppressWarnings("unused") private static class TestController {