GH-1226: Fix Test Harness

Resolves #1226

#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**

Author: garyrussell

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

@@ -34,6 +34,9 @@
 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;
@@ -63,6 +66,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) {
@@ -80,6 +85,7 @@ protected void processListener(MethodRabbitListenerEndpoint endpoint, RabbitList
 		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);
@@ -105,6 +111,31 @@ protected void processListener(MethodRabbitListenerEndpoint endpoint, RabbitList
 		super.processListener(endpoint, rabbitListener, proxy, target, beanName);
 	}
 
+	/**
+	 * 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) {
@@ -118,6 +149,18 @@ public <T> T getSpy(String id) {
 		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,28 +21,56 @@
 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;
 
 	private final ValueToReturn<T> callback;
 
 	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 T answer(InvocationOnMock invocation) throws Throwable {
 		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 T answer(InvocationOnMock invocation) throws Throwable {
 	 * @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.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 @@
  * @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 Void answer(InvocationOnMock invocation) throws Throwable {
 		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 CountDownLatch getLatch() {
 	 */
 	@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.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 void testProxiedListenerSpy() throws Exception {
 		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 static class Config {
 
 		@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 SimpleRabbitListenerContainerFactory rabbitListenerContainerFactory(Conne
 			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 void testOneWay() throws Exception {
 		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.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 void testOneWay() throws Exception {
 		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();