From f813be87e5b01297e0f80654bb8898fc2dabcdc5 Mon Sep 17 00:00:00 2001
From: Avgustin Marinov <Avgustin.Marinov@bosch.com>
Date: Wed, 11 Dec 2024 16:00:10 +0200
Subject: [PATCH] Refactor AfterTransactionCommitDefaultServiceExecutor (#2143)

fixes transaction in transaction after commit (or at least makes is cleaner)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
---
 ...ansactionCommitDefaultServiceExecutor.java | 71 +++++++++----------
 1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/executor/AfterTransactionCommitDefaultServiceExecutor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/executor/AfterTransactionCommitDefaultServiceExecutor.java
index 986ea1e65..25d28b8fe 100644
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/executor/AfterTransactionCommitDefaultServiceExecutor.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/executor/AfterTransactionCommitDefaultServiceExecutor.java
@@ -21,39 +21,36 @@ import org.springframework.transaction.support.TransactionSynchronizationManager
  * The class is thread safe.
  */
 @Slf4j
-public class AfterTransactionCommitDefaultServiceExecutor implements TransactionSynchronization, AfterTransactionCommitExecutor {
+public class AfterTransactionCommitDefaultServiceExecutor implements AfterTransactionCommitExecutor {
 
-    private static final ThreadLocal<List<Runnable>> THREAD_LOCAL_RUNNABLES = new ThreadLocal<>();
+    private static class TransactionSynchronizationImpl implements TransactionSynchronization {
 
-    @Override
-    // Exception squid:S1217 - Is aspectJ proxy
-    @SuppressWarnings({ "squid:S1217" })
-    public void afterCommit() {
-        final List<Runnable> afterCommitRunnables = THREAD_LOCAL_RUNNABLES.get();
-        if (afterCommitRunnables == null) {
-            log.trace("Transaction successfully committed, runnables is null");
-            return;
-        }
+        private final List<Runnable> afterCommitRunnables = new ArrayList<>();
 
-        // removes the runnables that will process, so they would be able to start new transactions and
-        // inserting new after commit hooks
-        THREAD_LOCAL_RUNNABLES.remove();
-        log.debug("Transaction successfully committed, executing {} runnables", afterCommitRunnables.size());
-        for (final Runnable afterCommitRunnable : afterCommitRunnables) {
-            log.debug("Executing runnable {}", afterCommitRunnable);
-            try {
-                afterCommitRunnable.run();
-            } catch (final RuntimeException e) {
-                log.error("Failed to execute runnable {}", afterCommitRunnable, e);
+        @Override
+        // Exception squid:S1217 - Is aspectJ proxy
+        @SuppressWarnings({ "squid:S1217" })
+        public void afterCommit() {
+            log.debug("Transaction successfully committed, executing {} runnables", afterCommitRunnables.size());
+            for (final Runnable afterCommitRunnable : afterCommitRunnables) {
+                log.debug("Executing runnable {}", afterCommitRunnable);
+                try {
+                    afterCommitRunnable.run();
+                } catch (final RuntimeException e) {
+                    log.error("Failed to execute runnable {}", afterCommitRunnable, e);
+                }
             }
         }
-    }
 
-    @Override
-    @SuppressWarnings({ "squid:S1217" })
-    public void afterCompletion(final int status) {
-        log.debug("Transaction completed after commit with status {}", status == STATUS_COMMITTED ? "COMMITTED" : "ROLLEDBACK");
-        THREAD_LOCAL_RUNNABLES.remove();
+        @Override
+        @SuppressWarnings({ "squid:S1217" })
+        public void afterCompletion(final int status) {
+            log.debug("Transaction completed after commit with status {}", status == STATUS_COMMITTED ? "COMMITTED" : "ROLLEDBACK");
+        }
+
+        private void afterCommit(final Runnable runnable) {
+            afterCommitRunnables.add(runnable);
+        }
     }
 
     @Override
@@ -62,17 +59,15 @@ public class AfterTransactionCommitDefaultServiceExecutor implements Transaction
     public void afterCommit(final Runnable runnable) {
         log.debug("Submitting new runnable {} to run after transaction commit", runnable);
         if (TransactionSynchronizationManager.isSynchronizationActive()) {
-            List<Runnable> localRunnables = THREAD_LOCAL_RUNNABLES.get();
-            if (localRunnables == null) {
-                localRunnables = new ArrayList<>();
-                THREAD_LOCAL_RUNNABLES.set(localRunnables);
-                TransactionSynchronizationManager.registerSynchronization(this);
-            }
-            localRunnables.add(runnable);
-            return;
+            TransactionSynchronizationManager.getSynchronizations().stream().filter(TransactionSynchronizationImpl.class::isInstance)
+                    .map(TransactionSynchronizationImpl.class::cast).findAny().orElseGet(() -> {
+                        final TransactionSynchronizationImpl newTS = new TransactionSynchronizationImpl();
+                        TransactionSynchronizationManager.registerSynchronization(newTS);
+                        return newTS;
+                    }).afterCommit(runnable);
+        } else {
+            log.info("Transaction synchronization is NOT ACTIVE/ INACTIVE. Executing right now runnable {}", runnable);
+            runnable.run();
         }
-        log.info("Transaction synchronization is NOT ACTIVE/ INACTIVE. Executing right now runnable {}", runnable);
-
-        runnable.run();
     }
 }
\ No newline at end of file