From a77b7bf0a45d77dc4639e4498beb6c2d4fe55303 Mon Sep 17 00:00:00 2001 From: Renan Kummer Date: Mon, 14 Oct 2024 19:39:36 -0300 Subject: [PATCH 1/3] feat!: EXPOSED-555 Allow read-only suspendable transactions Regular transaction management allow callers to configure if new transactions should be read-only. This change adds the same behavior to suspendable transactions. --- exposed-core/api/exposed-core.api | 8 ++-- .../transactions/experimental/Suspended.kt | 12 ++++-- exposed-tests/build.gradle.kts | 1 + .../tests/postgresql/ConnectionPoolTests.kt | 39 ++++++++++++++++++ .../tests/shared/ThreadLocalManagerTest.kt | 40 ++++++++++++++++--- gradle/libs.versions.toml | 1 + 6 files changed, 89 insertions(+), 12 deletions(-) diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index ad453dce25..26a7a383e4 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -3718,10 +3718,10 @@ public final class org/jetbrains/exposed/sql/transactions/TransactionStore : kot } public final class org/jetbrains/exposed/sql/transactions/experimental/SuspendedKt { - public static final fun newSuspendedTransaction (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; - public static synthetic fun newSuspendedTransaction$default (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; - public static final fun suspendedTransactionAsync (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; - public static synthetic fun suspendedTransactionAsync$default (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; + public static final fun newSuspendedTransaction (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Ljava/lang/Boolean;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public static synthetic fun newSuspendedTransaction$default (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Ljava/lang/Boolean;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; + public static final fun suspendedTransactionAsync (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Ljava/lang/Boolean;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public static synthetic fun suspendedTransactionAsync$default (Lkotlin/coroutines/CoroutineContext;Lorg/jetbrains/exposed/sql/Database;Ljava/lang/Integer;Ljava/lang/Boolean;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; public static final fun withSuspendTransaction (Lorg/jetbrains/exposed/sql/Transaction;Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public static synthetic fun withSuspendTransaction$default (Lorg/jetbrains/exposed/sql/Transaction;Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt index f6685e2423..92bafe0ef2 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt @@ -68,9 +68,10 @@ suspend fun newSuspendedTransaction( context: CoroutineContext? = null, db: Database? = null, transactionIsolation: Int? = null, + readOnly: Boolean? = null, statement: suspend Transaction.() -> T ): T = - withTransactionScope(context, null, db, transactionIsolation) { + withTransactionScope(context, null, db, transactionIsolation, readOnly) { suspendedTransactionAsyncInternal(true, statement).await() } @@ -102,10 +103,11 @@ suspend fun suspendedTransactionAsync( context: CoroutineContext? = null, db: Database? = null, transactionIsolation: Int? = null, + readOnly: Boolean? = null, statement: suspend Transaction.() -> T ): Deferred { val currentTransaction = TransactionManager.currentOrNull() - return withTransactionScope(context, null, db, transactionIsolation) { + return withTransactionScope(context, null, db, transactionIsolation, readOnly) { suspendedTransactionAsyncInternal(!holdsSameTransaction(currentTransaction), statement) } } @@ -129,6 +131,7 @@ private suspend fun withTransactionScope( currentTransaction: Transaction?, db: Database? = null, transactionIsolation: Int?, + readOnly: Boolean? = null, body: suspend TransactionScope.() -> T ): T { val currentScope = coroutineContext[TransactionScope] @@ -137,7 +140,10 @@ private suspend fun withTransactionScope( val manager = currentDatabase?.transactionManager ?: TransactionManager.manager val tx = lazy(LazyThreadSafetyMode.NONE) { - currentTransaction ?: manager.newTransaction(transactionIsolation ?: manager.defaultIsolationLevel) + currentTransaction ?: manager.newTransaction( + isolation = transactionIsolation ?: manager.defaultIsolationLevel, + readOnly = readOnly ?: manager.defaultReadOnly + ) } val element = TransactionCoroutineElement(tx, manager) diff --git a/exposed-tests/build.gradle.kts b/exposed-tests/build.gradle.kts index 5b42ba07b5..2d2af52eb7 100644 --- a/exposed-tests/build.gradle.kts +++ b/exposed-tests/build.gradle.kts @@ -38,6 +38,7 @@ dependencies { compileOnly(libs.h2) testCompileOnly(libs.sqlite.jdbc) testImplementation(libs.logcaptor) + testImplementation(libs.kotlinx.coroutines.test) } tasks.withType().configureEach { diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt index 9dc1c47d95..518324d8c3 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt @@ -2,6 +2,7 @@ package org.jetbrains.exposed.sql.tests.postgresql import com.zaxxer.hikari.HikariConfig import com.zaxxer.hikari.HikariDataSource +import kotlinx.coroutines.test.runTest import org.jetbrains.exposed.dao.id.IntIdTable import org.jetbrains.exposed.exceptions.ExposedSQLException import org.jetbrains.exposed.sql.Database @@ -13,6 +14,7 @@ import org.jetbrains.exposed.sql.tests.shared.assertEquals import org.jetbrains.exposed.sql.tests.shared.assertTrue import org.jetbrains.exposed.sql.tests.shared.expectException import org.jetbrains.exposed.sql.transactions.TransactionManager +import org.jetbrains.exposed.sql.transactions.experimental.newSuspendedTransaction import org.jetbrains.exposed.sql.transactions.transaction import org.junit.Assert import org.junit.Assume @@ -94,4 +96,41 @@ class ConnectionPoolTests : LogDbInTestName() { TransactionManager.closeAndUnregister(hikariPG) } + + @Test + fun testSuspendedReadOnlyModeWithHikariAndPostgres() = runTest { + Assume.assumeTrue(TestDB.POSTGRESQL in TestDB.enabledDialects()) + + val testTable = object : IntIdTable("HIKARI_TESTER") { } + + fun Transaction.getReadOnlyMode(): Boolean { + val mode = exec("SHOW transaction_read_only;") { + it.next() + it.getBoolean(1) + } + assertNotNull(mode) + return mode + } + + // read only mode should be set directly by hikari config + newSuspendedTransaction(db = hikariPG) { + assertTrue(getReadOnlyMode()) + + // table cannot be created in read-only mode + expectException { + SchemaUtils.create(testTable) + } + } + + // transaction setting should override hikari config + newSuspendedTransaction(transactionIsolation = Connection.TRANSACTION_SERIALIZABLE, readOnly = false, db = hikariPG) { + Assert.assertFalse(getReadOnlyMode()) + + // table can now be created and dropped + SchemaUtils.create(testTable) + SchemaUtils.drop(testTable) + } + + TransactionManager.closeAndUnregister(hikariPG) + } } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ThreadLocalManagerTest.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ThreadLocalManagerTest.kt index 7fdb5be533..a365306ed2 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ThreadLocalManagerTest.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ThreadLocalManagerTest.kt @@ -1,6 +1,8 @@ package org.jetbrains.exposed.sql.tests.shared +import kotlinx.coroutines.test.runTest import org.jetbrains.exposed.dao.id.IntIdTable +import org.jetbrains.exposed.exceptions.ExposedSQLException import org.jetbrains.exposed.sql.Database import org.jetbrains.exposed.sql.SchemaUtils import org.jetbrains.exposed.sql.insert @@ -9,6 +11,7 @@ import org.jetbrains.exposed.sql.tests.DatabaseTestsBase import org.jetbrains.exposed.sql.tests.TestDB import org.jetbrains.exposed.sql.tests.shared.dml.DMLTestsData import org.jetbrains.exposed.sql.transactions.TransactionManager +import org.jetbrains.exposed.sql.transactions.experimental.newSuspendedTransaction import org.jetbrains.exposed.sql.transactions.inTopLevelTransaction import org.jetbrains.exposed.sql.transactions.transaction import org.jetbrains.exposed.sql.transactions.transactionManager @@ -48,11 +51,7 @@ class ThreadLocalManagerTest : DatabaseTestsBase() { @Test fun testReadOnly() { - // Explanation: MariaDB driver never set readonly to true, MSSQL silently ignores the call, SQLite does not - // promise anything, H2 has very limited functionality - val excludeSettings = TestDB.ALL_H2 + TestDB.ALL_MARIADB + - listOf(TestDB.SQLITE, TestDB.SQLSERVER, TestDB.ORACLE) - withTables(excludeSettings = excludeSettings, RollbackTable) { + withTables(excludeSettings = READ_ONLY_EXCLUDED_VENDORS, RollbackTable) { assertFails { inTopLevelTransaction(db.transactionManager.defaultIsolationLevel, true) { maxAttempts = 1 @@ -61,8 +60,39 @@ class ThreadLocalManagerTest : DatabaseTestsBase() { }.message?.run { assertTrue(contains("read-only")) } ?: fail("message should not be null") } } + + @Test + fun testSuspendedReadOnly() = runTest { + Assume.assumeFalse(dialect in READ_ONLY_EXCLUDED_VENDORS) + + val database = dialect.connect() + newSuspendedTransaction(db = database, readOnly = true) { + expectException { + SchemaUtils.create(RollbackTable) + } + } + + transaction(db = database) { + SchemaUtils.create(RollbackTable) + } + + newSuspendedTransaction(db = database, readOnly = true) { + expectException { + RollbackTable.insert { it[value] = "random-something" } + } + } + + transaction(db = database) { + SchemaUtils.drop(RollbackTable) + } + } } object RollbackTable : IntIdTable("rollbackTable") { val value = varchar("value", 20) } + +// Explanation: MariaDB driver never set readonly to true, MSSQL silently ignores the call, SQLite does not +// promise anything, H2 has very limited functionality +private val READ_ONLY_EXCLUDED_VENDORS = + TestDB.ALL_H2 + TestDB.ALL_MARIADB + listOf(TestDB.SQLITE, TestDB.SQLSERVER, TestDB.ORACLE) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8000d5f770..fa85134b31 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -42,6 +42,7 @@ detekt-formatting = { group = "io.gitlab.arturbosch.detekt", name = "detekt-form kotlinx-coroutines = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-core", version.ref = "kotlinCoroutines" } kotlinx-coroutines-debug = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-debug", version.ref = "kotlinCoroutines" } +kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "kotlinCoroutines" } kotlinx-jvm-datetime = { group = "org.jetbrains.kotlinx", name = "kotlinx-datetime-jvm", version.ref = "kotlinx-datetime" } kotlinx-serialization = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-json", version.ref = "kotlinxSerialization" } From 7e13c348b603e4f2ed44351dd6055a47cb9a547a Mon Sep 17 00:00:00 2001 From: Renan Kummer Date: Tue, 26 Nov 2024 15:20:23 -0300 Subject: [PATCH 2/3] refactor: EXPOSED-555 Remove default value in private function argument --- .../exposed/sql/transactions/experimental/Suspended.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt index 92bafe0ef2..c571d94a5e 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt @@ -87,7 +87,7 @@ suspend fun Transaction.withSuspendTransaction( context: CoroutineContext? = null, statement: suspend Transaction.() -> T ): T = - withTransactionScope(context, this, db = null, transactionIsolation = null) { + withTransactionScope(context, this, db = null, transactionIsolation = null, readOnly = null) { suspendedTransactionAsyncInternal(false, statement).await() } @@ -131,7 +131,7 @@ private suspend fun withTransactionScope( currentTransaction: Transaction?, db: Database? = null, transactionIsolation: Int?, - readOnly: Boolean? = null, + readOnly: Boolean?, body: suspend TransactionScope.() -> T ): T { val currentScope = coroutineContext[TransactionScope] From 3a76a15cff4b19d10806f47b893b388572f54e3e Mon Sep 17 00:00:00 2001 From: Renan Kummer Date: Tue, 26 Nov 2024 16:17:34 -0300 Subject: [PATCH 3/3] refactor: EXPOSED-555 Extract common test code --- .../tests/postgresql/ConnectionPoolTests.kt | 39 +++++++------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt index 518324d8c3..99c0ca2954 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/postgresql/ConnectionPoolTests.kt @@ -17,10 +17,10 @@ import org.jetbrains.exposed.sql.transactions.TransactionManager import org.jetbrains.exposed.sql.transactions.experimental.newSuspendedTransaction import org.jetbrains.exposed.sql.transactions.transaction import org.junit.Assert +import org.junit.Assert.assertNotNull import org.junit.Assume import org.junit.Test import java.sql.Connection -import kotlin.test.assertNotNull class ConnectionPoolTests : LogDbInTestName() { private val hikariDataSourcePG by lazy { @@ -64,24 +64,13 @@ class ConnectionPoolTests : LogDbInTestName() { fun testReadOnlyModeWithHikariAndPostgres() { Assume.assumeTrue(TestDB.POSTGRESQL in TestDB.enabledDialects()) - val testTable = object : IntIdTable("HIKARI_TESTER") { } - - fun Transaction.getReadOnlyMode(): Boolean { - val mode = exec("SHOW transaction_read_only;") { - it.next() - it.getBoolean(1) - } - assertNotNull(mode) - return mode - } - // read only mode should be set directly by hikari config transaction(db = hikariPG) { assertTrue(getReadOnlyMode()) // table cannot be created in read-only mode expectException { - SchemaUtils.create(testTable) + SchemaUtils.create(TestTable) } } @@ -90,8 +79,8 @@ class ConnectionPoolTests : LogDbInTestName() { Assert.assertFalse(getReadOnlyMode()) // table can now be created and dropped - SchemaUtils.create(testTable) - SchemaUtils.drop(testTable) + SchemaUtils.create(TestTable) + SchemaUtils.drop(TestTable) } TransactionManager.closeAndUnregister(hikariPG) @@ -103,15 +92,6 @@ class ConnectionPoolTests : LogDbInTestName() { val testTable = object : IntIdTable("HIKARI_TESTER") { } - fun Transaction.getReadOnlyMode(): Boolean { - val mode = exec("SHOW transaction_read_only;") { - it.next() - it.getBoolean(1) - } - assertNotNull(mode) - return mode - } - // read only mode should be set directly by hikari config newSuspendedTransaction(db = hikariPG) { assertTrue(getReadOnlyMode()) @@ -134,3 +114,14 @@ class ConnectionPoolTests : LogDbInTestName() { TransactionManager.closeAndUnregister(hikariPG) } } + +private val TestTable = object : IntIdTable("HIKARI_TESTER") { } + +private fun Transaction.getReadOnlyMode(): Boolean { + val mode = exec("SHOW transaction_read_only;") { + it.next() + it.getBoolean(1) + } + assertNotNull(mode) + return mode == true +}