Refactoring in AbstractMethodMessageHandler

Split out the mechanics of invoking a HandlerMethod and handling the
result into a separate helper class.

See gh-21987

spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/AbstractExceptionHandlerMethodResolver.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.
@@ -85,7 +85,7 @@ public abstract class AbstractExceptionHandlerMethodResolver {
 	 * @return a Method to handle the exception, or {@code null} if none found
 	 */
 	@Nullable
-	public Method resolveMethod(Exception exception) {
+	public Method resolveMethod(Throwable exception) {
 		Method method = resolveMethodByExceptionType(exception.getClass());
 		if (method == null) {
 			Throwable cause = exception.getCause();

spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/reactive/AbstractMethodMessageHandler.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.
@@ -24,7 +24,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Predicate;
 
 import org.apache.commons.logging.Log;
@@ -35,11 +34,9 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.core.MethodIntrospector;
-import org.springframework.core.MethodParameter;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.lang.Nullable;
 import org.springframework.messaging.Message;
-import org.springframework.messaging.MessageHandlingException;
 import org.springframework.messaging.MessagingException;
 import org.springframework.messaging.ReactiveMessageHandler;
 import org.springframework.messaging.handler.HandlerMethod;
@@ -82,20 +79,14 @@ public abstract class AbstractMethodMessageHandler<T>
 	protected final Log logger = LogFactory.getLog(getClass());
 
 
+	@Nullable
+	private Predicate<Class<?>> handlerPredicate;
+
 	private ArgumentResolverConfigurer argumentResolverConfigurer = new ArgumentResolverConfigurer();
 
 	private ReturnValueHandlerConfigurer returnValueHandlerConfigurer = new ReturnValueHandlerConfigurer();
 
-	private final HandlerMethodArgumentResolverComposite argumentResolvers =
+	private final InvocableHelper invocableHelper = new InvocableHelper(this::createExceptionMethodResolverFor);
-			new HandlerMethodArgumentResolverComposite();
-
-	private final HandlerMethodReturnValueHandlerComposite returnValueHandlers =
-			new HandlerMethodReturnValueHandlerComposite();
-
-	private ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
-
-	@Nullable
-	private Predicate<Class<?>> handlerPredicate;
 
 	@Nullable
 	private ApplicationContext applicationContext;
@@ -104,12 +95,24 @@ public abstract class AbstractMethodMessageHandler<T>
 
 	private final MultiValueMap<String, T> destinationLookup = new LinkedMultiValueMap<>(64);
 
-	private final Map<Class<?>, AbstractExceptionHandlerMethodResolver> exceptionHandlerCache =
-			new ConcurrentHashMap<>(64);
 
-	private final Map<MessagingAdviceBean, AbstractExceptionHandlerMethodResolver> exceptionHandlerAdviceCache =
+	/**
-			new LinkedHashMap<>(64);
+	 * Configure a predicate to decide if which beans in the Spring context
+	 * should be checked to see if they have message handling methods.
+	 * <p>By default this is not set and sub-classes should configure it in
+	 * order to enable auto-detection of message handling methods.
+	 */
+	public void setHandlerPredicate(@Nullable Predicate<Class<?>> handlerPredicate) {
+		this.handlerPredicate = handlerPredicate;
+	}
 
+	/**
+	 * Return the {@link #setHandlerPredicate configured} handler predicate.
+	 */
+	@Nullable
+	public Predicate<Class<?>> getHandlerPredicate() {
+		return this.handlerPredicate;
+	}
 
 	/**
 	 * Configure custom resolvers for handler method arguments.
@@ -141,39 +144,20 @@ public abstract class AbstractMethodMessageHandler<T>
 		return this.returnValueHandlerConfigurer;
 	}
 
-	/**
-	 * Configure a predicate to decide if which beans in the Spring context
-	 * should be checked to see if they have message handling methods.
-	 * <p>By default this is not set and sub-classes should configure it in
-	 * order to enable auto-detection of message handling methods.
-	 */
-	public void setHandlerPredicate(@Nullable Predicate<Class<?>> handlerPredicate) {
-		this.handlerPredicate = handlerPredicate;
-	}
-
-	/**
-	 * Return the {@link #setHandlerPredicate configured} handler predicate.
-	 */
-	@Nullable
-	public Predicate<Class<?>> getHandlerPredicate() {
-		return this.handlerPredicate;
-	}
-
 	/**
 	 * Configure the registry for adapting various reactive types.
 	 * <p>By default this is an instance of {@link ReactiveAdapterRegistry} with
 	 * default settings.
 	 */
 	public void setReactiveAdapterRegistry(ReactiveAdapterRegistry registry) {
-		Assert.notNull(registry, "ReactiveAdapterRegistry is required");
+		this.invocableHelper.setReactiveAdapterRegistry(registry);
-		this.reactiveAdapterRegistry = registry;
 	}
 
 	/**
 	 * Return the configured registry for adapting reactive types.
 	 */
 	public ReactiveAdapterRegistry getReactiveAdapterRegistry() {
-		return this.reactiveAdapterRegistry;
+		return this.invocableHelper.getReactiveAdapterRegistry();
 	}
 
 	@Override
@@ -193,7 +177,7 @@ public abstract class AbstractMethodMessageHandler<T>
 	protected void registerExceptionHandlerAdvice(
 			MessagingAdviceBean bean, AbstractExceptionHandlerMethodResolver resolver) {
 
-		this.exceptionHandlerAdviceCache.put(bean, resolver);
+		this.invocableHelper.registerExceptionHandlerAdvice(bean, resolver);
 	}
 
 	/**
@@ -219,13 +203,13 @@ public abstract class AbstractMethodMessageHandler<T>
 		if (resolvers.isEmpty()) {
 			resolvers = new ArrayList<>(this.argumentResolverConfigurer.getCustomResolvers());
 		}
-		this.argumentResolvers.addResolvers(resolvers);
+		this.invocableHelper.addArgumentResolvers(resolvers);
 
 		List<? extends HandlerMethodReturnValueHandler> handlers = initReturnValueHandlers();
 		if (handlers.isEmpty()) {
 			handlers = new ArrayList<>(this.returnValueHandlerConfigurer.getCustomHandlers());
 		}
-		this.returnValueHandlers.addHandlers(handlers);
+		this.invocableHelper.addReturnValueHandlers(handlers);
 
 		initHandlerMethods();
 	}
@@ -379,21 +363,7 @@ public abstract class AbstractMethodMessageHandler<T>
 			return Mono.empty();
 		}
 		HandlerMethod handlerMethod = match.getHandlerMethod().createWithResolvedBean();
-		InvocableHandlerMethod invocable = new InvocableHandlerMethod(handlerMethod);
+		return this.invocableHelper.handleMessage(handlerMethod, message);
-		invocable.setArgumentResolvers(this.argumentResolvers.getResolvers());
-		if (logger.isDebugEnabled()) {
-			logger.debug("Invoking " + invocable.getShortLogMessage());
-		}
-		return invocable.invoke(message)
-				.flatMap(value -> {
-					MethodParameter returnType = invocable.getReturnType();
-					return this.returnValueHandlers.handleReturnValue(value, returnType, message);
-				})
-				.onErrorResume(throwable -> {
-					Exception ex = (throwable instanceof Exception) ? (Exception) throwable :
-							new MessageHandlingException(message, "HandlerMethod invocation error", throwable);
-					return processHandlerException(message, handlerMethod, ex);
-				});
 	}
 
 	@Nullable
@@ -482,68 +452,6 @@ public abstract class AbstractMethodMessageHandler<T>
 		logger.debug("No handlers for destination '" + destination + "'");
 	}
 
-
-	private Mono<Void> processHandlerException(Message<?> message, HandlerMethod handlerMethod, Exception ex) {
-		InvocableHandlerMethod exceptionInvocable = findExceptionHandler(handlerMethod, ex);
-		if (exceptionInvocable == null) {
-			logger.error("Unhandled exception from message handling method", ex);
-			return Mono.error(ex);
-		}
-		exceptionInvocable.setArgumentResolvers(this.argumentResolvers.getResolvers());
-		if (logger.isDebugEnabled()) {
-			logger.debug("Invoking " + exceptionInvocable.getShortLogMessage());
-		}
-		return exceptionInvocable.invoke(message, ex)
-				.flatMap(value -> {
-					MethodParameter returnType = exceptionInvocable.getReturnType();
-					return this.returnValueHandlers.handleReturnValue(value, returnType, message);
-				});
-	}
-
-	/**
-	 * Find an exception handling method for the given exception.
-	 * <p>The default implementation searches methods in the class hierarchy of
-	 * the HandlerMethod first and if not found, it continues searching for
-	 * additional handling methods registered via
-	 * {@link #registerExceptionHandlerAdvice(MessagingAdviceBean, AbstractExceptionHandlerMethodResolver)}.
-	 * @param handlerMethod the method where the exception was raised
-	 * @param exception the raised exception
-	 * @return a method to handle the exception, or {@code null}
-	 */
-	@Nullable
-	protected InvocableHandlerMethod findExceptionHandler(HandlerMethod handlerMethod, Exception exception) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Searching for methods to handle " + exception.getClass().getSimpleName());
-		}
-		Class<?> beanType = handlerMethod.getBeanType();
-		AbstractExceptionHandlerMethodResolver resolver = this.exceptionHandlerCache.get(beanType);
-		if (resolver == null) {
-			resolver = createExceptionMethodResolverFor(beanType);
-			this.exceptionHandlerCache.put(beanType, resolver);
-		}
-		InvocableHandlerMethod exceptionHandlerMethod = null;
-		Method method = resolver.resolveMethod(exception);
-		if (method != null) {
-			exceptionHandlerMethod = new InvocableHandlerMethod(handlerMethod.getBean(), method);
-		}
-		else {
-			for (MessagingAdviceBean advice : this.exceptionHandlerAdviceCache.keySet()) {
-				if (advice.isApplicableToBeanType(beanType)) {
-					resolver = this.exceptionHandlerAdviceCache.get(advice);
-					method = resolver.resolveMethod(exception);
-					if (method != null) {
-						exceptionHandlerMethod = new InvocableHandlerMethod(advice.resolveBean(), method);
-						break;
-					}
-				}
-			}
-		}
-		if (exceptionHandlerMethod != null) {
-			exceptionHandlerMethod.setArgumentResolvers(this.argumentResolvers.getResolvers());
-		}
-		return exceptionHandlerMethod;
-	}
-
 	/**
 	 * Create a concrete instance of {@link AbstractExceptionHandlerMethodResolver}
 	 * that finds exception handling methods based on some criteria, e.g. based

spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/reactive/InvocableHelper.java

@@ -0,0 +1,206 @@
+/*
+ * 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.
+ * 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.messaging.handler.invocation.reactive;
+
+import java.lang.reflect.Method;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import reactor.core.publisher.Mono;
+
+import org.springframework.core.MethodParameter;
+import org.springframework.core.ReactiveAdapterRegistry;
+import org.springframework.lang.Nullable;
+import org.springframework.messaging.Message;
+import org.springframework.messaging.handler.HandlerMethod;
+import org.springframework.messaging.handler.MessagingAdviceBean;
+import org.springframework.messaging.handler.invocation.AbstractExceptionHandlerMethodResolver;
+import org.springframework.util.Assert;
+
+/**
+ * Help to initialize and invoke an {@link InvocableHandlerMethod}, and to then
+ * apply return value handling and exception handling. Holds all necessary
+ * configuration necessary to do so.
+ *
+ * @author Rossen Stoyanchev
+ * @since 5.2
+ */
+class InvocableHelper {
+
+	private static Log logger = LogFactory.getLog(InvocableHelper.class);
+
+
+	private final HandlerMethodArgumentResolverComposite argumentResolvers =
+			new HandlerMethodArgumentResolverComposite();
+
+	private final HandlerMethodReturnValueHandlerComposite returnValueHandlers =
+			new HandlerMethodReturnValueHandlerComposite();
+
+	private ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
+
+	private final Function<Class<?>, AbstractExceptionHandlerMethodResolver> exceptionMethodResolverFactory;
+
+	private final Map<Class<?>, AbstractExceptionHandlerMethodResolver> exceptionHandlerCache =
+			new ConcurrentHashMap<>(64);
+
+	private final Map<MessagingAdviceBean, AbstractExceptionHandlerMethodResolver> exceptionHandlerAdviceCache =
+			new LinkedHashMap<>(64);
+
+
+	public InvocableHelper(
+			Function<Class<?>, AbstractExceptionHandlerMethodResolver> exceptionMethodResolverFactory) {
+
+		this.exceptionMethodResolverFactory = exceptionMethodResolverFactory;
+	}
+
+	/**
+	 * Add the arguments resolvers to use for message handling and exception
+	 * handling methods.
+	 */
+	public void addArgumentResolvers(List<? extends HandlerMethodArgumentResolver> resolvers) {
+		this.argumentResolvers.addResolvers(resolvers);
+	}
+
+	/**
+	 * Add the return value handlers to use for message handling and exception
+	 * handling methods.
+	 */
+	public void addReturnValueHandlers(List<? extends HandlerMethodReturnValueHandler> handlers) {
+		this.returnValueHandlers.addHandlers(handlers);
+	}
+
+	/**
+	 * Configure the registry for adapting various reactive types.
+	 * <p>By default this is an instance of {@link ReactiveAdapterRegistry} with
+	 * default settings.
+	 */
+	public void setReactiveAdapterRegistry(ReactiveAdapterRegistry registry) {
+		Assert.notNull(registry, "ReactiveAdapterRegistry is required");
+		this.reactiveAdapterRegistry = registry;
+	}
+
+	/**
+	 * Return the configured registry for adapting reactive types.
+	 */
+	public ReactiveAdapterRegistry getReactiveAdapterRegistry() {
+		return this.reactiveAdapterRegistry;
+	}
+
+	/**
+	 * Method to populate the MessagingAdviceBean cache (e.g. to support "global"
+	 * {@code @MessageExceptionHandler}).
+	 */
+	public void registerExceptionHandlerAdvice(
+			MessagingAdviceBean bean, AbstractExceptionHandlerMethodResolver resolver) {
+
+		this.exceptionHandlerAdviceCache.put(bean, resolver);
+	}
+
+
+	/**
+	 * Create {@link InvocableHandlerMethod} with the configured arg resolvers.
+	 * @param handlerMethod the target handler method to invoke
+	 * @return the created instance
+	 */
+
+	public InvocableHandlerMethod initMessageMappingMethod(HandlerMethod handlerMethod) {
+		InvocableHandlerMethod invocable = new InvocableHandlerMethod(handlerMethod);
+		invocable.setArgumentResolvers(this.argumentResolvers.getResolvers());
+		return invocable;
+	}
+
+	/**
+	 * Find an exception handling method for the given exception.
+	 * <p>The default implementation searches methods in the class hierarchy of
+	 * the HandlerMethod first and if not found, it continues searching for
+	 * additional handling methods registered via
+	 * {@link #registerExceptionHandlerAdvice}.
+	 * @param handlerMethod the method where the exception was raised
+	 * @param ex the exception raised or signaled
+	 * @return a method to handle the exception, or {@code null}
+	 */
+	@Nullable
+	public InvocableHandlerMethod initExceptionHandlerMethod(HandlerMethod handlerMethod, Throwable ex) {
+		if (logger.isDebugEnabled()) {
+			logger.debug("Searching for methods to handle " + ex.getClass().getSimpleName());
+		}
+		Class<?> beanType = handlerMethod.getBeanType();
+		AbstractExceptionHandlerMethodResolver resolver = this.exceptionHandlerCache.get(beanType);
+		if (resolver == null) {
+			resolver = this.exceptionMethodResolverFactory.apply(beanType);
+			this.exceptionHandlerCache.put(beanType, resolver);
+		}
+		InvocableHandlerMethod exceptionHandlerMethod = null;
+		Method method = resolver.resolveMethod(ex);
+		if (method != null) {
+			exceptionHandlerMethod = new InvocableHandlerMethod(handlerMethod.getBean(), method);
+		}
+		else {
+			for (MessagingAdviceBean advice : this.exceptionHandlerAdviceCache.keySet()) {
+				if (advice.isApplicableToBeanType(beanType)) {
+					resolver = this.exceptionHandlerAdviceCache.get(advice);
+					method = resolver.resolveMethod(ex);
+					if (method != null) {
+						exceptionHandlerMethod = new InvocableHandlerMethod(advice.resolveBean(), method);
+						break;
+					}
+				}
+			}
+		}
+		if (exceptionHandlerMethod != null) {
+			logger.debug("Found exception handler " + exceptionHandlerMethod.getShortLogMessage());
+			exceptionHandlerMethod.setArgumentResolvers(this.argumentResolvers.getResolvers());
+		}
+		else {
+			logger.error("No exception handling method", ex);
+		}
+		return exceptionHandlerMethod;
+	}
+
+
+	public Mono<Void> handleMessage(HandlerMethod handlerMethod, Message<?> message) {
+		InvocableHandlerMethod invocable = initMessageMappingMethod(handlerMethod);
+		if (logger.isDebugEnabled()) {
+			logger.debug("Invoking " + invocable.getShortLogMessage());
+		}
+		return invocable.invoke(message)
+				.flatMap(returnValue -> handleReturnValue(returnValue, invocable, message))
+				.onErrorResume(ex -> {
+					InvocableHandlerMethod exHandler = initExceptionHandlerMethod(handlerMethod, ex);
+					if (exHandler == null) {
+						return Mono.error(ex);
+					}
+					if (logger.isDebugEnabled()) {
+						logger.debug("Invoking " + exHandler.getShortLogMessage());
+					}
+					return exHandler.invoke(message, ex)
+							.flatMap(returnValue -> handleReturnValue(returnValue, exHandler, message));
+				});
+	}
+
+	private Mono<Void> handleReturnValue(
+			@Nullable Object returnValue, HandlerMethod handlerMethod, Message<?> message) {
+
+		MethodParameter returnType = handlerMethod.getReturnType();
+		return this.returnValueHandlers.handleReturnValue(returnValue, returnType, message);
+	}
+
+}