Spring transaction doesn't close/commit transaction at the end of `tr…

…ansaction` block

SpringTransactionManager has connection leak when used with `transaction` instead of @transaction annotation
#831

spring-transaction/src/main/kotlin/org/jetbrains/exposed/spring/SpringTransactionManager.kt

@@ -29,12 +29,17 @@ class SpringTransactionManager(private val _dataSource: DataSource,
 
     private val db = Database.connect(_dataSource) { this }
 
+    private val springTxKey = "SPRING_TX_KEY"
+
     override fun doBegin(transaction: Any, definition: TransactionDefinition) {
         super.doBegin(transaction, definition)
 
         if (TransactionSynchronizationManager.hasResource(_dataSource)) {
             currentOrNull() ?: initTransaction()
         }
+        if (!TransactionSynchronizationManager.hasResource(springTxKey)) {
+            TransactionSynchronizationManager.bindResource(springTxKey, transaction)
+        }
     }
 
     override fun doCleanupAfterCompletion(transaction: Any) {
@@ -67,9 +72,7 @@ class SpringTransactionManager(private val _dataSource: DataSource,
     }
 
     override fun newTransaction(isolation: Int, outerTransaction: Transaction?): Transaction {
-        val tDefinition = dataSource?.connection?.transactionIsolation?.takeIf { it != isolation }?.let {
-                DefaultTransactionDefinition().apply { isolationLevel = isolation }
-        }
+        val tDefinition = DefaultTransactionDefinition().apply { isolationLevel = isolation }
 
         getTransaction(tDefinition)
 
@@ -88,7 +91,12 @@ class SpringTransactionManager(private val _dataSource: DataSource,
 
     override fun currentOrNull(): Transaction? = TransactionSynchronizationManager.getResource(this) as Transaction?
 
-    private class SpringTransaction(override val connection: ExposedConnection<*>, override val db: Database, override val transactionIsolation: Int, override val outerTransaction: Transaction?) : TransactionInterface {
+    private inner class SpringTransaction(
+        override val connection: ExposedConnection<*>,
+        override val db: Database,
+        override val transactionIsolation: Int,
+        override val outerTransaction: Transaction?
+    ) : TransactionInterface {
 
         override fun commit() {
             connection.run {
@@ -102,8 +110,14 @@ class SpringTransactionManager(private val _dataSource: DataSource,
             connection.rollback()
         }
 
-        override fun close() { }
-
+        override fun close() {
+            if (TransactionSynchronizationManager.isActualTransactionActive()) {
+                TransactionSynchronizationManager.getResource(springTxKey)?.let { springTx ->
+                    this@SpringTransactionManager.doCleanupAfterCompletion(springTx)
+                    TransactionSynchronizationManager.unbindResource(springTxKey)
+                }
+            }
+        }
     }
 
 }

spring-transaction/src/test/kotlin/org.jetbrains.exposed.spring/EntityUpdateTest.kt

@@ -24,7 +24,7 @@ open class EntityUpdateTest : SpringTransactionTestBase() {
     }
 
     @Test @Transactional @Commit
-    fun test1() {
+    open fun test1() {
         SchemaUtils.create(t1)
         t1.insert {
             it[t1.c1] = "new"
@@ -33,14 +33,14 @@ open class EntityUpdateTest : SpringTransactionTestBase() {
     }
 
     @Test @Transactional @Commit
-    fun test2() {
+    open fun test2() {
         val entity = dao.findById(1) ?: fail()
         entity.c1 = "updated"
         Assert.assertEquals("updated", dao.findById(1)?.c1)
     }
 
     @Test @Transactional @Commit
-    fun test3() {
+    open fun test3() {
         val entity = dao.findById(1) ?: fail()
         Assert.assertEquals("updated", entity.c1)
         SchemaUtils.drop(t1)

spring-transaction/src/test/kotlin/org.jetbrains.exposed.spring/ExposedTransactionManagerTest.kt

@@ -4,6 +4,7 @@ import org.jetbrains.exposed.sql.SchemaUtils
 import org.jetbrains.exposed.sql.Table
 import org.jetbrains.exposed.sql.insert
 import org.jetbrains.exposed.sql.selectAll
+import org.jetbrains.exposed.sql.transactions.TransactionManager
 import org.jetbrains.exposed.sql.transactions.transaction
 import org.junit.Assert
 import org.junit.Test
@@ -12,6 +13,7 @@ import org.springframework.test.annotation.Repeat
 import org.springframework.transaction.PlatformTransactionManager
 import org.springframework.transaction.annotation.Transactional
 import java.util.*
+import kotlin.test.assertNull
 
 open class ExposedTransactionManagerTest : SpringTransactionTestBase() {
 
@@ -21,7 +23,7 @@ open class ExposedTransactionManagerTest : SpringTransactionTestBase() {
 
     @Test @Transactional @Commit
     @Repeat(5)
-    fun testConnection() {
+    open fun testConnection() {
         val pm = ctx.getBean(PlatformTransactionManager::class.java)
         if(pm !is SpringTransactionManager) error("Wrong txManager instance: ${pm.javaClass.name}")
 
@@ -36,7 +38,7 @@ open class ExposedTransactionManagerTest : SpringTransactionTestBase() {
 
     @Test @Transactional @Commit
     @Repeat(5)
-    fun testConnection2() {
+    open fun testConnection2() {
         SchemaUtils.create(t1)
         val rnd = Random().nextInt().toString()
         t1.insert {
@@ -47,7 +49,7 @@ open class ExposedTransactionManagerTest : SpringTransactionTestBase() {
         SchemaUtils.drop(t1)
     }
 
-    @Test @Transactional @Commit
+    @Test
     @Repeat(5)
     fun testConnectionWithoutAnnotation() {
         transaction {
@@ -58,6 +60,15 @@ open class ExposedTransactionManagerTest : SpringTransactionTestBase() {
             }
 
             Assert.assertEquals(t1.selectAll().single()[t1.c1], rnd)
+        }
+        assertNull(TransactionManager.currentOrNull())
+        transaction {
+            val rnd = Random().nextInt().toString()
+            t1.insert {
+                it[t1.c1] = rnd
+            }
+
+            Assert.assertEquals(2, t1.selectAll().count())
             SchemaUtils.drop(t1)
         }
     }

spring-transaction/src/test/kotlin/org.jetbrains.exposed.spring/SpringTransactionEntityTest.kt

@@ -7,6 +7,7 @@ import org.jetbrains.exposed.dao.id.UUIDTable
 import org.jetbrains.exposed.sql.SchemaUtils
 import org.jetbrains.exposed.sql.transactions.transaction
 import org.junit.Test
+import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.test.annotation.Commit
 import org.springframework.transaction.annotation.Transactional
 import java.util.*
@@ -36,6 +37,7 @@ class OrderDAO(id: EntityID<UUID>): UUIDEntity(id) {
     var product by OrderTable.product
 }
 
+@org.springframework.stereotype.Service
 @Transactional
 open class Service {
     open fun createCustomer(name: String): CustomerDAO {
@@ -56,18 +58,17 @@ open class Service {
     }
 
     open fun findOrderByProduct(product: String) : OrderDAO? {
-        return OrderDAO.find { OrderTable.product eq product }.singleOrNull()?.apply {
-            this.customer // load to cache
-        }
+        return OrderDAO.find { OrderTable.product eq product }.singleOrNull()
     }
 }
 
 open class SpringTransactionEntityTest : SpringTransactionTestBase() {
 
-    val service: Service = Service()
+    @Autowired
+    lateinit var service: Service
 
     @Test @Commit
-    fun test01(){
+    open fun test01() {
         transaction {
             SchemaUtils.create(CustomerTable, OrderTable)
         }
@@ -76,16 +77,18 @@ open class SpringTransactionEntityTest : SpringTransactionTestBase() {
         service.createOrder(customer, "Product1")
         val order = service.findOrderByProduct("Product1")
         assertNotNull(order)
-        assertEquals("Alice1", order.customer.name)
+        transaction {
+            assertEquals("Alice1", order.customer.name)
+        }
     }
 
     @Test @Commit
     fun test02(){
         service.doBoth("Bob", "Product2")
         val order = service.findOrderByProduct("Product2")
         assertNotNull(order)
-        assertEquals("Bob", order.customer.name)
         transaction {
+        assertEquals("Bob", order.customer.name)
             SchemaUtils.drop(CustomerTable, OrderTable)
         }
     }

spring-transaction/src/test/kotlin/org.jetbrains.exposed.spring/SpringTransactionTestBase.kt

@@ -27,6 +27,9 @@ open class TestConfig : TransactionManagementConfigurer {
     @Bean
     override fun annotationDrivenTransactionManager(): PlatformTransactionManager? = SpringTransactionManager(ds())
 
+    @Bean
+    open fun service() : Service = Service()
+
 }
 
 /**