Lazy resolution of the JMS message

Previously, any `javax.jms.Message` was converted eagerly to the Message
abstraction. This leads to unnecessary conversion if the Payload is not
requested by the underlying method (i.e. if the `javax.jms.Message` is
injected directly).

This commit returns a `Message` implementation that holds the
`javax.jms.Message` and lazily resolve the payload or the headers on
demand (that is the first time they are requested).

Issue: SPR-13777

spring-jms/src/main/java/org/springframework/jms/support/converter/MessagingMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.jms.support.converter;
 
-import java.util.Map;
 import javax.jms.JMSException;
 import javax.jms.Session;
 
@@ -24,7 +23,7 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.jms.support.JmsHeaderMapper;
 import org.springframework.jms.support.SimpleJmsHeaderMapper;
 import org.springframework.messaging.Message;
-import org.springframework.messaging.support.MessageBuilder;
+import org.springframework.messaging.MessageHeaders;
 import org.springframework.util.Assert;
 
 /**
@@ -104,12 +103,7 @@ public class MessagingMessageConverter implements MessageConverter, Initializing
 		if (message == null) {
 			return null;
 		}
-		Map<String, Object> mappedHeaders = this.headerMapper.toHeaders(message);
-		Object convertedObject = extractPayload(message);
-		MessageBuilder<Object> builder = (convertedObject instanceof org.springframework.messaging.Message) ?
-				MessageBuilder.fromMessage((org.springframework.messaging.Message<Object>) convertedObject) :
-				MessageBuilder.withPayload(convertedObject);
-		return builder.copyHeadersIfAbsent(mappedHeaders).build();
+		return new LazyResolutionMessage(message);
 	}
 
 	/**
@@ -127,4 +121,44 @@ public class MessagingMessageConverter implements MessageConverter, Initializing
 		return this.payloadConverter.toMessage(payload, session);
 	}
 
+	private MessageHeaders extractHeaders(javax.jms.Message message) {
+		return this.headerMapper.toHeaders(message);
+	}
+
+
+	private class LazyResolutionMessage implements Message<Object> {
+
+		private final javax.jms.Message message;
+
+		private Object payload;
+
+		private MessageHeaders headers;
+
+		public LazyResolutionMessage(javax.jms.Message message) {
+			this.message = message;
+		}
+
+		@Override
+		public Object getPayload() {
+			if (this.payload == null) {
+				try {
+					this.payload = extractPayload(this.message);
+				}
+				catch (JMSException ex) {
+					throw new MessageConversionException(
+							"Failed to extract payload from [" + this.message + "]", ex);
+				}
+			}
+			return this.payload;
+		}
+
+		@Override
+		public MessageHeaders getHeaders() {
+			if (this.headers == null) {
+				this.headers = extractHeaders(this.message);
+			}
+			return this.headers;
+		}
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -50,6 +50,8 @@ import static org.mockito.BDDMockito.*;
  */
 public class MessagingMessageListenerAdapterTests {
 
+	private static final Destination sharedReplyDestination = mock(Destination.class);
+
 	private final DefaultMessageHandlerMethodFactory factory = new DefaultMessageHandlerMethodFactory();
 
 	private final SampleBean sample = new SampleBean();
@@ -151,7 +153,6 @@ public class MessagingMessageListenerAdapterTests {
 
 	@Test
 	public void replyPayloadToQueue() throws JMSException {
-		Message<String> request = MessageBuilder.withPayload("Response").build();
 		Session session = mock(Session.class);
 		Queue replyDestination = mock(Queue.class);
 		given(session.createQueue("queueOut")).willReturn(replyDestination);
@@ -161,7 +162,7 @@ public class MessagingMessageListenerAdapterTests {
 		given(session.createTextMessage("Response")).willReturn(responseMessage);
 		given(session.createProducer(replyDestination)).willReturn(messageProducer);
 
-		MessagingMessageListenerAdapter listener = getPayloadInstance(request, "replyPayloadToQueue", Message.class);
+		MessagingMessageListenerAdapter listener = getPayloadInstance("Response", "replyPayloadToQueue", Message.class);
 		listener.onMessage(mock(javax.jms.Message.class), session);
 
 		verify(session).createQueue("queueOut");
@@ -172,7 +173,6 @@ public class MessagingMessageListenerAdapterTests {
 
 	@Test
 	public void replyPayloadToTopic() throws JMSException {
-		Message<String> request = MessageBuilder.withPayload("Response").build();
 		Session session = mock(Session.class);
 		Topic replyDestination = mock(Topic.class);
 		given(session.createTopic("topicOut")).willReturn(replyDestination);
@@ -182,7 +182,7 @@ public class MessagingMessageListenerAdapterTests {
 		given(session.createTextMessage("Response")).willReturn(responseMessage);
 		given(session.createProducer(replyDestination)).willReturn(messageProducer);
 
-		MessagingMessageListenerAdapter listener = getPayloadInstance(request, "replyPayloadToTopic", Message.class);
+		MessagingMessageListenerAdapter listener = getPayloadInstance("Response", "replyPayloadToTopic", Message.class);
 		listener.onMessage(mock(javax.jms.Message.class), session);
 
 		verify(session).createTopic("topicOut");
@@ -193,17 +193,13 @@ public class MessagingMessageListenerAdapterTests {
 
 	@Test
 	public void replyPayloadToDestination() throws JMSException {
-		Queue replyDestination = mock(Queue.class);
-		Message<String> request = MessageBuilder.withPayload("Response")
-				.setHeader("destination", replyDestination).build();
-
 		Session session = mock(Session.class);
 		MessageProducer messageProducer = mock(MessageProducer.class);
 		TextMessage responseMessage = mock(TextMessage.class);
 		given(session.createTextMessage("Response")).willReturn(responseMessage);
-		given(session.createProducer(replyDestination)).willReturn(messageProducer);
+		given(session.createProducer(sharedReplyDestination)).willReturn(messageProducer);
 
-		MessagingMessageListenerAdapter listener = getPayloadInstance(request, "replyPayloadToDestination", Message.class);
+		MessagingMessageListenerAdapter listener = getPayloadInstance("Response", "replyPayloadToDestination", Message.class);
 		listener.onMessage(mock(javax.jms.Message.class), session);
 
 		verify(session, times(0)).createQueue(anyString());
@@ -215,7 +211,6 @@ public class MessagingMessageListenerAdapterTests {
 	@Test
 	public void replyPayloadNoDestination() throws JMSException {
 		Queue replyDestination = mock(Queue.class);
-		Message<String> request = MessageBuilder.withPayload("Response").build();
 
 		Session session = mock(Session.class);
 		MessageProducer messageProducer = mock(MessageProducer.class);
@@ -224,7 +219,7 @@ public class MessagingMessageListenerAdapterTests {
 		given(session.createProducer(replyDestination)).willReturn(messageProducer);
 
 		MessagingMessageListenerAdapter listener =
-				getPayloadInstance(request, "replyPayloadNoDestination", Message.class);
+				getPayloadInstance("Response", "replyPayloadNoDestination", Message.class);
 		listener.setDefaultResponseDestination(replyDestination);
 		listener.onMessage(mock(javax.jms.Message.class), session);
 
@@ -243,7 +238,6 @@ public class MessagingMessageListenerAdapterTests {
 
 	private TextMessage testReplyWithJackson(String methodName, String replyContent) throws JMSException {
 		Queue replyDestination = mock(Queue.class);
-		Message<String> request = MessageBuilder.withPayload("Response").build();
 
 		Session session = mock(Session.class);
 		MessageProducer messageProducer = mock(MessageProducer.class);
@@ -251,7 +245,7 @@ public class MessagingMessageListenerAdapterTests {
 		given(session.createTextMessage(replyContent)).willReturn(responseMessage);
 		given(session.createProducer(replyDestination)).willReturn(messageProducer);
 
-		MessagingMessageListenerAdapter listener = getPayloadInstance(request, methodName, Message.class);
+		MessagingMessageListenerAdapter listener = getPayloadInstance("Response", methodName, Message.class);
 		MappingJackson2MessageConverter messageConverter = new MappingJackson2MessageConverter();
 		messageConverter.setTargetType(MessageType.TEXT);
 		listener.setMessageConverter(messageConverter);
@@ -321,8 +315,7 @@ public class MessagingMessageListenerAdapterTests {
 		}
 
 		public JmsResponse<String> replyPayloadToDestination(Message<String> input) {
-			return JmsResponse.forDestination(input.getPayload(),
-					input.getHeaders().get("destination", Destination.class));
+			return JmsResponse.forDestination(input.getPayload(), sharedReplyDestination);
 		}
 
 		public JmsResponse<String> replyPayloadNoDestination(Message<String> input) {

spring-jms/src/test/java/org/springframework/jms/support/converter/MessagingMessageConverterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -34,7 +34,6 @@ import static org.junit.Assert.*;
 import static org.mockito.BDDMockito.*;
 
 /**
- *
  * @author Stephane Nicoll
  */
 public class MessagingMessageConverterTests {
@@ -46,8 +45,8 @@ public class MessagingMessageConverterTests {
 
 	@Test
 	public void onlyHandlesMessage() throws JMSException {
-		thrown.expect(IllegalArgumentException.class);
-		converter.toMessage(new Object(), mock(Session.class));
+		this.thrown.expect(IllegalArgumentException.class);
+		this.converter.toMessage(new Object(), mock(Session.class));
 	}
 
 	@Test
@@ -57,43 +56,62 @@ public class MessagingMessageConverterTests {
 		ObjectMessage jmsMessage = mock(ObjectMessage.class);
 		given(session.createObjectMessage(payload)).willReturn(jmsMessage);
 
-		converter.toMessage(MessageBuilder.withPayload(payload).build(), session);
+		this.converter.toMessage(MessageBuilder.withPayload(payload).build(), session);
 		verify(session).createObjectMessage(payload);
 	}
 
 	@Test
 	public void fromNull() throws JMSException {
-		assertNull(converter.fromMessage(null));
+		assertNull(this.converter.fromMessage(null));
 	}
 
 	@Test
 	public void customPayloadConverter() throws JMSException {
 		TextMessage jmsMsg = new StubTextMessage("1224");
 
-		converter.setPayloadConverter(new SimpleMessageConverter() {
-			@Override
-			public Object fromMessage(javax.jms.Message message) throws JMSException, MessageConversionException {
-				TextMessage textMessage = (TextMessage) message;
-				return Long.parseLong(textMessage.getText());
-			}
-		});
+		this.converter.setPayloadConverter(new TestMessageConverter());
+		Message<?> msg = (Message<?>) this.converter.fromMessage(jmsMsg);
+		assertEquals(1224L, msg.getPayload());
+	}
 
-		Message<?> msg = (Message<?>) converter.fromMessage(jmsMsg);
+	@Test
+	public void payloadConversionLazilyInvoked() throws JMSException {
+		TextMessage jmsMsg = new StubTextMessage("1224");
+		TestMessageConverter converter = new TestMessageConverter();
+		this.converter.setPayloadConverter(converter);
+		Message<?> msg = (Message<?>) this.converter.fromMessage(jmsMsg);
+		assertEquals("Converter should not have been called yet", false, converter.called);
 		assertEquals(1224L, msg.getPayload());
+		assertEquals("Converter should have been called", true, converter.called);
 	}
 
 	@Test
-	public void payloadIsAMessage() throws JMSException {
-		final Message<String> message = MessageBuilder.withPayload("Test").setHeader("inside", true).build();
-		converter.setPayloadConverter(new SimpleMessageConverter() {
-			@Override
-			public Object fromMessage(javax.jms.Message jmsMessage) throws JMSException, MessageConversionException {
-				return message;
+	public void headerConversionLazilyInvoked() throws JMSException {
+		javax.jms.Message jmsMsg = mock(javax.jms.Message.class);
+		when(jmsMsg.getPropertyNames()).thenThrow(new IllegalArgumentException("Header failure"));
+
+		Message<?> msg = (Message<?>) this.converter.fromMessage(jmsMsg);
+
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("Header failure");
+		msg.getHeaders(); // Triggers headers resolution
+	}
+
+
+	static class TestMessageConverter extends SimpleMessageConverter {
+
+		private boolean called;
+
+		@Override
+		public Object fromMessage(javax.jms.Message message) throws JMSException, MessageConversionException {
+			if (this.called) {
+				throw new java.lang.IllegalStateException("Converter called twice");
 			}
-		});
-		Message<?> msg = (Message<?>) converter.fromMessage(new StubTextMessage());
-		assertEquals(message.getPayload(), msg.getPayload());
-		assertEquals(true, msg.getHeaders().get("inside"));
+			this.called = true;
+			TextMessage textMessage = (TextMessage) message;
+			return Long.parseLong(textMessage.getText());
+		}
+
 	}
 
 }