Revised method selection for JMS listeners (and their parameters)

Issue: SPR-13576

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<Method> annotatedMethods = new LinkedHashSet<Method>(1);
 			Class<?> targetClass = AopUtils.getTargetClass(bean);
-			ReflectionUtils.doWithMethods(targetClass, new ReflectionUtils.MethodCallback() {
+			Map<Method, Set<JmsListener>> annotatedMethods = MethodIntrospector.selectMethods(targetClass,
-				@Override
+					new MethodIntrospector.MetadataLookup<Set<JmsListener>>() {
-				public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
+						@Override
-					for (JmsListener jmsListener :
+						public Set<JmsListener> inspect(Method method) {
-							AnnotationUtils.getRepeatableAnnotations(method, JmsListener.class, JmsListeners.class)) {
+							Set<JmsListener> listenerMethods =
-						processJmsListener(jmsListener, method, bean);
+									AnnotationUtils.getRepeatableAnnotations(method, JmsListener.class, JmsListeners.class);
-						annotatedMethods.add(method);
+							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<Method, Set<JmsListener>> 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) {
+	protected void processJmsListener(JmsListener jmsListener, Method mostSpecificMethod, Object bean) {
-		if (AopUtils.isJdkDynamicProxy(bean)) {
+		Method invocableMethod = MethodIntrospector.selectInvocableMethod(mostSpecificMethod, bean.getClass());
-			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()));
-			}
-		}
 
 		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 [" +
+				throw new BeanInitializationException("Could not register JMS listener endpoint on [" +
-						method + "], no " + JmsListenerContainerFactory.class.getSimpleName() + " with id '" +
+						mostSpecificMethod + "], no " + JmsListenerContainerFactory.class.getSimpleName() +
-						containerFactoryBeanName + "' was found in the application context", ex);
+						" with id '" + containerFactoryBeanName + "' was found in the application context", ex);
 			}
 		}
 

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.
+	 * <p>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 '"
+				throw new IllegalStateException("Invalid @" + SendTo.class.getSimpleName() + " annotation on '" +
-						+ specificMethod + "' one destination must be set (got " + Arrays.toString(destinations) + ")");
+						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()

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());
+		assertEquals(SimpleMessageListenerTestBean.class, methodEndpoint.getBean().getClass());
-		assertNotNull(methodEndpoint.getMethod());
+		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 {
 

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 =
+		DefaultMessageListenerContainer messageListenerContainer = containerFactory.createListenerContainer(endpoint);
-				containerFactory.createListenerContainer(endpoint);
 		Object listener = messageListenerContainer.getMessageListener();
 		if (listener instanceof SessionAwareMessageListener) {
 			((SessionAwareMessageListener<Message>) 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) {
+	private MethodJmsListenerEndpoint createDefaultMethodJmsEndpoint(Class<?> clazz, String methodName, Class<?>... paramTypes) {
-		return createMethodJmsEndpoint(this.factory, getListenerMethod(methodName, parameterTypes));
+		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<String, Boolean> invocations = new HashMap<String, Boolean>();
 
-		public void expectFooBarUpperCase(@Payload String msg) {
+		public void handleIt(@Payload String msg, @Header("my-header") String myHeader) {
-			invocations.put("expectFooBarUpperCase", true);
+			invocations.put("handleIt", true);
 			assertEquals("Unexpected payload message", "FOO-BAR", msg);
+			assertEquals("Unexpected header value", "my-value", myHeader);
 		}
 	}