Skip to content

fix: EXPOSED-593 Rollback ExposedSQLException when use SpringTransactionManager#2398

Merged
bog-walk merged 10 commits intoJetBrains:mainfrom
FullOfOrange:fulloforange/spring-rollback-when-exposed-exception
Feb 27, 2025
Merged

fix: EXPOSED-593 Rollback ExposedSQLException when use SpringTransactionManager#2398
bog-walk merged 10 commits intoJetBrains:mainfrom
FullOfOrange:fulloforange/spring-rollback-when-exposed-exception

Conversation

@FullOfOrange
Copy link
Contributor

@FullOfOrange FullOfOrange commented Feb 8, 2025

Description

Summary of the change:

  • Add ExposedSpringTransactionAttributeSource to fix transaction not rollback when an ExposedSQLException thrown when using SpringTransactionManager.

  • Added @bean settings to spring-boot-starter

Detailed description:

  • What: Introduces ExposedSpringTransactionAttributeSource. This is an implementation of TransactionAttributeSource, a class for setting attributes on a transaction, and a class for adding ExposedSQLException as a rollback rule.
  • Why: Spring only rollback UncheckedExceptions when using the @Transactional annotation, but ExposedSQLException is a CheckedException and cannot be rolled back. This makes the behavior different from the default exposed transaction, and want to fix it.
  • How:
    • Implemented ExposedSpringTransactionAttributeSource to allow you to add an exception to the rollback rule.
      • If you use spring-boot-starter, it will automatically register the bean for you.
      • If you set it up yourself, you can register the ExposedSpringTransactionAttributeSource as a bean yourself and use it. (set up same as spring-boot-starter).

Type of Change

Please mark the relevant options with an "X":

  • Bug fix
  • New feature
  • Documentation update

Updates/remove existing public API methods:

  • Is breaking change

Affected databases:

  • MariaDB
  • Mysql5
  • Mysql8
  • Oracle
  • Postgres
  • SqlServer
  • H2
  • SQLite

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)
  • All public methods affected by my PR has up to date API docs
  • Documentation for my change is up to date

Related Issues

commit b1f340f
Author: 육진혁 (Ivan) <10372359+FullOfOrange@users.noreply.github.com>
Date:   Sat Feb 8 16:41:56 2025 +0900

    fix: detekt and api check

commit df9f29e
Merge: c7c781d a32e1f6
Author: 육진혁 (Ivan) <10372359+FullOfOrange@users.noreply.github.com>
Date:   Sat Feb 8 16:36:11 2025 +0900

    Merge branch 'main' into fulloforange/rollback-spring

commit c7c781d
Author: 육진혁 (Ivan) <10372359+FullOfOrange@users.noreply.github.com>
Date:   Sat Feb 8 16:33:54 2025 +0900

    Add ExposedSpringTransactionAttributeSource to rollback exception
@bog-walk bog-walk self-assigned this Feb 10, 2025
@bog-walk bog-walk linked an issue Feb 26, 2025 that may be closed by this pull request
@bog-walk bog-walk self-requested a review February 26, 2025 23:53
Copy link
Member

@bog-walk bog-walk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this PR @FullOfOrange

Just a request to edit one of the KDocs.

Please also consider rebasing from main.

Comment on lines 78 to 79
* @Primary annotation is used to avoid conflict with default TransactionAttributeSource bean
* than enable when use @EnableTransactionManagement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if you're actually seeing something else in the IDE, but this format (lines 78-79) will cause the lines to be ignored and not appear as part of the KDocs that show when the function is hovered over in the IDE. The KDocs attempts to resolve @Primary as a block tag, but it is not valid so it is ignored entirely.

Please quote the annotation in either backticks or single quotation marks to make it appear properly in KDocs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thank you for your review! I’ve applied your feedback.

@bog-walk bog-walk requested a review from e5l February 27, 2025 00:41
Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @FullOfOrange, thank you for the PR! LGTM

@bog-walk bog-walk merged commit 40955f3 into JetBrains:main Feb 27, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ExposedSQLException is not an runtime exception for Spring

3 participants