From 897557fd1b14a169e017f262b5336b84da23369e Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Thu, 2 Apr 2020 09:31:00 +0100
Subject: [PATCH] Fix faulty tests

The tests were doing Callable async request processing in the main
thread which was already bound (via HandlerInterceptor#preProcess).
This leads to an ISE from TransactionSynchronizationManager#bindResource
but it went unnoticed because there is no Servlet container and no
async dispatch to continue processing.

See gh-24835
---
 .../support/OpenEntityManagerInViewTests.java | 43 ++++++++++++++++---
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java b/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java
index cebeb2fe18..cba2b2dd38 100644
--- a/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java
+++ b/spring-orm/src/test/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewTests.java
@@ -17,6 +17,8 @@
 package org.springframework.orm.jpa.support;
 
 import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.persistence.EntityManager;
@@ -71,6 +73,8 @@ public class OpenEntityManagerInViewTests {
 
 	private ServletWebRequest webRequest;
 
+	private final TestTaskExecutor taskExecutor = new TestTaskExecutor();
+
 
 	@BeforeEach
 	public void setUp() {
@@ -144,10 +148,13 @@ public class OpenEntityManagerInViewTests {
 
 		AsyncWebRequest asyncWebRequest = new StandardServletAsyncWebRequest(this.request, this.response);
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(this.webRequest);
-		asyncManager.setTaskExecutor(new SyncTaskExecutor());
+		asyncManager.setTaskExecutor(this.taskExecutor);
 		asyncManager.setAsyncWebRequest(asyncWebRequest);
 		asyncManager.startCallableProcessing((Callable<String>) () -> "anything");
 
+		this.taskExecutor.await();
+		assertThat(asyncManager.getConcurrentResult()).as("Concurrent result ").isEqualTo("anything");
+
 		interceptor.afterConcurrentHandlingStarted(this.webRequest);
 		assertThat(TransactionSynchronizationManager.hasResource(factory)).isFalse();
 
@@ -198,10 +205,13 @@ public class OpenEntityManagerInViewTests {
 
 		AsyncWebRequest asyncWebRequest = new StandardServletAsyncWebRequest(this.request, this.response);
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(this.request);
-		asyncManager.setTaskExecutor(new SyncTaskExecutor());
+		asyncManager.setTaskExecutor(this.taskExecutor);
 		asyncManager.setAsyncWebRequest(asyncWebRequest);
 		asyncManager.startCallableProcessing((Callable<String>) () -> "anything");
 
+		this.taskExecutor.await();
+		assertThat(asyncManager.getConcurrentResult()).as("Concurrent result ").isEqualTo("anything");
+
 		interceptor.afterConcurrentHandlingStarted(this.webRequest);
 		assertThat(TransactionSynchronizationManager.hasResource(this.factory)).isFalse();
 
@@ -235,10 +245,13 @@ public class OpenEntityManagerInViewTests {
 
 		AsyncWebRequest asyncWebRequest = new StandardServletAsyncWebRequest(this.request, this.response);
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(this.request);
-		asyncManager.setTaskExecutor(new SyncTaskExecutor());
+		asyncManager.setTaskExecutor(this.taskExecutor);
 		asyncManager.setAsyncWebRequest(asyncWebRequest);
 		asyncManager.startCallableProcessing((Callable<String>) () -> "anything");
 
+		this.taskExecutor.await();
+		assertThat(asyncManager.getConcurrentResult()).as("Concurrent result ").isEqualTo("anything");
+
 		interceptor.afterConcurrentHandlingStarted(this.webRequest);
 		assertThat(TransactionSynchronizationManager.hasResource(this.factory)).isFalse();
 
@@ -360,10 +373,13 @@ public class OpenEntityManagerInViewTests {
 		given(asyncWebRequest.isAsyncStarted()).willReturn(true);
 
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(this.request);
-		asyncManager.setTaskExecutor(new SyncTaskExecutor());
+		asyncManager.setTaskExecutor(this.taskExecutor);
 		asyncManager.setAsyncWebRequest(asyncWebRequest);
 		asyncManager.startCallableProcessing((Callable<String>) () -> "anything");
 
+		this.taskExecutor.await();
+		assertThat(asyncManager.getConcurrentResult()).as("Concurrent result ").isEqualTo("anything");
+
 		assertThat(TransactionSynchronizationManager.hasResource(factory)).isFalse();
 		assertThat(TransactionSynchronizationManager.hasResource(factory2)).isFalse();
 		filter2.doFilter(this.request, this.response, filterChain3);
@@ -398,11 +414,26 @@ public class OpenEntityManagerInViewTests {
 
 
 	@SuppressWarnings("serial")
-	private static class SyncTaskExecutor extends SimpleAsyncTaskExecutor {
+	private static class TestTaskExecutor extends SimpleAsyncTaskExecutor {
+
+		private final CountDownLatch latch = new CountDownLatch(1);
 
 		@Override
 		public void execute(Runnable task, long startTimeout) {
-			task.run();
+			Runnable decoratedTask = () -> {
+				try {
+					task.run();
+				}
+				finally {
+					latch.countDown();
+				}
+			};
+			super.execute(decoratedTask, startTimeout);
+		}
+
+		void await() throws InterruptedException {
+			this.latch.await(5, TimeUnit.SECONDS);
 		}
 	}
+
 }