From 7317847aa72dedab4dc1a9fb8687b1e04960e202 Mon Sep 17 00:00:00 2001
From: rob bygrave <robin.bygrave@gmail.com>
Date: Fri, 24 Jul 2020 13:16:18 +1200
Subject: [PATCH] #2037 - Allow PostCommit transaction callback to recursively
 add another PostCommit callback

---
 .../server/transaction/JdbcTransaction.java   | 65 ++++++++++---------
 .../transaction/JdbcTransactionTest.java      | 36 ++++++++++
 2 files changed, 69 insertions(+), 32 deletions(-)

diff --git a/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java b/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java
index f07aad279..82dc5ee3b 100644
--- a/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java
+++ b/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java
@@ -347,55 +347,56 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes {
     callbackList.add(callback);
   }
 
-  private void firePreRollback() {
+  private void withEachCallback(Consumer<TransactionCallback> consumer) {
     if (callbackList != null) {
-      for (TransactionCallback callback : callbackList) {
-        try {
-          callback.preRollback();
-        } catch (Exception e) {
-          logger.error("Error executing preRollback callback", e);
-        }
+      // using old style loop to cater for case when new callbacks are added recursively (as otherwise iterator fails fast)
+      for (int i = 0; i < callbackList.size(); i++) {
+        consumer.accept(callbackList.get(i));
       }
     }
   }
 
-  private void firePostRollback() {
-    if (callbackList != null) {
-      for (TransactionCallback callback : callbackList) {
-        try {
-          callback.postRollback();
-        } catch (Exception e) {
-          logger.error("Error executing postRollback callback", e);
-        }
+  private void firePreRollback() {
+    withEachCallback(callback -> {
+      try {
+        callback.preRollback();
+      } catch (Exception e) {
+        logger.error("Error executing preRollback callback", e);
       }
-    }
+    });
+  }
+
+  private void firePostRollback() {
+    withEachCallback(callback -> {
+      try {
+        callback.postRollback();
+      } catch (Exception e) {
+        logger.error("Error executing postRollback callback", e);
+      }
+    });
     if (changeLogHolder != null) {
       changeLogHolder.postRollback();
     }
   }
 
   private void firePreCommit() {
-    if (callbackList != null) {
-      for (TransactionCallback callback : callbackList) {
-        try {
-          callback.preCommit();
-        } catch (Exception e) {
-          logger.error("Error executing preCommit callback", e);
-        }
+    withEachCallback(callback -> {
+      try {
+        callback.preCommit();
+      } catch (Exception e) {
+        logger.error("Error executing preCommit callback", e);
       }
-    }
+    });
   }
 
   private void firePostCommit() {
-    if (callbackList != null) {
-      for (TransactionCallback callback : callbackList) {
-        try {
-          callback.postCommit();
-        } catch (Exception e) {
-          logger.error("Error executing postCommit callback", e);
-        }
+    withEachCallback(callback -> {
+      try {
+        callback.postCommit();
+      } catch (Exception e) {
+        logger.error("Error executing postCommit callback", e);
       }
-    }
+    });
     if (changeLogHolder != null) {
       changeLogHolder.postCommit();
     }
diff --git a/src/test/java/io/ebeaninternal/server/transaction/JdbcTransactionTest.java b/src/test/java/io/ebeaninternal/server/transaction/JdbcTransactionTest.java
index fa0bf7b9b..7416ca58b 100644
--- a/src/test/java/io/ebeaninternal/server/transaction/JdbcTransactionTest.java
+++ b/src/test/java/io/ebeaninternal/server/transaction/JdbcTransactionTest.java
@@ -69,4 +69,40 @@ public class JdbcTransactionTest {
     }
   }
 
+  @Test
+  public void postNestedCallback() {
+    // test both pre-commit and post-commit to ensure callbacks don't disappear and new ones can be added
+    AtomicInteger preCommitCallCount = new AtomicInteger();
+    AtomicInteger postCommitCallCount = new AtomicInteger();
+
+    try (Transaction transaction = DB.beginTransaction()) {
+      DB.currentTransaction().register(
+        new TransactionCallbackAdapter() {
+          @Override
+          public void postCommit() {
+            postCommitCallCount.incrementAndGet();
+            DB.currentTransaction().register(
+              new TransactionCallbackAdapter() {
+                @Override
+                public void postCommit() {
+                  postCommitCallCount.incrementAndGet();
+                }
+              }
+            );
+          }
+
+          @Override
+          public void preCommit() {
+            preCommitCallCount.incrementAndGet();
+          }
+        }
+      );
+      EBasic basic = new EBasic("b1");
+      DB.save(basic);
+      transaction.commit();     // transaction will fail if recursive post-commit is failing
+
+      assertThat(preCommitCallCount.get()).isEqualTo(1);      // precommit executed once
+      assertThat(postCommitCallCount.get()).isEqualTo(2);     // postcommit executed twice
+    }
+  }
 }