incubator-seata icon indicating copy to clipboard operation
incubator-seata copied to clipboard

optimize: remove Saga annotation dependency on @LocalTCC

Open WangzJi opened this issue 6 months ago • 1 comments

  • [ ] I have registered the PR changes.

Ⅰ. Describe what this PR did

This PR removes the dependency of Saga annotation on @LocalTCC by introducing a new @TransactionParticipant annotation. The changes include:

  1. Core Changes:

    • Enhanced LocalTCCRemotingParser to support both @LocalTCC and @TransactionParticipant annotations
    • Fixed isService(Class<?>) method implementation in both tcc and compatible modules
  2. Test Coverage:

    • LocalTCCRemotingParserTest: Add unit tests for dual annotation support
    • TransactionParticipantIntegrationTest: Add integration tests for runtime behavior
    • AnnotationConflictTest: Add boundary tests for edge cases and conflicts

Ⅱ. Does this pull request fix one issue?

Closes #7439

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

  1. Run the test suite:
./mvnw test -Dtest=LocalTCCRemotingParserTest,TransactionParticipantIntegrationTest,AnnotationConflictTest
  1. Verify backward compatibility:
  • All existing @LocalTCC annotated services continue to work
  • New services can use either @LocalTCC or @TransactionParticipant
  • No breaking changes in existing functionality

Ⅴ. Special notes for reviews

  1. Backward Compatibility:

    • Maintains 100% compatibility with existing @LocalTCC usage
    • No breaking changes to existing services
  2. Code Quality:

    • All tests pass
    • Follows project coding standards
  3. Documentation:

    • Added clear documentation for @TransactionParticipant usage
    • Updated migration guide for existing users

WangzJi avatar Jun 16 '25 13:06 WangzJi

Codecov Report

:x: Patch coverage is 85.36585% with 6 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 61.04%. Comparing base (edacdd7) to head (8dd1230). :warning: Report is 2 commits behind head on 2.x.

Files with missing lines Patch % Lines
...moting/parser/SagaTransactionalRemotingParser.java 87.50% 2 Missing and 2 partials :warning:
...rm/tcc/remoting/parser/LocalTCCRemotingParser.java 77.77% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #7443      +/-   ##
============================================
- Coverage     61.20%   61.04%   -0.17%     
- Complexity      652      666      +14     
============================================
  Files          1310     1316       +6     
  Lines         49587    49800     +213     
  Branches       5837     5855      +18     
============================================
+ Hits          30349    30398      +49     
- Misses        16526    16694     +168     
+ Partials       2712     2708       -4     
Files with missing lines Coverage Δ
...rm/tcc/remoting/parser/LocalTCCRemotingParser.java 0.00% <ø> (ø)
...rm/tcc/remoting/parser/LocalTCCRemotingParser.java 87.50% <77.77%> (+37.50%) :arrow_up:
...moting/parser/SagaTransactionalRemotingParser.java 87.50% <87.50%> (ø)

... and 13 files with indirect coverage changes

Impacted file tree graph

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jun 16 '25 14:06 codecov[bot]

Hi all,@funky-eyes @YongGoose @pengten. Based on community feedback, I propose the following approach for decoupling Saga annotation from TCC:

  1. Create SagaTransactional annotation in saga-annotation module (independent from @LocalTCC)
  2. Implement SagaTransactionalRemotingParser to handle only SagaTransactional
  3. Register parser separately via SPI

Does this approach look good to the community? I'll proceed with implementation once confirmed. Thanks!

WangzJi avatar Jul 02 '25 05:07 WangzJi

Hi all,@funky-eyes @YongGoose @pengten. Based on community feedback, I propose the following approach for decoupling Saga annotation from TCC:

  1. Create SagaTransactional annotation in saga-annotation module (independent from @LocalTCC)
  2. Implement SagaTransactionalRemotingParser to handle only SagaTransactional
  3. Register parser separately via SPI

Does this approach look good to the community? I'll proceed with implementation once confirmed. Thanks!

Sounds good to me.

pengten avatar Jul 02 '25 06:07 pengten

Hi all,@funky-eyes @YongGoose @pengten. Based on community feedback, I propose the following approach for decoupling Saga annotation from TCC:

  1. Create SagaTransactional annotation in saga-annotation module (independent from @LocalTCC)
  2. Implement SagaTransactionalRemotingParser to handle only SagaTransactional
  3. Register parser separately via SPI

Does this approach look good to the community? I'll proceed with implementation once confirmed. Thanks!

Hi all,@funky-eyes @YongGoose @pengten. Based on community feedback, I propose the following approach for decoupling Saga annotation from TCC:

  1. Create SagaTransactional annotation in saga-annotation module (independent from @LocalTCC)
  2. Implement SagaTransactionalRemotingParser to handle only SagaTransactional
  3. Register parser separately via SPI

Does this approach look good to the community? I'll proceed with implementation once confirmed. Thanks!

I agree

funky-eyes avatar Aug 23 '25 12:08 funky-eyes

Please register your PR in those two files : https://github.com/apache/incubator-seata/blob/2.x/changes/zh-cn/2.x.md https://github.com/apache/incubator-seata/blob/2.x/changes/en-us/2.x.md

Done in 10acfb5.

WangzJi avatar Sep 02 '25 06:09 WangzJi