From da9c80c6048a184e8151638f7031e83a4d73281a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 9 Nov 2015 15:02:15 +0100 Subject: [PATCH] Revised method selection for JMS listeners (and their parameters) Issue: SPR-13576 --- ...msListenerAnnotationBeanPostProcessor.java | 61 ++++++-------- .../jms/config/MethodJmsListenerEndpoint.java | 40 +++++++--- ...tenerAnnotationBeanPostProcessorTests.java | 37 ++++++--- ...tenerContainerFactoryIntegrationTests.java | 80 ++++++++++++++----- 4 files changed, 138 insertions(+), 80 deletions(-) diff --git a/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java b/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java index 334a359891..b902a486e5 100644 --- a/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java +++ b/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java @@ -18,7 +18,6 @@ package org.springframework.jms.annotation; import java.lang.reflect.Method; import java.util.Collections; -import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -37,6 +36,7 @@ import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.config.ConfigurableBeanFactory; +import org.springframework.core.MethodIntrospector; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.jms.config.JmsListenerConfigUtils; @@ -48,7 +48,6 @@ import org.springframework.messaging.handler.annotation.support.DefaultMessageHa import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory; import org.springframework.messaging.handler.invocation.InvocableHandlerMethod; import org.springframework.util.Assert; -import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -194,26 +193,30 @@ public class JmsListenerAnnotationBeanPostProcessor @Override public Object postProcessAfterInitialization(final Object bean, String beanName) throws BeansException { if (!this.nonAnnotatedClasses.contains(bean.getClass())) { - final Set annotatedMethods = new LinkedHashSet(1); Class targetClass = AopUtils.getTargetClass(bean); - ReflectionUtils.doWithMethods(targetClass, new ReflectionUtils.MethodCallback() { - @Override - public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { - for (JmsListener jmsListener : - AnnotationUtils.getRepeatableAnnotations(method, JmsListener.class, JmsListeners.class)) { - processJmsListener(jmsListener, method, bean); - annotatedMethods.add(method); - } - } - }); + Map> annotatedMethods = MethodIntrospector.selectMethods(targetClass, + new MethodIntrospector.MetadataLookup>() { + @Override + public Set inspect(Method method) { + Set listenerMethods = + AnnotationUtils.getRepeatableAnnotations(method, JmsListener.class, JmsListeners.class); + return (!listenerMethods.isEmpty() ? listenerMethods : null); + } + }); if (annotatedMethods.isEmpty()) { this.nonAnnotatedClasses.add(bean.getClass()); if (logger.isTraceEnabled()) { - logger.trace("No @JmsListener annotations found on bean class: " + bean.getClass()); + logger.trace("No @JmsListener annotations found on bean type: " + bean.getClass()); } } else { // Non-empty set of methods + for (Map.Entry> entry : annotatedMethods.entrySet()) { + Method method = entry.getKey(); + for (JmsListener listener : entry.getValue()) { + processJmsListener(listener, method, bean); + } + } if (logger.isDebugEnabled()) { logger.debug(annotatedMethods.size() + " @JmsListener methods processed on bean '" + beanName + "': " + annotatedMethods); @@ -223,29 +226,13 @@ public class JmsListenerAnnotationBeanPostProcessor return bean; } - protected void processJmsListener(JmsListener jmsListener, Method method, Object bean) { - if (AopUtils.isJdkDynamicProxy(bean)) { - try { - // Found a @JmsListener method on the target class for this JDK proxy -> - // is it also present on the proxy itself? - method = bean.getClass().getMethod(method.getName(), method.getParameterTypes()); - } - catch (SecurityException ex) { - ReflectionUtils.handleReflectionException(ex); - } - catch (NoSuchMethodException ex) { - throw new IllegalStateException(String.format( - "@JmsListener method '%s' found on bean target class '%s', " + - "but not found in any interface(s) for bean JDK proxy. Either " + - "pull the method up to an interface or switch to subclass (CGLIB) " + - "proxies by setting proxy-target-class/proxyTargetClass " + - "attribute to 'true'", method.getName(), method.getDeclaringClass().getSimpleName())); - } - } + protected void processJmsListener(JmsListener jmsListener, Method mostSpecificMethod, Object bean) { + Method invocableMethod = MethodIntrospector.selectInvocableMethod(mostSpecificMethod, bean.getClass()); MethodJmsListenerEndpoint endpoint = new MethodJmsListenerEndpoint(); endpoint.setBean(bean); - endpoint.setMethod(method); + endpoint.setMethod(invocableMethod); + endpoint.setMostSpecificMethod(mostSpecificMethod); endpoint.setMessageHandlerMethodFactory(this.messageHandlerMethodFactory); endpoint.setBeanFactory(this.beanFactory); endpoint.setId(getEndpointId(jmsListener)); @@ -268,9 +255,9 @@ public class JmsListenerAnnotationBeanPostProcessor factory = this.beanFactory.getBean(containerFactoryBeanName, JmsListenerContainerFactory.class); } catch (NoSuchBeanDefinitionException ex) { - throw new BeanInitializationException("Could not register jms listener endpoint on [" + - method + "], no " + JmsListenerContainerFactory.class.getSimpleName() + " with id '" + - containerFactoryBeanName + "' was found in the application context", ex); + throw new BeanInitializationException("Could not register JMS listener endpoint on [" + + mostSpecificMethod + "], no " + JmsListenerContainerFactory.class.getSimpleName() + + " with id '" + containerFactoryBeanName + "' was found in the application context", ex); } } diff --git a/spring-jms/src/main/java/org/springframework/jms/config/MethodJmsListenerEndpoint.java b/spring-jms/src/main/java/org/springframework/jms/config/MethodJmsListenerEndpoint.java index 4855573d63..8d61641790 100644 --- a/spring-jms/src/main/java/org/springframework/jms/config/MethodJmsListenerEndpoint.java +++ b/spring-jms/src/main/java/org/springframework/jms/config/MethodJmsListenerEndpoint.java @@ -39,6 +39,7 @@ import org.springframework.util.StringUtils; * an incoming message for this endpoint. * * @author Stephane Nicoll + * @author Juergen Hoeller * @since 4.1 */ public class MethodJmsListenerEndpoint extends AbstractJmsListenerEndpoint { @@ -47,6 +48,8 @@ public class MethodJmsListenerEndpoint extends AbstractJmsListenerEndpoint { private Method method; + private Method mostSpecificMethod; + private MessageHandlerMethodFactory messageHandlerMethodFactory; private BeanFactory beanFactory; @@ -74,6 +77,29 @@ public class MethodJmsListenerEndpoint extends AbstractJmsListenerEndpoint { return this.method; } + /** + * Set the most specific method known for this endpoint's declaration. + *

In case of a proxy, this will be the method on the target class + * (if annotated itself, that is, if not just annotated in an interface). + * @since 4.2.3 + */ + public void setMostSpecificMethod(Method mostSpecificMethod) { + this.mostSpecificMethod = mostSpecificMethod; + } + + public Method getMostSpecificMethod() { + if (this.mostSpecificMethod != null) { + return this.mostSpecificMethod; + } + else if (AopUtils.isAopProxy(this.bean)) { + Class target = AopProxyUtils.ultimateTargetClass(this.bean); + return AopUtils.getMostSpecificMethod(getMethod(), target); + } + else { + return getMethod(); + } + } + /** * Set the {@link MessageHandlerMethodFactory} to use to build the * {@link InvocableHandlerMethod} responsible to manage the invocation @@ -134,8 +160,8 @@ public class MethodJmsListenerEndpoint extends AbstractJmsListenerEndpoint { if (ann != null) { Object[] destinations = ann.value(); if (destinations.length != 1) { - throw new IllegalStateException("Invalid @" + SendTo.class.getSimpleName() + " annotation on '" - + specificMethod + "' one destination must be set (got " + Arrays.toString(destinations) + ")"); + throw new IllegalStateException("Invalid @" + SendTo.class.getSimpleName() + " annotation on '" + + specificMethod + "' one destination must be set (got " + Arrays.toString(destinations) + ")"); } return resolve((String) destinations[0]); } @@ -154,16 +180,6 @@ public class MethodJmsListenerEndpoint extends AbstractJmsListenerEndpoint { } - private Method getMostSpecificMethod() { - if (AopUtils.isAopProxy(this.bean)) { - Class target = AopProxyUtils.ultimateTargetClass(this.bean); - return AopUtils.getMostSpecificMethod(getMethod(), target); - } - else { - return getMethod(); - } - } - @Override protected StringBuilder getEndpointDescription() { return super.getEndpointDescription() diff --git a/spring-jms/src/test/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessorTests.java b/spring-jms/src/test/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessorTests.java index 6c313b4f0d..ea1dbb70f3 100644 --- a/spring-jms/src/test/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessorTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -26,6 +26,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.BeanCreationException; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -58,8 +59,9 @@ public class JmsListenerAnnotationBeanPostProcessorTests { @Rule public final ExpectedException thrown = ExpectedException.none(); + @Test - public void simpleMessageListener() { + public void simpleMessageListener() throws Exception { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext( Config.class, SimpleMessageListenerTestBean.class); @@ -70,8 +72,9 @@ public class JmsListenerAnnotationBeanPostProcessorTests { JmsListenerEndpoint endpoint = container.getEndpoint(); assertEquals("Wrong endpoint type", MethodJmsListenerEndpoint.class, endpoint.getClass()); MethodJmsListenerEndpoint methodEndpoint = (MethodJmsListenerEndpoint) endpoint; - assertNotNull(methodEndpoint.getBean()); - assertNotNull(methodEndpoint.getMethod()); + assertEquals(SimpleMessageListenerTestBean.class, methodEndpoint.getBean().getClass()); + assertEquals(SimpleMessageListenerTestBean.class.getMethod("handleIt", String.class), methodEndpoint.getMethod()); + assertEquals(SimpleMessageListenerTestBean.class.getMethod("handleIt", String.class), methodEndpoint.getMostSpecificMethod()); SimpleMessageListenerContainer listenerContainer = new SimpleMessageListenerContainer(); methodEndpoint.setupListenerContainer(listenerContainer); @@ -83,14 +86,20 @@ public class JmsListenerAnnotationBeanPostProcessorTests { } @Test - public void metaAnnotationIsDiscovered() { + public void metaAnnotationIsDiscovered() throws Exception { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext( Config.class, MetaAnnotationTestBean.class); try { JmsListenerContainerTestFactory factory = context.getBean(JmsListenerContainerTestFactory.class); assertEquals("one container should have been registered", 1, factory.getListenerContainers().size()); + JmsListenerEndpoint endpoint = factory.getListenerContainers().get(0).getEndpoint(); + assertEquals("Wrong endpoint type", MethodJmsListenerEndpoint.class, endpoint.getClass()); + MethodJmsListenerEndpoint methodEndpoint = (MethodJmsListenerEndpoint) endpoint; + assertEquals(MetaAnnotationTestBean.class, methodEndpoint.getBean().getClass()); + assertEquals(MetaAnnotationTestBean.class.getMethod("handleIt", String.class), methodEndpoint.getMethod()); + assertEquals(MetaAnnotationTestBean.class.getMethod("handleIt", String.class), methodEndpoint.getMostSpecificMethod()); assertEquals("metaTestQueue", ((AbstractJmsListenerEndpoint) endpoint).getDestination()); } finally { @@ -99,13 +108,21 @@ public class JmsListenerAnnotationBeanPostProcessorTests { } @Test - public void sendToAnnotationFoundOnProxy() { + public void sendToAnnotationFoundOnProxy() throws Exception { ConfigurableApplicationContext context = new AnnotationConfigApplicationContext( Config.class, ProxyConfig.class, ProxyTestBean.class); try { JmsListenerContainerTestFactory factory = context.getBean(JmsListenerContainerTestFactory.class); assertEquals("one container should have been registered", 1, factory.getListenerContainers().size()); + JmsListenerEndpoint endpoint = factory.getListenerContainers().get(0).getEndpoint(); + assertEquals("Wrong endpoint type", MethodJmsListenerEndpoint.class, endpoint.getClass()); + MethodJmsListenerEndpoint methodEndpoint = (MethodJmsListenerEndpoint) endpoint; + assertTrue(AopUtils.isJdkDynamicProxy(methodEndpoint.getBean())); + assertTrue(methodEndpoint.getBean() instanceof SimpleService); + assertEquals(SimpleService.class.getMethod("handleIt", String.class), methodEndpoint.getMethod()); + assertEquals(ProxyTestBean.class.getMethod("handleIt", String.class), methodEndpoint.getMostSpecificMethod()); + Method m = ReflectionUtils.findMethod(endpoint.getClass(), "getDefaultResponseDestination"); ReflectionUtils.makeAccessible(m); Object destination = ReflectionUtils.invokeMethod(m, endpoint); @@ -132,7 +149,6 @@ public class JmsListenerAnnotationBeanPostProcessorTests { @JmsListener(destination = "testQueue") public void handleIt(String body) { } - } @@ -174,6 +190,7 @@ public class JmsListenerAnnotationBeanPostProcessorTests { } } + @Configuration @EnableTransactionManagement static class ProxyConfig { @@ -182,15 +199,15 @@ public class JmsListenerAnnotationBeanPostProcessorTests { public PlatformTransactionManager transactionManager() { return mock(PlatformTransactionManager.class); } - } + interface SimpleService { void handleIt(String body); - } + @Component static class ProxyTestBean implements SimpleService { @@ -199,10 +216,10 @@ public class JmsListenerAnnotationBeanPostProcessorTests { @JmsListener(destination = "testQueue") @SendTo("foobar") public void handleIt(String body) { - } } + @Component static class InvalidProxyTestBean implements SimpleService { diff --git a/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerFactoryIntegrationTests.java b/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerFactoryIntegrationTests.java index c4b8d99723..da2a90c146 100644 --- a/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerFactoryIntegrationTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerFactoryIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -29,12 +29,14 @@ import javax.jms.TextMessage; import org.junit.Before; import org.junit.Test; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.support.StaticListableBeanFactory; import org.springframework.jms.StubTextMessage; import org.springframework.jms.listener.DefaultMessageListenerContainer; import org.springframework.jms.listener.SessionAwareMessageListener; import org.springframework.jms.support.converter.MessageConversionException; import org.springframework.jms.support.converter.MessageConverter; +import org.springframework.messaging.handler.annotation.Header; import org.springframework.messaging.handler.annotation.Payload; import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory; import org.springframework.util.ReflectionUtils; @@ -53,27 +55,65 @@ public class JmsListenerContainerFactoryIntegrationTests { private final JmsEndpointSampleBean sample = new JmsEndpointSampleBean(); + private JmsEndpointSampleInterface listener = sample; + @Before public void setup() { initializeFactory(factory); } + @Test public void messageConverterUsedIfSet() throws JMSException { containerFactory.setMessageConverter(new UpperCaseMessageConverter()); - MethodJmsListenerEndpoint endpoint = createDefaultMethodJmsEndpoint("expectFooBarUpperCase", String.class); + MethodJmsListenerEndpoint endpoint = createDefaultMethodJmsEndpoint( + listener.getClass(), "handleIt", String.class, String.class); Message message = new StubTextMessage("foo-bar"); + message.setStringProperty("my-header", "my-value"); invokeListener(endpoint, message); - assertListenerMethodInvocation("expectFooBarUpperCase"); + assertListenerMethodInvocation("handleIt"); } + @Test + public void parameterAnnotationWithJdkProxy() throws JMSException { + ProxyFactory pf = new ProxyFactory(sample); + listener = (JmsEndpointSampleInterface) pf.getProxy(); + + containerFactory.setMessageConverter(new UpperCaseMessageConverter()); + + MethodJmsListenerEndpoint endpoint = createDefaultMethodJmsEndpoint( + JmsEndpointSampleInterface.class, "handleIt", String.class, String.class); + Message message = new StubTextMessage("foo-bar"); + message.setStringProperty("my-header", "my-value"); + + invokeListener(endpoint, message); + assertListenerMethodInvocation("handleIt"); + } + + @Test + public void parameterAnnotationWithCglibProxy() throws JMSException { + ProxyFactory pf = new ProxyFactory(sample); + pf.setProxyTargetClass(true); + listener = (JmsEndpointSampleBean) pf.getProxy(); + + containerFactory.setMessageConverter(new UpperCaseMessageConverter()); + + MethodJmsListenerEndpoint endpoint = createDefaultMethodJmsEndpoint( + JmsEndpointSampleBean.class, "handleIt", String.class, String.class); + Message message = new StubTextMessage("foo-bar"); + message.setStringProperty("my-header", "my-value"); + + invokeListener(endpoint, message); + assertListenerMethodInvocation("handleIt"); + } + + @SuppressWarnings("unchecked") private void invokeListener(JmsListenerEndpoint endpoint, Message message) throws JMSException { - DefaultMessageListenerContainer messageListenerContainer = - containerFactory.createListenerContainer(endpoint); + DefaultMessageListenerContainer messageListenerContainer = containerFactory.createListenerContainer(endpoint); Object listener = messageListenerContainer.getMessageListener(); if (listener instanceof SessionAwareMessageListener) { ((SessionAwareMessageListener) listener).onMessage(message, mock(Session.class)); @@ -87,40 +127,38 @@ public class JmsListenerContainerFactoryIntegrationTests { assertTrue("Method " + methodName + " should have been invoked", sample.invocations.get(methodName)); } - - private MethodJmsListenerEndpoint createMethodJmsEndpoint( - DefaultMessageHandlerMethodFactory factory, Method method) { + private MethodJmsListenerEndpoint createMethodJmsEndpoint(DefaultMessageHandlerMethodFactory factory, Method method) { MethodJmsListenerEndpoint endpoint = new MethodJmsListenerEndpoint(); - endpoint.setBean(sample); + endpoint.setBean(listener); endpoint.setMethod(method); endpoint.setMessageHandlerMethodFactory(factory); return endpoint; } - private MethodJmsListenerEndpoint createDefaultMethodJmsEndpoint(String methodName, Class... parameterTypes) { - return createMethodJmsEndpoint(this.factory, getListenerMethod(methodName, parameterTypes)); + private MethodJmsListenerEndpoint createDefaultMethodJmsEndpoint(Class clazz, String methodName, Class... paramTypes) { + return createMethodJmsEndpoint(this.factory, ReflectionUtils.findMethod(clazz, methodName, paramTypes)); } - private Method getListenerMethod(String methodName, Class... parameterTypes) { - Method method = ReflectionUtils.findMethod(JmsEndpointSampleBean.class, methodName, parameterTypes); - assertNotNull("no method found with name " + methodName + " and parameters " + Arrays.toString(parameterTypes)); - return method; - } - - private void initializeFactory(DefaultMessageHandlerMethodFactory factory) { factory.setBeanFactory(new StaticListableBeanFactory()); factory.afterPropertiesSet(); } - static class JmsEndpointSampleBean { + interface JmsEndpointSampleInterface { + + void handleIt(@Payload String msg, @Header("my-header") String myHeader); + } + + + static class JmsEndpointSampleBean implements JmsEndpointSampleInterface { private final Map invocations = new HashMap(); - public void expectFooBarUpperCase(@Payload String msg) { - invocations.put("expectFooBarUpperCase", true); + public void handleIt(@Payload String msg, @Header("my-header") String myHeader) { + invocations.put("handleIt", true); assertEquals("Unexpected payload message", "FOO-BAR", msg); + assertEquals("Unexpected header value", "my-value", myHeader); } }