Skip to content

Commit a42411f

Browse files
artembilanartembilan
committed
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
1 parent d8ec835 commit a42411f

File tree

3 files changed

+105
-102
lines changed

3 files changed

+105
-102
lines changed

‎spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java

Lines changed: 91 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public class MessagingMethodInvokerHelper<T> extends AbstractExpressionEvaluator
183183

184184
private final Map<Class<?>, HandlerMethod> handlerMessageMethods;
185185

186-
private final List<Map<Class<?>, HandlerMethod>> handlerMethodsList;
186+
private final List<Map<Class<?>, HandlerMethod>> handlerMethodsList = new LinkedList<>();
187187

188188
private HandlerMethod handlerMethod;
189189

@@ -258,10 +258,11 @@ private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annota
258258

259259
Assert.notNull(targetObject, "targetObject must not be null");
260260
this.targetObject = targetObject;
261-
createHandlerMethod();
261+
this.handlerMethod = createHandlerMethod(this.method);
262262
this.handlerMethods = null;
263263
this.handlerMessageMethods = null;
264-
this.handlerMethodsList = null;
264+
this.handlerMethodsList.add(
265+
Collections.singletonMap(this.handlerMethod.targetParameterType, this.handlerMethod));
265266
setDisplayString(targetObject, method);
266267

267268
JsonObjectMapper<?, ?> mapper;
@@ -274,6 +275,54 @@ private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annota
274275
this.jsonObjectMapper = mapper;
275276
}
276277

278+
private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annotation> annotationType,
279+
String methodName, Class<?> expectedType, boolean canProcessMessageList) {
280+
281+
this.annotationType = annotationType;
282+
this.methodName = methodName;
283+
this.canProcessMessageList = canProcessMessageList;
284+
Assert.notNull(targetObject, "targetObject must not be null");
285+
if (expectedType != null) {
286+
this.expectedType = TypeDescriptor.valueOf(expectedType);
287+
}
288+
else {
289+
this.expectedType = null;
290+
}
291+
this.targetObject = targetObject;
292+
Map<String, Map<Class<?>, HandlerMethod>> handlerMethodsForTarget =
293+
findHandlerMethodsForTarget(annotationType, methodName, expectedType != null);
294+
Map<Class<?>, HandlerMethod> methods = handlerMethodsForTarget.get(CANDIDATE_METHODS);
295+
Map<Class<?>, HandlerMethod> messageMethods = handlerMethodsForTarget.get(CANDIDATE_MESSAGE_METHODS);
296+
if ((methods.size() == 1 && messageMethods.isEmpty()) ||
297+
(messageMethods.size() == 1 && methods.isEmpty())) {
298+
if (methods.size() == 1) {
299+
this.handlerMethod = methods.values().iterator().next();
300+
}
301+
else {
302+
this.handlerMethod = messageMethods.values().iterator().next();
303+
}
304+
}
305+
else {
306+
this.handlerMethod = null;
307+
}
308+
309+
this.handlerMethods = methods;
310+
this.handlerMessageMethods = messageMethods;
311+
//TODO Consider to use global option to determine a precedence of methods
312+
this.handlerMethodsList.add(this.handlerMethods);
313+
this.handlerMethodsList.add(this.handlerMessageMethods);
314+
315+
setDisplayString(targetObject, methodName);
316+
JsonObjectMapper<?, ?> mapper;
317+
try {
318+
mapper = JsonObjectMapperProvider.newInstance();
319+
}
320+
catch (IllegalStateException e) {
321+
mapper = null;
322+
}
323+
this.jsonObjectMapper = mapper;
324+
}
325+
277326
/**
278327
* A {@code boolean} flag to use SpEL Expression evaluation or {@link InvocableHandlerMethod}
279328
* for target method invocation.
@@ -346,76 +395,28 @@ public boolean isRunning() {
346395
* Private constructors for internal use
347396
*/
348397

349-
private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annotation> annotationType,
350-
String methodName, Class<?> expectedType, boolean canProcessMessageList) {
351-
352-
this.annotationType = annotationType;
353-
this.methodName = methodName;
354-
this.canProcessMessageList = canProcessMessageList;
355-
Assert.notNull(targetObject, "targetObject must not be null");
356-
if (expectedType != null) {
357-
this.expectedType = TypeDescriptor.valueOf(expectedType);
358-
}
359-
else {
360-
this.expectedType = null;
361-
}
362-
this.targetObject = targetObject;
363-
Map<String, Map<Class<?>, HandlerMethod>> handlerMethodsForTarget =
364-
findHandlerMethodsForTarget(targetObject, annotationType, methodName, expectedType != null);
365-
Map<Class<?>, HandlerMethod> methods = handlerMethodsForTarget.get(CANDIDATE_METHODS);
366-
Map<Class<?>, HandlerMethod> messageMethods = handlerMethodsForTarget.get(CANDIDATE_MESSAGE_METHODS);
367-
if ((methods.size() == 1 && messageMethods.isEmpty()) ||
368-
(messageMethods.size() == 1 && methods.isEmpty())) {
369-
if (methods.size() == 1) {
370-
this.handlerMethod = methods.values().iterator().next();
371-
}
372-
else {
373-
this.handlerMethod = messageMethods.values().iterator().next();
374-
}
375-
this.handlerMethods = null;
376-
this.handlerMessageMethods = null;
377-
this.handlerMethodsList = null;
378-
}
379-
else {
380-
this.handlerMethod = null;
381-
this.handlerMethods = methods;
382-
this.handlerMessageMethods = messageMethods;
383-
this.handlerMethodsList = new LinkedList<>();
384-
385-
//TODO Consider to use global option to determine a precedence of methods
386-
this.handlerMethodsList.add(this.handlerMethods);
387-
this.handlerMethodsList.add(this.handlerMessageMethods);
388-
}
389-
setDisplayString(targetObject, methodName);
390-
JsonObjectMapper<?, ?> mapper;
391-
try {
392-
mapper = JsonObjectMapperProvider.newInstance();
393-
}
394-
catch (IllegalStateException e) {
395-
mapper = null;
396-
}
397-
this.jsonObjectMapper = mapper;
398-
}
399-
400398
private boolean isProvidedMessageHandlerFactoryBean() {
401399
BeanFactory beanFactory = getBeanFactory();
402400
return beanFactory != null
403401
&& beanFactory.containsBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME);
404402
}
405403

406-
private void createHandlerMethod() {
404+
private HandlerMethod createHandlerMethod(Method method) {
407405
try {
408-
InvocableHandlerMethod invocableHandlerMethod =
409-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(this.targetObject, this.method);
410-
this.handlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
411-
this.defaultHandlerMethod = null;
412-
checkSpelInvokerRequired(getTargetClass(this.targetObject), this.method, this.handlerMethod);
406+
InvocableHandlerMethod invocableHandlerMethod = createInvocableHandlerMethod(method);
407+
HandlerMethod handlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
408+
checkSpelInvokerRequired(getTargetClass(this.targetObject), method, handlerMethod);
409+
return handlerMethod;
413410
}
414411
catch (IneligibleMethodException e) {
415412
throw new IllegalArgumentException(e);
416413
}
417414
}
418415

416+
private InvocableHandlerMethod createInvocableHandlerMethod(Method method) {
417+
return this.messageHandlerMethodFactory.createInvocableHandlerMethod(this.targetObject, method);
418+
}
419+
419420
private void setDisplayString(Object targetObject, Object targetMethod) {
420421
StringBuilder sb = new StringBuilder(targetObject.getClass().getName());
421422
if (targetMethod instanceof Method) {
@@ -472,7 +473,7 @@ private T processInternal(ParametersWrapper parameters) throws Exception {
472473
if (!this.initialized) {
473474
initialize();
474475
}
475-
HandlerMethod candidate = this.findHandlerMethodForParameters(parameters);
476+
HandlerMethod candidate = findHandlerMethodForParameters(parameters);
476477
if (candidate == null) {
477478
candidate = this.defaultHandlerMethod;
478479
}
@@ -525,7 +526,13 @@ private synchronized void initialize() throws Exception {
525526
this.messageHandlerMethodFactory =
526527
beanFactory.getBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME,
527528
MessageHandlerMethodFactory.class);
528-
createHandlerMethod();
529+
this.handlerMethodsList
530+
.stream()
531+
.map(Map::values)
532+
.flatMap(Collection::stream)
533+
.forEach(handlerMethod ->
534+
handlerMethod.replaceInvocableHandlerMethod(
535+
createInvocableHandlerMethod(handlerMethod.invocableHandlerMethod.getMethod())));
529536
}
530537
else {
531538
if (beanFactory != null &&
@@ -645,7 +652,6 @@ else if (e instanceof IllegalStateException) {
645652
expression.getExpressionString() + " ]");
646653
}
647654
}
648-
649655
return invokeExpression(expression, parameters);
650656
}
651657
}
@@ -716,9 +722,8 @@ private boolean contentTypeIsJson(Message<?> message) {
716722
return contentType != null && contentType.toString().contains("json");
717723
}
718724

719-
private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(final Object targetObject,
720-
final Class<? extends Annotation> annotationType, final String methodNameArg,
721-
final boolean requiresReply) {
725+
private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(
726+
final Class<? extends Annotation> annotationType, final String methodNameArg, final boolean requiresReply) {
722727

723728
Map<String, Map<Class<?>, HandlerMethod>> methods = new HashMap<>();
724729

@@ -728,7 +733,7 @@ private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(fi
728733
final Map<Class<?>, HandlerMethod> fallbackMessageMethods = new HashMap<>();
729734
final AtomicReference<Class<?>> ambiguousFallbackType = new AtomicReference<>();
730735
final AtomicReference<Class<?>> ambiguousFallbackMessageGenericType = new AtomicReference<>();
731-
final Class<?> targetClass = getTargetClass(targetObject);
736+
final Class<?> targetClass = getTargetClass(this.targetObject);
732737

733738
final String methodNameToUse;
734739

@@ -779,11 +784,8 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
779784
HandlerMethod handlerMethod1;
780785
try {
781786
method1 = AopUtils.selectInvocableMethod(method1,
782-
org.springframework.util.ClassUtils.getUserClass(targetObject));
783-
InvocableHandlerMethod invocableHandlerMethod =
784-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, method1);
785-
handlerMethod1 = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
786-
checkSpelInvokerRequired(targetClass, method1, handlerMethod1);
787+
org.springframework.util.ClassUtils.getUserClass(this.targetObject));
788+
handlerMethod1 = createHandlerMethod(method1);
787789
}
788790
catch (IneligibleMethodException e) {
789791
if (logger.isDebugEnabled()) {
@@ -800,7 +802,7 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
800802
}
801803
if (AnnotationUtils.getAnnotation(method1, Default.class) != null) {
802804
Assert.state(this.defaultHandlerMethod == null,
803-
() -> "Only one method can be @Default, but there are more for: " + targetObject);
805+
() -> "Only one method can be @Default, but there are more for: " + this.targetObject);
804806
this.defaultHandlerMethod = handlerMethod1;
805807
}
806808
Class<?> targetParameterType = handlerMethod1.getTargetParameterType();
@@ -850,8 +852,7 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
850852

851853
if (candidateMethods.isEmpty() && candidateMessageMethods.isEmpty() && fallbackMethods.isEmpty()
852854
&& fallbackMessageMethods.isEmpty()) {
853-
findSingleSpecifMethodOnInterfacesIfProxy(targetObject, methodNameToUse, candidateMessageMethods,
854-
candidateMethods);
855+
findSingleSpecifMethodOnInterfacesIfProxy(methodNameToUse, candidateMessageMethods, candidateMethods);
855856
}
856857

857858
if (!candidateMethods.isEmpty() || !candidateMessageMethods.isEmpty()) {
@@ -875,7 +876,7 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
875876
if ("org.springframework.integration.gateway.RequestReplyExchanger".equals(iface.getName())) {
876877
frameworkMethods.add(targetClass.getMethod("exchange", Message.class));
877878
if (logger.isDebugEnabled()) {
878-
logger.debug(targetObject.getClass() +
879+
logger.debug(this.targetObject.getClass() +
879880
": Ambiguous fallback methods; using RequestReplyExchanger.exchange()");
880881
}
881882
}
@@ -886,12 +887,8 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
886887
}
887888
if (frameworkMethods.size() == 1) {
888889
Method frameworkMethod = org.springframework.util.ClassUtils.getMostSpecificMethod(
889-
frameworkMethods.get(0), targetObject.getClass());
890-
InvocableHandlerMethod invocableHandlerMethod =
891-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject,
892-
frameworkMethod);
893-
HandlerMethod theHandlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
894-
checkSpelInvokerRequired(targetClass, frameworkMethod, theHandlerMethod);
890+
frameworkMethods.get(0), this.targetObject.getClass());
891+
HandlerMethod theHandlerMethod = createHandlerMethod(frameworkMethod);
895892
methods.put(CANDIDATE_METHODS, Collections.singletonMap(Object.class, theHandlerMethod));
896893
methods.put(CANDIDATE_MESSAGE_METHODS, candidateMessageMethods);
897894
return methods;
@@ -916,17 +913,17 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
916913
return methods;
917914
}
918915

919-
private void findSingleSpecifMethodOnInterfacesIfProxy(final Object targetObject, final String methodName,
916+
private void findSingleSpecifMethodOnInterfacesIfProxy(final String methodName,
920917
Map<Class<?>, HandlerMethod> candidateMessageMethods,
921918
Map<Class<?>, HandlerMethod> candidateMethods) {
922-
if (AopUtils.isAopProxy(targetObject)) {
919+
if (AopUtils.isAopProxy(this.targetObject)) {
923920
final AtomicReference<Method> targetMethod = new AtomicReference<>();
924921
final AtomicReference<Class<?>> targetClass = new AtomicReference<>();
925-
Class<?>[] interfaces = ((Advised) targetObject).getProxiedInterfaces();
922+
Class<?>[] interfaces = ((Advised) this.targetObject).getProxiedInterfaces();
926923
for (Class<?> clazz : interfaces) {
927924
ReflectionUtils.doWithMethods(clazz, method1 -> {
928925
if (targetMethod.get() != null) {
929-
throw new IllegalStateException("Ambiguous method " + methodName + " on " + targetObject);
926+
throw new IllegalStateException("Ambiguous method " + methodName + " on " + this.targetObject);
930927
}
931928
else {
932929
targetMethod.set(method1);
@@ -937,11 +934,8 @@ private void findSingleSpecifMethodOnInterfacesIfProxy(final Object targetObject
937934
Method theMethod = targetMethod.get();
938935
if (theMethod != null) {
939936
theMethod = org.springframework.util.ClassUtils
940-
.getMostSpecificMethod(theMethod, targetObject.getClass());
941-
InvocableHandlerMethod invocableHandlerMethod =
942-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, theMethod);
943-
HandlerMethod theHandlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
944-
checkSpelInvokerRequired(targetClass.get(), theMethod, theHandlerMethod);
937+
.getMostSpecificMethod(theMethod, this.targetObject.getClass());
938+
HandlerMethod theHandlerMethod = createHandlerMethod(theMethod);
945939
Class<?> targetParameterType = theHandlerMethod.getTargetParameterType();
946940
if (theHandlerMethod.isMessageMethod()) {
947941
if (candidateMessageMethods.containsKey(targetParameterType)) {
@@ -1033,7 +1027,7 @@ private HandlerMethod findHandlerMethodForParameters(ParametersWrapper parameter
10331027

10341028
final Class<?> payloadType = parameters.getFirstParameterType();
10351029

1036-
HandlerMethod closestMatch = this.findClosestMatch(payloadType);
1030+
HandlerMethod closestMatch = findClosestMatch(payloadType);
10371031
if (closestMatch != null) {
10381032
return closestMatch;
10391033

@@ -1045,7 +1039,6 @@ private HandlerMethod findHandlerMethodForParameters(ParametersWrapper parameter
10451039
else {
10461040
return this.handlerMethods.get(Void.class);
10471041
}
1048-
10491042
}
10501043

10511044
private HandlerMethod findClosestMatch(Class<?> payloadType) {
@@ -1078,10 +1071,10 @@ private static class HandlerMethod {
10781071

10791072
private final String expressionString;
10801073

1081-
private final InvocableHandlerMethod invocableHandlerMethod;
1082-
10831074
private final boolean canProcessMessageList;
10841075

1076+
private InvocableHandlerMethod invocableHandlerMethod;
1077+
10851078
private volatile Expression expression;
10861079

10871080
private volatile TypeDescriptor targetParameterTypeDescriptor;
@@ -1109,6 +1102,9 @@ private static class HandlerMethod {
11091102
this.expressionString = generateExpression(this.invocableHandlerMethod.getMethod());
11101103
}
11111104

1105+
void replaceInvocableHandlerMethod(InvocableHandlerMethod newInvocableHandlerMethod) {
1106+
this.invocableHandlerMethod = newInvocableHandlerMethod;
1107+
}
11121108

11131109
@SuppressWarnings("unchecked")
11141110
public <T> T invoke(ParametersWrapper parameters) throws Exception {

0 commit comments

Comments
 (0)