From 79f78a36f7744eb24fb30677d80de02b8686cdb0 Mon Sep 17 00:00:00 2001
From: Oleg Zhurakousky <ozhurakousky@pivotal.io>
Date: Wed, 9 Jan 2019 21:38:20 +0100
Subject: [PATCH] INT-4571 Make MessageHandlerMethodFactory injectable

Make MessageHandlerMethodFactory injectable into MessagingMethodInvokerHelper
Allow 'handlerMethod' to be overriden
Deprecate HandlerMethodArgumentResolversHolder
Add 'integrationMessageHandlerMethodFactory' property to IntegrationContextUtils
Add test that actually validates that custom resolver gets picked up
---
 .../context/IntegrationContextUtils.java      |  5 +-
 .../HandlerMethodArgumentResolversHolder.java |  5 +-
 .../support/MessagingMethodInvokerHelper.java | 92 +++++++++++--------
 .../MethodInvokingMessageProcessorTests.java  | 65 ++++++++++++-
 4 files changed, 127 insertions(+), 40 deletions(-)

diff --git a/spring-integration-core/src/main/java/org/springframework/integration/context/IntegrationContextUtils.java b/spring-integration-core/src/main/java/org/springframework/integration/context/IntegrationContextUtils.java
index 5adc4954151..40e0bf66a41 100644
--- a/spring-integration-core/src/main/java/org/springframework/integration/context/IntegrationContextUtils.java
+++ b/spring-integration-core/src/main/java/org/springframework/integration/context/IntegrationContextUtils.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.
@@ -36,6 +36,7 @@
  * @author Josh Long
  * @author Artem Bilan
  * @author Gary Russell
+ * @author Oleg Zhurakousky
  */
 public abstract class IntegrationContextUtils {
 
@@ -105,6 +106,8 @@ public abstract class IntegrationContextUtils {
 
 	public static final String DISPOSABLES_BEAN_NAME = "integrationDisposableAutoCreatedBeans";
 
+	public static final String MESSAGE_HANDLER_FACTORY_BEAN_NAME = "integrationMessageHandlerMethodFactory";
+
 	/**
 	 * @param beanFactory BeanFactory for lookup, must not be null.
 	 * @return The {@link MetadataStore} bean whose name is "metadataStore".
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/support/HandlerMethodArgumentResolversHolder.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/support/HandlerMethodArgumentResolversHolder.java
index 242a40a68d4..8f29286dc13 100644
--- a/spring-integration-core/src/main/java/org/springframework/integration/handler/support/HandlerMethodArgumentResolversHolder.java
+++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/support/HandlerMethodArgumentResolversHolder.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2018 the original author or authors.
+ * Copyright 2017-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.
@@ -28,8 +28,9 @@
  * @author Gary Russell
  *
  * @since 5.0
- *
+ * @deprecated as of 5.1.2. Instead simply configure your own MessageHandlerMethodFactory as a bean.
  */
+@Deprecated
 public class HandlerMethodArgumentResolversHolder {
 
 	private final List<HandlerMethodArgumentResolver> resolvers;
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java
index 24cdfcc1609..331557f96b6 100644
--- a/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java
+++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java
@@ -94,6 +94,7 @@
 import org.springframework.messaging.handler.annotation.Headers;
 import org.springframework.messaging.handler.annotation.Payload;
 import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory;
+import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory;
 import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
 import org.springframework.messaging.handler.invocation.InvocableHandlerMethod;
 import org.springframework.messaging.handler.invocation.MethodArgumentResolutionException;
@@ -167,7 +168,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
 		SPEL_COMPILERS.put(SpelCompilerMode.MIXED, EXPRESSION_PARSER_MIXED);
 	}
 
-	private final DefaultMessageHandlerMethodFactory messageHandlerMethodFactory =
+	private MessageHandlerMethodFactory messageHandlerMethodFactory =
 			new DefaultMessageHandlerMethodFactory();
 
 	private final Object targetObject;
@@ -184,7 +185,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
 
 	private final List<Map<Class<?>, HandlerMethod>> handlerMethodsList;
 
-	private final HandlerMethod handlerMethod;
+	private HandlerMethod handlerMethod;
 
 	private final TypeDescriptor expectedType;
 
@@ -257,16 +258,7 @@ private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annota
 
 		Assert.notNull(targetObject, "targetObject must not be null");
 		this.targetObject = targetObject;
-		try {
-			InvocableHandlerMethod invocableHandlerMethod =
-					this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, method);
-			this.handlerMethod = new HandlerMethod(invocableHandlerMethod, canProcessMessageList);
-			this.defaultHandlerMethod = null;
-			checkSpelInvokerRequired(getTargetClass(targetObject), method, this.handlerMethod);
-		}
-		catch (IneligibleMethodException e) {
-			throw new IllegalArgumentException(e);
-		}
+		createHandlerMethod();
 		this.handlerMethods = null;
 		this.handlerMessageMethods = null;
 		this.handlerMethodsList = null;
@@ -295,7 +287,8 @@ public void setUseSpelInvoker(boolean useSpelInvoker) {
 	@Override
 	public void setBeanFactory(@NonNull BeanFactory beanFactory) {
 		super.setBeanFactory(beanFactory);
-		this.messageHandlerMethodFactory.setBeanFactory(beanFactory);
+		((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).setBeanFactory(beanFactory);
+
 		if (beanFactory instanceof ConfigurableListableBeanFactory) {
 			BeanExpressionResolver beanExpressionResolver = ((ConfigurableListableBeanFactory) beanFactory)
 					.getBeanExpressionResolver();
@@ -310,7 +303,7 @@ public void setBeanFactory(@NonNull BeanFactory beanFactory) {
 	public void setConversionService(ConversionService conversionService) {
 		super.setConversionService(conversionService);
 		if (conversionService != null) {
-			this.messageHandlerMethodFactory.setConversionService(conversionService);
+			((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).setConversionService(conversionService);
 		}
 	}
 
@@ -403,6 +396,24 @@ private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annota
 		this.jsonObjectMapper = mapper;
 	}
 
+	private boolean isProvidedMessageHandlerFactoryBean() {
+		return getBeanFactory() != null
+				&& getBeanFactory().containsBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME);
+	}
+
+	private void createHandlerMethod() {
+		try {
+			InvocableHandlerMethod invocableHandlerMethod =
+					this.messageHandlerMethodFactory.createInvocableHandlerMethod(this.targetObject, this.method);
+			this.handlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
+			this.defaultHandlerMethod = null;
+			checkSpelInvokerRequired(getTargetClass(this.targetObject), this.method, this.handlerMethod);
+		}
+		catch (IneligibleMethodException e) {
+			throw new IllegalArgumentException(e);
+		}
+	}
+
 	private void setDisplayString(Object targetObject, Object targetMethod) {
 		StringBuilder sb = new StringBuilder(targetObject.getClass().getName());
 		if (targetMethod instanceof Method) {
@@ -503,37 +514,46 @@ private void initializeHandler(HandlerMethod candidate) {
 		candidate.initialized = true;
 	}
 
+	@SuppressWarnings("deprecation")
 	private synchronized void initialize() throws Exception {
 		if (!this.initialized) {
-			BeanFactory beanFactory = getBeanFactory();
-			if (beanFactory != null &&
-					beanFactory.containsBean(IntegrationContextUtils.ARGUMENT_RESOLVER_MESSAGE_CONVERTER_BEAN_NAME)) {
+			if (isProvidedMessageHandlerFactoryBean()) {
+				logger.info("Overriding default instance of MessageHandlerMethodFactory with provided one.");
+				this.messageHandlerMethodFactory = getBeanFactory()
+						.getBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME, MessageHandlerMethodFactory.class);
+				createHandlerMethod();
+			}
+			else {
+				BeanFactory beanFactory = getBeanFactory();
+				if (beanFactory != null &&
+						beanFactory.containsBean(IntegrationContextUtils.ARGUMENT_RESOLVER_MESSAGE_CONVERTER_BEAN_NAME)) {
 
-				try {
-					MessageConverter messageConverter =
-							beanFactory.getBean(IntegrationContextUtils.ARGUMENT_RESOLVER_MESSAGE_CONVERTER_BEAN_NAME,
-									MessageConverter.class);
+					try {
+						MessageConverter messageConverter =
+								beanFactory.getBean(IntegrationContextUtils.ARGUMENT_RESOLVER_MESSAGE_CONVERTER_BEAN_NAME,
+										MessageConverter.class);
 
-					this.messageHandlerMethodFactory.setMessageConverter(messageConverter);
+						((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).setMessageConverter(messageConverter);
 
-					HandlerMethodArgumentResolversHolder handlerMethodArgumentResolversHolder =
-							beanFactory.getBean(this.canProcessMessageList
-											? IntegrationContextUtils.LIST_ARGUMENT_RESOLVERS_BEAN_NAME
-											: IntegrationContextUtils.ARGUMENT_RESOLVERS_BEAN_NAME,
-									HandlerMethodArgumentResolversHolder.class);
+						HandlerMethodArgumentResolversHolder handlerMethodArgumentResolversHolder =
+								beanFactory.getBean(this.canProcessMessageList
+												? IntegrationContextUtils.LIST_ARGUMENT_RESOLVERS_BEAN_NAME
+												: IntegrationContextUtils.ARGUMENT_RESOLVERS_BEAN_NAME,
+										HandlerMethodArgumentResolversHolder.class);
 
-					this.messageHandlerMethodFactory.setCustomArgumentResolvers(
-							handlerMethodArgumentResolversHolder.getResolvers());
+						((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).setCustomArgumentResolvers(
+								handlerMethodArgumentResolversHolder.getResolvers());
+					}
+					catch (NoSuchBeanDefinitionException e) {
+						configureLocalMessageHandlerFactory();
+					}
 				}
-				catch (NoSuchBeanDefinitionException e) {
+				else {
 					configureLocalMessageHandlerFactory();
 				}
-			}
-			else {
-				configureLocalMessageHandlerFactory();
+				((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).afterPropertiesSet();
 			}
 
-			this.messageHandlerMethodFactory.afterPropertiesSet();
 			prepareEvaluationContext();
 			this.initialized = true;
 		}
@@ -551,7 +571,7 @@ private void configureLocalMessageHandlerFactory() {
 			messageConverter = getBeanFactory()
 									.getBean(IntegrationContextUtils.ARGUMENT_RESOLVER_MESSAGE_CONVERTER_BEAN_NAME,
 											MessageConverter.class);
-			this.messageHandlerMethodFactory.setMessageConverter(messageConverter);
+			((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).setMessageConverter(messageConverter);
 		}
 		else {
 			messageConverter = new ConfigurableCompositeMessageConverter();
@@ -579,7 +599,7 @@ private void configureLocalMessageHandlerFactory() {
 
 		customArgumentResolvers.add(mapArgumentResolver);
 
-		this.messageHandlerMethodFactory.setCustomArgumentResolvers(customArgumentResolvers);
+		((DefaultMessageHandlerMethodFactory) this.messageHandlerMethodFactory).setCustomArgumentResolvers(customArgumentResolvers);
 	}
 
 	@SuppressWarnings("unchecked")
diff --git a/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java b/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java
index 59962ca1720..120074b4f2e 100644
--- a/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java
+++ b/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.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.
@@ -61,7 +61,9 @@
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.core.MethodParameter;
 import org.springframework.expression.Expression;
 import org.springframework.expression.spel.SpelCompilerMode;
 import org.springframework.expression.spel.SpelEvaluationException;
@@ -70,19 +72,26 @@
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.integration.annotation.ServiceActivator;
 import org.springframework.integration.annotation.UseSpelInvoker;
+import org.springframework.integration.channel.QueueChannel;
 import org.springframework.integration.config.EnableIntegration;
+import org.springframework.integration.context.IntegrationContextUtils;
 import org.springframework.integration.gateway.GatewayProxyFactoryBean;
 import org.springframework.integration.gateway.RequestReplyExchanger;
 import org.springframework.integration.handler.support.MessagingMethodInvokerHelper;
 import org.springframework.integration.support.MessageBuilder;
 import org.springframework.integration.test.util.TestUtils;
 import org.springframework.messaging.Message;
+import org.springframework.messaging.MessageChannel;
 import org.springframework.messaging.MessageHandler;
 import org.springframework.messaging.MessageHandlingException;
 import org.springframework.messaging.MessageHeaders;
 import org.springframework.messaging.MessagingException;
+import org.springframework.messaging.PollableChannel;
 import org.springframework.messaging.handler.annotation.Header;
 import org.springframework.messaging.handler.annotation.Payload;
+import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory;
+import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory;
+import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
 import org.springframework.messaging.support.GenericMessage;
 import org.springframework.util.StopWatch;
 
@@ -107,6 +116,60 @@ public class MethodInvokingMessageProcessorTests {
 	@Rule
 	public ExpectedException expected = ExpectedException.none();
 
+	@Test
+	public void testMessageHandlerMethodFactoryOverride() {
+		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(MyConfiguration.class)) {
+			MessageChannel channel = context.getBean("foo", MessageChannel.class);
+			channel.send(MessageBuilder.withPayload("Bob Smith").build());
+			PollableChannel out = context.getBean("out", PollableChannel.class);
+			assertEquals("Person: Bob Smith", out.receive().getPayload());
+		}
+	}
+
+	@EnableIntegration
+	public static class MyConfiguration {
+		@ServiceActivator(inputChannel = "foo", outputChannel = "out")
+		public String foo(Person person) {
+			return person.toString();
+		}
+
+		@Bean
+		public PollableChannel out() {
+			return new QueueChannel();
+		}
+
+		@Bean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME)
+		public MessageHandlerMethodFactory messageHandlerMethodFactory() {
+			DefaultMessageHandlerMethodFactory f = new DefaultMessageHandlerMethodFactory();
+			HandlerMethodArgumentResolver resolver = new HandlerMethodArgumentResolver() {
+
+				@Override
+				public boolean supportsParameter(MethodParameter parameter) {
+					return true;
+				}
+
+				@Override
+				public Object resolveArgument(MethodParameter parameter, Message<?> message) throws Exception {
+					String[] names = ((String) message.getPayload()).split(" ");
+					return new Person(names[0], names[1]);
+				}
+			};
+			f.setArgumentResolvers(Collections.singletonList(resolver));
+			f.afterPropertiesSet();
+			return f;
+		}
+
+		public static class Person {
+			private final String name;
+			public Person(String fname, String lname) {
+				this.name = fname + " " + lname;
+			}
+			public String toString() {
+				return "Person: " + name;
+			}
+		}
+	}
+
 	@Test
 	public void testHandlerInheritanceMethodImplInSuper() {
 		class A {