feat!: EXPOSED-109 change implement of spring transaction manager (#1840

)

* refactor: Implement a new one with AbstractPlatformTransactionManager

* refactor: remove unused property

* fix: remove nullable

* fix: add show sql feature

* fix: rebuild api file

* chore: remove property

* chore: add default show sql value

* fix: api definition

* feat: add default database config value

* feat :Add SmartTranscationObject implementation

* feat: close connection when transaction end

* chore: remove unused values

* feat: reset outer tx manager when transaction clean up

* test: Add outer transaction manager setting exception test

* chore: Add import

* test: add manager setting test

* chore: remake api file

* fix: fix when commit or rollback failure

* chore: fix detekt issue

* test: Add transactionAwareDataSourceProxy test

* chore: refactor spring transaction manager test

* refactor: remove default value in test

* refactor: apply code review

* refactor: apply test code review

* refactor: merge duplicate tests

* refactor: remove mockk and change exposed transaction object to private

* refactor: move database to private

* refactor: clean up transaction manager after test ended

* fix: while condition

* test: Add exposed and spring combine transaction

* fix: test

* fix: api dump

* refactor: apply connection spy suggestion

* refactor: apply code review

* refactor: apply code reivew

spring-transaction/api/spring-transaction.api

@@ -1,18 +1,5 @@
-public final class org/jetbrains/exposed/spring/SpringTransactionManager : org/springframework/jdbc/datasource/DataSourceTransactionManager, org/jetbrains/exposed/sql/transactions/TransactionManager {
-	public fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZZIJJ)V
-	public synthetic fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZZIJJILkotlin/jvm/internal/DefaultConstructorMarker;)V
-	public fun bindTransactionToThread (Lorg/jetbrains/exposed/sql/Transaction;)V
-	public fun currentOrNull ()Lorg/jetbrains/exposed/sql/Transaction;
-	public fun getDefaultIsolationLevel ()I
-	public fun getDefaultMaxRepetitionDelay ()J
-	public fun getDefaultMinRepetitionDelay ()J
-	public fun getDefaultReadOnly ()Z
-	public fun getDefaultRepetitionAttempts ()I
-	public fun newTransaction (IZLorg/jetbrains/exposed/sql/Transaction;)Lorg/jetbrains/exposed/sql/Transaction;
-	public fun setDefaultIsolationLevel (I)V
-	public fun setDefaultMaxRepetitionDelay (J)V
-	public fun setDefaultMinRepetitionDelay (J)V
-	public fun setDefaultReadOnly (Z)V
-	public fun setDefaultRepetitionAttempts (I)V
+public final class org/jetbrains/exposed/spring/SpringTransactionManager : org/springframework/transaction/support/AbstractPlatformTransactionManager {
+	public fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;Z)V
+	public synthetic fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
 }
 

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

@@ -6,182 +6,170 @@ import org.jetbrains.exposed.sql.StdOutSqlLogger
 import org.jetbrains.exposed.sql.Transaction
 import org.jetbrains.exposed.sql.addLogger
 import org.jetbrains.exposed.sql.exposedLogger
-import org.jetbrains.exposed.sql.statements.api.ExposedConnection
-import org.jetbrains.exposed.sql.statements.jdbc.JdbcConnectionImpl
-import org.jetbrains.exposed.sql.transactions.TransactionInterface
 import org.jetbrains.exposed.sql.transactions.TransactionManager
-import org.springframework.jdbc.datasource.ConnectionHolder
-import org.springframework.jdbc.datasource.DataSourceTransactionManager
+import org.jetbrains.exposed.sql.transactions.transactionManager
 import org.springframework.transaction.TransactionDefinition
 import org.springframework.transaction.TransactionSystemException
-import org.springframework.transaction.support.DefaultTransactionDefinition
+import org.springframework.transaction.support.AbstractPlatformTransactionManager
 import org.springframework.transaction.support.DefaultTransactionStatus
-import org.springframework.transaction.support.TransactionSynchronizationManager
+import org.springframework.transaction.support.SmartTransactionObject
 import javax.sql.DataSource
 
 class SpringTransactionManager(
     dataSource: DataSource,
-    databaseConfig: DatabaseConfig = DatabaseConfig { },
+    databaseConfig: DatabaseConfig = DatabaseConfig {},
     private val showSql: Boolean = false,
-    @Volatile override var defaultReadOnly: Boolean = databaseConfig.defaultReadOnly,
-    @Volatile override var defaultRepetitionAttempts: Int = databaseConfig.defaultRepetitionAttempts,
-    @Volatile override var defaultMinRepetitionDelay: Long = databaseConfig.defaultMinRepetitionDelay,
-    @Volatile override var defaultMaxRepetitionDelay: Long = databaseConfig.defaultMaxRepetitionDelay
-) : DataSourceTransactionManager(dataSource), TransactionManager {
+) : AbstractPlatformTransactionManager() {
 
-    init {
-        this.isRollbackOnCommitFailure = true
-    }
-
-    private val db = Database.connect(
-        datasource = dataSource,
-        databaseConfig = databaseConfig
-    ) { this }
+    private var _database: Database
 
-    @Volatile
-    override var defaultIsolationLevel: Int = -1
-        get() {
-            if (field == -1) {
-                field = Database.getDefaultIsolationLevel(db)
-            }
-            return field
-        }
+    private var _transactionManager: TransactionManager
 
-    private val transactionStackKey = "SPRING_TRANSACTION_STACK_KEY"
+    private val threadLocalTransactionManager: TransactionManager
+        get() = _transactionManager
 
-    private fun getTransactionStack(): List<TransactionManager> {
-        return TransactionSynchronizationManager.getResource(transactionStackKey)
-            ?.let { it as List<TransactionManager> }
-            ?: listOf()
+    init {
+        _database = Database.connect(
+            datasource = dataSource, databaseConfig = databaseConfig
+        ).apply {
+            _transactionManager = this.transactionManager
+        }
     }
 
-    private fun setTransactionStack(list: List<TransactionManager>) {
-        TransactionSynchronizationManager.unbindResourceIfPossible(transactionStackKey)
-        TransactionSynchronizationManager.bindResource(transactionStackKey, list)
-    }
+    override fun doGetTransaction(): Any {
+        val outerManager = TransactionManager.manager
+        val outer = threadLocalTransactionManager.currentOrNull()
 
-    private fun pushTransactionStack(transaction: TransactionManager) {
-        val transactionList = getTransactionStack()
-        setTransactionStack(transactionList + transaction)
+        return ExposedTransactionObject(
+            manager = threadLocalTransactionManager,
+            outerManager = outerManager,
+            outerTransaction = outer,
+        )
     }
 
-    private fun popTransactionStack() = setTransactionStack(getTransactionStack().dropLast(1))
-
-    private fun getLastTransactionStack() = getTransactionStack().lastOrNull()
-
     override fun doBegin(transaction: Any, definition: TransactionDefinition) {
-        super.doBegin(transaction, definition)
+        val trxObject = transaction as ExposedTransactionObject
+
+        val currentTransactionManager = trxObject.manager
+        TransactionManager.resetCurrent(currentTransactionManager)
 
-        if (TransactionSynchronizationManager.hasResource(obtainDataSource())) {
-            currentOrNull() ?: initTransaction(transaction)
+        currentTransactionManager.currentOrNull() ?: currentTransactionManager.newTransaction(
+            isolation = definition.isolationLevel,
+            readOnly = definition.isReadOnly,
+        ).apply {
+            if (showSql) {
+                addLogger(StdOutSqlLogger)
+            }
         }
+    }
 
-        pushTransactionStack(this@SpringTransactionManager)
+    override fun doCommit(status: DefaultTransactionStatus) {
+        val trxObject = status.transaction as ExposedTransactionObject
+        TransactionManager.resetCurrent(trxObject.manager)
+        trxObject.commit()
     }
 
-    override fun doCleanupAfterCompletion(transaction: Any) {
-        super.doCleanupAfterCompletion(transaction)
-        if (!TransactionSynchronizationManager.hasResource(obtainDataSource())) {
-            TransactionSynchronizationManager.unbindResourceIfPossible(this)
-        }
+    override fun doRollback(status: DefaultTransactionStatus) {
+        val trxObject = status.transaction as ExposedTransactionObject
+        TransactionManager.resetCurrent(trxObject.manager)
+        trxObject.rollback()
+    }
 
-        popTransactionStack()
-        TransactionManager.resetCurrent(getLastTransactionStack())
+    override fun doCleanupAfterCompletion(transaction: Any) {
+        val trxObject = transaction as ExposedTransactionObject
 
-        if (TransactionSynchronizationManager.isSynchronizationActive() && TransactionSynchronizationManager.getSynchronizations().isEmpty()) {
-            TransactionSynchronizationManager.clearSynchronization()
+        trxObject.cleanUpTransactionIfIsPossible {
+            closeStatementsAndConnections(it)
         }
-    }
 
-    override fun doSuspend(transaction: Any): Any {
-        TransactionSynchronizationManager.unbindResourceIfPossible(this)
-        return super.doSuspend(transaction)
+        trxObject.setCurrentToOuter()
     }
 
-    override fun doCommit(status: DefaultTransactionStatus) {
+    private fun closeStatementsAndConnections(transaction: Transaction) {
+        val currentStatement = transaction.currentStatement
         @Suppress("TooGenericExceptionCaught")
         try {
-            currentOrNull()?.commit()
-        } catch (e: Exception) {
-            throw TransactionSystemException(e.message.orEmpty(), e)
+            currentStatement?.let {
+                it.closeIfPossible()
+                transaction.currentStatement = null
+            }
+            transaction.closeExecutedStatements()
+        } catch (error: Exception) {
+            exposedLogger.warn("Statements close failed", error)
         }
-    }
 
-    override fun doRollback(status: DefaultTransactionStatus) {
         @Suppress("TooGenericExceptionCaught")
         try {
-            currentOrNull()?.rollback()
-        } catch (e: Exception) {
-            throw TransactionSystemException(e.message.orEmpty(), e)
+            transaction.close()
+        } catch (error: Exception) {
+            exposedLogger.warn("Transaction close failed: ${error.message}. Statement: $currentStatement", error)
         }
     }
 
-    override fun newTransaction(isolation: Int, readOnly: Boolean, outerTransaction: Transaction?): Transaction {
-        val tDefinition = DefaultTransactionDefinition().apply {
-            isReadOnly = readOnly
-            isolationLevel = isolation
-        }
-
-        val transactionStatus = (getTransaction(tDefinition) as DefaultTransactionStatus)
-        return currentOrNull() ?: initTransaction(transactionStatus.transaction)
+    override fun doSetRollbackOnly(status: DefaultTransactionStatus) {
+        val trxObject = status.transaction as ExposedTransactionObject
+        trxObject.setRollbackOnly()
     }
 
-    private fun initTransaction(transaction: Any): Transaction {
-        val connection = (TransactionSynchronizationManager.getResource(obtainDataSource()) as ConnectionHolder).connection
+    private data class ExposedTransactionObject(
+        val manager: TransactionManager,
+        val outerManager: TransactionManager,
+        private val outerTransaction: Transaction?,
+    ) : SmartTransactionObject {
 
-        @Suppress("TooGenericExceptionCaught")
-        val transactionImpl = try {
-            SpringTransaction(JdbcConnectionImpl(connection), db, defaultIsolationLevel, defaultReadOnly, currentOrNull(), transaction)
-        } catch (e: Exception) {
-            exposedLogger.error("Failed to start transaction. Connection will be closed.", e)
-            connection.close()
-            throw e
-        }
+        private var isRollback: Boolean = false
+        private var isCurrentTransactionEnded: Boolean = false
 
-        TransactionManager.resetCurrent(this)
-        return Transaction(transactionImpl).apply {
-            TransactionSynchronizationManager.bindResource(this@SpringTransactionManager, this)
-            if (showSql) {
-                addLogger(StdOutSqlLogger)
+        fun cleanUpTransactionIfIsPossible(block: (transaction: Transaction) -> Unit) {
+            val currentTransaction = getCurrentTransaction()
+            if (isCurrentTransactionEnded && currentTransaction != null) {
+                block(currentTransaction)
             }
         }
-    }
 
-    override fun currentOrNull(): Transaction? = TransactionSynchronizationManager.getResource(this) as Transaction?
-    override fun bindTransactionToThread(transaction: Transaction?) {
-        if (transaction != null) {
-            bindResourceForSure(this, transaction)
-        } else {
-            TransactionSynchronizationManager.unbindResourceIfPossible(this)
+        fun setCurrentToOuter() {
+            manager.bindTransactionToThread(outerTransaction)
+            TransactionManager.resetCurrent(outerManager)
         }
-    }
 
-    private fun bindResourceForSure(key: Any, value: Any) {
-        TransactionSynchronizationManager.unbindResourceIfPossible(key)
-        TransactionSynchronizationManager.bindResource(key, value)
-    }
-
-    private inner class SpringTransaction(
-        override val connection: ExposedConnection<*>,
-        override val db: Database,
-        override val transactionIsolation: Int,
-        override val readOnly: Boolean,
-        override val outerTransaction: Transaction?,
-        private val currentTransaction: Any,
-    ) : TransactionInterface {
-
-        override fun commit() {
-            connection.commit()
+        private fun hasOuterTransaction(): Boolean {
+            return outerTransaction != null
         }
 
-        override fun rollback() {
-            connection.rollback()
+        @Suppress("TooGenericExceptionCaught")
+        fun commit() {
+            try {
+                if (hasOuterTransaction().not()) {
+                    isCurrentTransactionEnded = true
+                    manager.currentOrNull()?.commit()
+                }
+            } catch (error: Exception) {
+                throw TransactionSystemException(error.message.orEmpty(), error)
+            }
         }
 
-        override fun close() {
-            if (TransactionSynchronizationManager.isActualTransactionActive()) {
-                this@SpringTransactionManager.doCleanupAfterCompletion(currentTransaction)
+        @Suppress("TooGenericExceptionCaught")
+        fun rollback() {
+            try {
+                if (hasOuterTransaction().not()) {
+                    isCurrentTransactionEnded = true
+                    manager.currentOrNull()?.rollback()
+                }
+            } catch (error: Exception) {
+                throw TransactionSystemException(error.message.orEmpty(), error)
             }
         }
+
+        fun getCurrentTransaction(): Transaction? = manager.currentOrNull()
+
+        fun setRollbackOnly() {
+            isRollback = true
+        }
+
+        override fun isRollbackOnly() = isRollback
+
+        override fun flush() {
+            // Do noting
+        }
     }
 }

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

@@ -0,0 +1,77 @@
+package org.jetbrains.exposed.spring
+
+import java.sql.Connection
+import java.sql.Savepoint
+
+internal class ConnectionSpy(private val connection: Connection) : Connection by connection {
+
+    var commitCallCount: Int = 0
+    var rollbackCallCount: Int = 0
+    var closeCallCount: Int = 0
+    var mockReadOnly: Boolean = false
+    var mockIsClosed: Boolean = false
+    var mockAutoCommit: Boolean = false
+    var mockTransactionIsolation: Int = Connection.TRANSACTION_READ_COMMITTED
+    var mockCommit: () -> Unit = {}
+    var mockRollback: () -> Unit = {}
+    private val callOrder = mutableListOf<String>()
+
+    fun verifyCallOrder(vararg functions: String): Boolean {
+        val indices = functions.map { callOrder.indexOf(it) }
+        return indices.none { it == -1 } && indices == indices.sorted()
+    }
+
+    fun clearMock() {
+        commitCallCount = 0
+        rollbackCallCount = 0
+        closeCallCount = 0
+        mockAutoCommit = false
+        mockReadOnly = false
+        mockIsClosed = false
+        mockTransactionIsolation = Connection.TRANSACTION_READ_COMMITTED
+        mockCommit = {}
+        mockRollback = {}
+        callOrder.clear()
+    }
+
+    override fun close() {
+        callOrder.add("close")
+        closeCallCount++
+    }
+
+    override fun setAutoCommit(autoCommit: Boolean) {
+        callOrder.add("setAutoCommit")
+        mockAutoCommit = autoCommit
+    }
+
+    override fun getAutoCommit() = mockAutoCommit
+    override fun commit() {
+        callOrder.add("commit")
+        commitCallCount++
+        mockCommit()
+    }
+
+    override fun rollback() {
+        callOrder.add("rollback")
+        rollbackCallCount++
+        mockRollback()
+    }
+
+    override fun rollback(savepoint: Savepoint?) = Unit
+    override fun isClosed(): Boolean {
+        callOrder.add("isClosed")
+        return mockIsClosed
+    }
+
+    override fun setReadOnly(readOnly: Boolean) {
+        callOrder.add("setReadOnly")
+        mockReadOnly = readOnly
+    }
+
+    override fun isReadOnly(): Boolean {
+        callOrder.add("isReadOnly")
+        return mockReadOnly
+    }
+
+    override fun getTransactionIsolation() = mockTransactionIsolation
+}