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.

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/BindingContextFactory.java

@@ -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<SyncHandlerMethodArgumentResolver> getInitBinderArgumentResolvers() {
-		return getAdapter().getInitBinderArgumentResolvers();
-	}
-
-	private List<HandlerMethodArgumentResolver> getArgumentResolvers() {
-		return getAdapter().getArgumentResolvers();
-	}
-
-	private ReactiveAdapterRegistry getAdapterRegistry() {
-		return getAdapter().getReactiveAdapterRegistry();
-	}
-
-	private Stream<Method> getInitBinderMethods(HandlerMethod handlerMethod) {
-		return getAdapter().getInitBinderMethods(handlerMethod.getBeanType()).stream();
-	}
-
-	private Stream<Method> 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<BindingContext> createBindingContext(HandlerMethod handlerMethod,
-			ServerWebExchange exchange) {
-
-		List<SyncInvocableHandlerMethod> 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<Void> initModel(HandlerMethod handlerMethod, BindingContext context,
-			ServerWebExchange exchange) {
-
-		List<Mono<HandlerResult>> 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<Mono<Void>> processModelMethodMonos(Object[] resultArr, BindingContext context) {
-		return Arrays.stream(resultArr)
-				.map(result -> processModelMethodResult((HandlerResult) result, context))
-				.collect(Collectors.toList());
-	}
-
-	private Mono<Void> 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);
-	}
-
-}

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.
+	 *
+	 * <p>This will wait for {@code @ModelAttribute} methods that return
+	 * {@code Mono<Void>} 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<Void> initModel(BindingContext bindingContext,
+			List<InvocableHandlerMethod> attributeMethods, ServerWebExchange exchange) {
+
+		List<Mono<HandlerResult>> 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<Void> 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.<Void>from(adapter.toPublisher(value));
+						}
+					}
+					else {
+						attributeType = type.resolve();
+					}
+
+					String name = getAttributeName(attributeType, handlerResult.getReturnTypeSource());
+					bindingContext.getModel().asMap().putIfAbsent(name, value);
+					return Mono.<Void>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);
+	}
+
+}

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<HandlerMethodArgumentResolver> 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<Class<?>, Set<Method>> 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<HandlerMethodArgumentResolver> getDefaultArgumentResolvers() {
@@ -305,10 +308,13 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, BeanFactory
 		InvocableHandlerMethod invocable = new InvocableHandlerMethod(handlerMethod);
 		invocable.setArgumentResolvers(getArgumentResolvers());
 
-		Mono<BindingContext> bindingContextMono =
-				this.bindingContextFactory.createBindingContext(handlerMethod, exchange);
+		BindingContext bindingContext = new InitBinderBindingContext(
+				getWebBindingInitializer(), getInitBinderMethods(handlerMethod));
 
-		return bindingContextMono.then(bindingContext ->
+		Mono<Void> 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<Method> getInitBinderMethods(Class<?> handlerType) {
-		return this.initBinderCache.computeIfAbsent(handlerType, aClass ->
+	private List<SyncInvocableHandlerMethod> getInitBinderMethods(HandlerMethod handlerMethod) {
+
+		Class<?> handlerType = handlerMethod.getBeanType();
+
+		Set<Method> 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<Method> getModelAttributeMethods(Class<?> handlerType) {
-		return this.modelAttributeCache.computeIfAbsent(handlerType, aClass ->
+	private List<InvocableHandlerMethod> getAttributeMethods(HandlerMethod handlerMethod) {
+
+		Class<?> handlerType = handlerMethod.getBeanType();
+
+		Set<Method> 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<HandlerResult> handleException(Throwable ex, HandlerMethod handlerMethod,

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<SyncInvocableHandlerMethod> binderMethods = getBinderMethods(controller);
+		List<InvocableHandlerMethod> 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<SyncInvocableHandlerMethod> getBinderMethods(Object controller) {
+		return MethodIntrospector
+				.selectMethods(controller.getClass(), INIT_BINDER_METHODS).stream()
+				.map(method -> new SyncInvocableHandlerMethod(controller, method))
+				.collect(Collectors.toList());
+	}
+
+	private List<InvocableHandlerMethod> 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 {