From 24301a7d6a4634e8bdc445923468c32225f6a673 Mon Sep 17 00:00:00 2001 From: biedongbin Date: Tue, 24 Mar 2026 10:10:13 +0800 Subject: [PATCH 1/2] feature: enhance saga annotation robustness with action status report and TccHook support (#7840) --- .../seata/common/ConfigurationKeys.java | 6 + .../org/apache/seata/common/Constants.java | 15 + .../interceptor/ActionInterceptorHandler.java | 48 ++- .../rm/tcc/api/BusinessActionContext.java | 25 ++ .../rm/tcc/api/BusinessActionContextTest.java | 111 ++++++ .../rm/SagaAnnotationResourceManager.java | 53 ++- .../rm/SagaAnnotationResourceManagerTest.java | 333 ++++++++++++++++++ 7 files changed, 589 insertions(+), 2 deletions(-) create mode 100644 integration-tx-api/src/test/java/org/apache/seata/rm/tcc/api/BusinessActionContextTest.java create mode 100644 saga/seata-saga-annotation/src/test/java/org/apache/seata/saga/rm/SagaAnnotationResourceManagerTest.java diff --git a/common/src/main/java/org/apache/seata/common/ConfigurationKeys.java b/common/src/main/java/org/apache/seata/common/ConfigurationKeys.java index b99c856a0fc..87a6c4189ac 100644 --- a/common/src/main/java/org/apache/seata/common/ConfigurationKeys.java +++ b/common/src/main/java/org/apache/seata/common/ConfigurationKeys.java @@ -224,6 +224,12 @@ public interface ConfigurationKeys { */ String CLIENT_SAGA_COMPENSATE_PERSIST_MODE_UPDATE = CLIENT_RM_PREFIX + "sagaCompensatePersistModeUpdate"; + /** + * The constant CLIENT_SAGA_ACTION_STATUS_REPORT_ENABLE. + * Enable action status report for SAGA/TCC annotation mode to handle empty compensation and suspension issues. + */ + String CLIENT_SAGA_ACTION_STATUS_REPORT_ENABLE = CLIENT_RM_PREFIX + "sagaActionStatusReportEnable"; + /** * The constant CLIENT_REPORT_RETRY_COUNT. */ diff --git a/common/src/main/java/org/apache/seata/common/Constants.java b/common/src/main/java/org/apache/seata/common/Constants.java index c386992d033..77edb852a27 100644 --- a/common/src/main/java/org/apache/seata/common/Constants.java +++ b/common/src/main/java/org/apache/seata/common/Constants.java @@ -250,4 +250,19 @@ public interface Constants { * CW stands for Cluster Watch */ String WATCH_EVENT_PREFIX = "CW:"; + + /** + * Action status key for action context + */ + String ACTION_STATUS = "actionStatus"; + + /** + * Action status: prepare method executed successfully + */ + String ACTION_STATUS_SUCCESS = "success"; + + /** + * Action status: prepare method execution failed + */ + String ACTION_STATUS_FAILED = "failed"; } diff --git a/integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandler.java b/integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandler.java index d47047b71c0..6cbc2580cd2 100644 --- a/integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandler.java +++ b/integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandler.java @@ -16,6 +16,7 @@ */ package org.apache.seata.integration.tx.api.interceptor; +import org.apache.seata.common.ConfigurationKeys; import org.apache.seata.common.Constants; import org.apache.seata.common.exception.FrameworkException; import org.apache.seata.common.exception.SkipCallbackWrapperException; @@ -23,6 +24,7 @@ import org.apache.seata.common.json.JsonUtil; import org.apache.seata.common.util.CollectionUtils; import org.apache.seata.common.util.NetUtil; +import org.apache.seata.config.ConfigurationFactory; import org.apache.seata.core.context.RootContext; import org.apache.seata.integration.tx.api.fence.DefaultCommonFenceHandler; import org.apache.seata.integration.tx.api.fence.hook.TccHook; @@ -53,6 +55,9 @@ public class ActionInterceptorHandler { private static final Logger LOGGER = LoggerFactory.getLogger(ActionInterceptorHandler.class); + private static final boolean ENABLE_ACTION_STATUS_REPORT = ConfigurationFactory.getInstance() + .getBoolean(ConfigurationKeys.CLIENT_SAGA_ACTION_STATUS_REPORT_ENABLE, false); + /** * Handler the Tx Aspect * @@ -111,7 +116,20 @@ public Object proceed( } } else { // Execute business, and return the business result - return targetCallback.execute(); + try { + Object result = targetCallback.execute(); + // Report action status: success (only for non-CommonFence mode) + if (ENABLE_ACTION_STATUS_REPORT) { + reportActionStatus(actionContext, Constants.ACTION_STATUS_SUCCESS); + } + return result; + } catch (Throwable t) { + // Report action status: failed (only for non-CommonFence mode) + if (ENABLE_ACTION_STATUS_REPORT) { + reportActionStatus(actionContext, Constants.ACTION_STATUS_FAILED); + } + throw t; + } } } finally { try { @@ -325,4 +343,32 @@ protected Map fetchActionRequestContext(Method method, Object[] } return context; } + + /** + * Report action status to TC + * + * @param actionContext the action context + * @param status the action status (success/failed) + */ + protected void reportActionStatus(BusinessActionContext actionContext, String status) { + try { + actionContext.setActionStatus(status); + actionContext.setUpdated(true); + BusinessActionContextUtil.reportContext(actionContext); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "Report action status: xid={}, branchId={}, status={}", + actionContext.getXid(), + actionContext.getBranchId(), + status); + } + } catch (Exception e) { + LOGGER.warn( + "Report action status failed: xid={}, branchId={}, status={}, error={}", + actionContext.getXid(), + actionContext.getBranchId(), + status, + e.getMessage()); + } + } } diff --git a/integration-tx-api/src/main/java/org/apache/seata/rm/tcc/api/BusinessActionContext.java b/integration-tx-api/src/main/java/org/apache/seata/rm/tcc/api/BusinessActionContext.java index a34b41e9ce7..b8041d75009 100644 --- a/integration-tx-api/src/main/java/org/apache/seata/rm/tcc/api/BusinessActionContext.java +++ b/integration-tx-api/src/main/java/org/apache/seata/rm/tcc/api/BusinessActionContext.java @@ -16,6 +16,7 @@ */ package org.apache.seata.rm.tcc.api; +import org.apache.seata.common.Constants; import org.apache.seata.core.model.BranchType; import org.apache.seata.integration.tx.api.interceptor.ActionContextUtil; @@ -233,6 +234,30 @@ public void setBranchType(BranchType branchType) { this.branchType = branchType; } + /** + * Gets action status. + * + * @return the action status + */ + public String getActionStatus() { + if (actionContext == null) { + return null; + } + Object status = actionContext.get(Constants.ACTION_STATUS); + return status != null ? status.toString() : null; + } + + /** + * Sets action status. + * + * @param status the action status + */ + public void setActionStatus(String status) { + if (actionContext != null && status != null) { + actionContext.put(Constants.ACTION_STATUS, status); + } + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); diff --git a/integration-tx-api/src/test/java/org/apache/seata/rm/tcc/api/BusinessActionContextTest.java b/integration-tx-api/src/test/java/org/apache/seata/rm/tcc/api/BusinessActionContextTest.java new file mode 100644 index 00000000000..fe9f5ef2337 --- /dev/null +++ b/integration-tx-api/src/test/java/org/apache/seata/rm/tcc/api/BusinessActionContextTest.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.rm.tcc.api; + +import org.apache.seata.common.Constants; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Test cases for BusinessActionContext action status methods. + */ +public class BusinessActionContextTest { + + @Test + public void testGetActionStatusWhenContextIsNull() { + BusinessActionContext context = new BusinessActionContext(); + assertNull(context.getActionStatus(), "Action status should be null when actionContext is null"); + } + + @Test + public void testGetActionStatusWhenNotSet() { + BusinessActionContext context = new BusinessActionContext(); + Map actionContext = new HashMap<>(); + context.setActionContext(actionContext); + + assertNull(context.getActionStatus(), "Action status should be null when not set"); + } + + @Test + public void testSetAndGetActionStatusSuccess() { + BusinessActionContext context = new BusinessActionContext(); + Map actionContext = new HashMap<>(); + context.setActionContext(actionContext); + + context.setActionStatus(Constants.ACTION_STATUS_SUCCESS); + + assertEquals(Constants.ACTION_STATUS_SUCCESS, context.getActionStatus(), "Action status should be 'success'"); + } + + @Test + public void testSetAndGetActionStatusFailed() { + BusinessActionContext context = new BusinessActionContext(); + Map actionContext = new HashMap<>(); + context.setActionContext(actionContext); + + context.setActionStatus(Constants.ACTION_STATUS_FAILED); + + assertEquals(Constants.ACTION_STATUS_FAILED, context.getActionStatus(), "Action status should be 'failed'"); + } + + @Test + public void testSetActionStatusWithNullStatus() { + BusinessActionContext context = new BusinessActionContext(); + Map actionContext = new HashMap<>(); + context.setActionContext(actionContext); + + context.setActionStatus(Constants.ACTION_STATUS_SUCCESS); + context.setActionStatus(null); + + assertEquals( + Constants.ACTION_STATUS_SUCCESS, + context.getActionStatus(), + "Action status should not change when setting null"); + } + + @Test + public void testSetActionStatusWithNullActionContext() { + BusinessActionContext context = new BusinessActionContext(); + + // Should not throw exception + assertDoesNotThrow( + () -> context.setActionStatus(Constants.ACTION_STATUS_SUCCESS), + "Setting action status with null actionContext should not throw exception"); + + assertNull(context.getActionStatus(), "Action status should still be null"); + } + + @Test + public void testActionStatusOverwrite() { + BusinessActionContext context = new BusinessActionContext(); + Map actionContext = new HashMap<>(); + context.setActionContext(actionContext); + + context.setActionStatus(Constants.ACTION_STATUS_SUCCESS); + assertEquals(Constants.ACTION_STATUS_SUCCESS, context.getActionStatus()); + + context.setActionStatus(Constants.ACTION_STATUS_FAILED); + assertEquals( + Constants.ACTION_STATUS_FAILED, + context.getActionStatus(), + "Action status should be overwritten to 'failed'"); + } +} diff --git a/saga/seata-saga-annotation/src/main/java/org/apache/seata/saga/rm/SagaAnnotationResourceManager.java b/saga/seata-saga-annotation/src/main/java/org/apache/seata/saga/rm/SagaAnnotationResourceManager.java index 471b3640cee..4f9cd27f52c 100644 --- a/saga/seata-saga-annotation/src/main/java/org/apache/seata/saga/rm/SagaAnnotationResourceManager.java +++ b/saga/seata-saga-annotation/src/main/java/org/apache/seata/saga/rm/SagaAnnotationResourceManager.java @@ -23,12 +23,15 @@ import org.apache.seata.core.model.BranchStatus; import org.apache.seata.core.model.BranchType; import org.apache.seata.core.model.Resource; +import org.apache.seata.integration.tx.api.fence.hook.TccHook; +import org.apache.seata.integration.tx.api.fence.hook.TccHookManager; import org.apache.seata.integration.tx.api.remoting.TwoPhaseResult; import org.apache.seata.rm.AbstractResourceManager; import org.apache.seata.rm.tcc.api.BusinessActionContext; import org.apache.seata.rm.tcc.api.BusinessActionContextUtil; import java.lang.reflect.Method; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -100,12 +103,15 @@ public BranchStatus branchRollback( String.format("SagaAnnotation resource is not available, resourceId: %s", resourceId)); } + BusinessActionContext businessActionContext = null; try { - BusinessActionContext businessActionContext = + businessActionContext = BusinessActionContextUtil.getBusinessActionContext(xid, branchId, resourceId, applicationData); Object[] args = this.getTwoPhaseRollbackArgs(resource, businessActionContext); BusinessActionContextUtil.setContext(businessActionContext); + doBeforeSagaAnnotationRollback(xid, branchId, resource.getActionName(), businessActionContext); + boolean result; Object ret = compensationMethod.invoke(targetBean, args); if (ret != null) { @@ -131,6 +137,7 @@ public BranchStatus branchRollback( LOGGER.error(msg, ExceptionUtil.unwrap(t)); return BranchStatus.PhaseTwo_RollbackFailed_Retryable; } finally { + doAfterSagaAnnotationRollback(xid, branchId, resource.getActionName(), businessActionContext); BusinessActionContextUtil.clear(); } } @@ -164,4 +171,48 @@ protected Object[] getTwoPhaseMethodParams( } return args; } + + /** + * to do some business operations before saga annotation rollback + * @param xid the xid + * @param branchId the branchId + * @param actionName the actionName + * @param context the business action context + */ + private void doBeforeSagaAnnotationRollback( + String xid, long branchId, String actionName, BusinessActionContext context) { + List hooks = TccHookManager.getHooks(); + if (hooks.isEmpty()) { + return; + } + for (TccHook hook : hooks) { + try { + hook.beforeTccRollback(xid, branchId, actionName, context); + } catch (Exception e) { + LOGGER.error("Failed execute beforeTccRollback in hook {}", e.getMessage(), e); + } + } + } + + /** + * to do some business operations after saga annotation rollback + * @param xid the xid + * @param branchId the branchId + * @param actionName the actionName + * @param context the business action context + */ + private void doAfterSagaAnnotationRollback( + String xid, long branchId, String actionName, BusinessActionContext context) { + List hooks = TccHookManager.getHooks(); + if (hooks.isEmpty()) { + return; + } + for (TccHook hook : hooks) { + try { + hook.afterTccRollback(xid, branchId, actionName, context); + } catch (Exception e) { + LOGGER.error("Failed execute afterTccRollback in hook {}", e.getMessage(), e); + } + } + } } diff --git a/saga/seata-saga-annotation/src/test/java/org/apache/seata/saga/rm/SagaAnnotationResourceManagerTest.java b/saga/seata-saga-annotation/src/test/java/org/apache/seata/saga/rm/SagaAnnotationResourceManagerTest.java new file mode 100644 index 00000000000..148f63ee072 --- /dev/null +++ b/saga/seata-saga-annotation/src/test/java/org/apache/seata/saga/rm/SagaAnnotationResourceManagerTest.java @@ -0,0 +1,333 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.saga.rm; + +import org.apache.seata.core.model.BranchStatus; +import org.apache.seata.core.model.BranchType; +import org.apache.seata.integration.tx.api.fence.hook.TccHook; +import org.apache.seata.integration.tx.api.fence.hook.TccHookManager; +import org.apache.seata.integration.tx.api.remoting.TwoPhaseResult; +import org.apache.seata.rm.tcc.api.BusinessActionContext; +import org.apache.seata.rm.tcc.api.BusinessActionContextUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Unit tests for SagaAnnotationResourceManager. + * + * Focus areas: + * 1. branchRollback with TccHook before/after callbacks + * 2. Hook exception handling (should not break rollback) + * 3. Different compensation return types (boolean, TwoPhaseResult, null) + * 4. Compensation exception handling + */ +public class SagaAnnotationResourceManagerTest { + + static { + System.setProperty("config.type", "file"); + System.setProperty("config.file.name", "file.conf"); + } + + private SagaAnnotationResourceManager resourceManager; + + @BeforeEach + void setUp() { + TccHookManager.clear(); + resourceManager = new SagaAnnotationResourceManager(); + } + + @AfterEach + void tearDown() { + TccHookManager.clear(); + BusinessActionContextUtil.clear(); + } + + // ---- Helper classes ---- + + public static class TestCompensationTarget { + public boolean compensate(BusinessActionContext context) { + return true; + } + + public boolean compensateFail(BusinessActionContext context) { + return false; + } + + public Boolean compensateReturnNull(BusinessActionContext context) { + return null; + } + + public TwoPhaseResult compensateWithResultSuccess(BusinessActionContext context) { + return new TwoPhaseResult(true, "ok"); + } + + public TwoPhaseResult compensateWithResultFail(BusinessActionContext context) { + return new TwoPhaseResult(false, "fail"); + } + + public boolean compensateThrow(BusinessActionContext context) { + throw new RuntimeException("compensation error"); + } + } + + public static class TrackingTccHook implements TccHook { + boolean beforeRollbackCalled = false; + boolean afterRollbackCalled = false; + boolean shouldThrowInBefore = false; + boolean shouldThrowInAfter = false; + + @Override + public void beforeTccPrepare(String xid, Long branchId, String actionName, BusinessActionContext context) {} + + @Override + public void afterTccPrepare(String xid, Long branchId, String actionName, BusinessActionContext context) {} + + @Override + public void beforeTccCommit(String xid, Long branchId, String actionName, BusinessActionContext context) {} + + @Override + public void afterTccCommit(String xid, Long branchId, String actionName, BusinessActionContext context) {} + + @Override + public void beforeTccRollback(String xid, Long branchId, String actionName, BusinessActionContext context) { + beforeRollbackCalled = true; + if (shouldThrowInBefore) { + throw new RuntimeException("hook error in beforeTccRollback"); + } + } + + @Override + public void afterTccRollback(String xid, Long branchId, String actionName, BusinessActionContext context) { + afterRollbackCalled = true; + if (shouldThrowInAfter) { + throw new RuntimeException("hook error in afterTccRollback"); + } + } + } + + private SagaAnnotationResource createResource(String actionName, String methodName) throws NoSuchMethodException { + SagaAnnotationResource resource = new SagaAnnotationResource(); + resource.setActionName(actionName); + resource.setTargetBean(new TestCompensationTarget()); + resource.setCompensationMethod( + TestCompensationTarget.class.getDeclaredMethod(methodName, BusinessActionContext.class)); + resource.setCompensationArgsClasses(new Class[] {BusinessActionContext.class}); + resource.setPhaseTwoCompensationKeys(new String[] {"unused"}); + return resource; + } + + // ---- Tests for hook invocation in branchRollback ---- + + @Test + void testBranchRollbackWithHooksInvoked() throws Exception { + TrackingTccHook hook = new TrackingTccHook(); + TccHookManager.registerHook(hook); + + SagaAnnotationResource resource = createResource("testAction", "compensate"); + resourceManager.getManagedResources().put("testAction", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 1L, "testAction", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + assertTrue(hook.beforeRollbackCalled, "beforeTccRollback should be called"); + assertTrue(hook.afterRollbackCalled, "afterTccRollback should be called"); + } + + @Test + void testBranchRollbackHookExceptionInBeforeDoesNotBreakRollback() throws Exception { + TrackingTccHook hook = new TrackingTccHook(); + hook.shouldThrowInBefore = true; + TccHookManager.registerHook(hook); + + SagaAnnotationResource resource = createResource("testAction2", "compensate"); + resourceManager.getManagedResources().put("testAction2", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 2L, "testAction2", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + assertTrue(hook.beforeRollbackCalled); + assertTrue(hook.afterRollbackCalled, "afterTccRollback should still be called even if before throws"); + } + + @Test + void testBranchRollbackHookExceptionInAfterDoesNotBreakRollback() throws Exception { + TrackingTccHook hook = new TrackingTccHook(); + hook.shouldThrowInAfter = true; + TccHookManager.registerHook(hook); + + SagaAnnotationResource resource = createResource("testAction3", "compensate"); + resourceManager.getManagedResources().put("testAction3", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 3L, "testAction3", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + assertTrue(hook.beforeRollbackCalled); + assertTrue(hook.afterRollbackCalled); + } + + @Test + void testBranchRollbackWithoutHooks() throws Exception { + SagaAnnotationResource resource = createResource("testAction4", "compensate"); + resourceManager.getManagedResources().put("testAction4", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 4L, "testAction4", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + } + + // ---- Tests for different compensation return types ---- + + @Test + void testBranchRollbackCompensationReturnsFalse() throws Exception { + SagaAnnotationResource resource = createResource("testAction5", "compensateFail"); + resourceManager.getManagedResources().put("testAction5", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 5L, "testAction5", null); + + assertEquals(BranchStatus.PhaseTwo_RollbackFailed_Retryable, status); + } + + @Test + void testBranchRollbackCompensationReturnsNull() throws Exception { + SagaAnnotationResource resource = createResource("testAction6", "compensateReturnNull"); + resourceManager.getManagedResources().put("testAction6", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 6L, "testAction6", null); + + // null return is treated as success + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + } + + @Test + void testBranchRollbackCompensationReturnsTwoPhaseResultSuccess() throws Exception { + SagaAnnotationResource resource = createResource("testAction7", "compensateWithResultSuccess"); + resourceManager.getManagedResources().put("testAction7", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 7L, "testAction7", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + } + + @Test + void testBranchRollbackCompensationReturnsTwoPhaseResultFail() throws Exception { + SagaAnnotationResource resource = createResource("testAction8", "compensateWithResultFail"); + resourceManager.getManagedResources().put("testAction8", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 8L, "testAction8", null); + + assertEquals(BranchStatus.PhaseTwo_RollbackFailed_Retryable, status); + } + + @Test + void testBranchRollbackCompensationThrowsException() throws Exception { + SagaAnnotationResource resource = createResource("testAction9", "compensateThrow"); + resourceManager.getManagedResources().put("testAction9", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 9L, "testAction9", null); + + assertEquals(BranchStatus.PhaseTwo_RollbackFailed_Retryable, status); + } + + // ---- Tests for hook invocation with compensation failure ---- + + @Test + void testBranchRollbackWithHooksWhenCompensationFails() throws Exception { + TrackingTccHook hook = new TrackingTccHook(); + TccHookManager.registerHook(hook); + + SagaAnnotationResource resource = createResource("testAction10", "compensateFail"); + resourceManager.getManagedResources().put("testAction10", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 10L, "testAction10", null); + + assertEquals(BranchStatus.PhaseTwo_RollbackFailed_Retryable, status); + assertTrue(hook.beforeRollbackCalled, "beforeTccRollback should be called even when compensation fails"); + assertTrue(hook.afterRollbackCalled, "afterTccRollback should be called in finally block"); + } + + @Test + void testBranchRollbackWithHooksWhenCompensationThrows() throws Exception { + TrackingTccHook hook = new TrackingTccHook(); + TccHookManager.registerHook(hook); + + SagaAnnotationResource resource = createResource("testAction11", "compensateThrow"); + resourceManager.getManagedResources().put("testAction11", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 11L, "testAction11", null); + + assertEquals(BranchStatus.PhaseTwo_RollbackFailed_Retryable, status); + assertTrue(hook.beforeRollbackCalled); + assertTrue(hook.afterRollbackCalled, "afterTccRollback should be called in finally block even on exception"); + } + + // ---- Tests for multiple hooks ---- + + @Test + void testBranchRollbackWithMultipleHooks() throws Exception { + TrackingTccHook hook1 = new TrackingTccHook(); + TrackingTccHook hook2 = new TrackingTccHook(); + TccHookManager.registerHook(hook1); + TccHookManager.registerHook(hook2); + + SagaAnnotationResource resource = createResource("testAction12", "compensate"); + resourceManager.getManagedResources().put("testAction12", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 12L, "testAction12", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + assertTrue(hook1.beforeRollbackCalled); + assertTrue(hook1.afterRollbackCalled); + assertTrue(hook2.beforeRollbackCalled); + assertTrue(hook2.afterRollbackCalled); + } + + @Test + void testBranchRollbackFirstHookThrowsSecondStillCalled() throws Exception { + TrackingTccHook hook1 = new TrackingTccHook(); + hook1.shouldThrowInBefore = true; + TrackingTccHook hook2 = new TrackingTccHook(); + TccHookManager.registerHook(hook1); + TccHookManager.registerHook(hook2); + + SagaAnnotationResource resource = createResource("testAction13", "compensate"); + resourceManager.getManagedResources().put("testAction13", resource); + + BranchStatus status = + resourceManager.branchRollback(BranchType.SAGA_ANNOTATION, "xid123", 13L, "testAction13", null); + + assertEquals(BranchStatus.PhaseTwo_Rollbacked, status); + assertTrue(hook1.beforeRollbackCalled); + // hook2.beforeRollbackCalled is NOT guaranteed because hook1 throws + // the loop iterates hooks sequentially, and the exception breaks the loop + assertTrue(hook2.afterRollbackCalled, "afterTccRollback should still call all hooks"); + } +} From 26eff1e290b79c421936e9029cd4011d413e0cb9 Mon Sep 17 00:00:00 2001 From: biedongbin Date: Tue, 31 Mar 2026 09:56:53 +0800 Subject: [PATCH 2/2] test: add tests for ActionInterceptorHandler reportActionStatus coverage (#7840) --- .../ActionInterceptorHandlerReportTest.java | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 integration-tx-api/src/test/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandlerReportTest.java diff --git a/integration-tx-api/src/test/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandlerReportTest.java b/integration-tx-api/src/test/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandlerReportTest.java new file mode 100644 index 00000000000..fdcff6a7541 --- /dev/null +++ b/integration-tx-api/src/test/java/org/apache/seata/integration/tx/api/interceptor/ActionInterceptorHandlerReportTest.java @@ -0,0 +1,205 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.integration.tx.api.interceptor; + +import org.apache.seata.common.Constants; +import org.apache.seata.common.executor.Callback; +import org.apache.seata.core.model.BranchType; +import org.apache.seata.integration.tx.api.fence.hook.TccHookManager; +import org.apache.seata.rm.tcc.api.BusinessActionContext; +import org.apache.seata.rm.tcc.api.BusinessActionContextUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; + +/** + * Tests for action status report functionality in ActionInterceptorHandler. + * + * Covers: + * 1. reportActionStatus method (success/failed/exception paths) + * 2. ENABLE_ACTION_STATUS_REPORT conditional branches in proceed method + */ +public class ActionInterceptorHandlerReportTest { + + private MockedStatic mockedContextUtil; + private boolean originalEnableActionStatusReport; + + @BeforeEach + void setUp() throws Exception { + TccHookManager.clear(); + mockedContextUtil = Mockito.mockStatic(BusinessActionContextUtil.class); + mockedContextUtil + .when(() -> BusinessActionContextUtil.reportContext(any())) + .thenReturn(true); + + // Enable action status report via reflection + originalEnableActionStatusReport = setEnableActionStatusReport(true); + } + + @AfterEach + void tearDown() throws Exception { + mockedContextUtil.close(); + TccHookManager.clear(); + + // Restore original value + setEnableActionStatusReport(originalEnableActionStatusReport); + } + + private boolean setEnableActionStatusReport(boolean value) throws Exception { + Field field = ActionInterceptorHandler.class.getDeclaredField("ENABLE_ACTION_STATUS_REPORT"); + field.setAccessible(true); + + // Use sun.misc.Unsafe to modify static final field (works on all Java versions) + Field theUnsafe = sun.misc.Unsafe.class.getDeclaredField("theUnsafe"); + theUnsafe.setAccessible(true); + sun.misc.Unsafe unsafe = (sun.misc.Unsafe) theUnsafe.get(null); + + Object base = unsafe.staticFieldBase(field); + long offset = unsafe.staticFieldOffset(field); + boolean original = unsafe.getBoolean(base, offset); + unsafe.putBoolean(base, offset, value); + return original; + } + + private BusinessActionContext createActionContext(String xid, long branchId) { + BusinessActionContext context = new BusinessActionContext(); + context.setXid(xid); + context.setBranchId(branchId); + context.setBranchType(BranchType.AT); + Map actionContext = new HashMap<>(); + context.setActionContext(actionContext); + return context; + } + + private TwoPhaseBusinessActionParam createParam(String actionName) { + TwoPhaseBusinessActionParam param = new TwoPhaseBusinessActionParam(); + param.setActionName(actionName); + param.setDelayReport(Boolean.TRUE); + param.setBranchType(BranchType.AT); + param.setUseCommonFence(false); + return param; + } + + // ---- Test reportActionStatus directly ---- + + @Test + void testReportActionStatusSuccess() { + ActionInterceptorHandler handler = new ActionInterceptorHandler(); + BusinessActionContext context = createActionContext("xid1", 1L); + + handler.reportActionStatus(context, Constants.ACTION_STATUS_SUCCESS); + + assertEquals(Constants.ACTION_STATUS_SUCCESS, context.getActionStatus()); + mockedContextUtil.verify(() -> BusinessActionContextUtil.reportContext(context)); + } + + @Test + void testReportActionStatusFailed() { + ActionInterceptorHandler handler = new ActionInterceptorHandler(); + BusinessActionContext context = createActionContext("xid2", 2L); + + handler.reportActionStatus(context, Constants.ACTION_STATUS_FAILED); + + assertEquals(Constants.ACTION_STATUS_FAILED, context.getActionStatus()); + } + + @Test + void testReportActionStatusExceptionHandledGracefully() { + ActionInterceptorHandler handler = new ActionInterceptorHandler(); + BusinessActionContext context = createActionContext("xid3", 3L); + + mockedContextUtil + .when(() -> BusinessActionContextUtil.reportContext(any())) + .thenThrow(new RuntimeException("report failed")); + + assertDoesNotThrow(() -> handler.reportActionStatus(context, Constants.ACTION_STATUS_SUCCESS)); + assertEquals(Constants.ACTION_STATUS_SUCCESS, context.getActionStatus()); + } + + // ---- Test ENABLE_ACTION_STATUS_REPORT conditional in proceed ---- + + @Test + void testProceedReportsSuccessWhenCallbackSucceeds() throws Throwable { + ActionInterceptorHandler handler = Mockito.spy(new ActionInterceptorHandler()); + Method method = TestTarget.class.getDeclaredMethod("execute", BusinessActionContext.class); + BusinessActionContext context = createActionContext("xid100", 100L); + Object[] arguments = new Object[] {context}; + TwoPhaseBusinessActionParam param = createParam("testAction"); + + Mockito.doReturn("branch123") + .when(handler) + .doTxActionLogStore( + any(Method.class), + any(), + any(TwoPhaseBusinessActionParam.class), + any(BusinessActionContext.class)); + + Callback successCallback = () -> "ok"; + handler.proceed(method, arguments, "xid100", param, successCallback); + + assertEquals(Constants.ACTION_STATUS_SUCCESS, context.getActionStatus()); + mockedContextUtil.verify(() -> BusinessActionContextUtil.reportContext(any()), Mockito.atLeast(1)); + } + + @Test + void testProceedReportsFailedWhenCallbackThrows() throws Throwable { + ActionInterceptorHandler handler = Mockito.spy(new ActionInterceptorHandler()); + Method method = TestTarget.class.getDeclaredMethod("execute", BusinessActionContext.class); + BusinessActionContext context = createActionContext("xid200", 200L); + Object[] arguments = new Object[] {context}; + TwoPhaseBusinessActionParam param = createParam("testAction"); + + Mockito.doReturn("branch456") + .when(handler) + .doTxActionLogStore( + any(Method.class), + any(), + any(TwoPhaseBusinessActionParam.class), + any(BusinessActionContext.class)); + + Callback failCallback = () -> { + throw new RuntimeException("business error"); + }; + + try { + handler.proceed(method, arguments, "xid200", param, failCallback); + } catch (RuntimeException e) { + // expected + } + + assertEquals(Constants.ACTION_STATUS_FAILED, context.getActionStatus()); + } + + // ---- Helper class for method reference ---- + + public static class TestTarget { + public Object execute(BusinessActionContext context) { + return "result"; + } + } +}