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**
This commit is contained in:
Gary Russell
2020-04-01 12:17:26 -04:00
committed by GitHub
parent 76e79012fe
commit fcca7a7b26
2 changed files with 71 additions and 6 deletions

View File

@@ -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<Class<?>, HandlerMethod> candidateMethods,

View File

@@ -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<T> {
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();
}
}
}