[#1145] Fix generic parameter type detection for batch listeners … (#1146)

* [#1145] Fix generic parameter type detection for batch listeners with typed 'org.springframework.messaging.Message's

* [#1145] Update integration test

* [#1145] Add myself as author

Author: KaiStapel

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/adapter/MessagingMessageListenerAdapter.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.
@@ -57,6 +57,7 @@
  * @author Stephane Nicoll
  * @author Gary Russell
  * @author Artem Bilan
+ * @author Kai Stapel
  *
  * @since 1.4
  */
@@ -381,7 +382,8 @@ private Type extractGenericParameterTypFromMethodParameter(MethodParameter metho
 				if (parameterizedType.getRawType().equals(Message.class)) {
 					genericParameterType = ((ParameterizedType) genericParameterType).getActualTypeArguments()[0];
 				}
-				else if (parameterizedType.getRawType().equals(List.class)
+				else if (this.isBatch
+						&& parameterizedType.getRawType().equals(List.class)
 						&& parameterizedType.getActualTypeArguments().length == 1) {
 
 					Type paramType = parameterizedType.getActualTypeArguments()[0];
@@ -392,7 +394,7 @@ else if (parameterizedType.getRawType().equals(List.class)
 					if (messageHasGeneric) {
 						genericParameterType = ((ParameterizedType) paramType).getActualTypeArguments()[0];
 					}
-					if (this.isBatch) {
+					else {
 						// when decoding batch messages we convert to the List's generic type
 						genericParameterType = paramType;
 					}

spring-rabbit/src/test/java/org/springframework/amqp/rabbit/annotation/EnableRabbitBatchJsonIntegrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 the original author or authors.
+ * Copyright 2019-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.
@@ -18,12 +18,14 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.UnsupportedEncodingException;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.amqp.core.MessageProperties;
 import org.springframework.amqp.rabbit.batch.SimpleBatchingStrategy;
 import org.springframework.amqp.rabbit.config.SimpleRabbitListenerContainerFactory;
 import org.springframework.amqp.rabbit.connection.CachingConnectionFactory;
@@ -32,6 +34,7 @@
 import org.springframework.amqp.rabbit.junit.RabbitAvailable;
 import org.springframework.amqp.rabbit.junit.RabbitAvailableCondition;
 import org.springframework.amqp.support.converter.Jackson2JsonMessageConverter;
+import org.springframework.amqp.support.converter.SimpleMessageConverter;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -43,6 +46,7 @@
 
 /**
  * @author Gary Russell
+ * @author Kai Stapel
  * @since 2.2
  *
  */
@@ -58,23 +62,29 @@ public class EnableRabbitBatchJsonIntegrationTests {
 	private Listener listener;
 
 	@Test
-	public void testSimpleList() throws InterruptedException {
-		this.template.convertAndSend("json.batch.1", new Foo("foo"));
-		this.template.convertAndSend("json.batch.1", new Foo("bar"));
+	public void testSimpleList() throws Exception {
+		this.template.send("json.batch.1", msg("{\"bar\":\"foo\"}"));
+		this.template.send("json.batch.1", msg("{\"bar\":\"bar\"}"));
 		assertThat(this.listener.foosLatch.await(10, TimeUnit.SECONDS)).isTrue();
 		assertThat(this.listener.foos.get(0).getBar()).isEqualTo("foo");
 		assertThat(this.listener.foos.get(1).getBar()).isEqualTo("bar");
 	}
 
 	@Test
-	public void testMessageList() throws InterruptedException {
-		this.template.convertAndSend("json.batch.2", new Foo("foo"));
-		this.template.convertAndSend("json.batch.2", new Foo("bar"));
+	public void testMessageList() throws Exception {
+		this.template.send("json.batch.2", msg("{\"bar\":\"foo\"}"));
+		this.template.send("json.batch.2", msg("{\"bar\":\"bar\"}"));
 		assertThat(this.listener.fooMessagesLatch.await(10, TimeUnit.SECONDS)).isTrue();
 		assertThat(this.listener.fooMessages.get(0).getPayload().getBar()).isEqualTo("foo");
 		assertThat(this.listener.fooMessages.get(1).getPayload().getBar()).isEqualTo("bar");
 	}
 
+	private org.springframework.amqp.core.Message msg(String body) throws UnsupportedEncodingException {
+		MessageProperties properties = new MessageProperties();
+		properties.setContentType("application/json");
+		return new org.springframework.amqp.core.Message(body.getBytes(SimpleMessageConverter.DEFAULT_CHARSET), properties);
+	}
+
 	@Configuration
 	@EnableRabbit
 	public static class Config {
@@ -84,6 +94,8 @@ public SimpleRabbitListenerContainerFactory rabbitListenerContainerFactory() {
 			SimpleRabbitListenerContainerFactory factory = new SimpleRabbitListenerContainerFactory();
 			factory.setConnectionFactory(connectionFactory());
 			factory.setBatchListener(true);
+			factory.setConsumerBatchEnabled(true);
+			factory.setBatchSize(2);
 			factory.setMessageConverter(converter());
 			return factory;
 		}

spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/adapter/MessagingMessageListenerAdapterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2019 the original author or authors.
+ * Copyright 2014-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.
@@ -25,8 +25,10 @@
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -51,6 +53,7 @@
  * @author Stephane Nicoll
  * @author Artem Bilan
  * @author Gary Russell
+ * @author Kai Stapel
  */
 public class MessagingMessageListenerAdapterTests {
 
@@ -212,6 +215,54 @@ public void genericMessageTest3() throws Exception {
 		assertThat(this.sample.payload.getClass()).isEqualTo(LinkedHashMap.class);
 	}
 
+	@Test
+	public void batchAmqpMessagesTest() {
+		// given
+		org.springframework.amqp.core.Message message1 = MessageTestUtils.createTextMessage("{ \"foo1\" : \"bar1\" }");
+		message1.getMessageProperties().setContentType("application/json");
+		Channel channel = mock(Channel.class);
+		BatchMessagingMessageListenerAdapter listener = getBatchInstance("withAmqpMessageBatch");
+		listener.setMessageConverter(new Jackson2JsonMessageConverter());
+
+		// when
+		listener.onMessageBatch(Arrays.asList(message1), channel);
+
+		// then
+		assertThat(this.sample.batchPayloads.get(0).getClass()).isEqualTo(String.class);
+	}
+
+	@Test
+	public void batchTypedMessagesTest() {
+		// given
+		org.springframework.amqp.core.Message message1 = MessageTestUtils.createTextMessage("{ \"foo1\" : \"bar1\" }");
+		message1.getMessageProperties().setContentType("application/json");
+		Channel channel = mock(Channel.class);
+		BatchMessagingMessageListenerAdapter listener = getBatchInstance("withTypedMessageBatch");
+		listener.setMessageConverter(new Jackson2JsonMessageConverter());
+
+		// when
+		listener.onMessageBatch(Arrays.asList(message1), channel);
+
+		// then
+		assertThat(this.sample.batchPayloads.get(0).getClass()).isEqualTo(Foo.class);
+	}
+
+	@Test
+	public void batchTypedObjectTest() {
+		// given
+		org.springframework.amqp.core.Message message1 = MessageTestUtils.createTextMessage("{ \"foo1\" : \"bar1\" }");
+		message1.getMessageProperties().setContentType("application/json");
+		Channel channel = mock(Channel.class);
+		BatchMessagingMessageListenerAdapter listener = getBatchInstance("withFooBatch");
+		listener.setMessageConverter(new Jackson2JsonMessageConverter());
+
+		// when
+		listener.onMessageBatch(Arrays.asList(message1), channel);
+
+		// then
+		assertThat(this.sample.batchPayloads.get(0).getClass()).isEqualTo(Foo.class);
+	}
+
 	protected MessagingMessageListenerAdapter getSimpleInstance(String methodName, Class<?>... parameterTypes) {
 		Method m = ReflectionUtils.findMethod(SampleBean.class, methodName, parameterTypes);
 		return createInstance(m, false);
@@ -246,6 +297,17 @@ protected MessagingMessageListenerAdapter createMultiInstance(Method m1, Method
 		return adapter;
 	}
 
+	protected BatchMessagingMessageListenerAdapter getBatchInstance(String methodName) {
+		Method m = ReflectionUtils.findMethod(SampleBean.class, methodName, List.class);
+		return createBatchInstance(m);
+	}
+
+	protected BatchMessagingMessageListenerAdapter createBatchInstance(Method m) {
+		BatchMessagingMessageListenerAdapter adapter = new BatchMessagingMessageListenerAdapter(null, m, false, null, null);
+		adapter.setHandlerAdapter(new HandlerAdapter(factory.createInvocableHandlerMethod(sample, m)));
+		return adapter;
+	}
+
 	private void initializeFactory(DefaultMessageHandlerMethodFactory factory) {
 		factory.setBeanFactory(new StaticListableBeanFactory());
 		factory.afterPropertiesSet();
@@ -254,6 +316,7 @@ private void initializeFactory(DefaultMessageHandlerMethodFactory factory) {
 	private static class SampleBean {
 
 		private Object payload;
+		private List<Object> batchPayloads;
 
 		SampleBean() {
 		}
@@ -295,6 +358,21 @@ public void withNonGenericMessage(@SuppressWarnings("rawtypes") Message message)
 			this.payload = message.getPayload();
 		}
 
+		@SuppressWarnings("unused")
+		public void withAmqpMessageBatch(List<org.springframework.amqp.core.Message> messageBatch) {
+			this.batchPayloads = messageBatch.stream().map(m -> new String(m.getBody())).collect(Collectors.toList());
+		}
+
+		@SuppressWarnings("unused")
+		public void withTypedMessageBatch(List<Message<Foo>> messageBatch) {
+			this.batchPayloads = messageBatch.stream().map(Message::getPayload).collect(Collectors.toList());
+		}
+
+		@SuppressWarnings("unused")
+		public void withFooBatch(List<Foo> messageBatch) {
+			this.batchPayloads = new ArrayList<>(messageBatch);
+		}
+
 		@SuppressWarnings("unused")
 		public String failWithReturn(Integer input) {
 			throw new IllegalArgumentException("Expected test exception");