From 451a9eb49348ecd5284a0cd074165cc5d9ab0af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gwendal=20Roue=CC=81?= Date: Sat, 30 Mar 2024 14:45:01 +0100 Subject: [PATCH 1/3] [BREAKING] Remove Configuration.defaultTransactionKind GRDB now defaults read-only transactions to DEFERRED, and write transactions to IMMEDIATE. See https://github.com/groue/GRDB.swift/issues/1483 for some context. --- GRDB/Core/Configuration.swift | 22 -- GRDB/Core/Database.swift | 33 ++- GRDB/Core/DatabasePool.swift | 11 +- GRDB/Core/DatabaseQueue.swift | 7 +- GRDB/Core/DatabaseSnapshot.swift | 4 - GRDB/Core/DatabaseSnapshotPool.swift | 4 - GRDB/Documentation.docc/DatabaseSharing.md | 3 +- .../Extension/Configuration.md | 1 - GRDB/Documentation.docc/Transactions.md | 19 +- TODO.md | 2 +- Tests/GRDBTests/BackupTestCase.swift | 4 +- Tests/GRDBTests/DatabaseQueueTests.swift | 2 - Tests/GRDBTests/DatabaseSavepointTests.swift | 228 +----------------- .../GRDBTests/DatabaseSnapshotPoolTests.swift | 13 +- Tests/GRDBTests/DatabaseSuspensionTests.swift | 2 +- Tests/GRDBTests/DatabaseTests.swift | 47 +++- 16 files changed, 88 insertions(+), 314 deletions(-) diff --git a/GRDB/Core/Configuration.swift b/GRDB/Core/Configuration.swift index 2c6e62498a..17a50eff8e 100644 --- a/GRDB/Core/Configuration.swift +++ b/GRDB/Core/Configuration.swift @@ -238,28 +238,6 @@ public struct Configuration { // MARK: - Transactions - /// The default kind of write transactions. - /// - /// The default is ``Database/TransactionKind/deferred``. - /// - /// You can change the default transaction kind. For example, you can force - /// all write transactions to be `IMMEDIATE`: - /// - /// ```swift - /// var config = Configuration() - /// config.defaultTransactionKind = .immediate - /// let dbQueue = try DatabaseQueue(configuration: config) - /// - /// // BEGIN IMMEDIATE TRANSACTION; ...; COMMIT TRANSACTION; - /// try dbQueue.write { db in ... } - /// ``` - /// - /// This property is ignored for read-only transactions. Those always open - /// `DEFERRED` SQLite transactions. - /// - /// Related SQLite documentation: - public var defaultTransactionKind: Database.TransactionKind = .deferred - /// A boolean value indicating whether it is valid to leave a transaction /// opened at the end of a database access method. /// diff --git a/GRDB/Core/Database.swift b/GRDB/Core/Database.swift index 7d1e6c2f8a..ec881c14e0 100644 --- a/GRDB/Core/Database.swift +++ b/GRDB/Core/Database.swift @@ -1282,14 +1282,10 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib /// Use ``inSavepoint(_:)`` instead. /// /// - parameters: - /// - kind: The transaction type (default nil). + /// - kind: The transaction type. /// - /// If nil, and the database connection is read-only, the transaction - /// kind is ``TransactionKind/deferred``. - /// - /// If nil, and the database connection is not read-only, the - /// transaction kind is the ``Configuration/defaultTransactionKind`` - /// of the ``configuration``. + /// If nil, the transaction kind is DEFERRED when the current + /// database access is read-only, and IMMEDIATE otherwise. /// - operations: A function that executes SQL statements and returns /// either ``TransactionCompletion/commit`` or ``TransactionCompletion/rollback``. /// - throws: A ``DatabaseError`` whenever an SQLite error occurs, or the @@ -1413,8 +1409,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib // By default, top level SQLite savepoints open a // deferred transaction. // - // But GRDB database configuration mandates a default transaction - // kind that we have to honor. + // But GRDB prefers immediate transactions for writes. // // Besides, starting some (?) SQLCipher/SQLite version, SQLite has a // bug. Returning 1 from `sqlite3_commit_hook` does not leave the @@ -1502,18 +1497,22 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib /// Related SQLite documentation: /// /// - parameters: - /// - kind: The transaction type (default nil). - /// - /// If nil, and the database connection is read-only, the transaction - /// kind is ``TransactionKind/deferred``. + /// - kind: The transaction type. /// - /// If nil, and the database connection is not read-only, the - /// transaction kind is the ``Configuration/defaultTransactionKind`` - /// of the ``configuration``. + /// If nil, the transaction kind is DEFERRED when the current + /// database access is read-only, and IMMEDIATE otherwise. /// - throws: A ``DatabaseError`` whenever an SQLite error occurs. public func beginTransaction(_ kind: TransactionKind? = nil) throws { // SQLite throws an error for non-deferred transactions when read-only. - let kind = kind ?? (isReadOnly ? .deferred : configuration.defaultTransactionKind) + // We prefer immediate transactions for writes, so that write + // transactions can not overlap. This reduces the opportunity for + // SQLITE_BUSY, which is immediately thrown whenever a transaction + // is upgraded after an initial read and a concurrent processes + // has acquired the write lock beforehand. This SQLITE_BUSY error + // can not be avoided with a busy timeout. + // + // See . + let kind = kind ?? (isReadOnly ? .deferred : .immediate) try execute(sql: "BEGIN \(kind.rawValue) TRANSACTION") assert(sqlite3_get_autocommit(sqliteConnection) == 0) } diff --git a/GRDB/Core/DatabasePool.swift b/GRDB/Core/DatabasePool.swift index 34a9e7fdeb..771f37038d 100644 --- a/GRDB/Core/DatabasePool.swift +++ b/GRDB/Core/DatabasePool.swift @@ -118,10 +118,6 @@ public final class DatabasePool { configuration.readonly = true - // Readers use deferred transactions by default. - // Other transaction kinds are forbidden by SQLite in read-only connections. - configuration.defaultTransactionKind = .deferred - // // > But there are some obscure cases where a query against a WAL-mode // > database can return SQLITE_BUSY, so applications should be prepared @@ -787,9 +783,10 @@ extension DatabasePool: DatabaseWriter { /// /// - precondition: This method is not reentrant. /// - parameters: - /// - kind: The transaction type (default nil). If nil, the transaction - /// type is the ``Configuration/defaultTransactionKind`` of the - /// the ``configuration``. + /// - kind: The transaction type. + /// + /// If nil, the transaction kind is DEFERRED when the database + /// connection is read-only, and IMMEDIATE otherwise. /// - updates: A function that updates the database. /// - throws: The error thrown by `updates`, or by the wrapping transaction. public func writeInTransaction( diff --git a/GRDB/Core/DatabaseQueue.swift b/GRDB/Core/DatabaseQueue.swift index ae763f694c..0e43acc422 100644 --- a/GRDB/Core/DatabaseQueue.swift +++ b/GRDB/Core/DatabaseQueue.swift @@ -352,9 +352,10 @@ extension DatabaseQueue: DatabaseWriter { /// ``` /// /// - parameters: - /// - kind: The transaction type (default nil). If nil, the transaction - /// type is the ``Configuration/defaultTransactionKind`` of the - /// the ``configuration``. + /// - kind: The transaction type. + /// + /// If nil, the transaction kind is DEFERRED when the database + /// connection is read-only, and IMMEDIATE otherwise. /// - updates: A function that updates the database. /// - throws: The error thrown by `updates`, or by the wrapping transaction. public func inTransaction( diff --git a/GRDB/Core/DatabaseSnapshot.swift b/GRDB/Core/DatabaseSnapshot.swift index 1381c1871c..197771b24a 100644 --- a/GRDB/Core/DatabaseSnapshot.swift +++ b/GRDB/Core/DatabaseSnapshot.swift @@ -126,10 +126,6 @@ public final class DatabaseSnapshot { // DatabaseSnapshot is read-only. configuration.readonly = true - // DatabaseSnapshot uses deferred transactions by default. - // Other transaction kinds are forbidden by SQLite in read-only connections. - configuration.defaultTransactionKind = .deferred - // DatabaseSnapshot keeps a long-lived transaction. configuration.allowsUnsafeTransactions = true diff --git a/GRDB/Core/DatabaseSnapshotPool.swift b/GRDB/Core/DatabaseSnapshotPool.swift index d17f672404..525fa1c745 100644 --- a/GRDB/Core/DatabaseSnapshotPool.swift +++ b/GRDB/Core/DatabaseSnapshotPool.swift @@ -242,10 +242,6 @@ public final class DatabaseSnapshotPool { // DatabaseSnapshotPool is read-only. configuration.readonly = true - // DatabaseSnapshotPool uses deferred transactions by default. - // Other transaction kinds are forbidden by SQLite in read-only connections. - configuration.defaultTransactionKind = .deferred - // DatabaseSnapshotPool keeps a long-lived transaction. configuration.allowsUnsafeTransactions = true diff --git a/GRDB/Documentation.docc/DatabaseSharing.md b/GRDB/Documentation.docc/DatabaseSharing.md index 50b9569f4c..29dd6b9b68 100644 --- a/GRDB/Documentation.docc/DatabaseSharing.md +++ b/GRDB/Documentation.docc/DatabaseSharing.md @@ -152,12 +152,11 @@ If several processes want to write in the database, configure the database pool ```swift var configuration = Configuration() -configuration.defaultTransactionKind = .immediate configuration.busyMode = .timeout(/* a TimeInterval */) let dbPool = try DatabasePool(path: ..., configuration: configuration) ``` -Both the `defaultTransactionKind` and `busyMode` are important for preventing `SQLITE_BUSY`. The `immediate` transaction kind prevents write transactions from overlapping, and the busy timeout has write transactions wait, instead of throwing `SQLITE_BUSY`, whenever another process is writing. +The busy timeout has write transactions wait, instead of throwing `SQLITE_BUSY`, whenever another process is writing. GRDB automatically opens all write transactions with the IMMEDIATE kind, preventing write transactions from overlapping. With such a setup, you will still get `SQLITE_BUSY` errors if the database remains locked by another process for longer than the specified timeout. You can catch those errors: diff --git a/GRDB/Documentation.docc/Extension/Configuration.md b/GRDB/Documentation.docc/Extension/Configuration.md index efb77b8a62..55f0d770d8 100644 --- a/GRDB/Documentation.docc/Extension/Configuration.md +++ b/GRDB/Documentation.docc/Extension/Configuration.md @@ -90,7 +90,6 @@ do { ### Configuring GRDB Connections - ``allowsUnsafeTransactions`` -- ``defaultTransactionKind`` - ``label`` - ``maximumReaderCount`` - ``observesSuspensionNotifications`` diff --git a/GRDB/Documentation.docc/Transactions.md b/GRDB/Documentation.docc/Transactions.md index 56dedce47b..2067a1f096 100644 --- a/GRDB/Documentation.docc/Transactions.md +++ b/GRDB/Documentation.docc/Transactions.md @@ -214,6 +214,8 @@ SQLite savepoints are more than nested transactions, though. For advanced uses, SQLite supports [three kinds of transactions](https://www.sqlite.org/lang_transaction.html): deferred (the default), immediate, and exclusive. +By default, GRDB opens DEFERRED transaction for reads, and IMMEDIATE transactions for writes. + The transaction kind can be chosen for individual transaction: ```swift @@ -222,20 +224,3 @@ let dbQueue = try DatabaseQueue(path: "/path/to/database.sqlite") // BEGIN EXCLUSIVE TRANSACTION ... try dbQueue.inTransaction(.exclusive) { db in ... } ``` - -It is also possible to configure the ``Configuration/defaultTransactionKind``: - -```swift -var config = Configuration() -config.defaultTransactionKind = .immediate - -let dbQueue = try DatabaseQueue( - path: "/path/to/database.sqlite", - configuration: config) - -// BEGIN IMMEDIATE TRANSACTION ... -try dbQueue.write { db in ... } - -// BEGIN IMMEDIATE TRANSACTION ... -try dbQueue.inTransaction { db in ... } -``` diff --git a/TODO.md b/TODO.md index 941eae011b..11e7d0d93e 100644 --- a/TODO.md +++ b/TODO.md @@ -90,7 +90,7 @@ - [X] GRDB7/BREAKING: insertAndFetch, saveAndFetch, and updateAndFetch no longer return optionals (32f41472) - [ ] GRDB7/BREAKING: AsyncValueObservation does not need any scheduler (83c0e643) - [X] GRDB7/BREAKING: Stop exporting SQLite (679d6463) -- [ ] GRDB7/BREAKING: Remove Configuration.defaultTransactionKind (2661ff46) +- [X] GRDB7/BREAKING: Remove Configuration.defaultTransactionKind (2661ff46) - [ ] GRDB7: Replace LockedBox with Mutex (00ccab06) - [ ] GRDB7: Sendable: BusyCallback (e0d8e20b) - [ ] GRDB7: Sendable: BusyMode (e0d8e20b) diff --git a/Tests/GRDBTests/BackupTestCase.swift b/Tests/GRDBTests/BackupTestCase.swift index 33b2f1ac6a..4ea5d049fd 100644 --- a/Tests/GRDBTests/BackupTestCase.swift +++ b/Tests/GRDBTests/BackupTestCase.swift @@ -84,7 +84,7 @@ class BackupTestCase: GRDBTestCase { let sourceDbPageCount = try setupBackupSource(source) try setupBackupDestination(destination) - try source.write { sourceDb in + try source.read { sourceDb in try destination.barrierWriteWithoutTransaction { destDb in XCTAssertThrowsError( try sourceDb.backup(to: destDb, pagesPerStep: 1) { progress in @@ -102,7 +102,7 @@ class BackupTestCase: GRDBTestCase { XCTAssertEqual(try Int.fetchOne(db, sql: "SELECT id FROM items")!, 1) } - try source.write { dbSource in + try source.read { dbSource in try destination.barrierWriteWithoutTransaction { dbDest in var progressCount: Int = 1 var isCompleted: Bool = false diff --git a/Tests/GRDBTests/DatabaseQueueTests.swift b/Tests/GRDBTests/DatabaseQueueTests.swift index 7bd5adf2b5..66f4cd5554 100644 --- a/Tests/GRDBTests/DatabaseQueueTests.swift +++ b/Tests/GRDBTests/DatabaseQueueTests.swift @@ -369,8 +369,6 @@ class DatabaseQueueTests: GRDBTestCase { func test_busy_timeout_and_IMMEDIATE_transactions_do_prevent_SQLITE_BUSY() throws { var configuration = dbConfiguration! // Test fails when this line is commented - configuration.defaultTransactionKind = .immediate - // Test fails when this line is commented configuration.busyMode = .timeout(10) let dbQueue = try makeDatabaseQueue(filename: "test") diff --git a/Tests/GRDBTests/DatabaseSavepointTests.swift b/Tests/GRDBTests/DatabaseSavepointTests.swift index 1483d1a682..bc727f28eb 100644 --- a/Tests/GRDBTests/DatabaseSavepointTests.swift +++ b/Tests/GRDBTests/DatabaseSavepointTests.swift @@ -96,226 +96,8 @@ class DatabaseSavepointTests: GRDBTestCase { XCTAssertThrowsError(try db.execute(sql: "COMMIT")) } } - - func testReleaseTopLevelSavepointFromDatabaseWithDefaultDeferredTransactions() throws { - dbConfiguration.defaultTransactionKind = .deferred - let dbQueue = try makeDatabaseQueue() - let observer = Observer() - dbQueue.add(transactionObserver: observer) - sqlQueries.removeAll() - try dbQueue.writeWithoutTransaction { db in - try insertItem(db, name: "item1") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item2") - return .commit - } - XCTAssertFalse(db.isInsideTransaction) - try insertItem(db, name: "item3") - } - - XCTAssertEqual(sqlQueries, [ - "INSERT INTO items (name) VALUES ('item1')", - "BEGIN DEFERRED TRANSACTION", - "INSERT INTO items (name) VALUES ('item2')", - "COMMIT TRANSACTION", - "INSERT INTO items (name) VALUES ('item3')" - ]) - XCTAssertEqual(try fetchAllItemNames(dbQueue), ["item1", "item2", "item3"]) - XCTAssertEqual(observer.allRecordedEvents.count, 3) - #if SQLITE_ENABLE_PREUPDATE_HOOK - XCTAssertEqual(observer.allRecordedPreUpdateEvents.count, 3) - #endif - } - - func testRollbackTopLevelSavepointFromDatabaseWithDefaultDeferredTransactions() throws { - dbConfiguration.defaultTransactionKind = .deferred - let dbQueue = try makeDatabaseQueue() - let observer = Observer() - dbQueue.add(transactionObserver: observer) - sqlQueries.removeAll() - try dbQueue.writeWithoutTransaction { db in - try insertItem(db, name: "item1") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item2") - return .rollback - } - XCTAssertFalse(db.isInsideTransaction) - try insertItem(db, name: "item3") - } - XCTAssertEqual(sqlQueries, [ - "INSERT INTO items (name) VALUES ('item1')", - "BEGIN DEFERRED TRANSACTION", - "INSERT INTO items (name) VALUES ('item2')", - "ROLLBACK TRANSACTION", - "INSERT INTO items (name) VALUES ('item3')" - ]) - XCTAssertEqual(try fetchAllItemNames(dbQueue), ["item1", "item3"]) - XCTAssertEqual(observer.allRecordedEvents.count, 3) - #if SQLITE_ENABLE_PREUPDATE_HOOK - XCTAssertEqual(observer.allRecordedPreUpdateEvents.count, 3) - #endif - } - - func testNestedSavepointFromDatabaseWithDefaultDeferredTransactions() throws { - dbConfiguration.defaultTransactionKind = .deferred - let dbQueue = try makeDatabaseQueue() - let observer = Observer() - dbQueue.add(transactionObserver: observer) - sqlQueries.removeAll() - try dbQueue.writeWithoutTransaction { db in - try insertItem(db, name: "item1") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item2") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item3") - return .commit - } - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item4") - return .commit - } - XCTAssertFalse(db.isInsideTransaction) - try insertItem(db, name: "item5") - } - XCTAssertEqual(sqlQueries, [ - "INSERT INTO items (name) VALUES ('item1')", - "BEGIN DEFERRED TRANSACTION", - "INSERT INTO items (name) VALUES ('item2')", - "SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item3')", - "RELEASE SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item4')", - "COMMIT TRANSACTION", - "INSERT INTO items (name) VALUES ('item5')" - ]) - XCTAssertEqual(try fetchAllItemNames(dbQueue), ["item1", "item2", "item3", "item4", "item5"]) - XCTAssertEqual(observer.allRecordedEvents.count, 5) - #if SQLITE_ENABLE_PREUPDATE_HOOK - XCTAssertEqual(observer.allRecordedPreUpdateEvents.count, 5) - #endif - try dbQueue.inDatabase { db in try db.execute(sql: "DELETE FROM items") } - observer.reset() - - sqlQueries.removeAll() - try dbQueue.writeWithoutTransaction { db in - try insertItem(db, name: "item1") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item2") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item3") - return .commit - } - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item4") - return .rollback - } - XCTAssertFalse(db.isInsideTransaction) - try insertItem(db, name: "item5") - } - XCTAssertEqual(sqlQueries, [ - "INSERT INTO items (name) VALUES ('item1')", - "BEGIN DEFERRED TRANSACTION", - "INSERT INTO items (name) VALUES ('item2')", - "SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item3')", - "RELEASE SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item4')", - "ROLLBACK TRANSACTION", - "INSERT INTO items (name) VALUES ('item5')" - ]) - XCTAssertEqual(try fetchAllItemNames(dbQueue), ["item1", "item5"]) - XCTAssertEqual(observer.allRecordedEvents.count, 5) - #if SQLITE_ENABLE_PREUPDATE_HOOK - XCTAssertEqual(observer.allRecordedPreUpdateEvents.count, 5) - #endif - try dbQueue.inDatabase { db in try db.execute(sql: "DELETE FROM items") } - observer.reset() - - sqlQueries.removeAll() - try dbQueue.writeWithoutTransaction { db in - try insertItem(db, name: "item1") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item2") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item3") - return .rollback - } - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item4") - return .commit - } - XCTAssertFalse(db.isInsideTransaction) - try insertItem(db, name: "item5") - } - XCTAssertEqual(sqlQueries, [ - "INSERT INTO items (name) VALUES ('item1')", - "BEGIN DEFERRED TRANSACTION", - "INSERT INTO items (name) VALUES ('item2')", - "SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item3')", - "ROLLBACK TRANSACTION TO SAVEPOINT grdb", - "RELEASE SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item4')", - "COMMIT TRANSACTION", - "INSERT INTO items (name) VALUES ('item5')" - ]) - XCTAssertEqual(try fetchAllItemNames(dbQueue), ["item1", "item2", "item4", "item5"]) - XCTAssertEqual(observer.allRecordedEvents.count, 4) - #if SQLITE_ENABLE_PREUPDATE_HOOK - XCTAssertEqual(observer.allRecordedPreUpdateEvents.count, 4) - #endif - try dbQueue.inDatabase { db in try db.execute(sql: "DELETE FROM items") } - observer.reset() - - sqlQueries.removeAll() - try dbQueue.writeWithoutTransaction { db in - try insertItem(db, name: "item1") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item2") - try db.inSavepoint { - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item3") - return .rollback - } - XCTAssertTrue(db.isInsideTransaction) - try insertItem(db, name: "item4") - return .rollback - } - XCTAssertFalse(db.isInsideTransaction) - try insertItem(db, name: "item5") - } - XCTAssertEqual(sqlQueries, [ - "INSERT INTO items (name) VALUES ('item1')", - "BEGIN DEFERRED TRANSACTION", - "INSERT INTO items (name) VALUES ('item2')", - "SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item3')", - "ROLLBACK TRANSACTION TO SAVEPOINT grdb", - "RELEASE SAVEPOINT grdb", - "INSERT INTO items (name) VALUES ('item4')", - "ROLLBACK TRANSACTION", - "INSERT INTO items (name) VALUES ('item5')" - ]) - XCTAssertEqual(try fetchAllItemNames(dbQueue), ["item1", "item5"]) - XCTAssertEqual(observer.allRecordedEvents.count, 4) - #if SQLITE_ENABLE_PREUPDATE_HOOK - XCTAssertEqual(observer.allRecordedPreUpdateEvents.count, 4) - #endif - try dbQueue.inDatabase { db in try db.execute(sql: "DELETE FROM items") } - observer.reset() - } - - func testReleaseTopLevelSavepointFromDatabaseWithDefaultImmediateTransactions() throws { - dbConfiguration.defaultTransactionKind = .immediate + + func testReleaseTopLevelSavepoint() throws { let dbQueue = try makeDatabaseQueue() let observer = Observer() dbQueue.add(transactionObserver: observer) @@ -344,8 +126,7 @@ class DatabaseSavepointTests: GRDBTestCase { #endif } - func testRollbackTopLevelSavepointFromDatabaseWithDefaultImmediateTransactions() throws { - dbConfiguration.defaultTransactionKind = .immediate + func testRollbackTopLevelSavepoint() throws { let dbQueue = try makeDatabaseQueue() let observer = Observer() dbQueue.add(transactionObserver: observer) @@ -374,8 +155,7 @@ class DatabaseSavepointTests: GRDBTestCase { #endif } - func testNestedSavepointFromDatabaseWithDefaultImmediateTransactions() throws { - dbConfiguration.defaultTransactionKind = .immediate + func testNestedSavepoint() throws { let dbQueue = try makeDatabaseQueue() let observer = Observer() dbQueue.add(transactionObserver: observer) diff --git a/Tests/GRDBTests/DatabaseSnapshotPoolTests.swift b/Tests/GRDBTests/DatabaseSnapshotPoolTests.swift index 6a58ff4056..0caf342139 100644 --- a/Tests/GRDBTests/DatabaseSnapshotPoolTests.swift +++ b/Tests/GRDBTests/DatabaseSnapshotPoolTests.swift @@ -42,7 +42,14 @@ final class DatabaseSnapshotPoolTests: GRDBTestCase { let dbPool = try makeDatabasePool() let counter = try Counter(dbPool: dbPool) // 0 try dbPool.write(counter.increment) // 1 - let snapshot = try dbPool.write { db in try DatabaseSnapshotPool(db) } // locked at 1 + // We can't open a DatabaseSnapshotPool from an IMMEDIATE + // transaction (as documented by sqlite3_snapshot_get). So we + // force a DEFERRED transaction: + var snapshot: DatabaseSnapshotPool! + try dbPool.writeInTransaction(.deferred) { db in + snapshot = try DatabaseSnapshotPool(db) // locked at 1 + return .commit + } try dbPool.write(counter.increment) // 2 try XCTAssertEqual(dbPool.read(counter.value), 2) @@ -55,7 +62,9 @@ final class DatabaseSnapshotPoolTests: GRDBTestCase { let dbPool = try makeDatabasePool() let counter = try Counter(dbPool: dbPool) // 0 try dbPool.write(counter.increment) // 1 - let snapshot = try dbPool.writeWithoutTransaction { db in try DatabaseSnapshotPool(db) } // locked at 1 + let snapshot = try dbPool.writeWithoutTransaction { db in + try DatabaseSnapshotPool(db) // locked at 1 + } try dbPool.write(counter.increment) // 2 try XCTAssertEqual(dbPool.read(counter.value), 2) diff --git a/Tests/GRDBTests/DatabaseSuspensionTests.swift b/Tests/GRDBTests/DatabaseSuspensionTests.swift index eea8a31c7e..ce205c6cf5 100644 --- a/Tests/GRDBTests/DatabaseSuspensionTests.swift +++ b/Tests/GRDBTests/DatabaseSuspensionTests.swift @@ -507,7 +507,7 @@ class DatabaseSuspensionTests : GRDBTestCase { try db.execute(sql: "SELECT * FROM sqlite_master") XCTAssertEqual(db.journalModeCache, "wal") } - try dbPool.write { db in + dbPool.writeWithoutTransaction { db in XCTAssertEqual(db.journalModeCache, "wal") } try dbPool.read { db in diff --git a/Tests/GRDBTests/DatabaseTests.swift b/Tests/GRDBTests/DatabaseTests.swift index 145340ab3c..064deb00a3 100644 --- a/Tests/GRDBTests/DatabaseTests.swift +++ b/Tests/GRDBTests/DatabaseTests.swift @@ -505,16 +505,54 @@ class DatabaseTests : GRDBTestCase { try dbQueue.inTransaction { db in .commit } } - func testExplicitTransactionManagement() throws { + func testImplicitTransactionManagement() throws { let dbQueue = try makeDatabaseQueue() + try dbQueue.read { db in + XCTAssertEqual(lastSQLQuery, "BEGIN DEFERRED TRANSACTION") + } + + try dbQueue.write { db in + XCTAssertEqual(lastSQLQuery, "BEGIN IMMEDIATE TRANSACTION") + } + try dbQueue.writeWithoutTransaction { db in try db.beginTransaction() - XCTAssertEqual(lastSQLQuery, "BEGIN DEFERRED TRANSACTION") + XCTAssertEqual(lastSQLQuery, "BEGIN IMMEDIATE TRANSACTION") try db.rollback() XCTAssertEqual(lastSQLQuery, "ROLLBACK TRANSACTION") - try db.beginTransaction(.immediate) - XCTAssertEqual(lastSQLQuery, "BEGIN IMMEDIATE TRANSACTION") + + try db.inSavepoint { + XCTAssertEqual(lastSQLQuery, "BEGIN IMMEDIATE TRANSACTION") + return .commit + } + XCTAssertEqual(lastSQLQuery, "COMMIT TRANSACTION") + + try db.readOnly { + try db.beginTransaction() + XCTAssertEqual(lastSQLQuery, "BEGIN DEFERRED TRANSACTION") + try db.rollback() + XCTAssertEqual(lastSQLQuery, "ROLLBACK TRANSACTION") + + try db.inSavepoint { + XCTAssertEqual(lastSQLQuery, "BEGIN DEFERRED TRANSACTION") + return .rollback + } + XCTAssertEqual(lastSQLQuery, "ROLLBACK TRANSACTION") + } + } + } + + func testExplicitTransactionManagement() throws { + let dbQueue = try makeDatabaseQueue() + + try dbQueue.writeWithoutTransaction { db in + try db.beginTransaction(.deferred) + XCTAssertEqual(lastSQLQuery, "BEGIN DEFERRED TRANSACTION") + try db.commit() + XCTAssertEqual(lastSQLQuery, "COMMIT TRANSACTION") + try db.beginTransaction(.exclusive) + XCTAssertEqual(lastSQLQuery, "BEGIN EXCLUSIVE TRANSACTION") try db.commit() XCTAssertEqual(lastSQLQuery, "COMMIT TRANSACTION") } @@ -561,7 +599,6 @@ class DatabaseTests : GRDBTestCase { } func testReadOnlyTransaction() throws { - dbConfiguration.defaultTransactionKind = .immediate let dbQueue = try makeDatabaseQueue() try dbQueue.inDatabase { db in do { From d96dc2d328830bdd61e3279fd292c62d542e58de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gwendal=20Roue=CC=81?= Date: Sun, 25 Aug 2024 08:47:58 +0200 Subject: [PATCH 2/3] Aim at debugging failing CI test --- Tests/GRDBTests/ValueObservationTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/GRDBTests/ValueObservationTests.swift b/Tests/GRDBTests/ValueObservationTests.swift index 325fb10c32..2d5b77e70b 100644 --- a/Tests/GRDBTests/ValueObservationTests.swift +++ b/Tests/GRDBTests/ValueObservationTests.swift @@ -1095,7 +1095,7 @@ class ValueObservationTests: GRDBTestCase { try Table("t").fetchCount($0) } - let initialValueExpectation = self.expectation(description: "") + let initialValueExpectation = self.expectation(description: "initialValue") #if SQLITE_ENABLE_SNAPSHOT || (!GRDBCUSTOMSQLITE && !GRDBCIPHER) initialValueExpectation.assertForOverFulfill = true #else @@ -1104,7 +1104,7 @@ class ValueObservationTests: GRDBTestCase { #endif initialValueExpectation.expectedFulfillmentCount = observationCount - let secondValueExpectation = self.expectation(description: "") + let secondValueExpectation = self.expectation(description: "secondValue") secondValueExpectation.expectedFulfillmentCount = observationCount var cancellables: [AnyDatabaseCancellable] = [] From 1c368ed18d34aeca2e236997688ea8284b302c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gwendal=20Roue=CC=81?= Date: Sun, 25 Aug 2024 10:39:38 +0200 Subject: [PATCH 3/3] Skip flaky test with SQLCipher 3 --- Tests/GRDBTests/ValueObservationTests.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/GRDBTests/ValueObservationTests.swift b/Tests/GRDBTests/ValueObservationTests.swift index 2d5b77e70b..326d536476 100644 --- a/Tests/GRDBTests/ValueObservationTests.swift +++ b/Tests/GRDBTests/ValueObservationTests.swift @@ -1083,6 +1083,13 @@ class ValueObservationTests: GRDBTestCase { // An attempt at finding a regression test for func testManyObservations() throws { + // TODO: Fix flaky test with SQLCipher 3 + #if GRDBCIPHER + if sqlite3_libversion_number() <= 3020001 { + throw XCTSkip("Skip flaky test with SQLCipher 3") + } + #endif + // We'll start many observations let observationCount = 100 dbConfiguration.maximumReaderCount = 5