Fix MMIHelper for reinitialization (#2786)

* Fix MMIHelper for reinitialization

If we provide a custom `MessageHandlerMethodFactory`, the
`MessagingMethodInvokerHelper` obtains its bean and reinitialize a
`handlerMethod`, but it is done only for single, explicit
and provided method.
In case of `Function` or `Consumer` we use local names for methods to
extract, but this is not populated to properties of the
`MessagingMethodInvokerHelper`

* Change an internal `MessagingMethodInvokerHelper` logic to recreate
`InvocableHandlerMethod` instances based on the `MessageHandlerMethodFactory`
bean for all the methods scanned on the target.
* Populate a `handlerMethodsList` for the purpose above even if we have
only one candidate
* Fix `AggregatorParserTests` to reflect the current logic around
`handlerMethodsList`
* Prove that new logic works well with a custom `MessageHandlerMethodFactory`
bean in the `MessagingAnnotationsWithBeanAnnotationTests`

**Cherry-pick to 5.1.x**

* * Move CTOR to the proper place

# Conflicts:
#	spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java
#	spring-integration-core/src/test/java/org/springframework/integration/config/AggregatorParserTests.java
This commit is contained in:
Artem Bilan
2019-03-07 12:48:15 -05:00
parent d8ec8352ff
commit a42411f2d6
3 changed files with 105 additions and 102 deletions

View File

@@ -183,7 +183,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
private final Map<Class<?>, HandlerMethod> handlerMessageMethods;
private final List<Map<Class<?>, HandlerMethod>> handlerMethodsList;
private final List<Map<Class<?>, HandlerMethod>> handlerMethodsList = new LinkedList<>();
private HandlerMethod handlerMethod;
@@ -258,10 +258,11 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
Assert.notNull(targetObject, "targetObject must not be null");
this.targetObject = targetObject;
createHandlerMethod();
this.handlerMethod = createHandlerMethod(this.method);
this.handlerMethods = null;
this.handlerMessageMethods = null;
this.handlerMethodsList = null;
this.handlerMethodsList.add(
Collections.singletonMap(this.handlerMethod.targetParameterType, this.handlerMethod));
setDisplayString(targetObject, method);
JsonObjectMapper<?, ?> mapper;
@@ -274,6 +275,54 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
this.jsonObjectMapper = mapper;
}
private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annotation> annotationType,
String methodName, Class<?> expectedType, boolean canProcessMessageList) {
this.annotationType = annotationType;
this.methodName = methodName;
this.canProcessMessageList = canProcessMessageList;
Assert.notNull(targetObject, "targetObject must not be null");
if (expectedType != null) {
this.expectedType = TypeDescriptor.valueOf(expectedType);
}
else {
this.expectedType = null;
}
this.targetObject = targetObject;
Map<String, Map<Class<?>, HandlerMethod>> handlerMethodsForTarget =
findHandlerMethodsForTarget(annotationType, methodName, expectedType != null);
Map<Class<?>, HandlerMethod> methods = handlerMethodsForTarget.get(CANDIDATE_METHODS);
Map<Class<?>, HandlerMethod> messageMethods = handlerMethodsForTarget.get(CANDIDATE_MESSAGE_METHODS);
if ((methods.size() == 1 && messageMethods.isEmpty()) ||
(messageMethods.size() == 1 && methods.isEmpty())) {
if (methods.size() == 1) {
this.handlerMethod = methods.values().iterator().next();
}
else {
this.handlerMethod = messageMethods.values().iterator().next();
}
}
else {
this.handlerMethod = null;
}
this.handlerMethods = methods;
this.handlerMessageMethods = messageMethods;
//TODO Consider to use global option to determine a precedence of methods
this.handlerMethodsList.add(this.handlerMethods);
this.handlerMethodsList.add(this.handlerMessageMethods);
setDisplayString(targetObject, methodName);
JsonObjectMapper<?, ?> mapper;
try {
mapper = JsonObjectMapperProvider.newInstance();
}
catch (IllegalStateException e) {
mapper = null;
}
this.jsonObjectMapper = mapper;
}
/**
* A {@code boolean} flag to use SpEL Expression evaluation or {@link InvocableHandlerMethod}
* for target method invocation.
@@ -346,76 +395,28 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
* Private constructors for internal use
*/
private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annotation> annotationType,
String methodName, Class<?> expectedType, boolean canProcessMessageList) {
this.annotationType = annotationType;
this.methodName = methodName;
this.canProcessMessageList = canProcessMessageList;
Assert.notNull(targetObject, "targetObject must not be null");
if (expectedType != null) {
this.expectedType = TypeDescriptor.valueOf(expectedType);
}
else {
this.expectedType = null;
}
this.targetObject = targetObject;
Map<String, Map<Class<?>, HandlerMethod>> handlerMethodsForTarget =
findHandlerMethodsForTarget(targetObject, annotationType, methodName, expectedType != null);
Map<Class<?>, HandlerMethod> methods = handlerMethodsForTarget.get(CANDIDATE_METHODS);
Map<Class<?>, HandlerMethod> messageMethods = handlerMethodsForTarget.get(CANDIDATE_MESSAGE_METHODS);
if ((methods.size() == 1 && messageMethods.isEmpty()) ||
(messageMethods.size() == 1 && methods.isEmpty())) {
if (methods.size() == 1) {
this.handlerMethod = methods.values().iterator().next();
}
else {
this.handlerMethod = messageMethods.values().iterator().next();
}
this.handlerMethods = null;
this.handlerMessageMethods = null;
this.handlerMethodsList = null;
}
else {
this.handlerMethod = null;
this.handlerMethods = methods;
this.handlerMessageMethods = messageMethods;
this.handlerMethodsList = new LinkedList<>();
//TODO Consider to use global option to determine a precedence of methods
this.handlerMethodsList.add(this.handlerMethods);
this.handlerMethodsList.add(this.handlerMessageMethods);
}
setDisplayString(targetObject, methodName);
JsonObjectMapper<?, ?> mapper;
try {
mapper = JsonObjectMapperProvider.newInstance();
}
catch (IllegalStateException e) {
mapper = null;
}
this.jsonObjectMapper = mapper;
}
private boolean isProvidedMessageHandlerFactoryBean() {
BeanFactory beanFactory = getBeanFactory();
return beanFactory != null
&& beanFactory.containsBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME);
}
private void createHandlerMethod() {
private HandlerMethod createHandlerMethod(Method method) {
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);
InvocableHandlerMethod invocableHandlerMethod = createInvocableHandlerMethod(method);
HandlerMethod handlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
checkSpelInvokerRequired(getTargetClass(this.targetObject), method, handlerMethod);
return handlerMethod;
}
catch (IneligibleMethodException e) {
throw new IllegalArgumentException(e);
}
}
private InvocableHandlerMethod createInvocableHandlerMethod(Method method) {
return this.messageHandlerMethodFactory.createInvocableHandlerMethod(this.targetObject, method);
}
private void setDisplayString(Object targetObject, Object targetMethod) {
StringBuilder sb = new StringBuilder(targetObject.getClass().getName());
if (targetMethod instanceof Method) {
@@ -472,7 +473,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
if (!this.initialized) {
initialize();
}
HandlerMethod candidate = this.findHandlerMethodForParameters(parameters);
HandlerMethod candidate = findHandlerMethodForParameters(parameters);
if (candidate == null) {
candidate = this.defaultHandlerMethod;
}
@@ -525,7 +526,13 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
this.messageHandlerMethodFactory =
beanFactory.getBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME,
MessageHandlerMethodFactory.class);
createHandlerMethod();
this.handlerMethodsList
.stream()
.map(Map::values)
.flatMap(Collection::stream)
.forEach(handlerMethod ->
handlerMethod.replaceInvocableHandlerMethod(
createInvocableHandlerMethod(handlerMethod.invocableHandlerMethod.getMethod())));
}
else {
if (beanFactory != null &&
@@ -645,7 +652,6 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
expression.getExpressionString() + " ]");
}
}
return invokeExpression(expression, parameters);
}
}
@@ -716,9 +722,8 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
return contentType != null && contentType.toString().contains("json");
}
private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(final Object targetObject,
final Class<? extends Annotation> annotationType, final String methodNameArg,
final boolean requiresReply) {
private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(
final Class<? extends Annotation> annotationType, final String methodNameArg, final boolean requiresReply) {
Map<String, Map<Class<?>, HandlerMethod>> methods = new HashMap<>();
@@ -728,7 +733,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
final Map<Class<?>, HandlerMethod> fallbackMessageMethods = new HashMap<>();
final AtomicReference<Class<?>> ambiguousFallbackType = new AtomicReference<>();
final AtomicReference<Class<?>> ambiguousFallbackMessageGenericType = new AtomicReference<>();
final Class<?> targetClass = getTargetClass(targetObject);
final Class<?> targetClass = getTargetClass(this.targetObject);
final String methodNameToUse;
@@ -779,11 +784,8 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
HandlerMethod handlerMethod1;
try {
method1 = AopUtils.selectInvocableMethod(method1,
org.springframework.util.ClassUtils.getUserClass(targetObject));
InvocableHandlerMethod invocableHandlerMethod =
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, method1);
handlerMethod1 = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
checkSpelInvokerRequired(targetClass, method1, handlerMethod1);
org.springframework.util.ClassUtils.getUserClass(this.targetObject));
handlerMethod1 = createHandlerMethod(method1);
}
catch (IneligibleMethodException e) {
if (logger.isDebugEnabled()) {
@@ -800,7 +802,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
}
if (AnnotationUtils.getAnnotation(method1, Default.class) != null) {
Assert.state(this.defaultHandlerMethod == null,
() -> "Only one method can be @Default, but there are more for: " + targetObject);
() -> "Only one method can be @Default, but there are more for: " + this.targetObject);
this.defaultHandlerMethod = handlerMethod1;
}
Class<?> targetParameterType = handlerMethod1.getTargetParameterType();
@@ -850,8 +852,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
if (candidateMethods.isEmpty() && candidateMessageMethods.isEmpty() && fallbackMethods.isEmpty()
&& fallbackMessageMethods.isEmpty()) {
findSingleSpecifMethodOnInterfacesIfProxy(targetObject, methodNameToUse, candidateMessageMethods,
candidateMethods);
findSingleSpecifMethodOnInterfacesIfProxy(methodNameToUse, candidateMessageMethods, candidateMethods);
}
if (!candidateMethods.isEmpty() || !candidateMessageMethods.isEmpty()) {
@@ -875,7 +876,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
if ("org.springframework.integration.gateway.RequestReplyExchanger".equals(iface.getName())) {
frameworkMethods.add(targetClass.getMethod("exchange", Message.class));
if (logger.isDebugEnabled()) {
logger.debug(targetObject.getClass() +
logger.debug(this.targetObject.getClass() +
": Ambiguous fallback methods; using RequestReplyExchanger.exchange()");
}
}
@@ -886,12 +887,8 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
}
if (frameworkMethods.size() == 1) {
Method frameworkMethod = org.springframework.util.ClassUtils.getMostSpecificMethod(
frameworkMethods.get(0), targetObject.getClass());
InvocableHandlerMethod invocableHandlerMethod =
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject,
frameworkMethod);
HandlerMethod theHandlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
checkSpelInvokerRequired(targetClass, frameworkMethod, theHandlerMethod);
frameworkMethods.get(0), this.targetObject.getClass());
HandlerMethod theHandlerMethod = createHandlerMethod(frameworkMethod);
methods.put(CANDIDATE_METHODS, Collections.singletonMap(Object.class, theHandlerMethod));
methods.put(CANDIDATE_MESSAGE_METHODS, candidateMessageMethods);
return methods;
@@ -916,17 +913,17 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
return methods;
}
private void findSingleSpecifMethodOnInterfacesIfProxy(final Object targetObject, final String methodName,
private void findSingleSpecifMethodOnInterfacesIfProxy(final String methodName,
Map<Class<?>, HandlerMethod> candidateMessageMethods,
Map<Class<?>, HandlerMethod> candidateMethods) {
if (AopUtils.isAopProxy(targetObject)) {
if (AopUtils.isAopProxy(this.targetObject)) {
final AtomicReference<Method> targetMethod = new AtomicReference<>();
final AtomicReference<Class<?>> targetClass = new AtomicReference<>();
Class<?>[] interfaces = ((Advised) targetObject).getProxiedInterfaces();
Class<?>[] interfaces = ((Advised) this.targetObject).getProxiedInterfaces();
for (Class<?> clazz : interfaces) {
ReflectionUtils.doWithMethods(clazz, method1 -> {
if (targetMethod.get() != null) {
throw new IllegalStateException("Ambiguous method " + methodName + " on " + targetObject);
throw new IllegalStateException("Ambiguous method " + methodName + " on " + this.targetObject);
}
else {
targetMethod.set(method1);
@@ -937,11 +934,8 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
Method theMethod = targetMethod.get();
if (theMethod != null) {
theMethod = org.springframework.util.ClassUtils
.getMostSpecificMethod(theMethod, targetObject.getClass());
InvocableHandlerMethod invocableHandlerMethod =
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, theMethod);
HandlerMethod theHandlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
checkSpelInvokerRequired(targetClass.get(), theMethod, theHandlerMethod);
.getMostSpecificMethod(theMethod, this.targetObject.getClass());
HandlerMethod theHandlerMethod = createHandlerMethod(theMethod);
Class<?> targetParameterType = theHandlerMethod.getTargetParameterType();
if (theHandlerMethod.isMessageMethod()) {
if (candidateMessageMethods.containsKey(targetParameterType)) {
@@ -1033,7 +1027,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
final Class<?> payloadType = parameters.getFirstParameterType();
HandlerMethod closestMatch = this.findClosestMatch(payloadType);
HandlerMethod closestMatch = findClosestMatch(payloadType);
if (closestMatch != null) {
return closestMatch;
@@ -1045,7 +1039,6 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
else {
return this.handlerMethods.get(Void.class);
}
}
private HandlerMethod findClosestMatch(Class<?> payloadType) {
@@ -1078,10 +1071,10 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
private final String expressionString;
private final InvocableHandlerMethod invocableHandlerMethod;
private final boolean canProcessMessageList;
private InvocableHandlerMethod invocableHandlerMethod;
private volatile Expression expression;
private volatile TypeDescriptor targetParameterTypeDescriptor;
@@ -1109,6 +1102,9 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
this.expressionString = generateExpression(this.invocableHandlerMethod.getMethod());
}
void replaceInvocableHandlerMethod(InvocableHandlerMethod newInvocableHandlerMethod) {
this.invocableHandlerMethod = newInvocableHandlerMethod;
}
@SuppressWarnings("unchecked")
public <T> T invoke(ParametersWrapper parameters) throws Exception {

View File

@@ -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.
@@ -22,7 +22,6 @@ import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@@ -170,7 +169,7 @@ public class AggregatorParserTests {
Object handlerMethods = new DirectFieldAccessor(new DirectFieldAccessor(new DirectFieldAccessor(accessor
.getPropertyValue("outputProcessor")).getPropertyValue("processor")).getPropertyValue("delegate"))
.getPropertyValue("handlerMethods");
assertNull(handlerMethods);
assertNotNull(handlerMethods);
Object handlerMethod = new DirectFieldAccessor(new DirectFieldAccessor(new DirectFieldAccessor(accessor
.getPropertyValue("outputProcessor")).getPropertyValue("processor")).getPropertyValue("delegate"))
.getPropertyValue("handlerMethod");
@@ -250,7 +249,7 @@ public class AggregatorParserTests {
MessagingMethodInvokerHelper<Long> methodInvokerHelper =
TestUtils.getPropertyValue(releaseStrategy, "adapter.delegate", MessagingMethodInvokerHelper.class);
Object handlerMethods = TestUtils.getPropertyValue(methodInvokerHelper, "handlerMethods");
assertNull(handlerMethods);
assertNotNull(handlerMethods);
Object handlerMethod = TestUtils.getPropertyValue(methodInvokerHelper, "handlerMethod");
assertTrue(handlerMethod.toString().contains("checkCompleteness"));
input.send(createMessage(1L, "correlationId", 4, 0, null));
@@ -261,7 +260,7 @@ public class AggregatorParserTests {
Assert.assertNull(reply);
input.send(createMessage(5L, "correlationId", 4, 3, null));
reply = outputChannel.receive(0);
Assert.assertNotNull(reply);
assertNotNull(reply);
assertEquals(11L, reply.getPayload());
}
@@ -275,7 +274,7 @@ public class AggregatorParserTests {
DirectFieldAccessor releaseStrategyAccessor = new DirectFieldAccessor(new DirectFieldAccessor(new DirectFieldAccessor(releaseStrategy)
.getPropertyValue("adapter")).getPropertyValue("delegate"));
Object handlerMethods = releaseStrategyAccessor.getPropertyValue("handlerMethods");
assertNull(handlerMethods);
assertNotNull(handlerMethods);
Object handlerMethod = releaseStrategyAccessor.getPropertyValue("handlerMethod");
assertTrue(handlerMethod.toString().contains("checkCompleteness"));
input.send(createMessage(1L, "correlationId", 4, 0, null));
@@ -286,7 +285,7 @@ public class AggregatorParserTests {
Assert.assertNull(reply);
input.send(createMessage(5L, "correlationId", 4, 3, null));
reply = outputChannel.receive(0);
Assert.assertNotNull(reply);
assertNotNull(reply);
assertEquals(11L, reply.getPayload());
}

View File

@@ -68,6 +68,7 @@ import org.springframework.integration.channel.DirectChannel;
import org.springframework.integration.channel.QueueChannel;
import org.springframework.integration.config.EnableIntegration;
import org.springframework.integration.config.EnableMessageHistory;
import org.springframework.integration.context.IntegrationContextUtils;
import org.springframework.integration.core.MessageSelector;
import org.springframework.integration.core.MessageSource;
import org.springframework.integration.endpoint.AbstractEndpoint;
@@ -81,6 +82,8 @@ import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;
import org.springframework.messaging.MessageHandler;
import org.springframework.messaging.PollableChannel;
import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory;
import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory;
import org.springframework.messaging.support.ErrorMessage;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.messaging.support.MessageBuilder;
@@ -244,6 +247,11 @@ public class MessagingAnnotationsWithBeanAnnotationTests {
private static final ExpressionParser PARSER = new SpelExpressionParser();
@Bean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME)
public MessageHandlerMethodFactory messageHandlerMethodFactory() {
return new DefaultMessageHandlerMethodFactory();
}
@Bean
public AtomicInteger counter() {
return new AtomicInteger();