Skip to content

Issue #24291 - Merge transactional attributes #24571

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 6 commits into from
Closed

Issue #24291 - Merge transactional attributes #24571

wants to merge 6 commits into from

Conversation

elab
Copy link

@elab elab commented Feb 22, 2020 •
edited
Loading

Changes to spring-tx as discribed in #24291

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 22, 2020
@elab elab changed the title Issue #24291 - Merge @Transactional annotations Issue #24291 - Merge transactional attributes Feb 23, 2020
@elab elab requested review from jhoeller and odrotbohm February 23, 2020 10:37
@elab
Copy link
Author

elab commented Feb 23, 2020

@odrotbohm Since transaction attributes are merged in this new implementation, it could be used in spring-data as is (i.e. without reverting the order of lookups), what do you think?

@elab
Copy link
Author

elab commented Feb 23, 2020

Looking though Reference Documentation (Data Access part) 5.2.3 I couldn't find anything that had to be changed.

@elab elab force-pushed the gh-24291 branch 4 times, most recently from 68dc863 to b06a5a9 Compare February 27, 2020 15:50
elab added 4 commits March 10, 2020 14:46
@elab
Refer to the targetClass instead of the declaringClass
3ddbe40 
(as described in comment and class javadoc)

See spring-projectsgh-24291
@elab
Merge policy for transactional attributes.
af2ba6a 
Merge transactional attributes from all relevant definition places,
according to their priority (instead of returning the first found one).

See spring-projectsgh-24291
@elab
rename AbstractFallbackTransactionAttributeSource to
555e0c1 
AbstractMergeTransactionAttributeSource
@elab
fix unit tests
82f519a
elab added 2 commits March 10, 2020 17:18
@elab
Add "declaring class of the specific method" to "priorities" (as it was
06d92c1 
in the original implementation), while also keeping the previously added
"target class"; improve JavaDoc.

The new priorities are:

  1. specific method; (this is just the proper name of the former
"method in the target class")
  2. declaring class of the specific method; (added)
  3. target class;
  4. method in the declaring class/interface;
  5. declaring class/interface.

instead of:

  1. method in the target class;
  2. target class;
  3. method in the declaring class/interface;
  4. declaring class/interface.
@elab
Add unit test for the use case from
97cd768 
spring-projects#24291 :

- base DAO class
- concrete DAO classes, which declare different transaction managers and
inherit transactional methods from the base DAO class
@rstoyanchev rstoyanchev added the in: data Issues in data modules (jdbc, orm, oxm, tx) label Nov 10, 2021
@jhoeller
Copy link
Contributor

It looks like we'll address both #24291 and #23473 through a new @TransactionConfig annotation, analogous to our existing @CacheConfig, or similar global settings which are not trying to rely on class-level @Transactional declarations (which brings in the undesirable implicit transactional-for-all-public-methods semantic whereas we only try to have certain settings shared for methods which are explicitly marked as transactional). From that perspective, we are not going to use a merging approach here.

Thanks for the PR, in any case!

@jhoeller jhoeller closed this Nov 23, 2023
@jhoeller jhoeller added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhoeller jhoeller Awaiting requested review from jhoeller

@odrotbohm odrotbohm Awaiting requested review from odrotbohm

Assignees
No one assigned
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) 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.
4 participants
@elab @jhoeller @rstoyanchev @spring-projects-issues