feat: Support regular spring JDBC transactions in the Exposed SpringTransactionManager

[bystam]

Description

The Exposed SpringTransactionManager currently does not make Spring JDBC constructs take part in the transaction. In short:

@Autowired
private lateinit var dataSource: DataSource

@Transactional
fun insertTwoThings() {
    val jdbcTemplate = JdbcTemplate(dataSource)
    // these two statements will NOT be run in a transaction
    jdbcTemplate.executeUpdate("INSERT INTO foo VALUES ('bar1')")
    jdbcTemplate.executeUpdate("INSERT INTO foo VALUES ('bar2')")
}

This draft tries to start supporting this by adding functionality to SpringTransactionManager borrowed from the basic DataSourceTransactionManager.

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.

• In short, Spring JDBC manages its transactions by pinning ConnectionHolder instances to thread locals inside the TransactionSynchronizationManager. Spring JDBC classes like JdbcTemplate then reuse those ConnectionHolder instances to make sure a single autocommit-disabled connection is shared throughout the transaction
• Since the Exposed SpringTransactionManager does not interact with TransactionSynchronizationManager at all - it does not influence transaction semantics in Spring JDBC.
• This PR makes a quick attempt at blending SpringTransactionManager and DataSourceTransactionManager in order to make @Transactional influence 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, the JpaTransactionManager introduces JPA-specific transaction behaviour but ALSO make sure to adhere to the expected semantics of Spring JDBC.

Why a draft?
This is written by basically cherry-picking DataSourceTransactionManager code into SpringTransactionManager. I feel like I managed to get a pretty decent high level understanding of how Spring JDBC manages transactions, but there's plenty of low level stuff in there I am uncertain about whether it made sense to bring or not.

---

Type of Change

Please mark the relevant options with an "X":

• Bug fix
• New feature
• Documentation update

Updates/remove existing public API methods:

• Is breaking change

Affected databases:

• MariaDB
• Mysql5
• Mysql8
• Oracle
• Postgres
• SqlServer
• H2
• SQLite

Checklist

• Unit tests are in place
• The build is green (including the Detekt check)
• All public methods affected by my PR has up to date API docs
• Documentation for my change is up to date

---

Related Issues

https://youtrack.jetbrains.com/issue/EXPOSED-657/Transaction-manager-provided-by-Exposed-is-not-compatible-with-JdbcTemplate

[bystam]

Apologize for the failing PR title/commit message/detekt checks. I will make sure to get that sorted.

[bystam]

Detect any existing ConnectionHolder set as a thread-local and reuse it

[bystam]

This class is actually more or less the thing that makes the whole implementation work.

For Exposed and Spring JDBC to co-exist, they need to both see the same connection. Spring JDBC fetches its connection by calling something like:

ConnectionHolder holder = (ConnectionHolder) TransactionSynchronizationManager.getResource(dataSource);
holder.getConnection()

so for them to share it, the Exposed transaction manager needs to install a ConnectionHolder that provides the same connection using TransactionSynchronizationManager.bindResource(dataSource, <some connection holder>). The ConnectionHolder class of spring can take a "pointer to a connection" called a ConnectionHandle, which is what I implement here.

This is done upstairs like

trxObject.setConnectionHolder(
    ConnectionHolder(ExposedConnectionHandle(transaction))
)
trxObject.isNewConnectionHolder = true
...

if (trxObject.isNewConnectionHolder){
    TransactionSynchronizationManager.bindResource(dataSource, trxObject.getConnectionHolder())
}

[bystam]

This ExposedConnectionHandle is the thing that makes this all work. See comment downstairs.

[bystam]

I obviously broke a bunch of other tests using this implementation 🙃 - looking into it

[FullOfOrange]

I think if you run detekt, the build will succeed, and if you also look at this link and fix the commit and pr messages, checks may be done!

Oops, didn't see your comment above, I left a comment because the detekt test failed at build time.

[bystam]

Okay - my attempt broke quite a lot of tests it looks like. Might have been a bit naive to simply blend them like I did. Maybe things need to be integrated more deeply into the Exposed TransactionManager rather than handled on the outside.

[FullOfOrange]

When I first proposed to re-implement the SpringTransactionManager, I thought about implementing an Exposed TransactionManager for Spring, but since it is hard work, I thought it would be more efficient to integrate ThreadLocalTransactionManager's transaction management with Spring's transaction lifecycle. (I thought the SpringTransactionManager would just set @transactional as a replacement for transaction {})

I used ThreadLocalTransactionManager to integrate with Spring, but it seemed a bit too much to fulfill the basic requirements of Spring, such as the SQLException not rollback issue. On second thought, it would be nice to rewrite the TransactionManager implementation, even if it's difficult.

[bystam]

Looking more closely at the failing tests - I got the sense that all the "extra stuff" that I cherry-picked from the DataSourceTransactionManager that manipulated the connection might have been what made it not work.

I decided to scratch most of that, since the regular Exposed TransactionManager handles a bunch of connection configuration itself.

What's left is simply the TransactionSynchronizationManager + ConnectionHolder dance - which I believe was the core part to make this actually work. Now the tests seem green to me!

The only bit I am curious about is whether it's important to set the isSynchronizedWithTransaction flag or not. If I use it exactly like DataSourceTransactionManager does then I get some failures, but that's because useSavepointForNestedTransaction is set to false. I don't totally know how to write a test that might reproduce any failures with misusing that flag - or if it even matters.

[bystam]

When I first proposed to re-implement the SpringTransactionManager, I thought about implementing an Exposed TransactionManager for Spring, but since it is hard work, I thought it would be more efficient to integrate ThreadLocalTransactionManager's transaction management with Spring's transaction lifecycle. (I thought the SpringTransactionManager would just set @transactional as a replacement for transaction {})

I used ThreadLocalTransactionManager to integrate with Spring, but it seemed a bit too much to fulfill the basic requirements of Spring, such as the SQLException not rollback issue. On second thought, it would be nice to rewrite the TransactionManager implementation, even if it's difficult.

I see! I think I managed to get it to work now actually. Though I am not entirely sure how I would write a test that might catch any nasty edge cases. All the existing tests and the new ones I added are green, though!

[bystam]

I created a copy of the existing ExposedTransactionManagerTest that runs basically the same tests but using springs JdbcClient to verify all the same cases.

Some of them behaved differently, but that is intentional. Exposed requires a transaction in order to operate, but JdbcClient does not - and instead runs in autocommit mode if none is present.

[bystam]

This behaviour is somewhat tricky. The full TransactionSynchronization behaviour is actually not supported right now - because we handle savepoints outside of Springs knowledge, which means that the savepoint callbacks specifically do not trigger.

While that might be a somewhat incomplete support - I personally feel that covering 95% of the common use cases is better than not covering them, as listening to savepoint callbacks should be a quite niche thing.

[joc-a]

Hi @bystam, thank you for your contribution. Please rebase on the main branch to make the Pull Request Title Validation check pass.

[bystam]

I feel like I made all the checks pass now - but it looks like some tests failed on the latest run. Those kinds of failures happen to me sporadically locally too, howver. And I don't think I caused them? Or? 🤔

Edit: I found it. An existing test accidentally had a teardown that unregistered the database set up by the base SpringTransactionTestBase, even though it only intended to clean up its own databases.

It failed sporadically because the tests were run in different order every time - and they only failed when they are run in a specific order.