From 489a6b20d2b6098ee87aa34d416cd3aae0b555e8 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Thu, 11 Dec 2014 18:51:18 +0200 Subject: [PATCH] AMQP-452: PRC: RemoteInvocResult for exceptions JIRA: https://jira.spring.io/browse/AMQP-452 When the `body` of the received `message` in the `AmqpInvokerServiceExporter` isn't an instance of `RemoteInvocation`, we should wrap an appropriate `IllegalArgumentException` to the `RemoteInvocationResult`. It allows for for `AmqpClientInterceptor` to unwrap the reply correctly and rethrow to the caller. Conflicts: spring-amqp/src/main/java/org/springframework/amqp/remoting/service/AmqpInvokerServiceExporter.java Resolved. --- .../service/AmqpInvokerServiceExporter.java | 16 +++--- .../amqp/remoting/RemotingTest.java | 50 +++++++++++++++++-- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/spring-amqp/src/main/java/org/springframework/amqp/remoting/service/AmqpInvokerServiceExporter.java b/spring-amqp/src/main/java/org/springframework/amqp/remoting/service/AmqpInvokerServiceExporter.java index 05c9a07c..bb673b2b 100644 --- a/spring-amqp/src/main/java/org/springframework/amqp/remoting/service/AmqpInvokerServiceExporter.java +++ b/spring-amqp/src/main/java/org/springframework/amqp/remoting/service/AmqpInvokerServiceExporter.java @@ -48,6 +48,7 @@ import org.springframework.remoting.support.RemoteInvocationResult; * "http://static.springsource.org/spring-amqp/reference/html/amqp.html#request-reply" >here. * * @author David Bilge + * @author Artem Bilan * @since 1.2 */ public class AmqpInvokerServiceExporter extends RemoteInvocationBasedExporter implements MessageListener { @@ -64,13 +65,16 @@ public class AmqpInvokerServiceExporter extends RemoteInvocationBasedExporter im } Object invocationRaw = messageConverter.fromMessage(message); - if (invocationRaw == null || !(invocationRaw instanceof RemoteInvocation)) { - send(new RuntimeException("The message does not contain a RemoteInvocation payload"), replyToAddress); - return; - } - RemoteInvocation invocation = (RemoteInvocation) invocationRaw; - RemoteInvocationResult remoteInvocationResult = invokeAndCreateResult(invocation, getService()); + RemoteInvocationResult remoteInvocationResult; + if (invocationRaw == null || !(invocationRaw instanceof RemoteInvocation)) { + remoteInvocationResult = new RemoteInvocationResult( + new IllegalArgumentException("The message does not contain a RemoteInvocation payload")); + } + else { + RemoteInvocation invocation = (RemoteInvocation) invocationRaw; + remoteInvocationResult = invokeAndCreateResult(invocation, getService()); + } send(remoteInvocationResult, replyToAddress); } diff --git a/spring-amqp/src/test/java/org/springframework/amqp/remoting/RemotingTest.java b/spring-amqp/src/test/java/org/springframework/amqp/remoting/RemotingTest.java index e3228089..dd8b870a 100644 --- a/spring-amqp/src/test/java/org/springframework/amqp/remoting/RemotingTest.java +++ b/spring-amqp/src/test/java/org/springframework/amqp/remoting/RemotingTest.java @@ -13,6 +13,12 @@ package org.springframework.amqp.remoting; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertThat; + +import java.util.concurrent.atomic.AtomicBoolean; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -29,16 +35,21 @@ import org.springframework.amqp.remoting.testservice.GeneralException; import org.springframework.amqp.remoting.testservice.SpecialException; import org.springframework.amqp.remoting.testservice.TestServiceImpl; import org.springframework.amqp.remoting.testservice.TestServiceInterface; +import org.springframework.amqp.support.converter.MessageConversionException; import org.springframework.amqp.support.converter.MessageConverter; +import org.springframework.amqp.support.converter.SimpleMessageConverter; /** * @author David Bilge + * @author Artem Bilan * @since 1.2 */ public class RemotingTest { private TestServiceInterface riggedProxy; + private AmqpInvokerServiceExporter serviceExporter; + /** * Set up a rig of directly wired-up proxy and service listener so that both can be tested together without needing * a running rabbit. @@ -47,11 +58,11 @@ public class RemotingTest { public void initializeTestRig() throws Exception { // Set up the service TestServiceInterface testService = new TestServiceImpl(); - final AmqpInvokerServiceExporter serviceExporter = new AmqpInvokerServiceExporter(); + this.serviceExporter = new AmqpInvokerServiceExporter(); final SentSavingTemplate sentSavingTemplate = new SentSavingTemplate(); - serviceExporter.setAmqpTemplate(sentSavingTemplate); - serviceExporter.setService(testService); - serviceExporter.setServiceInterface(TestServiceInterface.class); + this.serviceExporter.setAmqpTemplate(sentSavingTemplate); + this.serviceExporter.setService(testService); + this.serviceExporter.setServiceInterface(TestServiceInterface.class); // Set up the client AmqpProxyFactoryBean amqpProxyFactoryBean = new AmqpProxyFactoryBean(); @@ -100,4 +111,35 @@ public class RemotingTest { Assert.assertNotNull(returnedException); Assert.assertTrue(returnedException instanceof SpecialException); } + + @Test + public void testWrongRemoteInvocationArgument() { + MessageConverter messageConverter = this.serviceExporter.getMessageConverter(); + this.serviceExporter.setMessageConverter(new SimpleMessageConverter() { + + private AtomicBoolean invoked = new AtomicBoolean(); + + @Override + protected Message createMessage(Object object, MessageProperties messageProperties) + throws MessageConversionException { + Message message = super.createMessage(object, messageProperties); + if (!invoked.getAndSet(true)) { + messageProperties.setContentType(null); + } + return message; + } + + }); + + try { + riggedProxy.simpleStringReturningTestMethod("Test"); + } + catch (Exception e) { + assertThat(e, instanceOf(IllegalArgumentException.class)); + assertThat(e.getMessage(), containsString("The message does not contain a RemoteInvocation payload")); + } + + this.serviceExporter.setMessageConverter(messageConverter); + } + }