From 31a3607de6fd95a372e9330632a869db064892bd Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 14 Jan 2016 17:03:11 +0100 Subject: [PATCH] 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 --- .../converter/MessagingMessageConverter.java | 52 ++++++++++++--- .../MessagingMessageListenerAdapterTests.java | 27 +++----- .../MessagingMessageConverterTests.java | 66 ++++++++++++------- 3 files changed, 95 insertions(+), 50 deletions(-) diff --git a/spring-jms/src/main/java/org/springframework/jms/support/converter/MessagingMessageConverter.java b/spring-jms/src/main/java/org/springframework/jms/support/converter/MessagingMessageConverter.java index a4db74313e9..6b793fa3633 100644 --- a/spring-jms/src/main/java/org/springframework/jms/support/converter/MessagingMessageConverter.java +++ b/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 mappedHeaders = this.headerMapper.toHeaders(message); - Object convertedObject = extractPayload(message); - MessageBuilder builder = (convertedObject instanceof org.springframework.messaging.Message) ? - MessageBuilder.fromMessage((org.springframework.messaging.Message) 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 { + + 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; + } + } + } diff --git a/spring-jms/src/test/java/org/springframework/jms/listener/adapter/MessagingMessageListenerAdapterTests.java b/spring-jms/src/test/java/org/springframework/jms/listener/adapter/MessagingMessageListenerAdapterTests.java index d4b98753859..9f87143aa53 100644 --- a/spring-jms/src/test/java/org/springframework/jms/listener/adapter/MessagingMessageListenerAdapterTests.java +++ b/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 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 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 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 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 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 replyPayloadToDestination(Message input) { - return JmsResponse.forDestination(input.getPayload(), - input.getHeaders().get("destination", Destination.class)); + return JmsResponse.forDestination(input.getPayload(), sharedReplyDestination); } public JmsResponse replyPayloadNoDestination(Message input) { diff --git a/spring-jms/src/test/java/org/springframework/jms/support/converter/MessagingMessageConverterTests.java b/spring-jms/src/test/java/org/springframework/jms/support/converter/MessagingMessageConverterTests.java index 4841d039527..e390676bd4f 100644 --- a/spring-jms/src/test/java/org/springframework/jms/support/converter/MessagingMessageConverterTests.java +++ b/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 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()); + } + } }