spring-framework
spring-framework copied to clipboard
Propagate `CoroutineContext` in reactive transaction
Motivation:
- Fixes #27307
Modifications:
- Propagate caller's
CoroutineContext
when converting coroutines intoMono
.- In
CoroutinesUtils#invokeSuspendingFunction
- In
TransactionalOperatorExtensions
-
Flow<T>.transactional(...)
-
TransactionalOperator.executeAndAwait(...)
-
- In
- Change
TransactionalOperator.executeAndAwait(...)
's type parameter upper bound fromAny
toAny?
- For better usability.
@ks-yim Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.
@ks-yim Thank you for signing the Contributor License Agreement!
Moving this issue back to waiting for triage while we are waiting more information on https://github.com/spring-projects/spring-data-commons/issues/2532 side.
Are there any updates on this issue? It's a bit of a roadblock for us - we rely on the context being propagated so can't use transactions at the moment :(
@jamesbassett Any chance you could provide a repro since we have no feedback on that on the Spring Data issue?
@sdeleuze I have created a reproducible example here https://github.com/JoeMaher/tx-coroutine-context-loss-example, please let me know if you have any queries/concerns
Great, thanks, I will have a look shortly.
Hi @sdeleuze did you get a chance to look at @JoeMaher 's reproducer? 🤞
Sorry for the delay but I am currently focused on the latest bits of native support for Spring Framework 6 RC1.
Hi @sdeleuze just bumping this (now that Spring 6 is released!)
Thanks for your patience all.
I re-worked the PR after fixing the nullability issue via #29919 with this commit, I now need to have the confirmation from the Kotlin team this is ok to remove Job.Key
from the Coroutines context for this specific use case.
If validated, we may backport the fix to 5.3.x
.
@mp911de @poutsma Could you please have a look to this commit and this comment from Kotlin team to check if you see something against merging this on main
and 5.3.x
.
The limitation on Job
is coming from Coroutines implementation, looks like a pretty unusual use case, and given the fact that this popular use case is broken for months, merging this will a a significant improvement for a lot of users.
@mp911de @poutsma Could you please have a look to this commit and this comment from Kotlin team to check if you see something against merging this on
main
and5.3.x
.
I did look, and can follow along with the discussion, but this is too far out of my area of expertise to give any insightful comment. If the PR resolves #27307, and does not break any existing code, than it seems good to me.
The fix and tests for both functional and annotation variants are now merged in main
and will be available as of Spring Framework 6.0.5
and Spring Boot 3.0.3
.
I evaluated if it is possible to merge it on 5.3.x
but this is not straightforward as it would require to backport 2 other changes that are impacting the public APIs, so I chose to not backport it.
That's fantastic news @sdeleuze thank you so much for progressing this 🥳 I'll give it a test drive when Boot 3.0.3 drops (I imagine that's later this month?)
@jamesbassett there is the calendar with Spring Ecosystem releases https://calendar.spring.io/
Just dropping in to say thanks @sdeleuze I tested out Boot 3.0.3 today and can confirm that everything now works as expected (we're not losing context when using transactions) and executeAndAwait()
doesn't require the !!
(hold-my-beer operator) any more 🥳 Thanks for all your help ❤️