From 33a40f6cae7550c36f995a4ee0ff8765a1ca8038 Mon Sep 17 00:00:00 2001
From: Gary Russell <grussell@pivotal.io>
Date: Tue, 17 Nov 2015 15:37:31 -0500
Subject: [PATCH] INT-3884: Outbound Gateway replyChannel in Chain

JIRA: https://jira.spring.io/browse/INT-3884

Previously, an outbound gateway within a chain allowed a `reply-channel` attribute
but it was ignored. The gateway's output channel is set to send the message to
the next element in the chain, or the chain's output channel if the gateway is
the last element.

Tighten the parser logic to detect and disallow a reply channel within a chain.

Polishing
---
 .../xml/AbstractConsumerEndpointParser.java   | 21 +++++++-
 .../config/xml/EnricherParser.java            |  8 ++-
 .../xml/EnricherParserTests5-context.xml      | 16 +++---
 .../config/JmsOutboundGatewayParserTests.java |  9 ++++
 .../config/gatewayMaintainsReplyChannel.xml   | 52 +++++++++++++++----
 src/reference/asciidoc/whats-new.adoc         |  7 +++
 6 files changed, 93 insertions(+), 20 deletions(-)

diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java
index 16343ac985..b0f866b5e5 100644
--- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java
+++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.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.
@@ -95,12 +95,19 @@ public abstract class AbstractConsumerEndpointParser extends AbstractBeanDefinit
 		String inputChannelAttributeName = this.getInputChannelAttributeName();
 		boolean hasInputChannelAttribute = element.hasAttribute(inputChannelAttributeName);
 		if (parserContext.isNested()) {
+			String elementDescription = IntegrationNamespaceUtils.createElementDescription(element);
 			if (hasInputChannelAttribute) {
-				String elementDescription = IntegrationNamespaceUtils.createElementDescription(element);
 				parserContext.getReaderContext().error("The '" + inputChannelAttributeName
 						+ "' attribute isn't allowed for a nested (e.g. inside a <chain/>) endpoint element: "
 						+ elementDescription + ".", element);
 			}
+			if (!replyChannelInChainAllowed(element)) {
+				if (StringUtils.hasText(element.getAttribute("reply-channel"))) {
+					parserContext.getReaderContext().error("The 'reply-channel' attribute isn't"
+							+ " allowed for a nested (e.g. inside a <chain/>) outbound gateway element: "
+							+ elementDescription + ".", element);
+				}
+			}
 			return handlerBeanDefinition;
 		}
 		else {
@@ -168,4 +175,14 @@ public abstract class AbstractConsumerEndpointParser extends AbstractBeanDefinit
 		return null;
 	}
 
+	/**
+	 * Override to allow 'reply-channel' within a chain, for components where it
+	 * makes sense (e.g. enricher). Default is false for outbound gateways, else true.
+	 * @return true to allow a reply channel attribute within a chain.
+	 */
+	protected boolean replyChannelInChainAllowed(Element element) {
+		String localName = element.getLocalName();
+		return localName == null || !localName.contains("outbound-gateway");
+	}
+
 }
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/EnricherParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/EnricherParser.java
index 0f97644fdb..94543dec93 100644
--- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/EnricherParser.java
+++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/EnricherParser.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.
@@ -42,6 +42,7 @@ import org.springframework.util.xml.DomUtils;
  * @author Artem Bilan
  * @author Liujiong
  * @author Kris Jacyna
+ * @author Gary Russell
  * @since 2.1
  */
 public class EnricherParser extends AbstractConsumerEndpointParser {
@@ -201,4 +202,9 @@ public class EnricherParser extends AbstractConsumerEndpointParser {
 		return builder;
 	}
 
+	@Override
+	protected boolean replyChannelInChainAllowed(Element element) {
+		return true;
+	}
+
 }
diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EnricherParserTests5-context.xml b/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EnricherParserTests5-context.xml
index a6f75c7df6..72bf561dbb 100644
--- a/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EnricherParserTests5-context.xml
+++ b/spring-integration-core/src/test/java/org/springframework/integration/config/xml/EnricherParserTests5-context.xml
@@ -11,16 +11,20 @@
 
 	<channel id="requestChannel"/>
 
+	<channel id="replyChannel"/>
+
 	<channel id="errChannel"/>
 
 	<channel id="outputChannel">
 		<queue />
 	</channel>
 
-	<enricher id="enricher" 
-			input-channel="inputChannel" request-channel="requestChannel" 
-			output-channel="outputChannel" error-channel="errChannel">
-		<property name="name" expression="'Mr. ' + payload.name"/>
-	</enricher>
-
+	<chain input-channel="inputChannel" output-channel="outputChannel">
+		<enricher id="enricher"
+				  request-channel="requestChannel"
+				  reply-channel="replyChannel"
+				  error-channel="errChannel">
+			<property name="name" expression="'Mr. ' + payload.name"/>
+		</enricher>
+	</chain>
 </beans:beans>
diff --git a/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsOutboundGatewayParserTests.java b/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsOutboundGatewayParserTests.java
index ac37df71e7..bf36afd0f9 100644
--- a/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsOutboundGatewayParserTests.java
+++ b/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsOutboundGatewayParserTests.java
@@ -16,10 +16,13 @@
 
 package org.springframework.integration.jms.config;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -249,6 +252,12 @@ public class JmsOutboundGatewayParserTests {
 		String result = gateway.echo("hello");
 		verify(handler, times(1)).handleMessage(Mockito.any(Message.class));
 		assertEquals("hello", result);
+		JmsOutboundGateway gw1 = context.getBean("chain1$child.gateway.handler", JmsOutboundGateway.class);
+		MessageChannel out = TestUtils.getPropertyValue(gw1, "outputChannel", MessageChannel.class);
+		assertThat(out.getClass().getSimpleName(), equalTo("ReplyForwardingMessageChannel"));
+		JmsOutboundGateway gw2 = context.getBean("chain2$child.gateway.handler", JmsOutboundGateway.class);
+		out = TestUtils.getPropertyValue(gw2, "outputChannel", MessageChannel.class);
+		assertThat(out.getClass().getName(), containsString("MessageHandlerChain$"));
 		context.close();
 	}
 
diff --git a/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/gatewayMaintainsReplyChannel.xml b/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/gatewayMaintainsReplyChannel.xml
index 3e5cae5a01..9f0af69626 100644
--- a/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/gatewayMaintainsReplyChannel.xml
+++ b/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/gatewayMaintainsReplyChannel.xml
@@ -6,7 +6,7 @@
 		http://www.springframework.org/schema/integration/jms http://www.springframework.org/schema/integration/jms/spring-integration-jms.xsd"
 	xmlns:int="http://www.springframework.org/schema/integration"
 	xmlns:int-jms="http://www.springframework.org/schema/integration/jms">
-	
+
 	<int:message-history/>
 
 	<int:gateway id="gateway"
@@ -19,17 +19,47 @@
 			<int:header name="foo" value="bar"/>
 		</int:method>
     </int:gateway>
-		
+
 	<int:chain input-channel="requests">
-		<int-jms:outbound-gateway request-destination="requestQueueA" 
-	                              reply-destination="replyQueueB" 
+		<int-jms:outbound-gateway request-destination="requestQueueA"
+	                              reply-destination="replyQueueB"
 	                              connection-factory="connectionFactory"
 	                              receive-timeout="100000"
 	                              reply-timeout="200000"/>
 	</int:chain>
-	
-	
-	
+
+	<int:chain id="chain1" input-channel="chainWithGatewayAtEnd">
+		<int-jms:outbound-gateway id="gateway"
+		                          request-destination="requestQueueA"
+	                              reply-destination="replyQueueB"
+	                              connection-factory="connectionFactory"
+	                              receive-timeout="100000"
+	                              reply-timeout="200000"/>
+	</int:chain>
+
+	<int:chain id="chain2" input-channel="chainWithGatewayInMiddle">
+		<int-jms:outbound-gateway id="gateway"
+	                              request-destination="requestQueueA"
+	                              reply-destination="replyQueueB"
+	                              connection-factory="connectionFactory"
+	                              receive-timeout="100000"
+	                              reply-timeout="200000"/>
+		<int:transformer expression="payload"/>
+	</int:chain>
+
+<!-- INT-3884
+	<int:chain id="invalidChain" input-channel="chainWithInvalidGatewayReplyChannel">
+		<int-jms:outbound-gateway id="badChainGateway"
+		                          request-destination="requestQueueA"
+	                              reply-destination="replyQueueB"
+	                              connection-factory="connectionFactory"
+	                              receive-timeout="100000"
+	                              reply-channel="badOutputChannel"
+	                              reply-timeout="200000"/>
+	</int:chain>
+
+	<int:channel id="badOutputChannel"/>
+-->
 	<bean id="requestQueueA" class="org.apache.activemq.command.ActiveMQQueue">
 		<constructor-arg value="request.queueA"/>
 	</bean>
@@ -37,7 +67,7 @@
 	<bean id="replyQueueB" class="org.apache.activemq.command.ActiveMQQueue">
 		<constructor-arg value="reply.queueB"/>
 	</bean>
-	
+
 
 	<bean id="connectionFactory" class="org.springframework.jms.connection.CachingConnectionFactory">
 		<property name="targetConnectionFactory">
@@ -48,11 +78,11 @@
 		<property name="sessionCacheSize" value="10"/>
 		<property name="cacheProducers" value="false"/>
 	</bean>
-	
+
 	<int-jms:inbound-gateway id="inboundGateway" request-channel="jmsInput" request-destination="requestQueueA"/>
-	
+
 	<int:channel id="jmsInput"/>
-	
+
 <!--	<int:service-activator input-channel="jmsInput">-->
 <!--		<bean class="org.springframework.integration.jms.config.JmsOutboundGatewayParserTests$SampleService"/>-->
 <!--	</int:service-activator>-->
diff --git a/src/reference/asciidoc/whats-new.adoc b/src/reference/asciidoc/whats-new.adoc
index 95b3329a9c..29f9625004 100644
--- a/src/reference/asciidoc/whats-new.adoc
+++ b/src/reference/asciidoc/whats-new.adoc
@@ -13,3 +13,10 @@ development process.
 [[x4.3-general]]
 === General Changes
 
+==== Outbound Gateway within Chain
+
+Previously, it was possible to specify a `reply-channel` on an outbound gateway within a chain.
+It was completely ignored; the gateway's reply goes to the next chain element, or to the chain's output channel
+if the gateway is the last element.
+This condition is now detected and disallowed.
+If you have such configuration, simply remove the `reply-channel`.