From fcca7a7b2696fa6bebfcf35892077166e4541fb0 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Wed, 1 Apr 2020 12:17:26 -0400 Subject: [PATCH] GH-3230: MMIH: Fix Pausable/Lifecycle Methods Resolves https://github.com/spring-projects/spring-integration/issues/3230 Previously, methods with the same name as `Lifecycle` methods were unconditionally ignored. - Use logic similar to `ControlBusMethodFilter` to explicitly compare the candidate Method to those on `Paused` and `Lifecycle` and only ignore those. - Explicitly disallow these methods when named - previously the check was only performed if no method name was supplied. * - reinstate `Pausable/Lifecycle` methods if explicitly requested. - log `Pausable` methods that are not candidates. * - Fix typo in test method. **Cherry-pick to 5.2.x** --- .../support/MessagingMethodInvokerHelper.java | 20 +++++-- .../MethodInvokingMessageProcessorTests.java | 57 ++++++++++++++++++- 2 files changed, 71 insertions(+), 6 deletions(-) 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 0a6b08df65..df40a24f96 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -71,6 +71,7 @@ import org.springframework.integration.annotation.Payloads; import org.springframework.integration.annotation.ServiceActivator; import org.springframework.integration.annotation.UseSpelInvoker; import org.springframework.integration.context.IntegrationContextUtils; +import org.springframework.integration.core.Pausable; import org.springframework.integration.support.MutableMessage; import org.springframework.integration.support.NullAwarePayloadArgumentResolver; import org.springframework.integration.support.converter.ConfigurableCompositeMessageConverter; @@ -97,7 +98,6 @@ import org.springframework.messaging.handler.invocation.MethodArgumentResolution import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; -import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; @@ -823,9 +823,19 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator im methodToProcess.getDeclaringClass().equals(Proxy.class) || (this.requiresReply && void.class.equals(methodToProcess.getReturnType())) || (this.methodName != null && !this.methodName.equals(methodToProcess.getName())) || - (this.methodName == null && - ObjectUtils.containsElement(new String[] { "start", "stop", "isRunning" }, - methodToProcess.getName()))); + (this.methodName == null && isPausableMethod(methodToProcess))); + } + + private boolean isPausableMethod(Method pausableMethod) { + Class declaringClass = pausableMethod.getDeclaringClass(); + boolean pausable = (Pausable.class.isAssignableFrom(declaringClass) + || Lifecycle.class.isAssignableFrom(declaringClass)) + && ReflectionUtils.findMethod(Pausable.class, pausableMethod.getName(), + pausableMethod.getParameterTypes()) != null; + if (pausable && this.logger.isTraceEnabled()) { + this.logger.trace(pausableMethod + " is not considered a candidate method unless explicitly requested"); + } + return pausable; } private void populateHandlerMethod(Map, HandlerMethod> candidateMethods, 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 dfd0d77b8d..2206288e97 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-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -19,6 +19,7 @@ package org.springframework.integration.handler; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.fail; import static org.mockito.AdditionalAnswers.returnsFirstArg; import static org.mockito.ArgumentMatchers.anyString; @@ -51,6 +52,7 @@ import org.springframework.beans.DirectFieldAccessor; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.ApplicationContext; +import org.springframework.context.Lifecycle; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -1165,6 +1167,31 @@ public class MethodInvokingMessageProcessorTests { assertThat(result).isEqualTo("Foo,Bar"); } + @Test + void lifecycleOnly() { + assertThatIllegalStateException().isThrownBy(() -> + new MethodInvokingMessageProcessor(new LifecycleBean(), (String) null)) + .withMessageContaining("no eligible methods"); + } + + @Test + void lifecycleOnlyExplicitMethod() { + LifecycleBean targetObject = new LifecycleBean(); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(targetObject, "start"); + processor.setBeanFactory(mock(BeanFactory.class)); + Object result = processor.processMessage(new GenericMessage<>("testing")); + assertThat(targetObject.startCalled).isTrue(); + } + + @Test + void lifecycleWithValidStartMethod() { + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(new LifeCycleWithCustomStart(), + (String) null); + processor.setBeanFactory(mock(BeanFactory.class)); + Object result = processor.processMessage(new GenericMessage<>("testing")); + assertThat(result).isEqualTo("TESTING"); + } + public static class Employee { private T entity; @@ -1586,4 +1613,32 @@ public class MethodInvokingMessageProcessorTests { } + public static class LifecycleBean implements Lifecycle { + + boolean startCalled; + + @Override + public void start() { + this.startCalled = true; + } + + @Override + public void stop() { + } + + @Override + public boolean isRunning() { + return false; + } + + } + + public static class LifeCycleWithCustomStart extends LifecycleBean { + + public String start(String in) { + return in.toUpperCase(); + } + + } + }