Skip to content

Warning when EnableTransactionManagement has lower precedence than EnableMethodSecurity #17665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Warning when EnableTransactionManagement has lower precedence than EnableMethodSecurity #17665

wants to merge 1 commit into from

Conversation

@yybmion
Copy link
Contributor

@yybmion yybmion commented Aug 5, 2025

Adds validation to ensure @EnableTransactionManagement has higher precedence than @EnableMethodSecurity, allowing proper transaction rollback when @PostAuthorize fails.

  • Warn when transaction management has same or lower precedence than method security
  • Add comprehensive test coverage for various precedence scenarios

Fixes #17544

@yybmion
Warning when EnableTransactionManagement has lower precedence than En…
59f6c3d 
…ableMethodSecurity

Warns when transaction management has same or lower precedence to prevent
issues with @PostAuthorize rollback on methods with side effects.

Fixes: spring-projects#17544

Signed-off-by: yybmion <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 5, 2025
@jzheaux jzheaux added status: blocked An issue that's blocked on an external project change and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 11, 2025
@jzheaux
Copy link
Contributor

jzheaux commented Aug 11, 2025

Thanks for this PR, @yybmion. I'm having some internal conversations with the Framework team about this, so I'd like to hold off on this warning for the moment. I'll post my findings here.

@jzheaux jzheaux self-assigned this Aug 11, 2025
@jzheaux
Copy link
Contributor

jzheaux commented Aug 22, 2025

@yybmion, after some consultation, I think we should leave this to documentation. As such, I applied #17797 and this PR is no longer needed.

Thank you, I appreciate your efforts and sorry that we didn't move forward on this one.

@jzheaux jzheaux closed this Aug 22, 2025
@jzheaux jzheaux added status: declined A suggestion or change that we don't feel we should currently apply and removed status: blocked An issue that's blocked on an external project change labels Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jzheaux jzheaux

Labels

status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider warning when EnableTransactionManagement has lower precedence than EnableMethodSecurity

3 participants

@yybmion @jzheaux @spring-projects-issues