-
Notifications
You must be signed in to change notification settings - Fork 753
feat: EXPOSED-657 Support regular spring JDBC transactions in the Exposed SpringTransactionManager #2675
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
base: main
Are you sure you want to change the base?
Conversation
…ager This means that @transactional when using Exposed with Spring also makes Spring JDBC classes like JdbcTemplate partake in the transaction
…o the shared Spring-test database configuration
… as it seems like TransactionSynchronizaationUtils.triggerFlush might be for other things
…e SmartTransactionObject over JdbcTransactionObjectSupport
4d62171 to
ce506d6
Compare
|
|
||
| @OptIn(InternalApi::class) | ||
| ThreadLocalTransactionsStack.pushTransaction(suspendedObject.transaction) | ||
| TransactionSynchronizationManager.bindResource(dataSource, suspendedObject.connectionHolder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see in doSuspend() the line trxObject.connectionHolder = null, so
should be added here trxObject.connectionHolder = suspendedObject.connectionHolder?
Could it cause missing of connectionHolder in trxObject after one suspend/resume cycle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right that it looks strange from a pure symmetry point of view. It was a while ago that I actually built this but I went back and looked at the thing I modeled this after (the built-in Spring DataSourceTransactionManager).
In there, they don't seem to re-connect the connection holder from suspended object to the transaction object.
Also, I did some digging with the debugger. I hope this screenshot will be readable. But looking at the actual underlying arguments sent to doResume, it looks like the transaction and the suspension objects already contain the same connection holder - so I assume those lifecycle elements are already handled by AbstractPlatformTransactionManager.
| } catch (ex: Throwable) { | ||
| trxObject.connectionHolder = null | ||
| throw CannotCreateTransactionException("Could not open JDBC Connection for transaction", ex) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move the block
@OptIn(InternalApi::class)
ThreadLocalTransactionsStack.pushTransaction(newTransaction)to the end of this method?
I see a problem that Exposed transaction could be added to the stack, and lost there in the case of exception inside this block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely! Good idea!
|
@bystam Hi, thank you for the PR. Overall, it looks like a good idea to allow using Exposed’s transactions with other data sources inside Spring. I’m happy to merge it if there are no further concerns. I’ve left some comments in the review — please take a look when you have a moment. |
…'doBegin' to avoid it leaking on an exception thrown when setting the spring trx resources

Description
The Exposed
SpringTransactionManagercurrently does not make Spring JDBC constructs take part in the transaction. In short:This draft tries to start supporting this by adding functionality to
SpringTransactionManagerborrowed from the basicDataSourceTransactionManager.Detailed description:
Caveat: I am not an expert on neither Exposed or Spring JDBC. This is my best effort attempt at simply reading source code.
ConnectionHolderinstances to thread locals inside theTransactionSynchronizationManager. Spring JDBC classes likeJdbcTemplatethen reuse thoseConnectionHolderinstances to make sure a single autocommit-disabled connection is shared throughout the transactionSpringTransactionManagerdoes not interact withTransactionSynchronizationManagerat all - it does not influence transaction semantics in Spring JDBC.SpringTransactionManagerandDataSourceTransactionManagerin order to make@Transactionalinfluence both Exposed and Spring JDBC simultaneously.But why?
Since Exposed depends on
spring-jdbc, in my mind it makes sense that it would honor the transaction semantics of its dependency. In comparison, when using JPA/Hibernate in Spring, theJpaTransactionManagerintroduces JPA-specific transaction behaviour but ALSO make sure to adhere to the expected semantics of Spring JDBC.Copy submitted
I originally had this submitted here: #2400 but that one had the source branch set to the
mainbranch of my fork, which goes against the suggested review guidelines AFAIK and also made it extra complicated for me to sync the main repo into my fork.Type of Change
Please mark the relevant options with an "X":
Updates/remove existing public API methods:
Affected databases:
Checklist
Related Issues
https://youtrack.jetbrains.com/issue/EXPOSED-657/Transaction-manager-provided-by-Exposed-is-not-compatible-with-JdbcTemplate