Polish: simplify ControllerMethodResolver initialization

Author: rstoyanchev

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -24,8 +24,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Function;
-import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import org.apache.commons.logging.Log;
@@ -40,7 +38,6 @@
 import org.springframework.http.codec.HttpMessageReader;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.web.bind.annotation.InitBinder;
 import org.springframework.web.bind.annotation.ModelAttribute;
@@ -53,7 +50,7 @@
 import org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentResolver;
 import org.springframework.web.reactive.result.method.SyncInvocableHandlerMethod;
 
-import static org.springframework.core.MethodIntrospector.selectMethods;
+import static org.springframework.core.MethodIntrospector.*;
 
 /**
  * Package-private class to assist {@link RequestMappingHandlerAdapter} with
@@ -102,81 +99,112 @@ class ControllerMethodResolver {
 	private final Map<Class<?>, SessionAttributesHandler> sessionAttributesHandlerCache = new ConcurrentHashMap<>(64);
 
 
-	ControllerMethodResolver(ArgumentResolverConfigurer argumentResolvers,
-			List<HttpMessageReader<?>> messageReaders, ReactiveAdapterRegistry reactiveRegistry,
-			ConfigurableApplicationContext context) {
+	ControllerMethodResolver(ArgumentResolverConfigurer customResolvers, ReactiveAdapterRegistry reactiveRegistry,
+			ConfigurableApplicationContext context, List<HttpMessageReader<?>> readers) {
 
-		Assert.notNull(argumentResolvers, "ArgumentResolverConfigurer is required");
-		Assert.notNull(messageReaders, "'messageReaders' is required");
+		Assert.notNull(customResolvers, "ArgumentResolverConfigurer is required");
+		Assert.notNull(readers, "'messageReaders' is required");
 		Assert.notNull(reactiveRegistry, "ReactiveAdapterRegistry is required");
 		Assert.notNull(context, "ApplicationContext is required");
 
-		ArgumentResolverRegistrar registrar;
+		this.initBinderResolvers = initBinderResolvers(customResolvers, reactiveRegistry, context);
+		this.modelAttributeResolvers = modelMethodResolvers(customResolvers, reactiveRegistry, context);
+		this.requestMappingResolvers = requestMappingResolvers(customResolvers, reactiveRegistry, context, readers);
+		this.exceptionHandlerResolvers = exceptionHandlerResolvers(customResolvers, reactiveRegistry, context);
+		this.reactiveAdapterRegistry = reactiveRegistry;
 
-		registrar = ArgumentResolverRegistrar.configurer(argumentResolvers).basic();
-		addResolversTo(registrar, reactiveRegistry, context);
-		this.initBinderResolvers = registrar.getSyncResolvers();
+		initControllerAdviceCaches(context);
+	}
 
-		registrar = ArgumentResolverRegistrar.configurer(argumentResolvers).modelAttributeSupport();
-		addResolversTo(registrar, reactiveRegistry, context);
-		this.modelAttributeResolvers = registrar.getResolvers();
+	private List<SyncHandlerMethodArgumentResolver> initBinderResolvers(
+			ArgumentResolverConfigurer customResolvers, ReactiveAdapterRegistry reactiveRegistry,
+			ConfigurableApplicationContext context) {
 
-		registrar = ArgumentResolverRegistrar.configurer(argumentResolvers).fullSupport(messageReaders);
-		addResolversTo(registrar, reactiveRegistry, context);
-		this.requestMappingResolvers = registrar.getResolvers();
+		return initResolvers(customResolvers, reactiveRegistry, context, false, Collections.emptyList()).stream()
+				.filter(resolver -> resolver instanceof SyncHandlerMethodArgumentResolver)
+				.map(resolver -> (SyncHandlerMethodArgumentResolver) resolver)
+				.collect(Collectors.toList());
+	}
 
-		registrar = ArgumentResolverRegistrar.configurer(argumentResolvers).basic();
-		addResolversTo(registrar, reactiveRegistry, context);
-		this.exceptionHandlerResolvers = registrar.getResolvers();
+	private static List<HandlerMethodArgumentResolver> modelMethodResolvers(
+			ArgumentResolverConfigurer customResolvers, ReactiveAdapterRegistry reactiveRegistry,
+			ConfigurableApplicationContext context) {
 
-		this.reactiveAdapterRegistry = reactiveRegistry;
+		return initResolvers(customResolvers, reactiveRegistry, context, true, Collections.emptyList());
+	}
 
-		initControllerAdviceCaches(context);
+	private static List<HandlerMethodArgumentResolver> requestMappingResolvers(
+			ArgumentResolverConfigurer customResolvers, ReactiveAdapterRegistry reactiveRegistry,
+			ConfigurableApplicationContext context, List<HttpMessageReader<?>> readers) {
+
+		return initResolvers(customResolvers, reactiveRegistry, context, true, readers);
+	}
+
+	private static List<HandlerMethodArgumentResolver> exceptionHandlerResolvers(
+			ArgumentResolverConfigurer customResolvers, ReactiveAdapterRegistry reactiveRegistry,
+			ConfigurableApplicationContext context) {
+
+		return initResolvers(customResolvers, reactiveRegistry, context, false, Collections.emptyList());
 	}
 
-	private void addResolversTo(ArgumentResolverRegistrar registrar,
-			ReactiveAdapterRegistry reactiveRegistry, ConfigurableApplicationContext context) {
+	private static List<HandlerMethodArgumentResolver> initResolvers(ArgumentResolverConfigurer customResolvers,
+			ReactiveAdapterRegistry reactiveRegistry, ConfigurableApplicationContext context,
+			boolean supportDataBinding, List<HttpMessageReader<?>> readers) {
 
 		ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
+		boolean requestMappingMethod = !readers.isEmpty() && supportDataBinding;
 
 		// Annotation-based...
-		registrar.add(new RequestParamMethodArgumentResolver(beanFactory, reactiveRegistry, false));
-		registrar.add(new RequestParamMapMethodArgumentResolver(reactiveRegistry));
-		registrar.add(new PathVariableMethodArgumentResolver(beanFactory, reactiveRegistry));
-		registrar.add(new PathVariableMapMethodArgumentResolver(reactiveRegistry));
-		registrar.add(new MatrixVariableMethodArgumentResolver(beanFactory, reactiveRegistry));
-		registrar.add(new MatrixVariableMapMethodArgumentResolver(reactiveRegistry));
-		registrar.addIfRequestBody(readers -> new RequestBodyArgumentResolver(readers, reactiveRegistry));
-		registrar.addIfRequestBody(readers -> new RequestPartMethodArgumentResolver(readers, reactiveRegistry));
-		registrar.addIfModelAttribute(() -> new ModelAttributeMethodArgumentResolver(reactiveRegistry, false));
-		registrar.add(new RequestHeaderMethodArgumentResolver(beanFactory, reactiveRegistry));
-		registrar.add(new RequestHeaderMapMethodArgumentResolver(reactiveRegistry));
-		registrar.add(new CookieValueMethodArgumentResolver(beanFactory, reactiveRegistry));
-		registrar.add(new ExpressionValueMethodArgumentResolver(beanFactory, reactiveRegistry));
-		registrar.add(new SessionAttributeMethodArgumentResolver(beanFactory, reactiveRegistry));
-		registrar.add(new RequestAttributeMethodArgumentResolver(beanFactory, reactiveRegistry));
+		List<HandlerMethodArgumentResolver> result = new ArrayList<>();
+		result.add(new RequestParamMethodArgumentResolver(beanFactory, reactiveRegistry, false));
+		result.add(new RequestParamMapMethodArgumentResolver(reactiveRegistry));
+		result.add(new PathVariableMethodArgumentResolver(beanFactory, reactiveRegistry));
+		result.add(new PathVariableMapMethodArgumentResolver(reactiveRegistry));
+		result.add(new MatrixVariableMethodArgumentResolver(beanFactory, reactiveRegistry));
+		result.add(new MatrixVariableMapMethodArgumentResolver(reactiveRegistry));
+		if (!readers.isEmpty()) {
+			result.add(new RequestBodyArgumentResolver(readers, reactiveRegistry));
+			result.add(new RequestPartMethodArgumentResolver(readers, reactiveRegistry));
+		}
+		if (supportDataBinding) {
+			result.add(new ModelAttributeMethodArgumentResolver(reactiveRegistry, false));
+		}
+		result.add(new RequestHeaderMethodArgumentResolver(beanFactory, reactiveRegistry));
+		result.add(new RequestHeaderMapMethodArgumentResolver(reactiveRegistry));
+		result.add(new CookieValueMethodArgumentResolver(beanFactory, reactiveRegistry));
+		result.add(new ExpressionValueMethodArgumentResolver(beanFactory, reactiveRegistry));
+		result.add(new SessionAttributeMethodArgumentResolver(beanFactory, reactiveRegistry));
+		result.add(new RequestAttributeMethodArgumentResolver(beanFactory, reactiveRegistry));
 
 		// Type-based...
-		registrar.addIfRequestBody(readers -> new HttpEntityArgumentResolver(readers, reactiveRegistry));
-		registrar.add(new ModelArgumentResolver(reactiveRegistry));
-		registrar.addIfModelAttribute(() -> new ErrorsMethodArgumentResolver(reactiveRegistry));
-		registrar.add(new ServerWebExchangeArgumentResolver(reactiveRegistry));
-		registrar.add(new PrincipalArgumentResolver(reactiveRegistry));
-		registrar.addIfRequestBody(readers -> new SessionStatusMethodArgumentResolver());
-		registrar.add(new WebSessionArgumentResolver(reactiveRegistry));
+		if (!readers.isEmpty()) {
+			result.add(new HttpEntityArgumentResolver(readers, reactiveRegistry));
+		}
+		result.add(new ModelArgumentResolver(reactiveRegistry));
+		if (supportDataBinding) {
+			result.add(new ErrorsMethodArgumentResolver(reactiveRegistry));
+		}
+		result.add(new ServerWebExchangeArgumentResolver(reactiveRegistry));
+		result.add(new PrincipalArgumentResolver(reactiveRegistry));
+		if (requestMappingMethod) {
+			result.add(new SessionStatusMethodArgumentResolver());
+		}
+		result.add(new WebSessionArgumentResolver(reactiveRegistry));
 
 		// Custom...
-		registrar.addCustomResolvers();
+		result.addAll(customResolvers.getCustomResolvers());
 
 		// Catch-all...
-		registrar.add(new RequestParamMethodArgumentResolver(beanFactory, reactiveRegistry, true));
-		registrar.addIfModelAttribute(() -> new ModelAttributeMethodArgumentResolver(reactiveRegistry, true));
+		result.add(new RequestParamMethodArgumentResolver(beanFactory, reactiveRegistry, true));
+		if (supportDataBinding) {
+			result.add(new ModelAttributeMethodArgumentResolver(reactiveRegistry, true));
+		}
+
+		return result;
 	}
 
-	private void initControllerAdviceCaches(@Nullable ApplicationContext applicationContext) {
-		if (applicationContext == null) {
-			return;
-		}
+	private void initControllerAdviceCaches(ApplicationContext applicationContext) {
+
 		if (logger.isInfoEnabled()) {
 			logger.info("Looking for @ControllerAdvice: " + applicationContext);
 		}
@@ -354,84 +382,4 @@ public SessionAttributesHandler getSessionAttributesHandler(HandlerMethod handle
 			(AnnotationUtils.findAnnotation(method, RequestMapping.class) == null) &&
 					(AnnotationUtils.findAnnotation(method, ModelAttribute.class) != null);
 
-
-	private static class ArgumentResolverRegistrar {
-
-		private final List<HandlerMethodArgumentResolver> customResolvers;
-
-		private final List<HttpMessageReader<?>> messageReaders;
-
-		private final boolean modelAttributeSupported;
-
-		private final List<HandlerMethodArgumentResolver> result = new ArrayList<>();
-
-
-		private ArgumentResolverRegistrar(ArgumentResolverConfigurer resolvers,
-				List<HttpMessageReader<?>> messageReaders, boolean modelAttribute) {
-
-			this.customResolvers = resolvers.getCustomResolvers();
-			this.messageReaders = messageReaders;
-			this.modelAttributeSupported = modelAttribute;
-		}
-
-
-		public void add(HandlerMethodArgumentResolver resolver) {
-			this.result.add(resolver);
-		}
-
-		public void addIfRequestBody(Function<List<HttpMessageReader<?>>, HandlerMethodArgumentResolver> function) {
-			if (!CollectionUtils.isEmpty(this.messageReaders)) {
-				add(function.apply(this.messageReaders));
-			}
-		}
-
-		public void addIfModelAttribute(Supplier<HandlerMethodArgumentResolver> supplier) {
-			if (this.modelAttributeSupported) {
-				add(supplier.get());
-			}
-		}
-
-		public void addCustomResolvers() {
-			this.customResolvers.forEach(this::add);
-		}
-
-
-		public List<HandlerMethodArgumentResolver> getResolvers() {
-			return this.result;
-		}
-
-		public List<SyncHandlerMethodArgumentResolver> getSyncResolvers() {
-			return this.result.stream()
-					.filter(resolver -> resolver instanceof SyncHandlerMethodArgumentResolver)
-					.map(resolver -> (SyncHandlerMethodArgumentResolver) resolver)
-					.collect(Collectors.toList());
-		}
-
-		public static Builder configurer(ArgumentResolverConfigurer configurer) {
-			return new Builder(configurer);
-		}
-
-
-		public static class Builder {
-
-			private final ArgumentResolverConfigurer resolvers;
-
-			public Builder(ArgumentResolverConfigurer configurer) {
-				this.resolvers = configurer;
-			}
-
-			public ArgumentResolverRegistrar fullSupport(List<HttpMessageReader<?>> httpMessageReaders) {
-				return new ArgumentResolverRegistrar(this.resolvers, httpMessageReaders, true);
-			}
-
-			public ArgumentResolverRegistrar modelAttributeSupport() {
-				return new ArgumentResolverRegistrar(this.resolvers, Collections.emptyList(), true);
-			}
-
-			public ArgumentResolverRegistrar basic() {
-				return new ArgumentResolverRegistrar(this.resolvers, Collections.emptyList(), false);
-			}
-		}
-	}
-
 }

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

@@ -169,7 +169,7 @@ public void afterPropertiesSet() throws Exception {
 		}
 
 		this.methodResolver = new ControllerMethodResolver(this.argumentResolverConfigurer,
-				this.messageReaders, this.reactiveAdapterRegistry, this.applicationContext);
+				this.reactiveAdapterRegistry, this.applicationContext, this.messageReaders);
 
 		this.modelInitializer = new ModelInitializer(this.methodResolver, this.reactiveAdapterRegistry);
 	}

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolverTests.java

@@ -76,7 +76,7 @@ public void setUp() throws Exception {
 		applicationContext.refresh();
 
 		this.methodResolver = new ControllerMethodResolver(
-				resolvers, codecs.getReaders(), ReactiveAdapterRegistry.getSharedInstance(), applicationContext);
+				resolvers, ReactiveAdapterRegistry.getSharedInstance(), applicationContext, codecs.getReaders());
 
 		Method method = ResolvableMethod.on(TestController.class).mockCall(TestController::handle).method();
 		this.handlerMethod = new HandlerMethod(new TestController(), method);

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java

@@ -79,7 +79,7 @@ public void setUp() throws Exception {
 		resolverConfigurer.addCustomResolver(new ModelArgumentResolver(adapterRegistry));
 
 		ControllerMethodResolver methodResolver = new ControllerMethodResolver(
-				resolverConfigurer, Collections.emptyList(), adapterRegistry, new StaticApplicationContext());
+				resolverConfigurer, adapterRegistry, new StaticApplicationContext(), Collections.emptyList());
 
 		this.modelInitializer = new ModelInitializer(methodResolver, adapterRegistry);
 	}