Skip to content

Commit 005bc80

Browse files
artembilangaryrussell
artembilan
authored and
garyrussell
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
1 parent 3006e58 commit 005bc80

File tree

3 files changed

+107
-102
lines changed

3 files changed

+107
-102
lines changed

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

Lines changed: 96 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator im
175175

176176
private final Map<Class<?>, HandlerMethod> handlerMessageMethods;
177177

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

180180
private final TypeDescriptor expectedType;
181181

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

255255
Assert.notNull(targetObject, "targetObject must not be null");
256256
this.targetObject = targetObject;
257-
createHandlerMethod();
257+
this.handlerMethod = createHandlerMethod(this.method);
258258
this.handlerMethods = null;
259259
this.handlerMessageMethods = null;
260-
this.handlerMethodsList = null;
260+
this.handlerMethodsList.add(
261+
Collections.singletonMap(this.handlerMethod.targetParameterType, this.handlerMethod));
261262
setDisplayString(targetObject, method);
262263

263264
JsonObjectMapper<?, ?> mapper;
@@ -270,6 +271,54 @@ private MessagingMethodInvokerHelper(Object targetObject, Class<? extends Annota
270271
this.jsonObjectMapper = mapper;
271272
}
272273

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

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

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

414+
private InvocableHandlerMethod createInvocableHandlerMethod(Method method) {
415+
return this.messageHandlerMethodFactory.createInvocableHandlerMethod(this.targetObject, method);
416+
}
417+
417418
private void setDisplayString(Object targetObject, Object targetMethod) {
418419
StringBuilder sb = new StringBuilder(targetObject.getClass().getName());
419420
if (targetMethod instanceof Method) {
@@ -475,7 +476,7 @@ private Object processInternal(ParametersWrapper parameters) {
475476
if (!this.initialized) {
476477
initialize();
477478
}
478-
HandlerMethod candidate = this.findHandlerMethodForParameters(parameters);
479+
HandlerMethod candidate = findHandlerMethodForParameters(parameters);
479480
if (candidate == null) {
480481
candidate = this.defaultHandlerMethod;
481482
}
@@ -528,7 +529,13 @@ private synchronized void initialize() {
528529
this.messageHandlerMethodFactory =
529530
beanFactory.getBean(IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME,
530531
MessageHandlerMethodFactory.class);
531-
createHandlerMethod();
532+
this.handlerMethodsList
533+
.stream()
534+
.map(Map::values)
535+
.flatMap(Collection::stream)
536+
.forEach(handlerMethod ->
537+
handlerMethod.replaceInvocableHandlerMethod(
538+
createInvocableHandlerMethod(handlerMethod.invocableHandlerMethod.getMethod())));
532539
}
533540
else {
534541
if (beanFactory != null &&
@@ -648,11 +655,11 @@ private Object processInvokeExceptionAndFallbackToExpressionIfAny(HandlerMethod
648655
}
649656
else if (ex instanceof IllegalStateException && // NOSONAR complex boolean expression
650657
(!(ex.getCause() instanceof IllegalArgumentException) ||
651-
!ex.getStackTrace()[0].getClassName().equals(InvocableHandlerMethod.class.getName()) ||
652-
(!"argument type mismatch".equals(ex.getCause().getMessage()) &&
653-
// JVM generates GeneratedMethodAccessor### after several calls with less error
654-
// checking
655-
!ex.getCause().getMessage().startsWith("java.lang.ClassCastException@")))) {
658+
!ex.getStackTrace()[0].getClassName().equals(InvocableHandlerMethod.class.getName()) ||
659+
(!"argument type mismatch".equals(ex.getCause().getMessage()) &&
660+
// JVM generates GeneratedMethodAccessor### after several calls with less error
661+
// checking
662+
!ex.getCause().getMessage().startsWith("java.lang.ClassCastException@")))) {
656663
throw ex;
657664
}
658665

@@ -746,9 +753,8 @@ private boolean contentTypeIsJson(Message<?> message) {
746753
return contentType != null && contentType.toString().contains("json");
747754
}
748755

749-
private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(final Object targetObject,
750-
final Class<? extends Annotation> annotationType, final String methodNameArg,
751-
final boolean requiresReply) {
756+
private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(
757+
final Class<? extends Annotation> annotationType, final String methodNameArg, final boolean requiresReply) {
752758

753759
Map<String, Map<Class<?>, HandlerMethod>> methods = new HashMap<>();
754760

@@ -758,7 +764,7 @@ private Map<String, Map<Class<?>, HandlerMethod>> findHandlerMethodsForTarget(fi
758764
final Map<Class<?>, HandlerMethod> fallbackMessageMethods = new HashMap<>();
759765
final AtomicReference<Class<?>> ambiguousFallbackType = new AtomicReference<>();
760766
final AtomicReference<Class<?>> ambiguousFallbackMessageGenericType = new AtomicReference<>();
761-
final Class<?> targetClass = getTargetClass(targetObject);
767+
final Class<?> targetClass = getTargetClass(this.targetObject);
762768

763769
final String methodNameToUse;
764770

@@ -809,11 +815,8 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
809815
HandlerMethod handlerMethod1;
810816
try {
811817
method1 = AopUtils.selectInvocableMethod(method1,
812-
org.springframework.util.ClassUtils.getUserClass(targetObject));
813-
InvocableHandlerMethod invocableHandlerMethod =
814-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, method1);
815-
handlerMethod1 = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
816-
checkSpelInvokerRequired(targetClass, method1, handlerMethod1);
818+
org.springframework.util.ClassUtils.getUserClass(this.targetObject));
819+
handlerMethod1 = createHandlerMethod(method1);
817820
}
818821
catch (IneligibleMethodException e) {
819822
if (LOGGER.isDebugEnabled()) {
@@ -830,7 +833,7 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
830833
}
831834
if (AnnotationUtils.getAnnotation(method1, Default.class) != null) {
832835
Assert.state(this.defaultHandlerMethod == null,
833-
() -> "Only one method can be @Default, but there are more for: " + targetObject);
836+
() -> "Only one method can be @Default, but there are more for: " + this.targetObject);
834837
this.defaultHandlerMethod = handlerMethod1;
835838
}
836839
Class<?> targetParameterType = handlerMethod1.getTargetParameterType();
@@ -880,8 +883,7 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
880883

881884
if (candidateMethods.isEmpty() && candidateMessageMethods.isEmpty() && fallbackMethods.isEmpty()
882885
&& fallbackMessageMethods.isEmpty()) {
883-
findSingleSpecifMethodOnInterfacesIfProxy(targetObject, methodNameToUse, candidateMessageMethods,
884-
candidateMethods);
886+
findSingleSpecifMethodOnInterfacesIfProxy(methodNameToUse, candidateMessageMethods, candidateMethods);
885887
}
886888

887889
if (!candidateMethods.isEmpty() || !candidateMessageMethods.isEmpty()) {
@@ -905,7 +907,7 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
905907
if ("org.springframework.integration.gateway.RequestReplyExchanger".equals(iface.getName())) {
906908
frameworkMethods.add(targetClass.getMethod("exchange", Message.class));
907909
if (LOGGER.isDebugEnabled()) {
908-
LOGGER.debug(targetObject.getClass() +
910+
LOGGER.debug(this.targetObject.getClass() +
909911
": Ambiguous fallback methods; using RequestReplyExchanger.exchange()");
910912
}
911913
}
@@ -916,12 +918,8 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
916918
}
917919
if (frameworkMethods.size() == 1) {
918920
Method frameworkMethod = org.springframework.util.ClassUtils.getMostSpecificMethod(
919-
frameworkMethods.get(0), targetObject.getClass());
920-
InvocableHandlerMethod invocableHandlerMethod =
921-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject,
922-
frameworkMethod);
923-
HandlerMethod theHandlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
924-
checkSpelInvokerRequired(targetClass, frameworkMethod, theHandlerMethod);
921+
frameworkMethods.get(0), this.targetObject.getClass());
922+
HandlerMethod theHandlerMethod = createHandlerMethod(frameworkMethod);
925923
methods.put(CANDIDATE_METHODS, Collections.singletonMap(Object.class, theHandlerMethod));
926924
methods.put(CANDIDATE_MESSAGE_METHODS, candidateMessageMethods);
927925
return methods;
@@ -946,17 +944,17 @@ else if (!Modifier.isPublic(method1.getModifiers())) {
946944
return methods;
947945
}
948946

949-
private void findSingleSpecifMethodOnInterfacesIfProxy(final Object targetObject, final String methodName,
947+
private void findSingleSpecifMethodOnInterfacesIfProxy(final String methodName,
950948
Map<Class<?>, HandlerMethod> candidateMessageMethods,
951949
Map<Class<?>, HandlerMethod> candidateMethods) {
952-
if (AopUtils.isAopProxy(targetObject)) {
950+
if (AopUtils.isAopProxy(this.targetObject)) {
953951
final AtomicReference<Method> targetMethod = new AtomicReference<>();
954952
final AtomicReference<Class<?>> targetClass = new AtomicReference<>();
955-
Class<?>[] interfaces = ((Advised) targetObject).getProxiedInterfaces();
953+
Class<?>[] interfaces = ((Advised) this.targetObject).getProxiedInterfaces();
956954
for (Class<?> clazz : interfaces) {
957955
ReflectionUtils.doWithMethods(clazz, method1 -> {
958956
if (targetMethod.get() != null) {
959-
throw new IllegalStateException("Ambiguous method " + methodName + " on " + targetObject);
957+
throw new IllegalStateException("Ambiguous method " + methodName + " on " + this.targetObject);
960958
}
961959
else {
962960
targetMethod.set(method1);
@@ -967,11 +965,8 @@ private void findSingleSpecifMethodOnInterfacesIfProxy(final Object targetObject
967965
Method theMethod = targetMethod.get();
968966
if (theMethod != null) {
969967
theMethod = org.springframework.util.ClassUtils
970-
.getMostSpecificMethod(theMethod, targetObject.getClass());
971-
InvocableHandlerMethod invocableHandlerMethod =
972-
this.messageHandlerMethodFactory.createInvocableHandlerMethod(targetObject, theMethod);
973-
HandlerMethod theHandlerMethod = new HandlerMethod(invocableHandlerMethod, this.canProcessMessageList);
974-
checkSpelInvokerRequired(targetClass.get(), theMethod, theHandlerMethod);
968+
.getMostSpecificMethod(theMethod, this.targetObject.getClass());
969+
HandlerMethod theHandlerMethod = createHandlerMethod(theMethod);
975970
Class<?> targetParameterType = theHandlerMethod.getTargetParameterType();
976971
if (theHandlerMethod.isMessageMethod()) {
977972
if (candidateMessageMethods.containsKey(targetParameterType)) {
@@ -1064,7 +1059,7 @@ private HandlerMethod findHandlerMethodForParameters(ParametersWrapper parameter
10641059

10651060
final Class<?> payloadType = parameters.getFirstParameterType();
10661061

1067-
HandlerMethod closestMatch = this.findClosestMatch(payloadType);
1062+
HandlerMethod closestMatch = findClosestMatch(payloadType);
10681063
if (closestMatch != null) {
10691064
return closestMatch;
10701065

@@ -1076,7 +1071,6 @@ private HandlerMethod findHandlerMethodForParameters(ParametersWrapper parameter
10761071
else {
10771072
return this.handlerMethods.get(Void.class);
10781073
}
1079-
10801074
}
10811075

10821076
private HandlerMethod findClosestMatch(Class<?> payloadType) {
@@ -1109,10 +1103,10 @@ private static class HandlerMethod {
11091103

11101104
private final String expressionString;
11111105

1112-
private final InvocableHandlerMethod invocableHandlerMethod;
1113-
11141106
private final boolean canProcessMessageList;
11151107

1108+
private InvocableHandlerMethod invocableHandlerMethod;
1109+
11161110
private volatile Expression expression;
11171111

11181112
private volatile TypeDescriptor targetParameterTypeDescriptor;
@@ -1140,6 +1134,9 @@ private static class HandlerMethod {
11401134
this.expressionString = generateExpression(this.invocableHandlerMethod.getMethod());
11411135
}
11421136

1137+
void replaceInvocableHandlerMethod(InvocableHandlerMethod newInvocableHandlerMethod) {
1138+
this.invocableHandlerMethod = newInvocableHandlerMethod;
1139+
}
11431140

11441141
public Object invoke(ParametersWrapper parameters) {
11451142
Message<?> message = parameters.getMessage();

‎spring-integration-core/src/test/java/org/springframework/integration/config/AggregatorParserTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public void testPropertyAssignment() {
159159
Object handlerMethods = new DirectFieldAccessor(new DirectFieldAccessor(new DirectFieldAccessor(accessor
160160
.getPropertyValue("outputProcessor")).getPropertyValue("processor")).getPropertyValue("delegate"))
161161
.getPropertyValue("handlerMethods");
162-
assertThat(handlerMethods).isNull();
162+
assertThat(handlerMethods).isNotNull();
163163
Object handlerMethod = new DirectFieldAccessor(new DirectFieldAccessor(new DirectFieldAccessor(accessor
164164
.getPropertyValue("outputProcessor")).getPropertyValue("processor")).getPropertyValue("delegate"))
165165
.getPropertyValue("handlerMethod");
@@ -244,7 +244,7 @@ public void testAggregatorWithPojoReleaseStrategy() {
244244
MessagingMethodInvokerHelper methodInvokerHelper =
245245
TestUtils.getPropertyValue(releaseStrategy, "adapter.delegate", MessagingMethodInvokerHelper.class);
246246
Object handlerMethods = TestUtils.getPropertyValue(methodInvokerHelper, "handlerMethods");
247-
assertThat(handlerMethods).isNull();
247+
assertThat(handlerMethods).isNotNull();
248248
Object handlerMethod = TestUtils.getPropertyValue(methodInvokerHelper, "handlerMethod");
249249
assertThat(handlerMethod.toString().contains("checkCompleteness")).isTrue();
250250
input.send(createMessage(1L, "correlationId", 4, 0, null));
@@ -269,7 +269,7 @@ public void testAggregatorWithPojoReleaseStrategyAsCollection() {
269269
DirectFieldAccessor releaseStrategyAccessor = new DirectFieldAccessor(new DirectFieldAccessor(new DirectFieldAccessor(releaseStrategy)
270270
.getPropertyValue("adapter")).getPropertyValue("delegate"));
271271
Object handlerMethods = releaseStrategyAccessor.getPropertyValue("handlerMethods");
272-
assertThat(handlerMethods).isNull();
272+
assertThat(handlerMethods).isNotNull();
273273
Object handlerMethod = releaseStrategyAccessor.getPropertyValue("handlerMethod");
274274
assertThat(handlerMethod.toString().contains("checkCompleteness")).isTrue();
275275
input.send(createMessage(1L, "correlationId", 4, 0, null));

0 commit comments

Comments
 (0)