GH-1226: Fix Test Harness

Resolves https://github.com/spring-projects/spring-amqp/issues/1226

https://github.com/spring-projects/spring-amqp/issues/1157 changed the way we spy
listeners - to support CGLIB proxies (e.g. `@Transactional`).

Instead of spying the listener, it mocks the listener and sets a default answer
to call the real method on the delegate.

This broke when users used other answers, such as those provided by the framework.

Change the provided answers to subclass `ForwardsInvocation`.

Also fix `ConcurrentModificationException` in `getExceptions()`.

**cherry-pick to 2.2.x, 2.1.x**

spring-rabbit-test/src/main/java/org/springframework/amqp/rabbit/test/RabbitListenerTestHarness.java

@@ -36,6 +36,9 @@ import org.springframework.amqp.rabbit.annotation.RabbitHandler;
 import org.springframework.amqp.rabbit.annotation.RabbitListener;
 import org.springframework.amqp.rabbit.annotation.RabbitListenerAnnotationBeanPostProcessor;
 import org.springframework.amqp.rabbit.listener.MethodRabbitListenerEndpoint;
+import org.springframework.amqp.rabbit.test.mockito.LambdaAnswer;
+import org.springframework.amqp.rabbit.test.mockito.LambdaAnswer.ValueToReturn;
+import org.springframework.amqp.rabbit.test.mockito.LatchCountDownAndCallRealMethodAnswer;
 import org.springframework.aop.framework.ProxyFactoryBean;
 import org.springframework.core.annotation.AnnotationAttributes;
 import org.springframework.core.annotation.AnnotationUtils;
@@ -65,6 +68,8 @@ public class RabbitListenerTestHarness extends RabbitListenerAnnotationBeanPostP
 
 	private final Map<String, Object> listeners = new HashMap<>();
 
+	private final Map<String, Object> delegates = new HashMap<>();
+
 	private final AnnotationAttributes attributes;
 
 	public RabbitListenerTestHarness(AnnotationMetadata importMetadata) {
@@ -82,6 +87,7 @@ public class RabbitListenerTestHarness extends RabbitListenerAnnotationBeanPostP
 		String id = rabbitListener.id();
 		if (StringUtils.hasText(id)) {
 			if (this.attributes.getBoolean("spy")) {
+				this.delegates.put(id, proxy);
 				proxy = Mockito.mock(AopTestUtils.getUltimateTargetObject(proxy).getClass(),
 						AdditionalAnswers.delegatesTo(proxy));
 				this.listeners.put(id, proxy);
@@ -107,6 +113,31 @@ public class RabbitListenerTestHarness extends RabbitListenerAnnotationBeanPostP
 		return super.processListener(endpoint, rabbitListener, proxy, target, beanName); // NOSONAR proxy is not null
 	}
 
+	/**
+	 * Return a {@link LatchCountDownAndCallRealMethodAnswer} that is properly configured
+	 * to invoke the listener.
+	 * @param id the listener id.
+	 * @param count the count.
+	 * @return the answer.
+	 * @since 2.1.16
+	 */
+	public LatchCountDownAndCallRealMethodAnswer getLatchAnswerFor(String id, int count) {
+		return new LatchCountDownAndCallRealMethodAnswer(count, this.delegates.get(id));
+	}
+
+	/**
+	 * Return a {@link LambdaAnswer} that is properly configured to invoke the listener.
+	 * @param <T> the return type.
+	 * @param id the listener id.
+	 * @param callRealMethod true to call the real method.
+	 * @param callback the callback.
+	 * @return the answer.
+	 * @since 2.1.16
+	 */
+	public <T> LambdaAnswer<T> getLambdaAnswerFor(String id, boolean callRealMethod, ValueToReturn<T> callback) {
+		return new LambdaAnswer<>(callRealMethod, callback, this.delegates.get(id));
+	}
+
 	public InvocationData getNextInvocationDataFor(String id, long wait, TimeUnit unit) throws InterruptedException {
 		CaptureAdvice advice = this.listenerCapture.get(id);
 		if (advice != null) {
@@ -120,6 +151,18 @@ public class RabbitListenerTestHarness extends RabbitListenerAnnotationBeanPostP
 		return (T) this.listeners.get(id);
 	}
 
+	/**
+	 * Get the actual listener object (not the spy).
+	 * @param <T> the type.
+	 * @param id the id.
+	 * @return the listener.
+	 * @since 2.1.16
+	 */
+	@SuppressWarnings("unchecked")
+	public <T> T getDelegate(String id) {
+		return (T) this.delegates.get(id);
+	}
+
 	private static final class CaptureAdvice implements MethodInterceptor {
 
 		private final BlockingQueue<InvocationData> invocationData = new LinkedBlockingQueue<>();

spring-rabbit-test/src/main/java/org/springframework/amqp/rabbit/test/mockito/LambdaAnswer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2019 the original author or authors.
+ * Copyright 2016-2020 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.
@@ -21,18 +21,24 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
+import org.mockito.internal.stubbing.defaultanswers.ForwardsInvocations;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
+import org.springframework.lang.Nullable;
+
 /**
  * An {@link Answer} to optionally call the real method and allow returning a
  * custom result. Captures any exceptions thrown.
  *
+ * @param <T> the return type.
+ *
  * @author Gary Russell
  * @since 1.6
  *
  */
-public class LambdaAnswer<T> implements Answer<T> {
+@SuppressWarnings("serial")
+public class LambdaAnswer<T> extends ForwardsInvocations {
 
 	private final boolean callRealMethod;
 
@@ -40,9 +46,31 @@ public class LambdaAnswer<T> implements Answer<T> {
 
 	private final Set<Exception> exceptions = Collections.synchronizedSet(new LinkedHashSet<>());
 
+	private final boolean hasDelegate;
+
+	/**
+	 * Deprecated.
+	 * @param callRealMethod true to call the real method.
+	 * @param callback the callback.
+	 * @deprecated in favor of {@link #LambdaAnswer(boolean, ValueToReturn, Object)}.
+	 */
+	@Deprecated
 	public LambdaAnswer(boolean callRealMethod, ValueToReturn<T> callback) {
+		this(callRealMethod, callback, null);
+	}
+
+	/**
+	 * Construct an instance with the provided properties. Use the test harness to get an
+	 * instance with the proper delegate.
+	 * @param callRealMethod true to call the real method.
+	 * @param callback the call back to receive the result.
+	 * @param delegate the delegate.
+	 */
+	public LambdaAnswer(boolean callRealMethod, ValueToReturn<T> callback, @Nullable Object delegate) {
+		super(delegate);
 		this.callRealMethod = callRealMethod;
 		this.callback = callback;
+		this.hasDelegate = delegate != null;
 	}
 
 	@SuppressWarnings("unchecked")
@@ -51,7 +79,12 @@ public class LambdaAnswer<T> implements Answer<T> {
 		T result = null;
 		try {
 			if (this.callRealMethod) {
-				result = (T) invocation.callRealMethod();
+				if (this.hasDelegate) {
+					result = (T) super.answer(invocation);
+				}
+				else {
+					result = (T) invocation.callRealMethod();
+				}
 			}
 			return this.callback.apply(invocation, result);
 		}
@@ -67,7 +100,9 @@ public class LambdaAnswer<T> implements Answer<T> {
 	 * @since 2.2.3
 	 */
 	public Collection<Exception> getExceptions() {
-		return Collections.unmodifiableCollection(this.exceptions);
+		synchronized (this.exceptions) {
+			return new LinkedHashSet<>(this.exceptions);
+		}
 	}
 
 	@FunctionalInterface

spring-rabbit-test/src/main/java/org/springframework/amqp/rabbit/test/mockito/LatchCountDownAndCallRealMethodAnswer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2019 the original author or authors.
+ * Copyright 2016-2020 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.
@@ -21,7 +21,9 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
+import org.mockito.internal.stubbing.defaultanswers.ForwardsInvocations;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
@@ -35,23 +37,48 @@ import org.springframework.lang.Nullable;
  * @since 1.6
  *
  */
-public class LatchCountDownAndCallRealMethodAnswer implements Answer<Void> {
+@SuppressWarnings("serial")
+public class LatchCountDownAndCallRealMethodAnswer extends ForwardsInvocations {
 
 	private final CountDownLatch latch;
 
 	private final Set<Exception> exceptions = Collections.synchronizedSet(new LinkedHashSet<>());
 
+	private final boolean hasDelegate;
+
 	/**
+	 * Get an instance with no delegate.
+	 * @deprecated in favor of
+	 * {@link #LatchCountDownAndCallRealMethodAnswer(int, Object)}.
 	 * @param count to set in a {@link CountDownLatch}.
 	 */
+	@Deprecated
 	public LatchCountDownAndCallRealMethodAnswer(int count) {
+		this(count, null);
+	}
+
+	/**
+	 * Get an instance with the provided properties. Use the test harness to get an
+	 * instance with the proper delegate.
+	 * @param count the count.
+	 * @param delegate the delegate.
+	 * @since 2.1.16
+	 */
+	public LatchCountDownAndCallRealMethodAnswer(int count, @Nullable Object delegate) {
+		super(delegate);
 		this.latch = new CountDownLatch(count);
+		this.hasDelegate = delegate != null;
 	}
 
 	@Override
-	public Void answer(InvocationOnMock invocation) throws Throwable {
+	public Object answer(InvocationOnMock invocation) throws Throwable {
 		try {
-			invocation.callRealMethod();
+			if (this.hasDelegate) {
+				return super.answer(invocation);
+			}
+			else {
+				invocation.callRealMethod();
+			}
 		}
 		catch (Exception e) {
 			this.exceptions.add(e);
@@ -63,6 +90,16 @@ public class LatchCountDownAndCallRealMethodAnswer implements Answer<Void> {
 		return null;
 	}
 
+	/**
+	 * Wait for the latch to count down.
+	 * @param timeout the timeout in seconds.
+	 * @return the result of awaiting on the latch; true if counted down.
+	 * @throws InterruptedException if the thread is interrupted.
+	 * @since 2.1.16
+	 */
+	public boolean await(int timeout) throws InterruptedException {
+		return this.latch.await(timeout, TimeUnit.SECONDS);
+	}
 
 	public CountDownLatch getLatch() {
 		return latch;
@@ -75,7 +112,9 @@ public class LatchCountDownAndCallRealMethodAnswer implements Answer<Void> {
 	 */
 	@Nullable
 	public Collection<Exception> getExceptions() {
-		return Collections.unmodifiableCollection(this.exceptions);
+		synchronized (this.exceptions) {
+			return new LinkedHashSet<>(this.exceptions);
+		}
 	}
 
 }

spring-rabbit-test/src/test/java/org/springframework/amqp/rabbit/test/RabbitListenerProxyTest.java

@@ -21,8 +21,6 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.verify;
 
-import java.util.concurrent.TimeUnit;
-
 import org.junit.jupiter.api.Test;
 
 import org.springframework.amqp.core.AnonymousQueue;
@@ -68,13 +66,14 @@ public class RabbitListenerProxyTest {
 		Listener listener = this.harness.getSpy("foo");
 		assertThat(listener).isNotNull();
 
-		LatchCountDownAndCallRealMethodAnswer answer = new LatchCountDownAndCallRealMethodAnswer(1);
+		LatchCountDownAndCallRealMethodAnswer answer = this.harness.getLatchAnswerFor("foo", 1);
 		doAnswer(answer).when(listener).foo(anyString());
 
 		this.rabbitTemplate.convertAndSend(this.queue.getName(), "foo");
 
-		assertThat(answer.getLatch().await(10, TimeUnit.SECONDS)).isTrue();
+		assertThat(answer.await(10)).isTrue();
 		verify(listener).foo("foo");
+		assertThat(answer.getExceptions()).isEmpty();
 	}
 
 	@Configuration
@@ -84,7 +83,7 @@ public class RabbitListenerProxyTest {
 
 		@Bean
 		public Listener listener() {
-			return (Listener) new ProxyFactory(new Listener()).getProxy();
+			return (Listener) new ProxyFactory(new Listener(dependency())).getProxy();
 		}
 
 		@Bean
@@ -113,12 +112,26 @@ public class RabbitListenerProxyTest {
 			containerFactory.setConnectionFactory(cf);
 			return containerFactory;
 		}
+
+		@Bean
+		public String dependency() {
+			return "dependency";
+		}
+
 	}
 
 	public static class Listener {
 
+		private final String dependency;
+
+		public Listener(String dependency) {
+			this.dependency = dependency;
+		}
+
 		@RabbitListener(id = "foo", queues = "#{queue.name}")
 		public void foo(String foo) {
+			this.dependency.substring(0);
 		}
 	}
+
 }

spring-rabbit-test/src/test/java/org/springframework/amqp/rabbit/test/examples/ExampleRabbitListenerSpyAndCaptureTest.java

@@ -88,14 +88,14 @@ public class ExampleRabbitListenerSpyAndCaptureTest {
 		Listener listener = this.harness.getSpy("bar");
 		assertThat(listener).isNotNull();
 
-		LatchCountDownAndCallRealMethodAnswer answer = new LatchCountDownAndCallRealMethodAnswer(2);
+		LatchCountDownAndCallRealMethodAnswer answer = this.harness.getLatchAnswerFor("bar", 3);
 		doAnswer(answer).when(listener).foo(anyString(), anyString());
 
 		this.rabbitTemplate.convertAndSend(this.queue2.getName(), "bar");
 		this.rabbitTemplate.convertAndSend(this.queue2.getName(), "baz");
 		this.rabbitTemplate.convertAndSend(this.queue2.getName(), "ex");
 
-		assertThat(answer.getLatch().await(10, TimeUnit.SECONDS)).isTrue();
+		assertThat(answer.await(10)).isTrue();
 		verify(listener).foo("bar", this.queue2.getName());
 		verify(listener).foo("baz", this.queue2.getName());
 

spring-rabbit-test/src/test/java/org/springframework/amqp/rabbit/test/examples/ExampleRabbitListenerSpyTest.java

@@ -21,8 +21,6 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.verify;
 
-import java.util.concurrent.TimeUnit;
-
 import org.junit.jupiter.api.Test;
 
 import org.springframework.amqp.core.AnonymousQueue;
@@ -84,13 +82,13 @@ public class ExampleRabbitListenerSpyTest {
 		Listener listener = this.harness.getSpy("bar");
 		assertThat(listener).isNotNull();
 
-		LatchCountDownAndCallRealMethodAnswer answer = new LatchCountDownAndCallRealMethodAnswer(2);
+		LatchCountDownAndCallRealMethodAnswer answer = this.harness.getLatchAnswerFor("bar", 2);
 		doAnswer(answer).when(listener).foo(anyString(), anyString());
 
 		this.rabbitTemplate.convertAndSend(this.queue2.getName(), "bar");
 		this.rabbitTemplate.convertAndSend(this.queue2.getName(), "baz");
 
-		assertThat(answer.getLatch().await(10, TimeUnit.SECONDS)).isTrue();
+		assertThat(answer.await(10)).isTrue();
 		verify(listener).foo("bar", this.queue2.getName());
 		verify(listener).foo("baz", this.queue2.getName());
 	}

spring-rabbit-test/src/test/java/org/springframework/amqp/rabbit/test/mockito/AnswerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2019 the original author or authors.
+ * Copyright 2016-2020 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.
@@ -35,15 +35,17 @@ public class AnswerTests {
 
 	@Test
 	public void testLambda() {
-		Foo foo = spy(new Foo());
-		willAnswer(new LambdaAnswer<String>(true, (i, r) -> r + r)).given(foo).foo(anyString());
+		Foo delegate = new Foo();
+		Foo foo = spy(delegate);
+		willAnswer(new LambdaAnswer<String>(true, (i, r) -> r + r, delegate)).given(foo).foo(anyString());
 		assertThat(foo.foo("foo")).isEqualTo("FOOFOO");
-		willAnswer(new LambdaAnswer<String>(true, (i, r) -> r + i.getArguments()[0])).given(foo).foo(anyString());
+		willAnswer(new LambdaAnswer<String>(true, (i, r) -> r + i.getArguments()[0], delegate))
+				.given(foo).foo(anyString());
 		assertThat(foo.foo("foo")).isEqualTo("FOOfoo");
 		willAnswer(new LambdaAnswer<String>(false, (i, r) ->
-			"" + i.getArguments()[0] + i.getArguments()[0])).given(foo).foo(anyString());
+			"" + i.getArguments()[0] + i.getArguments()[0], delegate)).given(foo).foo(anyString());
 		assertThat(foo.foo("foo")).isEqualTo("foofoo");
-		LambdaAnswer<String> answer = new LambdaAnswer<>(true, (inv, result) -> result);
+		LambdaAnswer<String> answer = new LambdaAnswer<>(true, (inv, result) -> result, delegate);
 		willAnswer(answer).given(foo).foo("fail");
 		assertThatIllegalArgumentException().isThrownBy(() -> foo.foo("fail"));
 		Collection<Exception> exceptions = answer.getExceptions();

src/reference/asciidoc/testing.adoc

@@ -74,13 +74,13 @@ The following example shows how to use `LatchCountDownAndCallRealMethodAnswer`:
 ====
 [source, java]
 ----
-LatchCountDownAndCallRealMethodAnswer answer = new LatchCountDownAndCallRealMethodAnswer(2);
+LatchCountDownAndCallRealMethodAnswer answer = this.harness.getLatchAnswerFor("myListener", 2);
 doAnswer(answer)
     .when(listener).foo(anyString(), anyString());
 
 ...
 
-assertTrue(answer.getLatch().await(10, TimeUnit.SECONDS));
+assertThat(answer.await(10)).isTrue();
 ----
 ====
 
@@ -126,6 +126,7 @@ assertEquals("thingthing", thing.thing("thing"));
 Starting with version 2.2.3, the answers capture any exceptions thrown by the method under test.
 Use `answer.getExceptions()` to get a reference to them.
 
+When used in conjunction with the <<test-harness>> use `harness.getLambdaAnswerFor("listenerId", true, ...)` to get a properly constructed answer for the listener.
 
 [[test-harness]]
 ==== `@RabbitListenerTest` and `RabbitListenerTestHarness`
@@ -197,13 +198,13 @@ public class MyTests {
         Listener listener = this.harness.getSpy("bar");
         assertNotNull(listener);
 
-        LatchCountDownAndCallRealMethodAnswer answer = new LatchCountDownAndCallRealMethodAnswer(2); <3>
+        LatchCountDownAndCallRealMethodAnswer answer = this.harness.getLatchAnswerFor("bar", 2); <3>
         doAnswer(answer).when(listener).foo(anyString(), anyString()); <4>
 
         this.rabbitTemplate.convertAndSend(this.queue2.getName(), "bar");
         this.rabbitTemplate.convertAndSend(this.queue2.getName(), "baz");
 
-        assertTrue(answer.getLatch().await(10, TimeUnit.SECONDS));
+        assertTrue(answer.await(10));
         verify(listener).foo("bar", this.queue2.getName());
         verify(listener).foo("baz", this.queue2.getName());
     }
@@ -220,6 +221,7 @@ suspended in the `RabbitTemplate` waiting for the reply.
 <3> In this case, we're only using a send operation so we need a latch to wait for the asynchronous call to the listener
 on the container thread.
 We use one of the link:#mockito-answer[Answer<?>] implementations to help with that.
+IMPORTANT: Due to the way the listener is spied, it is important to use `harness.getLatchAnswerFor()` to get a properly configured answer for the spy.
 
 <4> Configure the spy to invoke the `Answer`.
 ====
@@ -311,6 +313,9 @@ to suspend the test thread.
 <5> When the listener throws an exception, it is available in the `throwable` property of the invocation data.
 ====
 
+IMPORTANT: When using custom `Answer<?>` s with the harness, in order to operate properly, such answers should subclass `ForwardsInvocation` and get the actual listener (not the spy) from the harness (`getDelegate("myListener")`) and call `super.answer(invocation)`.
+See the provided <<mockito-answer>> source code for examples.
+
 [[test-template]]
 ==== Using `TestRabbitTemplate`