fix:事务后置回调中开启事务死循环 (#621)

ZPZP1 · zhangpeng · web-flow · commit c1f12172e271 · 2024-01-17T10:23:25.000+08:00
Co-authored-by: zhangpeng &lt;xinniankuailezp@163.com&gt;
diff --git a/dynamic-datasource-spring-boot-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v1/DsTransactionalTest.java b/dynamic-datasource-spring-boot-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v1/DsTransactionalTest.java
@@ -61,29 +61,13 @@ public void testDsTransactional() {
         PlaceOrderRequest placeOrderRequest = new PlaceOrderRequest(1, 1, 22, OrderStatus.INIT);
 
         //商品不足
-        TransactionContext.registerSynchronization(new TransactionSynchronization() {
-            @Override
-            public void afterCompletion(int status) {
-                if (status == STATUS_ROLLED_BACK) {
-                    placeOrderRequest.setOrderStatus(OrderStatus.FAIL);
-                }
-            }
-        });
         assertThrows(RuntimeException.class, () -> orderService.placeOrder(placeOrderRequest));
         assertThat(placeOrderRequest.getOrderStatus()).isEqualTo(OrderStatus.FAIL);
         assertThat(orderService.selectOrders()).isEmpty();
         assertThat(accountService.selectAccount()).isEqualTo(new Account(1, 50.0));
         assertThat(productService.selectProduct()).isEqualTo(new Product(1, 10.0, 20));
 
         //账户不足
-        TransactionContext.registerSynchronization(new TransactionSynchronization() {
-            @Override
-            public void afterCompletion(int status) {
-                if (status == STATUS_ROLLED_BACK) {
-                    placeOrderRequest.setOrderStatus(OrderStatus.FAIL);
-                }
-            }
-        });
         placeOrderRequest.setAmount(6);
         placeOrderRequest.setOrderStatus(OrderStatus.INIT);
         assertThrows(RuntimeException.class, () -> orderService.placeOrder(placeOrderRequest));
@@ -93,12 +77,6 @@ public void afterCompletion(int status) {
         assertThat(productService.selectProduct()).isEqualTo(new Product(1, 10.0, 20));
 
         //正常下单
-        TransactionContext.registerSynchronization(new TransactionSynchronization() {
-            @Override
-            public void afterCommit() {
-                placeOrderRequest.setOrderStatus(OrderStatus.SUCCESS);
-            }
-        });
         placeOrderRequest.setAmount(5);
         placeOrderRequest.setOrderStatus(OrderStatus.INIT);
         assertThat(orderService.placeOrder(placeOrderRequest)).isEqualTo(OrderStatus.INIT);
diff --git a/dynamic-datasource-spring-boot-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v1/service/tx/OrderService.java b/dynamic-datasource-spring-boot-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v1/service/tx/OrderService.java
@@ -17,7 +17,9 @@
 
 import com.baomidou.dynamic.datasource.annotation.DS;
 import com.baomidou.dynamic.datasource.annotation.DSTransactional;
+import com.baomidou.dynamic.datasource.tx.TransactionContext;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.support.TransactionSynchronization;
 
 import javax.sql.DataSource;
 import java.sql.*;
@@ -40,6 +42,29 @@ public OrderService(AccountService accountService, ProductService productService
 
     @DSTransactional
     public int placeOrder(PlaceOrderRequest request) {
+        TransactionContext.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCompletion(int status) {
+                if (status == STATUS_ROLLED_BACK) {
+                    request.setOrderStatus(OrderStatus.FAIL);
+                }
+            }
+        });
+        TransactionContext.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCompletion(int status) {
+                if (status == STATUS_ROLLED_BACK) {
+                    request.setOrderStatus(OrderStatus.FAIL);
+                }
+            }
+        });
+        TransactionContext.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCommit() {
+                request.setOrderStatus(OrderStatus.SUCCESS);
+            }
+        });
+
         try (Connection connection = dataSource.getConnection()) {
             Integer userId = request.getUserId();
             Integer productId = request.getProductId();
diff --git a/dynamic-datasource-spring-boot3-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v3/DsTransactionalTest.java b/dynamic-datasource-spring-boot3-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v3/DsTransactionalTest.java
@@ -61,29 +61,13 @@ public void testDsTransactional() {
         PlaceOrderRequest placeOrderRequest = new PlaceOrderRequest(1, 1, 22, OrderStatus.INIT);
 
         //商品不足
-        TransactionContext.registerSynchronization(new TransactionSynchronization() {
-            @Override
-            public void afterCompletion(int status) {
-                if (status == STATUS_ROLLED_BACK) {
-                    placeOrderRequest.setOrderStatus(OrderStatus.FAIL);
-                }
-            }
-        });
         assertThrows(RuntimeException.class, () -> orderService.placeOrder(placeOrderRequest));
         assertThat(placeOrderRequest.getOrderStatus()).isEqualTo(OrderStatus.FAIL);
         assertThat(orderService.selectOrders()).isEmpty();
         assertThat(accountService.selectAccount()).isEqualTo(new Account(1, 50.0));
         assertThat(productService.selectProduct()).isEqualTo(new Product(1, 10.0, 20));
 
         //账户不足
-        TransactionContext.registerSynchronization(new TransactionSynchronization() {
-            @Override
-            public void afterCompletion(int status) {
-                if (status == STATUS_ROLLED_BACK) {
-                    placeOrderRequest.setOrderStatus(OrderStatus.FAIL);
-                }
-            }
-        });
         placeOrderRequest.setAmount(6);
         placeOrderRequest.setOrderStatus(OrderStatus.INIT);
         assertThrows(RuntimeException.class, () -> orderService.placeOrder(placeOrderRequest));
@@ -93,12 +77,6 @@ public void afterCompletion(int status) {
         assertThat(productService.selectProduct()).isEqualTo(new Product(1, 10.0, 20));
 
         //正常下单
-        TransactionContext.registerSynchronization(new TransactionSynchronization() {
-            @Override
-            public void afterCommit() {
-                placeOrderRequest.setOrderStatus(OrderStatus.SUCCESS);
-            }
-        });
         placeOrderRequest.setAmount(5);
         placeOrderRequest.setOrderStatus(OrderStatus.INIT);
         assertThat(orderService.placeOrder(placeOrderRequest)).isEqualTo(OrderStatus.INIT);
diff --git a/dynamic-datasource-spring-boot3-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v3/service/tx/OrderService.java b/dynamic-datasource-spring-boot3-starter/src/test/java/com/baomidou/dynamic/datasource/fixture/v3/service/tx/OrderService.java
@@ -17,7 +17,9 @@
 
 import com.baomidou.dynamic.datasource.annotation.DS;
 import com.baomidou.dynamic.datasource.annotation.DSTransactional;
+import com.baomidou.dynamic.datasource.tx.TransactionContext;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.support.TransactionSynchronization;
 
 import javax.sql.DataSource;
 import java.sql.*;
@@ -40,6 +42,29 @@ public OrderService(AccountService accountService, ProductService productService
 
     @DSTransactional
     public int placeOrder(PlaceOrderRequest request) {
+        TransactionContext.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCompletion(int status) {
+                if (status == STATUS_ROLLED_BACK) {
+                    request.setOrderStatus(OrderStatus.FAIL);
+                }
+            }
+        });
+        TransactionContext.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCompletion(int status) {
+                if (status == STATUS_ROLLED_BACK) {
+                    request.setOrderStatus(OrderStatus.FAIL);
+                }
+            }
+        });
+        TransactionContext.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCommit() {
+                request.setOrderStatus(OrderStatus.SUCCESS);
+            }
+        });
+
         try (Connection connection = dataSource.getConnection()) {
             Integer userId = request.getUserId();
             Integer productId = request.getProductId();
diff --git a/dynamic-datasource-spring/src/main/java/com/baomidou/dynamic/datasource/tx/TransactionContext.java b/dynamic-datasource-spring/src/main/java/com/baomidou/dynamic/datasource/tx/TransactionContext.java
@@ -81,6 +81,9 @@ public static void registerSynchronization(TransactionSynchronization synchroniz
         if (Objects.isNull(synchronization)) {
             throw new IllegalArgumentException("TransactionSynchronization must not be null");
         }
+        if (DsStrUtils.isEmpty(TransactionContext.getXID())) {
+            throw new IllegalStateException("Transaction is not active");
+        }
         Set<TransactionSynchronization> synchs = SYNCHRONIZATION_HOLDER.get();
         synchs.add(synchronization);
     }
diff --git a/dynamic-datasource-spring/src/main/java/com/baomidou/dynamic/datasource/tx/TransactionalTemplate.java b/dynamic-datasource-spring/src/main/java/com/baomidou/dynamic/datasource/tx/TransactionalTemplate.java
@@ -113,21 +113,22 @@ private Object doExecute(TransactionalExecutor transactionalExecutor) throws Thr
         boolean state = true;
         Object o;
         String xid = LocalTxUtil.startTransaction();
+        boolean shouldInvokeAction = TransactionContext.getSynchronizations().isEmpty();
         try {
             o = transactionalExecutor.execute();
         } catch (Exception e) {
             state = !isRollback(e, transactionInfo);
             throw e;
         } finally {
-            invokeBeforeCompletion();
+            invokeBeforeCompletion(shouldInvokeAction);
             if (state) {
-                invokeBeforeCommit();
+                invokeBeforeCommit(shouldInvokeAction);
                 LocalTxUtil.commit(xid);
-                invokeAfterCommit();
-                invokeAfterCompletion(TransactionSynchronization.STATUS_COMMITTED);
+                invokeAfterCommit(shouldInvokeAction);
+                invokeAfterCompletion(TransactionSynchronization.STATUS_COMMITTED, shouldInvokeAction);
             } else {
                 LocalTxUtil.rollback(xid);
-                invokeAfterCompletion(TransactionSynchronization.STATUS_ROLLED_BACK);
+                invokeAfterCompletion(TransactionSynchronization.STATUS_ROLLED_BACK, shouldInvokeAction);
             }
         }
         return o;
@@ -241,8 +242,8 @@ public boolean isNotEmpty(Object[] array) {
     /**
      * Invoke before commit.
      */
-    public void invokeBeforeCommit() {
-        if (shouldInvokeAction()) {
+    public void invokeBeforeCommit(boolean shouldInvokeAction) {
+        if (shouldInvokeAction) {
             for (TransactionSynchronization synchronization : TransactionContext.getSynchronizations()) {
                 synchronization.beforeCommit(false);
             }
@@ -252,8 +253,8 @@ public void invokeBeforeCommit() {
     /**
      * Invoke before completion .
      */
-    public void invokeBeforeCompletion() {
-        if (shouldInvokeAction()) {
+    public void invokeBeforeCompletion(boolean shouldInvokeAction) {
+        if (shouldInvokeAction) {
             for (TransactionSynchronization synchronization : TransactionContext.getSynchronizations()) {
                 synchronization.beforeCompletion();
             }
@@ -263,8 +264,8 @@ public void invokeBeforeCompletion() {
     /**
      * Invoke after commit.
      */
-    public void invokeAfterCommit() {
-        if (shouldInvokeAction()) {
+    public void invokeAfterCommit(boolean shouldInvokeAction) {
+        if (shouldInvokeAction) {
             for (TransactionSynchronization synchronization : TransactionContext.getSynchronizations()) {
                 synchronization.afterCommit();
             }
@@ -274,22 +275,12 @@ public void invokeAfterCommit() {
     /**
      * Invoke after completion.
      */
-    public void invokeAfterCompletion(int status) {
-        if (shouldInvokeAction()) {
+    public void invokeAfterCompletion(int status, boolean shouldInvokeAction) {
+        if (shouldInvokeAction) {
             for (TransactionSynchronization synchronization : TransactionContext.getSynchronizations()) {
                 synchronization.afterCompletion(status);
             }
+            TransactionContext.removeSynchronizations();
         }
-        TransactionContext.removeSynchronizations();
-    }
-
-    /**
-     * Should invoke action boolean.
-     *
-     * @return the boolean
-     */
-    public boolean shouldInvokeAction() {
-        //If there is a savepoint, the action should not be executed
-        return !ConnectionFactory.hasSavepoint(TransactionContext.getXID());
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -81,6 +81,9 @@ public static void registerSynchronization(TransactionSynchronization synchroniz`
`81`	`81`	`if (Objects.isNull(synchronization)) {`
`82`	`82`	`throw new IllegalArgumentException("TransactionSynchronization must not be null");`
`83`	`83`	`}`
	`84`	`+ if (DsStrUtils.isEmpty(TransactionContext.getXID())) {`
	`85`	`+ throw new IllegalStateException("Transaction is not active");`
	`86`	`+ }`
`84`	`87`	`Set<TransactionSynchronization> synchs = SYNCHRONIZATION_HOLDER.get();`
`85`	`88`	`synchs.add(synchronization);`
`86`	`89`	`}`