Skip to content
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.

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

Rename TransactionSynchronizationManager#currentTransaction to something more meaningful #23086

Closed
michael-simons opened this issue Jun 5, 2019 · 8 comments
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@michael-simons
Copy link
Contributor

I am working currently on implementing reactive Transactions with our new Neo4j reactive driver and need to access the reactive transaction synchronisation manager through

...reactive.TransactionSynchronizationManager#currentTransaction

The JavaDoc says "…Return the TransactionSynchronizationManager of the current transaction…"

I understand that TransactionSynchronizationManager is merely a wrapper around the TransactionContext.

Neither the manager nor the context are the transaction itself and therefor I think that the currentTransaction() would benefit from another name, like current TransactionSynchronizationManager() or currentTransactionContext(); it's intentions would be clearer.

cc @mp911de

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 5, 2019
@sbrannen sbrannen added status: pending-design-work Needs design work before any code can be developed in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 5, 2019
@sbrannen sbrannen added this to the 5.2 M3 milestone Jun 5, 2019
@sbrannen
Copy link
Member

sbrannen commented Jun 5, 2019

Tentatively slated for 5.2 M3

@sbrannen sbrannen changed the title Method naming: TransactionSynchronizationManager#currentTransaction is irritating Rename TransactionSynchronizationManager#currentTransaction to something more meaningful Jun 5, 2019
@mp911de
Copy link
Member

mp911de commented Jun 5, 2019

To add a bit of background: The reactive TransactionSynchronizationManager isn't associated with a transaction/TransactionContext, it is a wrapper to interact with resources and synchronizations stored in TransactionContext.

Renaming that method to forCurrentTransaction() could be more expressive. current TransactionSynchronizationManager is rather bulky. As Javadoc could be always improved, we could refine it along the lines of Return the TransactionSynchronizationManager that is associated with the current transaction context.

@michael-simons
Copy link
Contributor Author

A big +1 for forCurrentTransaction(). That reads nice and shows the purpose.

@sbrannen
Copy link
Member

sbrannen commented Jun 5, 2019

A big +1 for forCurrentTransaction(). That reads nice and shows the purpose.

That sounds better to me as well.

So, unless someone objects, I'll go ahead and make the proposed name change and Javadoc improvement.

@snicoll
Copy link
Member

snicoll commented Jun 8, 2019

@sbrannen shouldn't we keep the existing method in a deprecated fashion at least for one feature release?

@jhoeller
Copy link
Contributor

jhoeller commented Jun 8, 2019

@snicoll this just affects the reactive transaction support, so it's new API in 5.2 anyway.

@davidkiss
Copy link

This is a breaking change for spring-data-r2dbc v1.0 M2 (see spring-projects/spring-data-r2dbc#154) and spring-fu's latest version.

@sbrannen
Copy link
Member

This is a breaking change for spring-data-r2dbc v1.0 M2 (see spring-projects/spring-data-r2dbc#154) and spring-fu's latest version.

As @jhoeller already pointed out in #23086 (comment), the API in question is new in Spring Framework 5.2 and is therefore allowed to be renamed before the 5.2 GA release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants