Skip to content

Rename TransactionSynchronizationManager#currentTransaction to something more meaningful #23086

@michael-simons

Description

@michael-simons
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

Activity

added
in: dataIssues in data modules (jdbc, orm, oxm, tx)
and removed on Jun 5, 2019
added this to the 5.2 M3 milestone on Jun 5, 2019
sbrannen

sbrannen commented on Jun 5, 2019

@sbrannen
Member

Tentatively slated for 5.2 M3

changed the title [-]Method naming: TransactionSynchronizationManager#currentTransaction is irritating[/-] [+]Rename TransactionSynchronizationManager#currentTransaction to something more meaningful[/+] on Jun 5, 2019
mp911de

mp911de commented on Jun 5, 2019

@mp911de
Member

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

michael-simons commented on Jun 5, 2019

@michael-simons
ContributorAuthor

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

sbrannen

sbrannen commented on Jun 5, 2019

@sbrannen
Member

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.

self-assigned this
on Jun 5, 2019
added a commit that references this issue on Jun 5, 2019
2ed9a61

7 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @sbrannen@snicoll@michael-simons@mp911de@jhoeller

      Issue actions

        Rename TransactionSynchronizationManager#currentTransaction to something more meaningful · Issue #23086 · spring-projects/spring-framework