From dfa061f1ccadb3482387ecccb2401ff42ee913f5 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Mon, 31 Oct 2016 13:33:48 -0400 Subject: [PATCH] INT-4151: MessageSource Advice Improvement JIRA: https://jira.spring.io/browse/INT-4151 If the `MessageSource` is already a proxy, we only advise the `receive()` method. If it's not, we advise all methods, which is incorrect. * Use `NameMatchMethodPointcutAdvisor` in all advising cases * Prove with the test case that only `receive()` method is advised for the `MessageSource` proxy --- .../aop/AbstractMessageSourceAdvice.java | 3 +- .../endpoint/SourcePollingChannelAdapter.java | 12 ++++--- .../endpoint/PollerAdviceTests.java | 36 ++++++++++++++++++- .../endpoint/compound-trigger-context.xml | 3 ++ 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/aop/AbstractMessageSourceAdvice.java b/spring-integration-core/src/main/java/org/springframework/integration/aop/AbstractMessageSourceAdvice.java index cce00b80bc..b3eb49ea71 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/aop/AbstractMessageSourceAdvice.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/aop/AbstractMessageSourceAdvice.java @@ -34,8 +34,7 @@ public abstract class AbstractMessageSourceAdvice implements MethodInterceptor { @Override public final Object invoke(MethodInvocation invocation) throws Throwable { Object target = invocation.getThis(); - if (!(target instanceof MessageSource) - || !invocation.getMethod().getName().equals("receive")) { + if (!(target instanceof MessageSource)) { return invocation.proceed(); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java index cec3b059ad..22887b405f 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java @@ -134,15 +134,13 @@ public class SourcePollingChannelAdapter extends AbstractPollingEndpoint if (AopUtils.isAopProxy(this.source)) { this.appliedAdvices.forEach(((Advised) this.source)::removeAdvice); for (Advice advice : chain) { - NameMatchMethodPointcutAdvisor sourceAdvisor = new NameMatchMethodPointcutAdvisor(advice); - sourceAdvisor.addMethodName("receive"); - ((Advised) this.source).addAdvisor(sourceAdvisor); + ((Advised) this.source).addAdvisor(adviceToReceiveAdvisor(advice)); } } else { ProxyFactory proxyFactory = new ProxyFactory(this.source); for (Advice advice : chain) { - proxyFactory.addAdvice(advice); + proxyFactory.addAdvisor(adviceToReceiveAdvisor(advice)); } this.source = (MessageSource) proxyFactory.getProxy(getBeanClassLoader()); } @@ -150,6 +148,12 @@ public class SourcePollingChannelAdapter extends AbstractPollingEndpoint this.appliedAdvices.addAll(chain); } + private NameMatchMethodPointcutAdvisor adviceToReceiveAdvisor(Advice advice) { + NameMatchMethodPointcutAdvisor sourceAdvisor = new NameMatchMethodPointcutAdvisor(advice); + sourceAdvisor.addMethodName("receive"); + return sourceAdvisor; + } + @Override protected void doStart() { if (this.source instanceof Lifecycle) { diff --git a/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java b/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java index 4a734e9669..842c9bd5b0 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/endpoint/PollerAdviceTests.java @@ -17,6 +17,7 @@ package org.springframework.integration.endpoint; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.greaterThan; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -344,10 +345,21 @@ public class PollerAdviceTests { assertTrue(source.latch.await(10, TimeUnit.SECONDS)); assertNotNull(TestUtils.getPropertyValue(adapter, "trigger.override")); adapter.stop(); + OtherAdvice sourceAdvice = ctx.getBean(OtherAdvice.class); + int count = sourceAdvice.calls; + assertThat(count, greaterThan(0)); + ((Foo) adapter.getMessageSource()).otherMethod(); + assertEquals(count, sourceAdvice.calls); ctx.close(); } - public static class Source implements MessageSource { + public interface Foo { + + void otherMethod(); + + } + + public static class Source implements MessageSource, Foo { private final CountDownLatch latch = new CountDownLatch(5); @@ -357,6 +369,28 @@ public class PollerAdviceTests { return null; } + @Override + public void otherMethod() { + + } + + } + + public static class OtherAdvice extends AbstractMessageSourceAdvice { + + private int calls; + + @Override + public boolean beforeReceive(MessageSource source) { + this.calls++; + return true; + } + + @Override + public Message afterReceive(Message result, MessageSource source) { + return result; + } + } @Configuration diff --git a/spring-integration-core/src/test/java/org/springframework/integration/endpoint/compound-trigger-context.xml b/spring-integration-core/src/test/java/org/springframework/integration/endpoint/compound-trigger-context.xml index d0d01cdc52..428830e06b 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/endpoint/compound-trigger-context.xml +++ b/spring-integration-core/src/test/java/org/springframework/integration/endpoint/compound-trigger-context.xml @@ -13,6 +13,7 @@ + @@ -29,4 +30,6 @@ + +