From 4f83a953bfe4a14e0dadda01c93b055c5bc1bf61 Mon Sep 17 00:00:00 2001
From: Artem Bilan <abilan@pivotal.io>
Date: Tue, 6 Jun 2017 14:29:59 -0400
Subject: [PATCH] SCSt-913: Fix AMQP Inbound Retry logic

Relates to spring-cloud/spring-cloud-stream#913
and https://github.com/spring-projects/spring-integration/commit/914094e51d1c70567b524bf415d26e4f736a5f0b

Also see https://github.com/spring-projects/spring-integration-kafka/pull/163

The `RetryTemplate` and `errorChannel` are mutually exclusive options:
```
Assert.state(getErrorChannel() == null, "Cannot have an 'errorChannel' property when a 'RetryTemplate' is "
	+ "provided; use an 'ErrorMessageSendingRecoverer' in the 'recoveryCallback' property to "
	+ "send an error message when retries are exhausted");
```
Therefore we don't have any error handling functionality if there is a retry but no `RecoveryCallback`.
So, don't add `RetryContext` to `ThreadLocal<AttributeAccessor>` if we don't have `RecoveryCallback` provided,
because that value is out of use then.

To make code more logical perform `attributesHolder.remove()` only for the non-retryable branch.
With that refactoring the missed `attributesHolder.remove()` has been observed in the `AmqpInboundGateway`
Add `attributesHolder.remove()` to the `RetryListener.close()` for retryable branch

**Cherry-pick to 4.3.x**
---
 .../amqp/inbound/AmqpInboundChannelAdapter.java  | 16 ++++++++++------
 .../amqp/inbound/AmqpInboundGateway.java         | 13 ++++++++++---
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundChannelAdapter.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundChannelAdapter.java
index 56baf86be1..ecd7e1ec5d 100644
--- a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundChannelAdapter.java
+++ b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundChannelAdapter.java
@@ -199,7 +199,12 @@ public class AmqpInboundChannelAdapter extends MessageProducerSupport implements
 		public void onMessage(final Message message, final Channel channel) throws Exception {
 			try {
 				if (AmqpInboundChannelAdapter.this.retryTemplate == null) {
-					processMessage(message, channel);
+					try {
+						processMessage(message, channel);
+					}
+					finally {
+						attributesHolder.remove();
+					}
 				}
 				else {
 					AmqpInboundChannelAdapter.this.retryTemplate.execute(new RetryCallback<Object, RuntimeException>() {
@@ -222,9 +227,6 @@ public class AmqpInboundChannelAdapter extends MessageProducerSupport implements
 					throw e;
 				}
 			}
-			finally {
-				attributesHolder.remove();
-			}
 		}
 
 		private void processMessage(Message message, Channel channel) {
@@ -246,14 +248,16 @@ public class AmqpInboundChannelAdapter extends MessageProducerSupport implements
 
 		@Override
 		public <T, E extends Throwable> boolean open(RetryContext context, RetryCallback<T, E> callback) {
-			attributesHolder.set(context);
+			if (AmqpInboundChannelAdapter.this.recoveryCallback != null) {
+				attributesHolder.set(context);
+			}
 			return true;
 		}
 
 		@Override
 		public <T, E extends Throwable> void close(RetryContext context, RetryCallback<T, E> callback,
 				Throwable throwable) {
-			// Empty
+			attributesHolder.remove();
 		}
 
 		@Override
diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundGateway.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundGateway.java
index be06eb6c6e..e9a0b42c84 100644
--- a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundGateway.java
+++ b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundGateway.java
@@ -255,7 +255,12 @@ public class AmqpInboundGateway extends MessagingGatewaySupport {
 		@Override
 		public void onMessage(final Message message, final Channel channel) throws Exception {
 			if (AmqpInboundGateway.this.retryTemplate == null) {
-				doOnMessage(message, channel);
+				try {
+					doOnMessage(message, channel);
+				}
+				finally {
+					attributesHolder.remove();
+				}
 			}
 			else {
 				AmqpInboundGateway.this.retryTemplate.execute(new RetryCallback<Object, RuntimeException>() {
@@ -354,14 +359,16 @@ public class AmqpInboundGateway extends MessagingGatewaySupport {
 
 		@Override
 		public <T, E extends Throwable> boolean open(RetryContext context, RetryCallback<T, E> callback) {
-			attributesHolder.set(context);
+			if (AmqpInboundGateway.this.recoveryCallback != null) {
+				attributesHolder.set(context);
+			}
 			return true;
 		}
 
 		@Override
 		public <T, E extends Throwable> void close(RetryContext context, RetryCallback<T, E> callback,
 				Throwable throwable) {
-			// Empty
+			attributesHolder.remove();
 		}
 
 		@Override