diff --git a/apple/Packages/TableProCore/Sources/TableProDatabase/ConnectionManager.swift b/apple/Packages/TableProCore/Sources/TableProDatabase/ConnectionManager.swift index 196219fed0..c7fdd087a2 100644 --- a/apple/Packages/TableProCore/Sources/TableProDatabase/ConnectionManager.swift +++ b/apple/Packages/TableProCore/Sources/TableProDatabase/ConnectionManager.swift @@ -16,6 +16,7 @@ public final class ConnectionManager: @unchecked Sendable { private var blockingTeardowns: Set = [] private var attemptGenerations: [UUID: Int] = [:] private var lastTeardownId = 0 + private var sessionEndObservers: [@Sendable (UUID, any DatabaseDriver) -> Void] = [] public init( driverFactory: DriverFactory, @@ -213,9 +214,10 @@ public final class ConnectionManager: @unchecked Sendable { /// `retireTeardown` cannot clear an entry that is not installed yet. private func claimTeardown(for connectionId: UUID) -> ConnectionTeardown? { lock.lock() - defer { lock.unlock() } guard let session = sessions.removeValue(forKey: connectionId) else { - return teardowns[connectionId] + let running = teardowns[connectionId] + lock.unlock() + return running } lastTeardownId += 1 let teardown = ConnectionTeardown(id: lastTeardownId) @@ -232,9 +234,21 @@ public final class ConnectionManager: @unchecked Sendable { if session.driver.holdsSuspensionBlockingResource { blockingTeardowns.insert(connectionId) } + let observers = sessionEndObservers + lock.unlock() + // After the unlock: an observer may call back into the manager, and this lock is not recursive. + for observer in observers { + observer(connectionId, session.driver) + } return teardown } + public func observeSessionEnds(_ observer: @escaping @Sendable (UUID, any DatabaseDriver) -> Void) { + lock.lock() + defer { lock.unlock() } + sessionEndObservers.append(observer) + } + /// Runs before `ConnectionTeardown.finish()`, so a waiter it wakes already reads the cleared state. private func retireTeardown(_ teardownId: Int, for connectionId: UUID) { lock.lock() diff --git a/apple/Packages/TableProCore/Sources/TableProDatabase/DriverTransactionState.swift b/apple/Packages/TableProCore/Sources/TableProDatabase/DriverTransactionState.swift index 3128df9d5f..201355116a 100644 --- a/apple/Packages/TableProCore/Sources/TableProDatabase/DriverTransactionState.swift +++ b/apple/Packages/TableProCore/Sources/TableProDatabase/DriverTransactionState.swift @@ -4,5 +4,7 @@ public enum DriverTransactionState: Sendable, Equatable { case idle case explicitTransaction case implicitTransaction + // A statement failed inside the transaction: the server refuses everything until a ROLLBACK, and a COMMIT rolls back. + case abortedTransaction case unknown } diff --git a/apple/Packages/TableProCore/Sources/TableProDatabase/WriteTransactionPolicy.swift b/apple/Packages/TableProCore/Sources/TableProDatabase/WriteTransactionPolicy.swift index d7a53e025e..695da2b08a 100644 --- a/apple/Packages/TableProCore/Sources/TableProDatabase/WriteTransactionPolicy.swift +++ b/apple/Packages/TableProCore/Sources/TableProDatabase/WriteTransactionPolicy.swift @@ -8,9 +8,10 @@ public enum WriteTransactionPolicy { ) -> Bool { guard supportsTransactions else { return false } switch state { - case .idle, .implicitTransaction: + case .idle: return true - case .explicitTransaction: + // MySQL's START TRANSACTION commits the work an autocommit-off session holds, so the write joins it. + case .explicitTransaction, .implicitTransaction, .abortedTransaction: return false case .unknown: return statementCount > 1 diff --git a/apple/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift b/apple/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift index f8b5e12054..4006f68639 100644 --- a/apple/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift +++ b/apple/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift @@ -570,4 +570,72 @@ struct ConnectionManagerTests { #expect(ssh.tunnelledConnectionIds == [connection.id]) } + + @Test("A replaced or disconnected session reaches the observer with the driver it ended") + func sessionEndsReachTheObserver() async throws { + let factory = MockDriverFactory() + let manager = ConnectionManager(driverFactory: factory, secureStore: MockSecureStore()) + let connection = DatabaseConnection(name: "Test", type: DatabaseType(rawValue: "mock")) + let ended = EndedSessions() + manager.observeSessionEnds { id, driver in + ended.record(id, driver, stillListed: false) + } + + let first = MockDatabaseDriver() + factory.drivers["mock"] = first + _ = try await manager.connect(connection) + #expect(ended.records.isEmpty) + + let second = MockDatabaseDriver() + factory.drivers["mock"] = second + _ = try await manager.connect(connection) + await manager.disconnect(connection.id) + + let records = ended.records + let endedIds = records.map { $0.id } + #expect(endedIds == [connection.id, connection.id]) + #expect(records.first?.driver === first) + #expect(records.last?.driver === second) + } + + @Test("An observer that reads the manager back runs after the session is gone, outside the lock") + func observerCanReadTheManager() async throws { + let factory = MockDriverFactory() + factory.drivers["mock"] = MockDatabaseDriver() + let manager = ConnectionManager(driverFactory: factory, secureStore: MockSecureStore()) + let connection = DatabaseConnection(name: "Test", type: DatabaseType(rawValue: "mock")) + let ended = EndedSessions() + manager.observeSessionEnds { id, driver in + ended.record(id, driver, stillListed: manager.session(for: id) != nil) + } + + _ = try await manager.connect(connection) + await manager.disconnect(connection.id) + + let stillListed = ended.records.map { $0.stillListed } + #expect(stillListed == [false]) + } +} + +private final class EndedSessions: @unchecked Sendable { + struct Record { + let id: UUID + let driver: any DatabaseDriver + let stillListed: Bool + } + + private let lock = NSLock() + private var recorded: [Record] = [] + + var records: [Record] { + lock.lock() + defer { lock.unlock() } + return recorded + } + + func record(_ id: UUID, _ driver: any DatabaseDriver, stillListed: Bool) { + lock.lock() + defer { lock.unlock() } + recorded.append(Record(id: id, driver: driver, stillListed: stillListed)) + } } diff --git a/apple/Packages/TableProCore/Tests/TableProDatabaseTests/WriteTransactionTests.swift b/apple/Packages/TableProCore/Tests/TableProDatabaseTests/WriteTransactionTests.swift index 0b36c5efd4..12385dec7c 100644 --- a/apple/Packages/TableProCore/Tests/TableProDatabaseTests/WriteTransactionTests.swift +++ b/apple/Packages/TableProCore/Tests/TableProDatabaseTests/WriteTransactionTests.swift @@ -111,7 +111,7 @@ private final class ProtocolDefaultDriver: DatabaseDriver, @unchecked Sendable { struct WriteTransactionPolicyTests { @Test("A driver without transactions never opens one") func withoutTransactions() { - for state in [DriverTransactionState.idle, .explicitTransaction, .implicitTransaction, .unknown] { + for state in [DriverTransactionState.idle, .explicitTransaction, .implicitTransaction, .abortedTransaction, .unknown] { for count in 1...2 { #expect( WriteTransactionPolicy.opensTransaction( @@ -128,17 +128,31 @@ struct WriteTransactionPolicyTests { #expect(WriteTransactionPolicy.opensTransaction(supportsTransactions: true, state: .idle, statementCount: 2)) } - @Test("A transaction the server opened because autocommit is off still wraps and commits") - func implicitTransactionWraps() { + @Test("A transaction the server opened because autocommit is off is joined, since a BEGIN would commit it") + func implicitTransactionIsJoined() { #expect( WriteTransactionPolicy.opensTransaction( supportsTransactions: true, state: .implicitTransaction, statementCount: 1 - ) + ) == false ) #expect( WriteTransactionPolicy.opensTransaction( supportsTransactions: true, state: .implicitTransaction, statementCount: 2 - ) + ) == false + ) + } + + @Test("A transaction a failed statement aborted is joined, so the server's refusal reaches the caller") + func abortedTransactionIsJoined() { + #expect( + WriteTransactionPolicy.opensTransaction( + supportsTransactions: true, state: .abortedTransaction, statementCount: 1 + ) == false + ) + #expect( + WriteTransactionPolicy.opensTransaction( + supportsTransactions: true, state: .abortedTransaction, statementCount: 2 + ) == false ) } @@ -192,14 +206,27 @@ struct DatabaseDriverWriteTests { #expect(driver.events == [.execute("UPDATE t SET v = 1")]) } - @Test("A session with autocommit off is wrapped and committed") - func implicitTransactionWraps() async throws { + @Test("A session with autocommit off runs the batch inside the work it holds, with no BEGIN or COMMIT") + func implicitTransactionIsJoined() async throws { let driver = RecordingWriteDriver() driver.scriptedState = .implicitTransaction - _ = try await driver.executeWrite(["UPDATE t SET v = 1"]) + _ = try await driver.executeWrite(["INSERT a", "INSERT b"]) - #expect(driver.events == [.begin(.readWrite), .execute("UPDATE t SET v = 1"), .commit]) + #expect(driver.events == [.execute("INSERT a"), .execute("INSERT b")]) + } + + @Test("A session in an aborted transaction sends the statement bare and leaves the rollback to the user") + func abortedTransactionIsJoined() async throws { + let driver = RecordingWriteDriver() + driver.scriptedState = .abortedTransaction + driver.failingStatement = "UPDATE t SET v = 1" + + await #expect(throws: WriteFailure.self) { + _ = try await driver.executeWrite(["UPDATE t SET v = 1"]) + } + + #expect(driver.events == [.execute("UPDATE t SET v = 1")]) } @Test("An unknown state leaves a single statement bare and wraps a batch") diff --git a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift index f2aa64f37a..19219de1b6 100644 --- a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift +++ b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift @@ -579,14 +579,21 @@ public final class OracleCoreConnection: @unchecked Sendable { } } - private func recordSuccess(of role: OracleTransactionRole, on session: Int) { - state.withLock { $0.transaction.statementSucceeded(role, on: session) } + private func recordSuccess(of role: OracleTransactionRole, on session: Int) async { + var answer: Bool? + if role == .dataDefinition, holdsTransaction { + answer = await serverHoldsTransaction() + } + let serverHoldsTransaction = answer + state.withLock { + $0.transaction.statementSucceeded(role, on: session, serverHoldsTransaction: serverHoldsTransaction) + } } /// Called under the query gate, so the answer about the transaction comes from the connection the refused - /// `COMMIT` or `ROLLBACK` ran on. + /// statement ran on. private func recordFailure(of role: OracleTransactionRole) async { - guard role == .endsTransaction, holdsTransaction else { return } + guard role.mayEndTransactionWhenRefused, holdsTransaction else { return } let serverHoldsTransaction = await serverHoldsTransaction() state.withLock { $0.transaction.statementFailed(role, serverHoldsTransaction: serverHoldsTransaction) } } @@ -765,7 +772,7 @@ public final class OracleCoreConnection: @unchecked Sendable { let zones = await sessionZones(on: connection) return try await collectRows(statement, options: admitted.options, zones: zones, on: connection) } - recordSuccess(of: role, on: admitted.session) + await recordSuccess(of: role, on: admitted.session) forgetSessionZonesIfChanged(by: query) await queryGate.release() return result @@ -872,7 +879,7 @@ public final class OracleCoreConnection: @unchecked Sendable { let zones = await sessionZones(on: connection) try await streamRows(query, options: admitted.options, zones: zones, on: connection, continuation: continuation) } - recordSuccess(of: role, on: admitted.session) + await recordSuccess(of: role, on: admitted.session) forgetSessionZonesIfChanged(by: query) await queryGate.release() continuation.finish() diff --git a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleSessionTransaction.swift b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleSessionTransaction.swift index 9c5bb23cff..6961481637 100644 --- a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleSessionTransaction.swift +++ b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleSessionTransaction.swift @@ -8,7 +8,7 @@ import Foundation /// driver's commit flag and the server commits it in the same round trip. /// /// A transaction opens with ``open()``, which is what the app's own writes call, or with a statement that opens one -/// (``OracleTransactionRole/opensTransaction``). It ends with a `COMMIT` or `ROLLBACK`, whoever sends it. +/// (``OracleTransactionRole/opensTransaction``). It ends with a `COMMIT`, a `ROLLBACK` or DDL, whoever sends it. /// /// The transaction belongs to the connection its first statement ran on. A connection that closes takes the /// transaction with it, so a statement that finds the transaction's connection replaced fails with @@ -31,34 +31,40 @@ struct OracleSessionTransaction: Equatable, Sendable { return false case .opensTransaction, .endsTransaction: guard isOpen else { return false } - case .other: + case .dataDefinition, .other: guard isOpen else { return true } } try bind(to: session) return false } - /// A statement that opens a transaction opens it here only once Oracle has accepted it, and a `COMMIT` or - /// `ROLLBACK` ends it. - mutating func statementSucceeded(_ role: OracleTransactionRole, on session: Int) { + // DDL commits around itself, but DDL on a private temporary table can leave the transaction open: the server decides. + mutating func statementSucceeded( + _ role: OracleTransactionRole, + on session: Int, + serverHoldsTransaction: Bool? = nil + ) { switch role { case .opensTransaction: isOpen = true self.session = session case .endsTransaction: close() + case .dataDefinition: + if serverHoldsTransaction == false { close() } case .query, .other: break } } - /// A `COMMIT` or `ROLLBACK` the server refused ends the transaction only when the server no longer holds one: a - /// malformed one leaves it open, and a commit a deferred constraint rolled back (ORA-02091) ends it. + /// A `COMMIT`, `ROLLBACK` or DDL statement the server refused ends the transaction only when the server no longer + /// holds one: a malformed one leaves it open, a commit a deferred constraint rolled back (ORA-02091) ends it, and + /// so does DDL that failed after the commit Oracle runs before it. /// /// `serverHoldsTransaction` is nil when there was no live connection to ask. The transaction is then left as it /// is, and the next write finds out whether its connection survived. mutating func statementFailed(_ role: OracleTransactionRole, serverHoldsTransaction: Bool?) { - guard role == .endsTransaction, serverHoldsTransaction == false else { return } + guard role.mayEndTransactionWhenRefused, serverHoldsTransaction == false else { return } close() } diff --git a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleTransactionRole.swift b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleTransactionRole.swift index b0f322bffc..5860771ab8 100644 --- a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleTransactionRole.swift +++ b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleTransactionRole.swift @@ -15,8 +15,12 @@ enum OracleTransactionRole: Equatable, Sendable { /// `COMMIT`, or a `ROLLBACK` that is not to a savepoint. case endsTransaction - /// Everything else: DML, DDL, PL/SQL, `CALL`, `ROLLBACK TO`, and the `FORCE` forms of `COMMIT` and `ROLLBACK`, - /// which settle an in-doubt distributed transaction rather than the session's own. + // Oracle commits before and after DDL (SQL Reference, COMMIT). ALTER SESSION and ALTER SYSTEM are + // session and system control, which commit nothing, and the schema switch sends ALTER SESSION. + case dataDefinition + + /// Everything else: DML, PL/SQL, `CALL`, `ROLLBACK TO`, session and system control, and the `FORCE` forms of + /// `COMMIT` and `ROLLBACK`, which settle an in-doubt distributed transaction rather than the session's own. case other /// How much of a statement is read for its first words. A dump can hold a statement millions of characters long, @@ -39,11 +43,24 @@ enum OracleTransactionRole: Equatable, Sendable { case "ROLLBACK": let word = Self.skippingWork(&reader) self = word == "TO" || word == "FORCE" ? .other : .endsTransaction + case "CREATE", "DROP", "TRUNCATE", "RENAME", "GRANT", "REVOKE", "COMMENT", "AUDIT", "NOAUDIT", "ANALYZE", + "PURGE", "FLASHBACK": + self = .dataDefinition + case "ALTER": + let word = reader.nextWord() + self = word == "SESSION" || word == "SYSTEM" ? .other : .dataDefinition + case "ASSOCIATE", "DISASSOCIATE": + self = reader.nextWord() == "STATISTICS" ? .dataDefinition : .other default: self = .other } } + // A refused COMMIT, ROLLBACK or DDL statement may still have ended the transaction, so only the server can say. + var mayEndTransactionWhenRefused: Bool { + self == .endsTransaction || self == .dataDefinition + } + private static func skippingWork(_ reader: inout HeaderReader) -> String? { let word = reader.nextWord() guard word == "WORK" else { return word } diff --git a/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleSessionTransactionTests.swift b/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleSessionTransactionTests.swift index c5a325377d..a178506f34 100644 --- a/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleSessionTransactionTests.swift +++ b/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleSessionTransactionTests.swift @@ -132,6 +132,85 @@ struct OracleSessionTransactionTests { #expect(transaction.isOpen) } + @Test func testDataDefinitionOutsideATransactionCommitsAsItRuns() throws { + var transaction = OracleSessionTransaction() + #expect(try transaction.admit(.dataDefinition, on: 1)) + transaction.statementSucceeded(.dataDefinition, on: 1) + #expect(!transaction.isOpen) + } + + @Test func testDataDefinitionEndsTheTransactionOnceOracleRunsIt() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + #expect(!(try transaction.admit(.dataDefinition, on: 1))) + #expect(transaction.isOpen) + + transaction.statementSucceeded(.dataDefinition, on: 1, serverHoldsTransaction: false) + #expect(!transaction.isOpen) + #expect(try transaction.admit(.other, on: 1)) + } + + @Test func testDataDefinitionTheServerRanInsideTheTransactionKeepsItOpen() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + _ = try transaction.admit(.dataDefinition, on: 1) + + transaction.statementSucceeded(.dataDefinition, on: 1, serverHoldsTransaction: true) + #expect(transaction.isOpen) + #expect(!(try transaction.admit(.other, on: 1))) + } + + @Test func testDataDefinitionWithNoConnectionToAskLeavesTheTransaction() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + _ = try transaction.admit(.dataDefinition, on: 1) + + transaction.statementSucceeded(.dataDefinition, on: 1, serverHoldsTransaction: nil) + #expect(transaction.isOpen) + } + + @Test func testRefusedDataDefinitionEndsTheTransactionWhenTheServerHoldsNone() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + _ = try transaction.admit(.dataDefinition, on: 1) + transaction.statementFailed(.dataDefinition, serverHoldsTransaction: false) + #expect(!transaction.isOpen) + } + + @Test func testRefusedDataDefinitionLeavesTheTransactionOpenWhenTheServerStillHoldsIt() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + _ = try transaction.admit(.dataDefinition, on: 1) + transaction.statementFailed(.dataDefinition, serverHoldsTransaction: true) + #expect(transaction.isOpen) + #expect(!(try transaction.admit(.other, on: 1))) + } + + @Test func testRefusedDataDefinitionWithNoConnectionToAskLeavesTheTransaction() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + _ = try transaction.admit(.dataDefinition, on: 1) + transaction.statementFailed(.dataDefinition, serverHoldsTransaction: nil) + #expect(transaction.isOpen) + } + + @Test func testDataDefinitionOnAReplacedConnectionReportsTheTransactionLost() throws { + var transaction = OracleSessionTransaction() + transaction.open() + _ = try transaction.admit(.other, on: 1) + + if let error = #expect(throws: (any Error).self, performing: { try transaction.admit(.dataDefinition, on: 2) }) { + #expect(error as? OracleCoreError == .transactionLost) + } + #expect(!transaction.isOpen) + } + @Test func testOpeningAnOpenTransactionKeepsItsConnection() throws { var transaction = OracleSessionTransaction() transaction.open() diff --git a/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleTransactionRoleTests.swift b/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleTransactionRoleTests.swift index e54664037a..e9f07b54bb 100644 --- a/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleTransactionRoleTests.swift +++ b/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleTransactionRoleTests.swift @@ -64,13 +64,50 @@ struct OracleTransactionRoleTests { #expect(OracleTransactionRole(of: "SELECT \(values) FROM dual") == .query) } + @Test func testDataDefinitionIsReadAsTheCommitOracleRunsAroundIt() { + for sql in [ + "CREATE TABLE t (a NUMBER)", + "create or replace view v AS SELECT 1 a FROM dual", + "-- keep\nCREATE INDEX t_a ON t (a)", + "ALTER TABLE t ADD (b NUMBER)", + "alter index t_a rebuild", + "ALTER USER hr IDENTIFIED BY secret", + "DROP TABLE t PURGE", + "TRUNCATE TABLE logs", + "RENAME t TO t2", + "GRANT SELECT ON emp TO hr", + "REVOKE SELECT ON emp FROM hr", + "COMMENT ON TABLE emp IS 'staff'", + "AUDIT SELECT ON emp", + "NOAUDIT SELECT ON emp", + "ANALYZE TABLE emp COMPUTE STATISTICS", + "ASSOCIATE STATISTICS WITH PACKAGES emp_stats", + "DISASSOCIATE STATISTICS FROM PACKAGES emp_stats", + "PURGE RECYCLEBIN", + "FLASHBACK TABLE emp TO BEFORE DROP", + ] { + #expect(OracleTransactionRole(of: sql) == .dataDefinition, "\(sql)") + } + } + + @Test func testSessionAndSystemControlCommitNothing() { + for sql in [ + "ALTER SESSION SET CURRENT_SCHEMA = \"HR\"", + "alter session set nls_date_format = 'YYYY-MM-DD'", + "/* switch */ ALTER SESSION SET CURRENT_SCHEMA = hr", + "ALTER SYSTEM FLUSH SHARED_POOL", + "SET ROLE ALL", + ] { + #expect(OracleTransactionRole(of: sql) == .other, "\(sql)") + } + } + @Test func testEverythingElseIsOrdinaryWork() { for sql in [ "INSERT INTO emp VALUES (1)", "UPDATE emp SET sal = sal * 2", "DELETE FROM emp", "MERGE INTO emp USING dual ON (1 = 1) WHEN MATCHED THEN UPDATE SET sal = 1", - "CREATE TABLE t (a NUMBER)", "BEGIN NULL; END;", "DECLARE v NUMBER; BEGIN v := 1; END;", "CALL p()", diff --git a/apple/TableProMobile/TableProMobile/AppState.swift b/apple/TableProMobile/TableProMobile/AppState.swift index 70deca10ba..084bb272ba 100644 --- a/apple/TableProMobile/TableProMobile/AppState.swift +++ b/apple/TableProMobile/TableProMobile/AppState.swift @@ -30,6 +30,8 @@ final class AppState { private(set) var sampleResetRevision = 0 let onboarding: OnboardingPreferences let connectionManager: ConnectionManager + let sessionTransactions: SessionTransactionMonitor + let sessionRedialer: SessionRedialer let backgroundRelease: BackgroundReleaseCoordinator let queryActivities = QueryActivityController() let syncCoordinator: IOSSyncCoordinator @@ -56,7 +58,8 @@ final class AppState { sampleInstaller: SampleDatabaseInstaller = .live, localDatabaseFiles: LocalDatabaseFileLocator = .live, bookmarkStore: FileBookmarkStore = FileBookmarkStore(), - libraryPublisher: ConnectionLibraryPublisher? = nil + libraryPublisher: ConnectionLibraryPublisher? = nil, + driverFactory injectedDriverFactory: (any DriverFactory)? = nil ) { self.sampleInstaller = sampleInstaller self.libraryPublisher = libraryPublisher ?? .live() @@ -69,7 +72,8 @@ final class AppState { queryHistory = QueryHistoryStorage(directory: libraryDirectory) groupStorage = GroupPersistence(directory: libraryDirectory) tagStorage = TagPersistence(directory: libraryDirectory) - let driverFactory = IOSDriverFactory(bookmarkStore: bookmarkStore, localFiles: localDatabaseFiles) + let driverFactory: any DriverFactory = injectedDriverFactory + ?? IOSDriverFactory(bookmarkStore: bookmarkStore, localFiles: localDatabaseFiles) self.secureStore = secureStore let sshProvider = IOSSSHProvider(secureStore: secureStore, container: localDatabaseFiles.container) self.sshProvider = sshProvider @@ -79,6 +83,16 @@ final class AppState { sshProvider: sshProvider ) self.connectionManager = connectionManager + let sessionTransactions = SessionTransactionMonitor { [connectionManager] id in + connectionManager.session(for: id)?.driver + } + self.sessionTransactions = sessionTransactions + connectionManager.observeSessionEnds { [weak sessionTransactions] id, driver in + Task { @MainActor in + sessionTransactions?.sessionEnded(id, driver: driver) + } + } + self.sessionRedialer = SessionRedialer(connectionManager: connectionManager) self.backgroundRelease = BackgroundReleaseCoordinator(connectionManager: connectionManager) loadPersistedData() diff --git a/apple/TableProMobile/TableProMobile/Coordinators/ConnectionCoordinator.swift b/apple/TableProMobile/TableProMobile/Coordinators/ConnectionCoordinator.swift index e306a1f1d4..fa4fb42f78 100644 --- a/apple/TableProMobile/TableProMobile/Coordinators/ConnectionCoordinator.swift +++ b/apple/TableProMobile/TableProMobile/Coordinators/ConnectionCoordinator.swift @@ -21,6 +21,9 @@ final class ConnectionCoordinator { private(set) var isSwitching = false private(set) var isReconnecting = false + @ObservationIgnored private var sessionCheck: Task? + @ObservationIgnored private var statementFailedDuringCheck = false + @ObservationIgnored private var metadataRefused = false var failureAlertMessage: String? var showFailureAlert = false @@ -172,11 +175,14 @@ final class ConnectionCoordinator { tables = existingTables await loadDatabases() await loadSchemas() + await refreshTransaction() guard attemptToken == token else { return } phase = .connected appState.libraryPreferences.recordConnected(connection.id) return } catch { + guard attemptToken == token else { return } + if await keepsRefusingSession(existing, token: token, refusal: error) { return } guard attemptToken == token else { return } session = nil await appState.connectionManager.disconnect(connection.id) @@ -200,6 +206,7 @@ final class ConnectionCoordinator { tables = newTables await loadDatabases() await loadSchemas() + await refreshTransaction() guard attemptToken == token else { return } phase = .connected appState.libraryPreferences.recordConnected(connection.id) @@ -228,23 +235,71 @@ final class ConnectionCoordinator { } func reconnectIfNeeded() async { - guard let session, !isSwitching, !isReconnecting, connectTask == nil else { return } - do { - _ = try await session.driver.ping() - return - } catch { - // Ping failed; fall through to actual reconnect path below. + await restoreSession(after: .whileIdle) + } + + enum SessionLoss { + // Nothing was in flight, so the server rolled the transaction back. + case whileIdle + // A statement was in flight, and it may have committed the transaction before the reply was lost. + case duringWork + // The transaction is gone either way, which is what was asked for. + case duringRollBack + } + + // True when the redial dropped a held transaction and the outcome is already settled for the user. + @discardableResult + private func restoreSession(after loss: SessionLoss) async -> Bool { + // One check at a time, and a statement that failed while it ran makes the loss ambiguous. + if let sessionCheck { + if loss == .duringWork { statementFailedDuringCheck = true } + return await sessionCheck.value + } + statementFailedDuringCheck = false + let check = Task { + let settled = await checkSession(after: loss) + sessionCheck = nil + return settled } + sessionCheck = check + return await check.value + } + private func checkSession(after loss: SessionLoss) async -> Bool { + guard let session, !isSwitching, !isReconnecting, connectTask == nil else { return false } let token = attemptToken + var workWasRunning = appState.sessionTransactions.isBusy(connection.id) + + if await takesNewerSession(than: session, token: token) { return false } + if appState.connectionManager.session(for: connection.id) != nil, await Self.answersPing(session.driver) { + return false + } + guard attemptToken == token else { return false } + // Another window can finish a whole redial while this ping is out. + if await takesNewerSession(than: session, token: token) { return false } + + workWasRunning = workWasRunning || appState.sessionTransactions.isBusy(connection.id) + let heldTransaction = transactionHold != nil isReconnecting = true defer { isReconnecting = false } do { - let newSession = try await appState.connectionManager.connect(connection, prompter: prompts) - guard attemptToken == token else { return } - self.session = newSession + let outcome = try await appState.sessionRedialer.redial( + dialTarget, + schema: supportsSchemas ? activeSchema : nil, + prompter: prompts + ) + guard attemptToken == token else { return false } + await takeSession(outcome.session, token: token, dialedHere: outcome.dialedHere) + guard attemptToken == token, heldTransaction else { return false } + appState.sessionTransactions.transactionLost(connection.id) + let settledLoss: SessionLoss = workWasRunning || statementFailedDuringCheck ? .duringWork : loss + if let message = Self.lostTransactionMessage(settledLoss, connectionName: displayName) { + failureAlertMessage = message + showFailureAlert = true + } + return true } catch { - guard attemptToken == token else { return } + guard attemptToken == token else { return false } let context = ErrorContext( operation: "reconnect", databaseType: connection.type, @@ -253,7 +308,198 @@ final class ConnectionCoordinator { ) phase = .error(ErrorClassifier.classify(error, context: context)) self.session = nil + return false + } + } + + private func takesNewerSession(than held: ConnectionSession, token: UUID) async -> Bool { + // A redial another window started is not ready to share until its schema is back. + await appState.sessionRedialer.settled(connection.id) + guard attemptToken == token, + let live = appState.connectionManager.session(for: connection.id), + live.driver !== held.driver else { + return false + } + await takeSession(live, token: token, dialedHere: false) + return true + } + + private var dialTarget: DatabaseConnection { + guard supportsDatabaseSwitching, !activeDatabase.isEmpty else { return connection } + var target = connection + target.database = activeDatabase + return target + } + + // A session another window dialed sits on that window's database and schema, so this one + // follows it before any write can name a table from the old list. + private func takeSession(_ newSession: ConnectionSession, token: UUID, dialedHere: Bool) async { + let driver = newSession.driver + let fetched: [TableInfo]? + do { + fetched = try await driver.fetchTables(schema: nil) + } catch { + Self.logger.warning("Failed to load tables on the new session: \(error.localizedDescription, privacy: .public)") + fetched = nil + } + guard attemptToken == token else { return } + var movedPlacement = false + if !dialedHere { + let database = sessionDatabase(of: newSession) + if supportsDatabaseSwitching, !database.isEmpty, database != activeDatabase { + activeDatabase = database + movedPlacement = true + } + } + // The dial restored this window's schema, or another window's; a failed restore leaves the default. + if supportsSchemas, let schema = driver.currentSchema, schema != activeSchema { + activeSchema = schema + movedPlacement = true + } + if let fetched { + tables = fetched + } else { + metadataRefused = true + if movedPlacement { tables = [] } + } + session = newSession + await refreshTransaction() + } + + // A transaction the server aborted refuses catalog reads, yet the session still holds the user's work. + private func keepsRefusingSession(_ existing: ConnectionSession, token: UUID, refusal: any Error) async -> Bool { + guard await Self.answersPing(existing.driver), attemptToken == token else { return false } + await refreshTransaction() + guard attemptToken == token, + appState.sessionTransactions.hold(for: connection.id, on: existing.driver) != nil else { return false } + Self.logger.info("Kept a session that refused its catalog: \(refusal.localizedDescription, privacy: .public)") + let database = sessionDatabase(of: existing) + if supportsDatabaseSwitching, !database.isEmpty { + activeDatabase = database + } + metadataRefused = true + session = existing + phase = .connected + appState.libraryPreferences.recordConnected(connection.id) + return true + } + + private func sessionDatabase(of taken: ConnectionSession) -> String { + guard let live = appState.connectionManager.session(for: connection.id), live.driver === taken.driver else { + return taken.activeDatabase + } + return live.activeDatabase + } + + static func lostTransactionMessage(_ loss: SessionLoss, connectionName: String) -> String? { + switch loss { + case .whileIdle: + String( + format: String( + localized: "The connection to %@ was lost while it waited, and the server rolled back its open transaction. It has been reconnected; run the statements again." + ), + connectionName + ) + case .duringWork: + String( + format: String( + localized: "The connection to %@ was lost before the server answered, so its open transaction may have been committed or rolled back. It has been reconnected; check which of its changes were saved before running them again." + ), + connectionName + ) + case .duringRollBack: + nil + } + } + + private static func answersPing(_ driver: any DatabaseDriver) async -> Bool { + do { + _ = try await driver.ping() + return true + } catch { + return false + } + } + + // MARK: - Transactions + + var transactionHold: SessionTransactionMonitor.Hold? { + appState.sessionTransactions.hold(for: connection.id, on: session?.driver) + } + + var transactionToken: SessionTransactionMonitor.HoldToken? { + appState.sessionTransactions.token(for: connection.id, on: session?.driver) + } + + func track(failed: (T) -> Bool = { _ in false }, _ work: () async throws -> T) async rethrows -> T { + do { + let value = try await appState.sessionTransactions.track(connection.id, work) + if failed(value) { + await restoreSession(after: .duringWork) + } + await reloadMetadataIfReleased() + return value + } catch { + await restoreSession(after: .duringWork) + throw error + } + } + + // The token is what the indicator showed when the user asked, so a swapped session or newer work refuses. + func commitTransaction(as token: SessionTransactionMonitor.HoldToken) async throws { + do { + try await appState.sessionTransactions.commit(connection.id, as: token) + } catch { + guard await !restoreSession(after: .duringWork) else { return } + throw error + } + await reloadMetadataIfReleased() + } + + func rollBackTransaction(as token: SessionTransactionMonitor.HoldToken) async throws { + do { + try await appState.sessionTransactions.rollBack(connection.id, as: token) + } catch { + guard await !restoreSession(after: .duringRollBack) else { return } + throw error + } + await reloadMetadataIfReleased() + } + + func refreshTransaction() async { + await appState.sessionTransactions.refresh(connection.id) + } + + // A Roll Back from any window can undo DDL; a failed read keeps the list it has. + func reloadAfterRollBack() async { + guard !metadataRefused else { + await reloadMetadataIfReleased() + return + } + guard let session else { return } + do { + let reloaded = try await session.driver.fetchTables(schema: supportsSchemas ? activeSchema : nil) + guard self.session?.driver === session.driver else { return } + tables = reloaded + } catch { + Self.logger.warning("Failed to reload tables after a Roll Back: \(error.localizedDescription, privacy: .public)") + } + } + + private func reloadMetadataIfReleased() async { + guard metadataRefused, let session, + appState.sessionTransactions.hold(for: connection.id, on: session.driver) == nil else { return } + do { + let reloaded = try await session.driver.fetchTables(schema: nil) + guard self.session?.driver === session.driver else { return } + tables = reloaded + metadataRefused = false + } catch { + Self.logger.warning("Tables are still refused: \(error.localizedDescription, privacy: .public)") + return } + await loadDatabases() + await loadSchemas() } // MARK: - Database / Schema Switching @@ -281,6 +527,7 @@ final class ConnectionCoordinator { showFailureAlert = true } } + await refreshTransaction() } private func reconnectWithDatabase(_ database: String) async { @@ -349,6 +596,8 @@ final class ConnectionCoordinator { failureAlertMessage = String(localized: "Failed to refresh tables") showFailureAlert = true } + // With autocommit off, a MySQL read opens a transaction. + await refreshTransaction() } // MARK: - Query History diff --git a/apple/TableProMobile/TableProMobile/Coordinators/SessionRedialer.swift b/apple/TableProMobile/TableProMobile/Coordinators/SessionRedialer.swift new file mode 100644 index 0000000000..8c939e84ff --- /dev/null +++ b/apple/TableProMobile/TableProMobile/Coordinators/SessionRedialer.swift @@ -0,0 +1,58 @@ +import Foundation +import os +import TableProDatabase +import TableProModels + +// Each scene has its own coordinator on the one shared session, so two windows coming back at once +// would each dial, and the later attempt cancels the earlier one's. +@MainActor +final class SessionRedialer { + struct Outcome { + let session: ConnectionSession + let dialedHere: Bool + } + + private static let logger = Logger(subsystem: "com.TablePro", category: "SessionRedialer") + + private let connectionManager: ConnectionManager + private var dials: [UUID: Task] = [:] + + init(connectionManager: ConnectionManager) { + self.connectionManager = connectionManager + } + + // The schema goes back inside the dial, so a window that joins or adopts it never reads the default schema. + func redial( + _ connection: DatabaseConnection, + schema: String?, + prompter: (any ConnectionPrompter)? + ) async throws -> Outcome { + if let inFlight = dials[connection.id] { + return Outcome(session: try await inFlight.value, dialedHere: false) + } + let manager = connectionManager + let dial = Task { + let session = try await manager.connect(connection, prompter: prompter) + await Self.restore(schema, on: session.driver) + return session + } + dials[connection.id] = dial + defer { + if dials[connection.id] == dial { dials[connection.id] = nil } + } + return Outcome(session: try await dial.value, dialedHere: true) + } + + func settled(_ connectionId: UUID) async { + _ = try? await dials[connectionId]?.value + } + + private static func restore(_ schema: String?, on driver: any DatabaseDriver) async { + guard let schema, let current = driver.currentSchema, current != schema else { return } + do { + try await driver.switchSchema(to: schema) + } catch { + logger.warning("Failed to restore schema \(schema, privacy: .public): \(error.localizedDescription, privacy: .public)") + } + } +} diff --git a/apple/TableProMobile/TableProMobile/Drivers/DuckDBDriver.swift b/apple/TableProMobile/TableProMobile/Drivers/DuckDBDriver.swift index 65e0e3081e..46c12c2605 100644 --- a/apple/TableProMobile/TableProMobile/Drivers/DuckDBDriver.swift +++ b/apple/TableProMobile/TableProMobile/Drivers/DuckDBDriver.swift @@ -47,8 +47,9 @@ nonisolated final class DuckDBDriver: DatabaseDriver, @unchecked Sendable { fileAccess.end() } + // In-process, so an open handle is a live session; an aborted transaction refuses every statement until ROLLBACK. func ping() async throws -> Bool { - _ = try await actor.query("SELECT 1") + guard await actor.isOpen else { throw DuckDBDriverError.notConnected } return true } @@ -206,6 +207,8 @@ actor DuckDBActor { if let conn { interruptHandle.attach(conn) } } + var isOpen: Bool { database != nil && connection != nil } + func close() { interruptHandle.detach() closeStreamResult() diff --git a/apple/TableProMobile/TableProMobile/Drivers/MySQLDriver.swift b/apple/TableProMobile/TableProMobile/Drivers/MySQLDriver.swift index 08fa33ebd1..fca3ef02c7 100644 --- a/apple/TableProMobile/TableProMobile/Drivers/MySQLDriver.swift +++ b/apple/TableProMobile/TableProMobile/Drivers/MySQLDriver.swift @@ -314,7 +314,12 @@ nonisolated final class MySQLDriver: DatabaseDriver, @unchecked Sendable { } func sessionTransactionState() async -> DriverTransactionState { - await actor.transactionState() + guard Self.reportsSessionStatusFlags(for: databaseType, banner: serverVersion) else { return .unknown } + return await actor.transactionState() + } + + static func reportsSessionStatusFlags(for databaseType: DatabaseType, banner: String?) -> Bool { + serverFlavor(for: databaseType, banner: banner).reportsSessionStatusFlags } /// Databend reports no `NO_BACKSLASH_ESCAPES` flag, so its session says nothing. diff --git a/apple/TableProMobile/TableProMobile/Drivers/PostgreSQLDriver.swift b/apple/TableProMobile/TableProMobile/Drivers/PostgreSQLDriver.swift index 4dc014aa63..a82a6028ae 100644 --- a/apple/TableProMobile/TableProMobile/Drivers/PostgreSQLDriver.swift +++ b/apple/TableProMobile/TableProMobile/Drivers/PostgreSQLDriver.swift @@ -88,7 +88,7 @@ nonisolated final class PostgreSQLDriver: DatabaseDriver, @unchecked Sendable { } func ping() async throws -> Bool { - _ = try await actor.execute("SELECT 1") + try await actor.ping() return true } @@ -133,10 +133,13 @@ nonisolated final class PostgreSQLDriver: DatabaseDriver, @unchecked Sendable { let task = Task { do { switch try await actor.beginStream(query: query, id: id, readsOnly: readsOnly) { - case .commandOk(let affectedRows): + case .commandOk(let affectedRows, let statusMessage): if affectedRows != 0 { continuation.yield(.rowsAffected(affectedRows)) } + if let statusMessage { + continuation.yield(.statusMessage(statusMessage)) + } case .tuples(let columns): continuation.yield(.columns(columns)) var emitted = 0 @@ -258,16 +261,42 @@ nonisolated final class PostgreSQLDriver: DatabaseDriver, @unchecked Sendable { } nonisolated enum PostgreSQLSessionTransaction { - static func state(from status: PGTransactionStatusType) -> DriverTransactionState { + static func state( + from status: PGTransactionStatusType, + statusAtSend: PGTransactionStatusType? = nil + ) -> DriverTransactionState { switch status { case PQTRANS_IDLE: return .idle - case PQTRANS_INTRANS, PQTRANS_INERROR, PQTRANS_ACTIVE: + case PQTRANS_INTRANS: return .explicitTransaction + case PQTRANS_INERROR: + return .abortedTransaction + // A statement still in flight says nothing about the transaction around it; the status before it went out does. + case PQTRANS_ACTIVE: + guard let statusAtSend, statusAtSend != PQTRANS_ACTIVE else { return .unknown } + return state(from: statusAtSend) default: return .unknown } } + + // An aborted block refuses SELECT 1 on a live session, an open one takes its snapshot from it, and a stream would end. + static func pingSendsStatement(in status: PGTransactionStatusType) -> Bool { + status == PQTRANS_IDLE + } + + // A COMMIT in an aborted block answers the tag ROLLBACK with no error, so it would read as a success. + static func statusMessage( + statusAtSend: PGTransactionStatusType, + commandTag: String, + statusAfter: PGTransactionStatusType + ) -> String? { + guard statusAtSend == PQTRANS_INERROR, commandTag == "ROLLBACK", statusAfter == PQTRANS_IDLE else { + return nil + } + return String(localized: "The failed transaction was rolled back. None of its changes were kept.") + } } // MARK: - PostgreSQL Actor (thread-safe C API access) @@ -373,7 +402,31 @@ private actor PostgreSQLActor { func transactionState() -> DriverTransactionState { guard let conn else { return .unknown } - return PostgreSQLSessionTransaction.state(from: PQtransactionStatus(conn)) + return PostgreSQLSessionTransaction.state( + from: PQtransactionStatus(conn), + statusAtSend: stream?.transactionStatusAtSend + ) + } + + func ping() throws { + guard let conn else { throw PostgreSQLError.notConnected } + guard PostgreSQLSessionTransaction.pingSendsStatement(in: PQtransactionStatus(conn)) else { + try checkSocket(conn) + return + } + _ = try execute("SELECT 1") + } + + // Measured on the Mac driver: one read leaves a closed session CONNECTION_OK, the second turns it CONNECTION_BAD. + private func checkSocket(_ conn: OpaquePointer) throws { + if PQstatus(conn) == CONNECTION_OK { + _ = PQconsumeInput(conn) + _ = PQisBusy(conn) + _ = PQconsumeInput(conn) + } + guard PQstatus(conn) == CONNECTION_OK else { + throw PostgreSQLError.connectionFailed(Self.message(from: conn)) + } } /// libpq keeps the value the server last reported in a `ParameterStatus`, so a `SET` the user ran is already in it. @@ -494,7 +547,8 @@ private actor PostgreSQLActor { let state = PostgreSQLStreamState( id: id, cancelsAbandonedOutput: cancelsOutput, - cancelsUnreadRows: cancelsOutput && readsOnly + cancelsUnreadRows: cancelsOutput && readsOnly, + transactionStatusAtSend: PQtransactionStatus(conn) ) try send(query, id: id, on: conn) guard PQsetSingleRowMode(conn) == 1 else { @@ -503,7 +557,7 @@ private actor PostgreSQLActor { } guard let firstResult = PQgetResult(conn) else { markDrained(state) - return .commandOk(affectedRows: 0) + return .commandOk(affectedRows: 0, statusMessage: nil) } switch PQresultStatus(firstResult) { @@ -519,13 +573,21 @@ private actor PostgreSQLActor { return .tuples(columns) case PGRES_COMMAND_OK: let affected = Int(String(cString: PQcmdTuples(firstResult))) ?? 0 + let commandTag = String(cString: PQcmdStatus(firstResult)) PQclear(firstResult) if let error = drain(state).error { throw error } - return .commandOk(affectedRows: affected) + return .commandOk( + affectedRows: affected, + statusMessage: PostgreSQLSessionTransaction.statusMessage( + statusAtSend: state.transactionStatusAtSend, + commandTag: commandTag, + statusAfter: self.conn.map { PQtransactionStatus($0) } ?? PQTRANS_UNKNOWN + ) + ) default: state.pendingResult = firstResult if let error = drain(state).error { throw error } - return .commandOk(affectedRows: 0) + return .commandOk(affectedRows: 0, statusMessage: nil) } } @@ -746,7 +808,7 @@ private actor PostgreSQLActor { nonisolated enum PGBeginStreamResult: Sendable { case tuples([ColumnInfo]) - case commandOk(affectedRows: Int) + case commandOk(affectedRows: Int, statusMessage: String?) } /// Confined to `PostgreSQLActor`, which owns the connection the stream reads. @@ -757,14 +819,21 @@ nonisolated private final class PostgreSQLStreamState { /// A cancel rolls back the whole statement, so rows nobody will read are cut short only for a read: an /// `UPDATE ... RETURNING` past the row cap has to finish, or its write is undone. let cancelsUnreadRows: Bool + let transactionStatusAtSend: PGTransactionStatusType /// A result read before its turn: the first row, or what the row-cap check took off the wire. var pendingResult: OpaquePointer? var isDrained = false - init(id: UInt64, cancelsAbandonedOutput: Bool, cancelsUnreadRows: Bool) { + init( + id: UInt64, + cancelsAbandonedOutput: Bool, + cancelsUnreadRows: Bool, + transactionStatusAtSend: PGTransactionStatusType + ) { self.id = id self.cancelsAbandonedOutput = cancelsAbandonedOutput self.cancelsUnreadRows = cancelsUnreadRows + self.transactionStatusAtSend = transactionStatusAtSend } } diff --git a/apple/TableProMobile/TableProMobile/Helpers/ConnectionListRowModel.swift b/apple/TableProMobile/TableProMobile/Helpers/ConnectionListRowModel.swift index 26515ede43..72cfb18712 100644 --- a/apple/TableProMobile/TableProMobile/Helpers/ConnectionListRowModel.swift +++ b/apple/TableProMobile/TableProMobile/Helpers/ConnectionListRowModel.swift @@ -29,12 +29,14 @@ nonisolated struct ConnectionListRowModel: Hashable, Sendable { let isFavorite: Bool let showsFavoriteMark: Bool let isMacOnly: Bool + let holdsTransaction: Bool init( connection: DatabaseConnection, section: LibrarySectionKind, tags allTags: [ConnectionTag], - groups: [ConnectionGroup] + groups: [ConnectionGroup], + holdsTransaction: Bool = false ) { id = connection.id title = connection.name.isEmpty ? connection.host : connection.name @@ -45,6 +47,7 @@ nonisolated struct ConnectionListRowModel: Hashable, Sendable { isFavorite = connection.isFavorite showsFavoriteMark = connection.isFavorite && section == .connections isMacOnly = MacOnlyConnection.isMacOnly(connection) + self.holdsTransaction = holdsTransaction let tagsById = Dictionary(allTags.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) let resolved = connection.tagIds.compactMap { tagsById[$0] } @@ -73,6 +76,9 @@ nonisolated struct ConnectionListRowModel: Hashable, Sendable { if isFavorite { parts.append(String(localized: "Favorite")) } + if holdsTransaction { + parts.append(String(localized: "Transaction open")) + } return parts.joined(separator: ", ") } } diff --git a/apple/TableProMobile/TableProMobile/Helpers/RowInserter.swift b/apple/TableProMobile/TableProMobile/Helpers/RowInserter.swift index bba4154fad..70a972e605 100644 --- a/apple/TableProMobile/TableProMobile/Helpers/RowInserter.swift +++ b/apple/TableProMobile/TableProMobile/Helpers/RowInserter.swift @@ -68,6 +68,17 @@ nonisolated enum RowInserter { ) guard !statements.isEmpty else { throw IntentDataError.noInsertableValues(table) } - return try await driver.executeWrite(statements) + // The write joins an autocommit-off transaction, and this session closes right after, which would roll it back. + guard await driver.sessionTransactionState() == .implicitTransaction else { + return try await driver.executeWrite(statements) + } + do { + let affected = try await driver.executeWrite(statements) + try await driver.commitTransaction() + return affected + } catch { + try? await driver.rollbackTransaction() + throw error + } } } diff --git a/apple/TableProMobile/TableProMobile/Localizable.xcstrings b/apple/TableProMobile/TableProMobile/Localizable.xcstrings index 0d9f0aa858..b55bb20728 100644 --- a/apple/TableProMobile/TableProMobile/Localizable.xcstrings +++ b/apple/TableProMobile/TableProMobile/Localizable.xcstrings @@ -3753,6 +3753,12 @@ "Command" : { "extractionState" : "manual", "generatesSymbol" : false + }, + "Commit" : { + + }, + "Commit the Transaction?" : { + }, "Computed by the database" : { "localizations" : { @@ -9431,6 +9437,9 @@ } } } + }, + "Its changes are saved to the database." : { + }, "Its subgroups are deleted too. Their connections move to Ungrouped." : { "localizations" : { @@ -9459,6 +9468,9 @@ } } } + }, + "Its uncommitted changes are discarded." : { + }, "JSON" : { "extractionState" : "manual", @@ -13116,6 +13128,9 @@ "Pin" : { "extractionState" : "manual", "generatesSymbol" : false + }, + "Other work ran on this connection before you confirmed. Check the transaction and try again." : { + }, "PING failed: %@" : { "extractionState" : "manual", @@ -14698,6 +14713,15 @@ } } } + }, + "Roll Back" : { + + }, + "Roll Back the Transaction?" : { + + }, + "Roll it back to continue." : { + }, "Rosette" : { "extractionState" : "manual", @@ -16955,6 +16979,12 @@ } } } + }, + "Switch" : { + + }, + "Switch Database?" : { + }, "Switching..." : { "extractionState" : "stale", @@ -18440,6 +18470,12 @@ } } } + }, + "The connection to %@ was lost before the server answered, so its open transaction may have been committed or rolled back. It has been reconnected; check which of its changes were saved before running them again." : { + + }, + "The connection to %@ was lost while it waited, and the server rolled back its open transaction. It has been reconnected; run the statements again." : { + }, "The connection was lost while a transaction was open. Check which of its changes were saved before running them again." : { "extractionState" : "manual", @@ -18562,6 +18598,9 @@ }, "The database name is longer than the 128 bytes FreeTDS takes." : { + }, + "The database “%@” has an open transaction. Switching rolls it back and discards its uncommitted changes." : { + }, "The encrypted file is corrupt or incomplete" : { "extractionState" : "manual", @@ -18592,6 +18631,9 @@ } } } + }, + "The failed transaction was rolled back. None of its changes were kept." : { + }, "The FreeTDS configuration could not be written: %@" : { "extractionState" : "manual", @@ -19388,6 +19430,9 @@ } } } + }, + "The session that held this transaction has ended." : { + }, "The server's SSH host key has changed. Open this connection in TablePro to check it." : { "localizations" : { @@ -19575,6 +19620,9 @@ } } } + }, + "The transaction on this connection can no longer be committed. Roll it back." : { + }, "The transaction on this connection is still open. Commit or roll it back." : { @@ -20802,6 +20850,27 @@ } } } + }, + "Transaction Failed" : { + + }, + "Transaction Open" : { + + }, + "Transaction failed on %1$@, %2$@. Roll it back to continue." : { + + }, + "Transaction failed on %@. Roll it back to continue." : { + + }, + "Transaction open" : { + + }, + "Transaction open on %1$@, %2$@" : { + + }, + "Transaction open on %@" : { + }, "Transfer" : { "extractionState" : "manual", diff --git a/apple/TableProMobile/TableProMobile/Platform/SessionTransactionMonitor.swift b/apple/TableProMobile/TableProMobile/Platform/SessionTransactionMonitor.swift new file mode 100644 index 0000000000..5fd3d75b8a --- /dev/null +++ b/apple/TableProMobile/TableProMobile/Platform/SessionTransactionMonitor.swift @@ -0,0 +1,177 @@ +import Foundation +import Observation +import os +import TableProDatabase + +@MainActor @Observable +final class SessionTransactionMonitor { + nonisolated enum Hold: Sendable, Equatable { + case open + case aborted + } + + // What a confirmation was opened for: a session, and the work it had seen. Work since then makes it stale. + struct HoldToken { + let driver: any DatabaseDriver + let workGeneration: Int + } + + private struct Entry { + let driver: any DatabaseDriver + var hold: Hold + } + + private static let logger = Logger(subsystem: "com.TablePro", category: "SessionTransactionMonitor") + + private var entries: [UUID: Entry] = [:] + private var workCounts: [UUID: Int] = [:] + private var rollBackCounts: [UUID: Int] = [:] + @ObservationIgnored private var refreshGenerations: [UUID: Int] = [:] + @ObservationIgnored private var workGenerations: [UUID: Int] = [:] + @ObservationIgnored private let liveDriver: (UUID) -> (any DatabaseDriver)? + + init(liveDriver: @escaping (UUID) -> (any DatabaseDriver)?) { + self.liveDriver = liveDriver + } + + func hold(for connectionId: UUID) -> Hold? { + entries[connectionId]?.hold + } + + // A window still on a session another window replaced must not show, or act on, the newer session's transaction. + func hold(for connectionId: UUID, on driver: (any DatabaseDriver)?) -> Hold? { + guard let driver, let entry = entries[connectionId], entry.driver === driver else { return nil } + return entry.hold + } + + func token(for connectionId: UUID, on driver: (any DatabaseDriver)?) -> HoldToken? { + guard let driver, hold(for: connectionId, on: driver) != nil else { return nil } + return HoldToken(driver: driver, workGeneration: workGenerations[connectionId, default: 0]) + } + + func rollBackCount(for connectionId: UUID) -> Int { + rollBackCounts[connectionId, default: 0] + } + + func isBusy(_ connectionId: UUID) -> Bool { + workCounts[connectionId, default: 0] > 0 + } + + func track(_ connectionId: UUID, _ work: () async throws -> T) async rethrows -> T { + workCounts[connectionId, default: 0] += 1 + workGenerations[connectionId, default: 0] += 1 + do { + let value = try await work() + await refresh(connectionId) + endWork(connectionId) + return value + } catch { + await refresh(connectionId) + endWork(connectionId) + throw error + } + } + + func refresh(_ connectionId: UUID) async { + guard let driver = liveDriver(connectionId) else { + entries[connectionId] = nil + return + } + let generation = refreshGenerations[connectionId, default: 0] + 1 + refreshGenerations[connectionId] = generation + let state = await driver.sessionTransactionState() + // A newer read, or a session that replaced this driver while it answered, owns the entry now. + guard refreshGenerations[connectionId] == generation, liveDriver(connectionId) === driver else { return } + apply(state, from: driver, to: connectionId) + } + + func commit(_ connectionId: UUID, as token: HoldToken) async throws { + let driver = try actionDriver(for: connectionId, token: token) + try await track(connectionId) { + let generation = workGenerations[connectionId, default: 0] + let state = await driver.sessionTransactionState() + guard liveDriver(connectionId) === driver else { throw TransactionActionError.sessionReplaced } + // Work another window ran during the read may have ended this transaction and opened another. + guard workGenerations[connectionId, default: 0] == generation else { + throw TransactionActionError.transactionChanged + } + if state == .abortedTransaction { + entries[connectionId] = Entry(driver: driver, hold: .aborted) + throw TransactionActionError.cannotCommit + } + try await driver.commitTransaction() + } + } + + func rollBack(_ connectionId: UUID, as token: HoldToken) async throws { + let driver = try actionDriver(for: connectionId, token: token) + try await track(connectionId) { + try await driver.rollbackTransaction() + rollBackCounts[connectionId, default: 0] += 1 + } + } + + // Rows a view read inside a transaction its session lost are as stale as after a Roll Back. + func transactionLost(_ connectionId: UUID) { + rollBackCounts[connectionId, default: 0] += 1 + } + + func sessionEnded(_ connectionId: UUID, driver: any DatabaseDriver) { + guard let entry = entries[connectionId], entry.driver === driver else { return } + entries[connectionId] = nil + } + + private func actionDriver(for connectionId: UUID, token: HoldToken) throws -> any DatabaseDriver { + let live = liveDriver(connectionId) + guard let live, live === token.driver, let entry = entries[connectionId], entry.driver === live else { + if let entry = entries[connectionId], entry.driver !== live { + entries[connectionId] = nil + } + Self.logger.info("Refused a transaction action: the session that held it is gone") + throw TransactionActionError.sessionReplaced + } + guard workGenerations[connectionId, default: 0] == token.workGeneration else { + Self.logger.info("Refused a transaction action: other work ran on the session before it was confirmed") + throw TransactionActionError.transactionChanged + } + return live + } + + private func apply(_ state: DriverTransactionState, from driver: any DatabaseDriver, to connectionId: UUID) { + switch state { + case .explicitTransaction, .implicitTransaction: + entries[connectionId] = Entry(driver: driver, hold: .open) + case .abortedTransaction: + entries[connectionId] = Entry(driver: driver, hold: .aborted) + case .idle: + entries[connectionId] = nil + case .unknown: + if let entry = entries[connectionId], entry.driver !== driver { + entries[connectionId] = nil + } + } + } + + private func endWork(_ connectionId: UUID) { + workGenerations[connectionId, default: 0] += 1 + let remaining = workCounts[connectionId, default: 1] - 1 + workCounts[connectionId] = remaining > 0 ? remaining : nil + } +} + +nonisolated enum TransactionActionError: Error, LocalizedError, Equatable { + case cannotCommit + case sessionReplaced + case transactionChanged + + var errorDescription: String? { + switch self { + case .cannotCommit: + String(localized: "The transaction on this connection can no longer be committed. Roll it back.") + case .sessionReplaced: + String(localized: "The session that held this transaction has ended.") + case .transactionChanged: + String(localized: "Other work ran on this connection before you confirmed. Check the transaction and try again.") + } + } +} diff --git a/apple/TableProMobile/TableProMobile/ViewModels/QueryEditorViewModel.swift b/apple/TableProMobile/TableProMobile/ViewModels/QueryEditorViewModel.swift index 9f0a29f6e0..8ef5a317b3 100644 --- a/apple/TableProMobile/TableProMobile/ViewModels/QueryEditorViewModel.swift +++ b/apple/TableProMobile/TableProMobile/ViewModels/QueryEditorViewModel.swift @@ -21,6 +21,7 @@ final class QueryEditorViewModel { case nothing case applied(count: Int) case openTransaction + case abortedTransaction /// The session was in a transaction at some point, or cannot say, so whether earlier work stands is not known. case unknown } @@ -173,6 +174,9 @@ final class QueryEditorViewModel { switch (interruption.standingWork, interruption.kind) { case (.openTransaction, _): return String(localized: "The transaction on this connection is still open. Commit or roll it back.") + // A COMMIT in an aborted PostgreSQL block rolls back without an error, so committing is not offered. + case (.abortedTransaction, _): + return String(localized: "The transaction on this connection can no longer be committed. Roll it back.") case (.unknown, _): return nil case (.nothing, _): @@ -428,6 +432,7 @@ final class QueryEditorViewModel { observedStates: [DriverTransactionState], now: DriverTransactionState ) -> StandingWork { + if now == .abortedTransaction { return .abortedTransaction } if now == .explicitTransaction || now == .implicitTransaction { return .openTransaction } guard now == .idle, observedStates.allSatisfy({ $0 == .idle }) else { return .unknown } return position > 1 ? .applied(count: position - 1) : .nothing diff --git a/apple/TableProMobile/TableProMobile/Views/ConnectedView.swift b/apple/TableProMobile/TableProMobile/Views/ConnectedView.swift index 3591dfd8fa..2b1aeb0c84 100644 --- a/apple/TableProMobile/TableProMobile/Views/ConnectedView.swift +++ b/apple/TableProMobile/TableProMobile/Views/ConnectedView.swift @@ -8,12 +8,14 @@ struct ConnectedView: View { @Environment(ScenePresenter.self) private var presenter @Environment(\.scenePhase) private var scenePhase @Environment(\.dismiss) private var dismiss + @Environment(\.horizontalSizeClass) private var horizontalSizeClass let connection: DatabaseConnection @State private var coordinator: ConnectionCoordinator? @State private var hapticSuccess = false @State private var hapticError = false @State private var showDeletedAlert = false + @State private var databaseSwitchAwaitingRollBack: String? private var displayTitle: String { connection.name.isEmpty ? connection.host : connection.name @@ -159,17 +161,21 @@ struct ConnectedView: View { tabChrome(coordinator) { TableListView(connectionId: connection.id) } + // Side by side, the detail column already shows the bar. + .transactionBar(for: coordinator, isEnabled: horizontalSizeClass == .compact) } detail: { NavigationStack { if let table = coordinator.selectedTable { DataBrowserView(table: table) .environment(coordinator) + .transactionBar(for: coordinator) } else { ContentUnavailableView( "No Table Selected", systemImage: "tablecells", description: Text("Pick a table to browse its rows.") ) + .transactionBar(for: coordinator) } } .id(TableBrowserIdentity(coordinator.selectedTable, coordinator.activeDatabase)) @@ -179,20 +185,29 @@ struct ConnectedView: View { Tab("Query", systemImage: "terminal", value: .query) { NavigationStack { tabChrome(coordinator) { QueryEditorView() } + .transactionBar(for: coordinator) } } Tab("History", systemImage: "clock", value: .history) { NavigationStack { tabChrome(coordinator) { QueryHistoryView() } + .transactionBar(for: coordinator) } } Tab("Info", systemImage: "info.circle", value: .info) { NavigationStack { tabChrome(coordinator) { ConnectionInfoView() } + .transactionBar(for: coordinator) } } } .tabViewStyle(.sidebarAdaptable) + .transactionAccessory(for: coordinator) + .onChange(of: appState.sessionTransactions.rollBackCount(for: connection.id)) { + Task { await coordinator.reloadAfterRollBack() } + } + // tabChrome sits inside each stack; this also reaches pushed row screens and their sheets, which track writes. + .environment(coordinator) .background { Button("") { coordinator.selectedTab = .tables } .keyboardShortcut("1", modifiers: .command) @@ -246,6 +261,24 @@ struct ConnectedView: View { } message: { Text(coordinator.failureAlertMessage ?? "") } + .confirmationDialog( + String(localized: "Switch Database?"), + isPresented: Binding( + get: { databaseSwitchAwaitingRollBack != nil }, + set: { if !$0 { databaseSwitchAwaitingRollBack = nil } } + ), + titleVisibility: .visible, + presenting: databaseSwitchAwaitingRollBack + ) { database in + Button(String(localized: "Switch"), role: .destructive) { + Task { await coordinator.switchDatabase(to: database) } + } + } message: { _ in + Text(String( + format: String(localized: "The database “%@” has an open transaction. Switching rolls it back and discards its uncommitted changes."), + coordinator.activeDatabase + )) + } .userActivity(SceneIntent.viewConnectionActivity, isActive: appState.offersHandoff(for: connection)) { activity in activity.title = connection.name.isEmpty ? connection.host : connection.name activity.isEligibleForHandoff = true @@ -289,7 +322,7 @@ struct ConnectedView: View { Menu { ForEach(coordinator.databases, id: \.self) { db in Button { - Task { await coordinator.switchDatabase(to: db) } + requestDatabaseSwitch(to: db, on: coordinator) } label: { if db == coordinator.activeDatabase { Label(db, systemImage: "checkmark") @@ -327,4 +360,13 @@ struct ConnectedView: View { } } } + + private func requestDatabaseSwitch(to database: String, on coordinator: ConnectionCoordinator) { + if TransactionIndicatorContent.databaseSwitchEndsSession(for: coordinator.connection.type), + coordinator.transactionHold != nil { + databaseSwitchAwaitingRollBack = database + } else { + Task { await coordinator.switchDatabase(to: database) } + } + } } diff --git a/apple/TableProMobile/TableProMobile/Views/ConnectionListRow.swift b/apple/TableProMobile/TableProMobile/Views/ConnectionListRow.swift index 53dd164a33..a1a333bffc 100644 --- a/apple/TableProMobile/TableProMobile/Views/ConnectionListRow.swift +++ b/apple/TableProMobile/TableProMobile/Views/ConnectionListRow.swift @@ -84,6 +84,13 @@ struct ConnectionListRow: View { } .frame(maxWidth: .infinity, alignment: .leading) + if model.holdsTransaction { + Image(systemName: TransactionIndicatorContent.openSymbolName) + .font(.caption) + .foregroundStyle(.orange) + .accessibilityHidden(true) + } + if model.showsFavoriteMark { Image(systemName: "star.fill") .font(.caption) diff --git a/apple/TableProMobile/TableProMobile/Views/ConnectionListView.swift b/apple/TableProMobile/TableProMobile/Views/ConnectionListView.swift index e75024649e..a72e1492b6 100644 --- a/apple/TableProMobile/TableProMobile/Views/ConnectionListView.swift +++ b/apple/TableProMobile/TableProMobile/Views/ConnectionListView.swift @@ -385,7 +385,8 @@ struct ConnectionListView: View { connection: connection, section: section, tags: appState.tags, - groups: appState.groups + groups: appState.groups, + holdsTransaction: appState.sessionTransactions.hold(for: connection.id) != nil ), isRenaming: renamingRow == rowId, onOpen: { open(connection.id) }, diff --git a/apple/TableProMobile/TableProMobile/Views/DataBrowserView.swift b/apple/TableProMobile/TableProMobile/Views/DataBrowserView.swift index 8e36da9eb8..277486fffb 100644 --- a/apple/TableProMobile/TableProMobile/Views/DataBrowserView.swift +++ b/apple/TableProMobile/TableProMobile/Views/DataBrowserView.swift @@ -128,14 +128,23 @@ struct DataBrowserView: View { showMatchedRow = true } } - .onChange(of: activeSchema) { _, newSchema in - guard reference == nil else { return } + // A redial or another window's database switch hands this table a new session; a referenced table keeps its schema. + .onChange(of: BrowseSource(session: session, schema: browserSchema)) { _, source in + guard source.driver != nil else { return } viewModel.attach( session: session, table: table, databaseType: connection.type, - host: connection.host, schema: newSchema + host: connection.host, schema: source.schema ) Task { await viewModel.load(isInitial: true) } } + .onChange(of: appState.sessionTransactions.rollBackCount(for: connection.id)) { + Task { await viewModel.load() } + } + // With autocommit off, a MySQL read opens a transaction. + .onChange(of: viewModel.isLoading || viewModel.isPageLoading) { _, loading in + guard !loading else { return } + Task { await coordinator.refreshTransaction() } + } .onDisappear { viewModel.cancel() } .sheet(isPresented: $showInsertSheet) { insertSheet } .sheet(isPresented: $showFilterSheet) { @@ -203,6 +212,7 @@ struct DataBrowserView: View { .sensoryFeedback(.error, trigger: hapticError) .navigationDestination(isPresented: $showStructure) { StructureView(table: table, session: session, databaseType: connection.type, schema: viewModel.schema) + .transactionBar(for: coordinator) } .navigationDestination(isPresented: $showMatchedRow) { rowDetail(index: 0) @@ -543,7 +553,7 @@ struct DataBrowserView: View { } private func performDelete(_ pkValues: [(column: String, value: String)]) async { - let success = await viewModel.deleteRow(pkValues: pkValues) + let success = await coordinator.track(failed: { !$0 }) { await viewModel.deleteRow(pkValues: pkValues) } if success { hapticSuccess.toggle() } else { @@ -551,3 +561,13 @@ struct DataBrowserView: View { } } } + +private struct BrowseSource: Equatable { + let driver: ObjectIdentifier? + let schema: String? + + init(session: ConnectionSession?, schema: String?) { + driver = session.map { ObjectIdentifier($0.driver) } + self.schema = schema + } +} diff --git a/apple/TableProMobile/TableProMobile/Views/ForeignKeyNavigation.swift b/apple/TableProMobile/TableProMobile/Views/ForeignKeyNavigation.swift index 4abb307a66..36a2859b27 100644 --- a/apple/TableProMobile/TableProMobile/Views/ForeignKeyNavigation.swift +++ b/apple/TableProMobile/TableProMobile/Views/ForeignKeyNavigation.swift @@ -54,6 +54,7 @@ private struct ForeignKeyPreviewModifier: ViewModifier { reference: reference ) .environment(coordinator) + .transactionBar(for: coordinator) } } } diff --git a/apple/TableProMobile/TableProMobile/Views/InsertRowView.swift b/apple/TableProMobile/TableProMobile/Views/InsertRowView.swift index 7984621ab9..d401d256ba 100644 --- a/apple/TableProMobile/TableProMobile/Views/InsertRowView.swift +++ b/apple/TableProMobile/TableProMobile/Views/InsertRowView.swift @@ -12,6 +12,7 @@ struct InsertRowView: View { var onInserted: (() -> Void)? @Environment(\.dismiss) private var dismiss + @Environment(ConnectionCoordinator.self) private var coordinator /// A column absent from this dictionary is left out of the `INSERT` so the database applies /// its own default. Keying by name rather than by position is what keeps the state from @@ -295,7 +296,7 @@ struct InsertRowView: View { defer { isSaving = false } do { - try await session.driver.executeWrite([sql]) + _ = try await coordinator.track { try await session.driver.executeWrite([sql]) } hapticSuccess.toggle() onInserted?() dismiss() diff --git a/apple/TableProMobile/TableProMobile/Views/QueryEditorView.swift b/apple/TableProMobile/TableProMobile/Views/QueryEditorView.swift index b29d800f28..dd6c091306 100644 --- a/apple/TableProMobile/TableProMobile/Views/QueryEditorView.swift +++ b/apple/TableProMobile/TableProMobile/Views/QueryEditorView.swift @@ -616,7 +616,9 @@ struct QueryEditorView: View { let progressUpdater = startActivityProgressUpdater(token: token) - await viewModel.run(driver: session.driver, script: script) + await coordinator.track(failed: { _ in QueryExecutionOutcome(phase: viewModel.phase) == .failed }) { + await viewModel.run(driver: session.driver, script: script) + } progressUpdater.cancel() let phase = viewModel.phase diff --git a/apple/TableProMobile/TableProMobile/Views/RowDetailView.swift b/apple/TableProMobile/TableProMobile/Views/RowDetailView.swift index 28b0e74c67..574a554bc7 100644 --- a/apple/TableProMobile/TableProMobile/Views/RowDetailView.swift +++ b/apple/TableProMobile/TableProMobile/Views/RowDetailView.swift @@ -3,6 +3,8 @@ import TableProDatabase import TableProModels struct RowDetailView: View { + // Tracks writes and shows the transaction; `coordinator` is set only where a referenced table can be opened. + @Environment(ConnectionCoordinator.self) private var connectionCoordinator @State private var viewModel: RowDetailViewModel @State private var fkPreviewItem: FKPreviewItem? @State private var showShareSheet = false @@ -79,6 +81,8 @@ struct RowDetailView: View { rowNavigator } } + .transactionBar(for: connectionCoordinator) + .leavesAfterRollBack(of: connectionCoordinator, isTableRow: viewModel.table != nil, isEditing: viewModel.isEditing) .navigationBarBackButtonHidden(viewModel.isEditing) .holdsScene(withUnsavedChanges: viewModel.hasUnsavedEdits) .navigationTitle(viewModel.table?.name ?? String(format: String(localized: "Row %d of %d"), viewModel.currentIndex + 1, viewModel.rows.count)) @@ -363,7 +367,9 @@ struct RowDetailView: View { } private func handleSave() async { - let success = await viewModel.saveChanges() + let success = await connectionCoordinator.track(failed: { _ in viewModel.operationError != nil }) { + await viewModel.saveChanges() + } if viewModel.pendingWriteConfirmation { showSaveConfirmation = true return @@ -376,7 +382,9 @@ struct RowDetailView: View { } private func executePendingSave() async { - let success = await viewModel.executePendingSave() + let success = await connectionCoordinator.track(failed: { _ in viewModel.operationError != nil }) { + await viewModel.executePendingSave() + } if success { hapticSuccess.toggle() } else { diff --git a/apple/TableProMobile/TableProMobile/Views/TableListView.swift b/apple/TableProMobile/TableProMobile/Views/TableListView.swift index 697fb8480a..e959ba0bdf 100644 --- a/apple/TableProMobile/TableProMobile/Views/TableListView.swift +++ b/apple/TableProMobile/TableProMobile/Views/TableListView.swift @@ -163,7 +163,7 @@ struct TableListView: View { let quoted = SQLBuilder.qualifiedIdentifier( table: table.name, schema: activeSchema, for: connection.type ) - try await driver.executeWrite(["TRUNCATE TABLE \(quoted)"]) + _ = try await coordinator.track { try await driver.executeWrite(["TRUNCATE TABLE \(quoted)"]) } await coordinator.refreshTables() } catch { errorMessage = error.localizedDescription @@ -190,7 +190,7 @@ struct TableListView: View { let quoted = SQLBuilder.qualifiedIdentifier( table: table.name, schema: activeSchema, for: connection.type ) - try await driver.executeWrite(["DROP TABLE \(quoted)"]) + _ = try await coordinator.track { try await driver.executeWrite(["DROP TABLE \(quoted)"]) } await coordinator.refreshTables() } catch { errorMessage = error.localizedDescription diff --git a/apple/TableProMobile/TableProMobile/Views/TransactionIndicatorContent.swift b/apple/TableProMobile/TableProMobile/Views/TransactionIndicatorContent.swift new file mode 100644 index 0000000000..9fa2503199 --- /dev/null +++ b/apple/TableProMobile/TableProMobile/Views/TransactionIndicatorContent.swift @@ -0,0 +1,81 @@ +import Foundation +import TableProModels + +nonisolated struct TransactionIndicatorContent: Equatable, Sendable { + nonisolated enum CommitOffer: Equatable, Sendable { + case hidden + case runs + case asksFirst + } + + static let openSymbolName = "hourglass" + static let abortedSymbolName = "exclamationmark.triangle.fill" + static let commitSymbolName = "checkmark" + static let rollBackSymbolName = "arrow.uturn.backward" + + let isAborted: Bool + let title: String + let subtitle: String + let accessibilityLabel: String + let commit: CommitOffer + + var symbolName: String { + isAborted ? Self.abortedSymbolName : Self.openSymbolName + } + + init( + isAborted: Bool, + connectionName: String, + activeDatabase: String, + switchesDatabases: Bool, + safeModeLevel: SafeModeLevel + ) { + self.isAborted = isAborted + // SQLite, DuckDB and Oracle keep a restored or empty activeDatabase that names nothing on the server. + let database = switchesDatabases && !activeDatabase.isEmpty ? activeDatabase : nil + + if isAborted { + title = String(localized: "Transaction Failed") + subtitle = String(localized: "Roll it back to continue.") + if let database { + accessibilityLabel = String( + format: String(localized: "Transaction failed on %1$@, %2$@. Roll it back to continue."), + database, + connectionName + ) + } else { + accessibilityLabel = String( + format: String(localized: "Transaction failed on %@. Roll it back to continue."), + connectionName + ) + } + commit = .hidden + return + } + + title = String(localized: "Transaction Open") + subtitle = database ?? connectionName + if let database { + accessibilityLabel = String( + format: String(localized: "Transaction open on %1$@, %2$@"), + database, + connectionName + ) + } else { + accessibilityLabel = String(format: String(localized: "Transaction open on %@"), connectionName) + } + switch safeModeLevel.writePermission { + case .proceed: + commit = .runs + case .requiresConfirmation: + commit = .asksFirst + case .blocked: + commit = .hidden + } + } + + // Mirrors ConnectionCoordinator.switchDatabase: only these engines dial a new session to switch. + static func databaseSwitchEndsSession(for type: DatabaseType) -> Bool { + type == .postgresql || type == .redshift + } +} diff --git a/apple/TableProMobile/TableProMobile/Views/TransactionIndicatorView.swift b/apple/TableProMobile/TableProMobile/Views/TransactionIndicatorView.swift new file mode 100644 index 0000000000..9c2105b91a --- /dev/null +++ b/apple/TableProMobile/TableProMobile/Views/TransactionIndicatorView.swift @@ -0,0 +1,250 @@ +import SwiftUI +import TableProModels + +extension View { + func transactionAccessory(for coordinator: ConnectionCoordinator) -> some View { + modifier(TransactionAccessoryPlacement(coordinator: coordinator)) + } + + func transactionBar(for coordinator: ConnectionCoordinator, isEnabled: Bool = true) -> some View { + modifier(TransactionBarPlacement(coordinator: coordinator, isEnabled: isEnabled)) + } + + // A table row is a copy of the grid page, which reloads after a Roll Back; an edit in progress finishes first. + func leavesAfterRollBack(of coordinator: ConnectionCoordinator, isTableRow: Bool, isEditing: Bool) -> some View { + modifier(RollBackDismissal(coordinator: coordinator, isTableRow: isTableRow, isEditing: isEditing)) + } +} + +private struct RollBackDismissal: ViewModifier { + @Environment(AppState.self) private var appState + @Environment(\.dismiss) private var dismiss + let coordinator: ConnectionCoordinator + let isTableRow: Bool + let isEditing: Bool + + @State private var rolledBack = false + + func body(content: Content) -> some View { + content + .onChange(of: appState.sessionTransactions.rollBackCount(for: coordinator.connection.id)) { + guard isTableRow else { return } + rolledBack = true + if !isEditing { dismiss() } + } + .onChange(of: isEditing) { _, editing in + if !editing, rolledBack { dismiss() } + } + } +} + +// The 26.0 tabViewBottomAccessory(content:) overload is avoided: it draws an empty capsule while nothing is held. +private struct TransactionAccessoryPlacement: ViewModifier { + @Environment(AppState.self) private var appState + let coordinator: ConnectionCoordinator + + func body(content: Content) -> some View { + if #available(iOS 26.1, *) { + content.tabViewBottomAccessory(isEnabled: coordinator.transactionHold != nil) { + TransactionAccessory(coordinator: coordinator, transactions: appState.sessionTransactions) + } + } else { + content + } + } +} + +// Goes on each screen, outside its own bar: on a navigation container it covers a pushed screen's last rows. +private struct TransactionBarPlacement: ViewModifier { + @Environment(AppState.self) private var appState + let coordinator: ConnectionCoordinator + let isEnabled: Bool + + func body(content: Content) -> some View { + if #available(iOS 26.1, *) { + content + } else { + content.bottomSafeAreaBar { + if isEnabled, coordinator.transactionHold != nil { + TransactionIndicatorView(coordinator: coordinator, transactions: appState.sessionTransactions) + .padding(.horizontal, 16) + .padding(.vertical, 8) + } + } + } + } +} + +@available(iOS 26.1, *) +private struct TransactionAccessory: View { + @Environment(\.tabViewBottomAccessoryPlacement) private var placement + let coordinator: ConnectionCoordinator + let transactions: SessionTransactionMonitor + + var body: some View { + TransactionIndicatorView( + coordinator: coordinator, + transactions: transactions, + prefersCompact: placement == .inline + ) + .padding(.horizontal, 16) + } +} + +struct TransactionIndicatorView: View { + @Environment(\.dynamicTypeSize) private var dynamicTypeSize + let coordinator: ConnectionCoordinator + let transactions: SessionTransactionMonitor + var prefersCompact = false + + // What each dialog was opened for, so a swapped session or newer work refuses rather than acts. + @State private var rollBackTarget: SessionTransactionMonitor.HoldToken? + @State private var commitTarget: SessionTransactionMonitor.HoldToken? + + private var connectionId: UUID { coordinator.connection.id } + private var isBusy: Bool { transactions.isBusy(connectionId) } + private var isCompact: Bool { prefersCompact || dynamicTypeSize.isAccessibilitySize } + + var body: some View { + if let hold = coordinator.transactionHold { + bar(TransactionIndicatorContent( + isAborted: hold == .aborted, + connectionName: coordinator.displayName, + activeDatabase: coordinator.activeDatabase, + switchesDatabases: coordinator.supportsDatabaseSwitching, + safeModeLevel: coordinator.connection.safeModeLevel + )) + } + } + + // A button title never truncates: where the titles do not fit, as on an iPhone in portrait, the buttons show symbols. + private func bar(_ content: TransactionIndicatorContent) -> some View { + ViewThatFits(in: .horizontal) { + row(content, iconOnly: isCompact) + row(content, iconOnly: true) + } + .confirmationDialog( + String(localized: "Roll Back the Transaction?"), + isPresented: isPresenting($rollBackTarget), + titleVisibility: .visible, + presenting: rollBackTarget + ) { target in + Button(String(localized: "Roll Back"), role: .destructive) { + perform { try await coordinator.rollBackTransaction(as: target) } + } + } message: { _ in + Text("Its uncommitted changes are discarded.") + } + .confirmationDialog( + String(localized: "Commit the Transaction?"), + isPresented: isPresenting($commitTarget), + titleVisibility: .visible, + presenting: commitTarget + ) { target in + Button(String(localized: "Commit")) { + // The level can change on another device while the dialog is up. + guard !coordinator.connection.safeModeLevel.blocksWrites else { return } + perform { try await coordinator.commitTransaction(as: target) } + } + } message: { _ in + Text("Its changes are saved to the database.") + } + } + + private func row(_ content: TransactionIndicatorContent, iconOnly: Bool) -> some View { + HStack(spacing: 12) { + summary(content) + Spacer(minLength: 8) + HStack(spacing: 8) { + rollBackButton + if content.commit != .hidden { + commitButton(content.commit) + } + } + .buttonStyle(.bordered) + .controlSize(.small) + .modifier(TransactionButtonLabels(isCompact: iconOnly)) + .disabled(isBusy) + } + } + + private func summary(_ content: TransactionIndicatorContent) -> some View { + HStack(spacing: 8) { + if isBusy { + ProgressView() + .controlSize(.small) + } else { + Image(systemName: content.symbolName) + .foregroundStyle(content.isAborted ? Color.red : Color.orange) + } + VStack(alignment: .leading, spacing: 1) { + Text(content.title) + .font(.subheadline.weight(.semibold)) + if !isCompact { + Text(content.subtitle) + .font(.caption) + .foregroundStyle(.secondary) + } + } + .lineLimit(1) + } + .accessibilityElement(children: .ignore) + .accessibilityLabel(content.accessibilityLabel) + } + + // Never gated by Safe Mode: Roll Back is the one way out that keeps nothing. + private var rollBackButton: some View { + Button(role: .destructive) { + rollBackTarget = coordinator.transactionToken + } label: { + Label("Roll Back", systemImage: TransactionIndicatorContent.rollBackSymbolName) + } + } + + private func commitButton(_ offer: TransactionIndicatorContent.CommitOffer) -> some View { + Button { + guard let target = coordinator.transactionToken else { return } + if offer == .asksFirst { + commitTarget = target + } else { + perform { try await coordinator.commitTransaction(as: target) } + } + } label: { + Label("Commit", systemImage: TransactionIndicatorContent.commitSymbolName) + } + } + + private func isPresenting(_ target: Binding) -> Binding { + Binding( + get: { target.wrappedValue != nil }, + set: { if !$0 { target.wrappedValue = nil } } + ) + } + + // The coordinator's alert lives on the TabView, so it outlasts this bar when a failure ends the hold. + private func perform(_ action: @escaping @MainActor () async throws -> Void) { + Task { + do { + try await action() + } catch is CancellationError { + return + } catch { + guard !coordinator.showFailureAlert else { return } + coordinator.failureAlertMessage = error.localizedDescription + coordinator.showFailureAlert = true + } + } + } +} + +private struct TransactionButtonLabels: ViewModifier { + let isCompact: Bool + + func body(content: Content) -> some View { + if isCompact { + content.labelStyle(.iconOnly) + } else { + content.labelStyle(.titleOnly) + } + } +} diff --git a/apple/TableProMobile/TableProMobileTests/BackgroundReleaseCoordinatorTests.swift b/apple/TableProMobile/TableProMobileTests/BackgroundReleaseCoordinatorTests.swift index ef68fed9b1..3b27921ff3 100644 --- a/apple/TableProMobile/TableProMobileTests/BackgroundReleaseCoordinatorTests.swift +++ b/apple/TableProMobile/TableProMobileTests/BackgroundReleaseCoordinatorTests.swift @@ -26,19 +26,6 @@ private final class SpyBackgroundTaskAsserter: BackgroundTaskAsserting { } } -private final class StubDriverFactory: DriverFactory, @unchecked Sendable { - var drivers: [String: any DatabaseDriver] = [:] - - func createDriver(for connection: DatabaseConnection, password: String?) throws -> any DatabaseDriver { - guard let driver = drivers[connection.type.rawValue] else { - throw ConnectionError.driverNotFound(connection.type.rawValue) - } - return driver - } - - func supportedTypes() -> [DatabaseType] { drivers.keys.map(DatabaseType.init(rawValue:)) } -} - private func makeConnection(_ type: String) -> DatabaseConnection { DatabaseConnection(name: "Test", type: DatabaseType(rawValue: type)) } diff --git a/apple/TableProMobile/TableProMobileTests/ConnectionCoordinatorReconnectTests.swift b/apple/TableProMobile/TableProMobileTests/ConnectionCoordinatorReconnectTests.swift new file mode 100644 index 0000000000..282388cf53 --- /dev/null +++ b/apple/TableProMobile/TableProMobileTests/ConnectionCoordinatorReconnectTests.swift @@ -0,0 +1,347 @@ +import Foundation +import TableProDatabase +@testable import TableProMobile +import TableProModels +import Testing + +@MainActor +@Suite("Connection coordinator reconnect", .timeLimit(.minutes(1))) +struct ConnectionCoordinatorReconnectTests { + private let fixture: AppStateFixture + private let factory = StubDriverFactory() + private let appState: AppState + private let connection = DatabaseConnection( + name: "Prod", + type: .mysql, + host: "db.example.com", + port: 3_306, + database: "app_prod" + ) + + init() throws { + fixture = try AppStateFixture() + appState = fixture.makeState(syncEnabled: false, driverFactory: factory) + } + + private var monitor: SessionTransactionMonitor { appState.sessionTransactions } + + private func driver(tables: [String] = [], state: DriverTransactionState = .idle) -> MockDatabaseDriver { + let driver = MockDatabaseDriver() + driver.scriptedTables = tables.map { TableInfo(name: $0) } + driver.scriptedTransactionState = state + return driver + } + + private func dial(_ driver: MockDatabaseDriver, database: String? = nil) async throws { + var target = connection + if let database { target.database = database } + factory.queue(driver) + _ = try await appState.connectionManager.connect(target) + } + + private func attachedCoordinator() async -> ConnectionCoordinator { + let coordinator = ConnectionCoordinator(connection: connection, appState: appState) + await coordinator.connect() + return coordinator + } + + private func isConnected(_ coordinator: ConnectionCoordinator) -> Bool { + if case .connected = coordinator.phase { return true } + return false + } + + private func tableNames(_ coordinator: ConnectionCoordinator) -> [String] { + coordinator.tables.map { $0.name } + } + + @Test("A redial dials the database the toolbar shows and reloads its tables") + func redialDialsTheActiveDatabase() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let coordinator = await attachedCoordinator() + await coordinator.switchDatabase(to: "app_staging") + #expect(coordinator.activeDatabase == "app_staging") + + first.pingError = MockDatabaseDriver.MockError.scripted + let second = driver(tables: ["staging_orders"]) + factory.queue(second) + await coordinator.reconnectIfNeeded() + + let dialedDatabase = factory.dialed.last?.database + #expect(dialedDatabase == "app_staging") + #expect(coordinator.session?.driver === second) + #expect(coordinator.activeDatabase == "app_staging") + #expect(tableNames(coordinator) == ["staging_orders"]) + #expect(isConnected(coordinator)) + } + + @Test("A session another window dialed is taken without a ping, with its database and tables") + func newerSessionIsAdoptedWithItsDatabase() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let coordinator = await attachedCoordinator() + #expect(coordinator.activeDatabase == "app_prod") + + let other = driver(tables: ["staging_orders"]) + try await dial(other, database: "app_staging") + let dialsBefore = factory.dialed.count + await coordinator.reconnectIfNeeded() + + #expect(first.pingCount == 0) + #expect(factory.dialed.count == dialsBefore) + #expect(coordinator.session?.driver === other) + #expect(coordinator.activeDatabase == "app_staging") + #expect(tableNames(coordinator) == ["staging_orders"]) + } + + @Test("Two windows coming back at once dial the connection once") + func twoWindowsDialOnce() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let left = await attachedCoordinator() + let right = await attachedCoordinator() + let dialsBefore = factory.dialed.count + + first.pingError = MockDatabaseDriver.MockError.scripted + let second = driver(tables: ["orders"]) + factory.queue(second) + async let leftDone: Void = left.reconnectIfNeeded() + async let rightDone: Void = right.reconnectIfNeeded() + _ = await (leftDone, rightDone) + + #expect(factory.dialed.count == dialsBefore + 1) + #expect(left.session?.driver === second) + #expect(right.session?.driver === second) + #expect(isConnected(left)) + #expect(isConnected(right)) + } + + @Test("A redial over an open transaction tells the user the server rolled it back") + func redialOverAHoldShowsTheAlert() async throws { + let first = driver(tables: ["orders"], state: .explicitTransaction) + try await dial(first) + let coordinator = await attachedCoordinator() + #expect(monitor.hold(for: connection.id) == .open) + + first.pingError = MockDatabaseDriver.MockError.scripted + factory.queue(driver(tables: ["orders"])) + await coordinator.reconnectIfNeeded() + + #expect(coordinator.showFailureAlert) + #expect(coordinator.failureAlertMessage == ConnectionCoordinator.lostTransactionMessage(.whileIdle, connectionName: "Prod")) + #expect(monitor.hold(for: connection.id) == nil) + } + + @Test("A redial with nothing held reconnects quietly") + func redialWithoutAHoldIsQuiet() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let coordinator = await attachedCoordinator() + + first.pingError = MockDatabaseDriver.MockError.scripted + factory.queue(driver(tables: ["orders"])) + await coordinator.reconnectIfNeeded() + + #expect(!coordinator.showFailureAlert) + #expect(isConnected(coordinator)) + #expect(monitor.rollBackCount(for: connection.id) == 0) + } + + @Test("A Commit whose reply is lost redials and says its outcome is not known") + func failedCommitOnADeadSessionRedials() async throws { + let first = driver(tables: ["orders"], state: .explicitTransaction) + try await dial(first) + let coordinator = await attachedCoordinator() + #expect(monitor.hold(for: connection.id) == .open) + let token = try #require(coordinator.transactionToken) + + first.commitError = MockDatabaseDriver.MockError.scripted + first.pingError = MockDatabaseDriver.MockError.scripted + let second = driver(tables: ["orders"]) + factory.queue(second) + try await coordinator.commitTransaction(as: token) + await ObservedCondition.wait { monitor.hold(for: connection.id) == nil } + + #expect(coordinator.session?.driver === second) + #expect(coordinator.showFailureAlert) + #expect(coordinator.failureAlertMessage == ConnectionCoordinator.lostTransactionMessage(.duringWork, connectionName: "Prod")) + #expect(isConnected(coordinator)) + } + + @Test("A foreground check while tracked work is out says the outcome is not known") + func foregroundCheckDuringWorkIsAmbiguous() async throws { + let first = driver(tables: ["orders"], state: .explicitTransaction) + try await dial(first) + let coordinator = await attachedCoordinator() + let gate = TestGate() + let work = Task { await coordinator.track { await gate.enter() } } + await gate.waitUntilEntered() + + first.pingError = MockDatabaseDriver.MockError.scripted + factory.queue(driver(tables: ["orders"])) + await coordinator.reconnectIfNeeded() + await gate.open() + await work.value + + #expect(coordinator.failureAlertMessage == ConnectionCoordinator.lostTransactionMessage(.duringWork, connectionName: "Prod")) + } + + @Test("A Roll Back that loses the connection redials without an alert: the transaction is gone as asked") + func lostRollBackRedialsQuietly() async throws { + let first = driver(tables: ["orders"], state: .explicitTransaction) + try await dial(first) + let coordinator = await attachedCoordinator() + let token = try #require(coordinator.transactionToken) + + first.rollbackError = MockDatabaseDriver.MockError.scripted + first.pingError = MockDatabaseDriver.MockError.scripted + let second = driver(tables: ["orders"]) + factory.queue(second) + try await coordinator.rollBackTransaction(as: token) + await ObservedCondition.wait { monitor.hold(for: connection.id) == nil } + + #expect(coordinator.session?.driver === second) + #expect(!coordinator.showFailureAlert) + // Row screens and grids leave or reload on this, as after a Roll Back that went through. + #expect(monitor.rollBackCount(for: connection.id) == 1) + } + + @Test("A window left on a replaced session shows no hold, and its Commit refuses rather than commit the newer one") + func staleWindowRefusesTheNewerSessionsCommit() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let coordinator = await attachedCoordinator() + let other = driver(tables: ["staging_orders"], state: .explicitTransaction) + try await dial(other, database: "app_staging") + await monitor.refresh(connection.id) + + #expect(monitor.hold(for: connection.id) == .open) + #expect(coordinator.transactionHold == nil) + #expect(coordinator.transactionToken == nil) + await #expect(throws: TransactionActionError.sessionReplaced) { + try await coordinator.commitTransaction(as: SessionTransactionMonitor.HoldToken(driver: first, workGeneration: 0)) + } + + #expect(!other.didCommitTransaction) + #expect(coordinator.session?.driver === other) + #expect(coordinator.activeDatabase == "app_staging") + #expect(coordinator.transactionHold == .open) + } + + @Test("Two windows sharing a redial both read the schema it restored") + func sharedRedialRestoresTheSchemaFirst() async throws { + let warehouse = DatabaseConnection( + name: "Warehouse", + type: .postgresql, + host: "pg.example.com", + port: 5_432, + database: "analytics" + ) + let first = driver(tables: ["facts"]) + first.currentSchema = "public" + factory.queue(first) + _ = try await appState.connectionManager.connect(warehouse) + let left = ConnectionCoordinator(connection: warehouse, appState: appState) + await left.connect() + await left.switchSchema(to: "sales") + let right = ConnectionCoordinator(connection: warehouse, appState: appState) + await right.connect() + #expect(right.activeSchema == "sales") + let dialsBefore = factory.dialed.count + + first.pingError = MockDatabaseDriver.MockError.scripted + let second = driver(tables: ["facts"]) + second.currentSchema = "public" + factory.queue(second) + async let leftDone: Void = left.reconnectIfNeeded() + async let rightDone: Void = right.reconnectIfNeeded() + _ = await (leftDone, rightDone) + + #expect(factory.dialed.count == dialsBefore + 1) + #expect(second.currentSchema == "sales") + #expect(left.activeSchema == "sales") + #expect(right.activeSchema == "sales") + } + + @Test("A Roll Back reloads the table list, so a dropped table it undid comes back") + func rollBackReloadsTables() async throws { + let first = driver(tables: ["orders"], state: .explicitTransaction) + try await dial(first) + let coordinator = await attachedCoordinator() + first.scriptedTables = [] + await coordinator.refreshTables() + #expect(coordinator.tables.isEmpty) + + first.scriptedTables = [TableInfo(name: "orders")] + await coordinator.reloadAfterRollBack() + + #expect(tableNames(coordinator) == ["orders"]) + } + + @Test("Refreshing the table list shows a transaction a read opened") + func tableRefreshPicksUpAReadTransaction() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let coordinator = await attachedCoordinator() + #expect(coordinator.transactionHold == nil) + + first.scriptedTransactionState = .implicitTransaction + await coordinator.refreshTables() + + #expect(coordinator.transactionHold == .open) + } + + @Test("A Commit refused on a live session reports the error and keeps the session") + func refusedCommitOnALiveSessionThrows() async throws { + let first = driver(tables: ["orders"], state: .explicitTransaction) + try await dial(first) + let coordinator = await attachedCoordinator() + first.scriptedTransactionState = .abortedTransaction + let token = try #require(coordinator.transactionToken) + + await #expect(throws: TransactionActionError.cannotCommit) { + try await coordinator.commitTransaction(as: token) + } + + #expect(coordinator.session?.driver === first) + #expect(monitor.hold(for: connection.id) == .aborted) + #expect(!coordinator.showFailureAlert) + } + + @Test("Tracked work that fails on a dead session redials") + func failedWorkRedials() async throws { + let first = driver(tables: ["orders"]) + try await dial(first) + let coordinator = await attachedCoordinator() + + first.pingError = MockDatabaseDriver.MockError.scripted + let second = driver(tables: ["orders"]) + factory.queue(second) + let ranIntoAnError = await coordinator.track(failed: { $0 }) { true } + + #expect(ranIntoAnError) + #expect(coordinator.session?.driver === second) + } + + @Test("A reattached session whose catalog an aborted transaction refuses is kept, and reloads after Roll Back") + func refusedMetadataReloadsAfterRollBack() async throws { + let first = driver(state: .abortedTransaction) + first.fetchTablesError = MockDatabaseDriver.MockError.scripted + try await dial(first) + let coordinator = await attachedCoordinator() + + #expect(isConnected(coordinator)) + #expect(coordinator.session?.driver === first) + #expect(monitor.hold(for: connection.id) == .aborted) + #expect(coordinator.tables.isEmpty) + + first.fetchTablesError = nil + first.scriptedTables = [TableInfo(name: "orders")] + first.scriptedTransactionState = .idle + try await coordinator.rollBackTransaction(as: #require(coordinator.transactionToken)) + + #expect(first.didRollbackTransaction) + #expect(monitor.hold(for: connection.id) == nil) + #expect(tableNames(coordinator) == ["orders"]) + } +} diff --git a/apple/TableProMobile/TableProMobileTests/ConnectionLibrary/ConnectionListRowModelTests.swift b/apple/TableProMobile/TableProMobileTests/ConnectionLibrary/ConnectionListRowModelTests.swift index c0e87219ab..4bb5812e19 100644 --- a/apple/TableProMobile/TableProMobileTests/ConnectionLibrary/ConnectionListRowModelTests.swift +++ b/apple/TableProMobile/TableProMobileTests/ConnectionLibrary/ConnectionListRowModelTests.swift @@ -57,6 +57,27 @@ struct ConnectionListRowModelTests { #expect(!row.accessibilityLabel.contains("·")) } + @Test("A row whose session holds a transaction says so after Favorite, since its mark is not read on its own") + func transactionIsSpokenAfterFavorite() { + let connection = DatabaseConnection(name: "Prod", type: .postgresql, isFavorite: true) + + let holding = ConnectionListRowModel( + connection: connection, + section: .connections, + tags: [], + groups: [], + holdsTransaction: true + ) + let idle = ConnectionListRowModel(connection: connection, section: .connections, tags: [], groups: []) + + #expect(holding.holdsTransaction) + #expect(holding.accessibilityLabel.hasSuffix( + [String(localized: "Favorite"), String(localized: "Transaction open")].joined(separator: ", ") + )) + #expect(!idle.holdsTransaction) + #expect(!idle.accessibilityLabel.contains(String(localized: "Transaction open"))) + } + @Test("A row that opens only on a Mac is marked, and VoiceOver names the engine and the reason") func macOnlyRowIsMarked() { let mongo = DatabaseConnection(name: "Events", type: .mongodb, host: "db.acme.io") diff --git a/apple/TableProMobile/TableProMobileTests/Drivers/DuckDBDriverTests.swift b/apple/TableProMobile/TableProMobileTests/Drivers/DuckDBDriverTests.swift index cdcf4ded22..732a17dd9a 100644 --- a/apple/TableProMobile/TableProMobileTests/Drivers/DuckDBDriverTests.swift +++ b/apple/TableProMobile/TableProMobileTests/Drivers/DuckDBDriverTests.swift @@ -128,6 +128,40 @@ final class DuckDBDriverTests: XCTestCase { XCTAssertTrue(alive) } + func testPingInsideAnAbortedTransactionKeepsTheSession() async throws { + let driver = try XCTUnwrap(driver) + _ = try await driver.execute(query: "CREATE TABLE keep (x INTEGER)") + _ = try await driver.execute(query: "INSERT INTO keep VALUES (1)") + _ = try await driver.execute(query: "CREATE TABLE u (x INTEGER PRIMARY KEY)") + _ = try await driver.execute(query: "BEGIN TRANSACTION") + _ = try await driver.execute(query: "INSERT INTO u VALUES (1)") + do { + _ = try await driver.execute(query: "INSERT INTO u VALUES (1)") + XCTFail("A duplicate key must fail") + } catch {} + do { + _ = try await driver.execute(query: "SELECT 1") + XCTFail("An aborted DuckDB transaction must refuse a statement") + } catch {} + + let alive = try await driver.ping() + XCTAssertTrue(alive) + + _ = try await driver.execute(query: "ROLLBACK") + let kept = try await driver.execute(query: "SELECT x FROM keep") + XCTAssertEqual(kept.rows.first?.first ?? nil, "1") + } + + func testPingAfterDisconnectFails() async throws { + let closed = DuckDBDriver(source: .inMemory) + try await closed.connect() + try await closed.disconnect() + do { + _ = try await closed.ping() + XCTFail("A closed DuckDB session must fail its ping") + } catch DuckDBDriverError.notConnected {} + } + func testServerVersionIsReported() async throws { let driver = try XCTUnwrap(driver) XCTAssertNotNil(driver.serverVersion) diff --git a/apple/TableProMobile/TableProMobileTests/Drivers/MySQLDriverTransactionTests.swift b/apple/TableProMobile/TableProMobileTests/Drivers/MySQLDriverTransactionTests.swift index 2493d2a2da..3299ed8c72 100644 --- a/apple/TableProMobile/TableProMobileTests/Drivers/MySQLDriverTransactionTests.swift +++ b/apple/TableProMobile/TableProMobileTests/Drivers/MySQLDriverTransactionTests.swift @@ -97,6 +97,21 @@ struct MySQLSessionTransactionTests { #expect(MySQLSessionTransaction.state(infoResult: 0, serverStatus: 0) == .idle) } + @Test("MySQL, MariaDB and TiDB read the session from the reply's status flags") + func measuredFlavorsReportTheStatusFlags() { + #expect(MySQLDriver.reportsSessionStatusFlags(for: .mysql, banner: "8.4.11")) + #expect(MySQLDriver.reportsSessionStatusFlags(for: .mariadb, banner: "10.6.28-MariaDB-ubu2204")) + #expect(MySQLDriver.reportsSessionStatusFlags(for: .tidb, banner: "8.0.11-TiDB-v8.5.0")) + #expect(MySQLDriver.reportsSessionStatusFlags(for: .mysql, banner: nil)) + } + + @Test("OceanBase and Databend report an unknown session, since their status flags were never measured") + func unmeasuredFlavorsReportNothing() { + #expect(!MySQLDriver.reportsSessionStatusFlags(for: .oceanbase, banner: "5.7.25-OceanBase_CE-v4.2.1")) + #expect(!MySQLDriver.reportsSessionStatusFlags(for: .oceanbase, banner: nil)) + #expect(!MySQLDriver.reportsSessionStatusFlags(for: .mysql, banner: "8.0.17-v1.2.615-nightly")) + } + @Test("Unrelated status bits do not change the answer") func unrelatedBitsAreIgnored() { let noBackslashEscapes = UInt32(SERVER_STATUS_NO_BACKSLASH_ESCAPES) diff --git a/apple/TableProMobile/TableProMobileTests/Drivers/PostgreSQLTransactionStatementTests.swift b/apple/TableProMobile/TableProMobileTests/Drivers/PostgreSQLTransactionStatementTests.swift index dc3a19533a..a066876055 100644 --- a/apple/TableProMobile/TableProMobileTests/Drivers/PostgreSQLTransactionStatementTests.swift +++ b/apple/TableProMobile/TableProMobileTests/Drivers/PostgreSQLTransactionStatementTests.swift @@ -25,15 +25,79 @@ struct PostgreSQLSessionTransactionTests { #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_IDLE) == .idle) } - @Test("Every transaction block reports the user's own transaction") - func openTransactionBlocks() { + @Test("An open transaction block reports the user's own transaction") + func openTransactionBlock() { #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_INTRANS) == .explicitTransaction) - #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_INERROR) == .explicitTransaction) - #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE) == .explicitTransaction) + } + + @Test("A block a failed statement aborted reports a transaction that can no longer commit") + func abortedTransactionBlock() { + #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_INERROR) == .abortedTransaction) + } + + @Test("A statement in flight reports the transaction it was sent in") + func statementInFlightReportsStatusAtSend() { + #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE, statusAtSend: PQTRANS_IDLE) == .idle) + #expect( + PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE, statusAtSend: PQTRANS_INTRANS) + == .explicitTransaction + ) + #expect( + PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE, statusAtSend: PQTRANS_INERROR) + == .abortedTransaction + ) + } + + @Test("A statement in flight with no status recorded at send reports an unknown state") + func statementInFlightWithoutStatusAtSend() { + #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE) == .unknown) + #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE, statusAtSend: PQTRANS_ACTIVE) == .unknown) + #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_ACTIVE, statusAtSend: PQTRANS_UNKNOWN) == .unknown) } @Test("A connection libpq cannot read reports an unknown state") func unknownStatus() { #expect(PostgreSQLSessionTransaction.state(from: PQTRANS_UNKNOWN) == .unknown) } + + @Test("Ping sends a statement only on an idle session") + func pingSendsStatementOnlyWhenIdle() { + #expect(PostgreSQLSessionTransaction.pingSendsStatement(in: PQTRANS_IDLE)) + for status in [PQTRANS_INTRANS, PQTRANS_INERROR, PQTRANS_ACTIVE, PQTRANS_UNKNOWN] { + #expect(!PostgreSQLSessionTransaction.pingSendsStatement(in: status)) + } + } + + @Test("A COMMIT the server answered with ROLLBACK in an aborted block says nothing was kept") + func commitInAbortedBlockSaysItRolledBack() { + #expect( + PostgreSQLSessionTransaction.statusMessage( + statusAtSend: PQTRANS_INERROR, commandTag: "ROLLBACK", statusAfter: PQTRANS_IDLE + ) == "The failed transaction was rolled back. None of its changes were kept." + ) + } + + @Test("A rollback to a savepoint, of a live transaction, or any other command adds no message") + func otherCommandsAddNoMessage() { + #expect( + PostgreSQLSessionTransaction.statusMessage( + statusAtSend: PQTRANS_INERROR, commandTag: "ROLLBACK", statusAfter: PQTRANS_INTRANS + ) == nil + ) + #expect( + PostgreSQLSessionTransaction.statusMessage( + statusAtSend: PQTRANS_INTRANS, commandTag: "ROLLBACK", statusAfter: PQTRANS_IDLE + ) == nil + ) + #expect( + PostgreSQLSessionTransaction.statusMessage( + statusAtSend: PQTRANS_INTRANS, commandTag: "COMMIT", statusAfter: PQTRANS_IDLE + ) == nil + ) + #expect( + PostgreSQLSessionTransaction.statusMessage( + statusAtSend: PQTRANS_IDLE, commandTag: "INSERT 0 1", statusAfter: PQTRANS_IDLE + ) == nil + ) + } } diff --git a/apple/TableProMobile/TableProMobileTests/Helpers/RowInserterTests.swift b/apple/TableProMobile/TableProMobileTests/Helpers/RowInserterTests.swift index 91bb1b3866..6906dca2bd 100644 --- a/apple/TableProMobile/TableProMobileTests/Helpers/RowInserterTests.swift +++ b/apple/TableProMobile/TableProMobileTests/Helpers/RowInserterTests.swift @@ -98,19 +98,39 @@ struct RowInserterTests { #expect(!driver.didCommitTransaction) } - @Test("wraps and commits when autocommit is off, so the rows persist") - func implicitTransactionWraps() async throws { + @Test("joins the transaction autocommit off opened and commits it, so the rows persist") + func implicitTransactionIsCommitted() async throws { let driver = makeDriver(results: [ok(), ok()]) driver.scriptedTransactionState = .implicitTransaction let rows = [ PayloadRow(values: ["name": .text("Ada")]), PayloadRow(values: ["name": .text("Grace")]) ] - _ = try await RowInserter.insert( - driver: driver, table: "people", type: .postgresql, schema: nil, qualifier: nil, rows: rows + let affected = try await RowInserter.insert( + driver: driver, table: "people", type: .mysql, schema: nil, qualifier: nil, rows: rows ) - #expect(driver.beganTransactionModes == [.readWrite]) + #expect(affected == 2) + #expect(!driver.didBeginTransaction) #expect(driver.didCommitTransaction) + #expect(!driver.didRollbackTransaction) + } + + @Test("rolls back the autocommit-off transaction when a row fails") + func implicitTransactionRollsBackOnFailure() async throws { + let driver = makeDriver(results: [ok(), .failure(MockDatabaseDriver.MockError.scripted)]) + driver.scriptedTransactionState = .implicitTransaction + let rows = [ + PayloadRow(values: ["name": .text("Ada")]), + PayloadRow(values: ["name": .text("Grace")]) + ] + await #expect(throws: (any Error).self) { + _ = try await RowInserter.insert( + driver: driver, table: "people", type: .mysql, schema: nil, qualifier: nil, rows: rows + ) + } + #expect(!driver.didBeginTransaction) + #expect(!driver.didCommitTransaction) + #expect(driver.didRollbackTransaction) } @Test("does not open a transaction for a single row on a driver that cannot report its session") diff --git a/apple/TableProMobile/TableProMobileTests/Mocks/MockDatabaseDriver.swift b/apple/TableProMobile/TableProMobileTests/Mocks/MockDatabaseDriver.swift index 8998a650c7..f5b2a658fa 100644 --- a/apple/TableProMobile/TableProMobileTests/Mocks/MockDatabaseDriver.swift +++ b/apple/TableProMobile/TableProMobileTests/Mocks/MockDatabaseDriver.swift @@ -19,6 +19,10 @@ final class MockDatabaseDriver: DatabaseDriver, @unchecked Sendable { var scriptedTransactionStates: [DriverTransactionState] = [] /// Answered in order, one per call, then nil. var scriptedLexicalFacts: [SQLSessionLexicalFacts?] = [] + var pingError: Error? + var fetchTablesError: Error? + var commitError: Error? + var rollbackError: Error? private(set) var executedQueries: [String] = [] private(set) var fetchColumnsCalls: Int = 0 @@ -29,6 +33,7 @@ final class MockDatabaseDriver: DatabaseDriver, @unchecked Sendable { private(set) var didBeginTransaction = false private(set) var didCommitTransaction = false private(set) var didRollbackTransaction = false + private(set) var pingCount = 0 var supportsSchemas: Bool = false var currentSchema: String? @@ -46,12 +51,17 @@ final class MockDatabaseDriver: DatabaseDriver, @unchecked Sendable { var beforeDisconnect: (@Sendable () async -> Void)? var beforeExecute: (@Sendable () async -> Void)? + var beforeTransactionState: (@Sendable () async -> Void)? func connect() async throws {} func disconnect() async throws { await beforeDisconnect?() } - func ping() async throws -> Bool { true } + func ping() async throws -> Bool { + pingCount += 1 + if let pingError { throw pingError } + return true + } func cancelCurrentQuery() async throws {} func execute(query: String) async throws -> QueryResult { @@ -66,7 +76,10 @@ final class MockDatabaseDriver: DatabaseDriver, @unchecked Sendable { } } - func fetchTables(schema: String?) async throws -> [TableInfo] { scriptedTables } + func fetchTables(schema: String?) async throws -> [TableInfo] { + if let fetchTablesError { throw fetchTablesError } + return scriptedTables + } func fetchColumns(table: String, schema: String?) async throws -> [ColumnInfo] { fetchColumnsCalls += 1 @@ -86,7 +99,9 @@ final class MockDatabaseDriver: DatabaseDriver, @unchecked Sendable { func fetchDatabases() async throws -> [String] { scriptedDatabases } func fetchSchemas() async throws -> [String] { scriptedSchemas } func switchDatabase(to name: String) async throws {} - func switchSchema(to name: String) async throws {} + func switchSchema(to name: String) async throws { + currentSchema = name + } func beginTransaction() async throws { didBeginTransaction = true } func beginTransaction(mode: PluginTransactionAccessMode) async throws { @@ -94,14 +109,23 @@ final class MockDatabaseDriver: DatabaseDriver, @unchecked Sendable { didBeginTransaction = true } - func commitTransaction() async throws { didCommitTransaction = true } - func rollbackTransaction() async throws { didRollbackTransaction = true } + func commitTransaction() async throws { + if let commitError { throw commitError } + didCommitTransaction = true + } + + func rollbackTransaction() async throws { + if let rollbackError { throw rollbackError } + didRollbackTransaction = true + } + func sessionLexicalFacts() async -> SQLSessionLexicalFacts? { guard !scriptedLexicalFacts.isEmpty else { return nil } return scriptedLexicalFacts.removeFirst() } func sessionTransactionState() async -> DriverTransactionState { + await beforeTransactionState?() guard !scriptedTransactionStates.isEmpty else { return scriptedTransactionState } return scriptedTransactionStates.removeFirst() } diff --git a/apple/TableProMobile/TableProMobileTests/Mocks/StubDriverFactory.swift b/apple/TableProMobile/TableProMobileTests/Mocks/StubDriverFactory.swift new file mode 100644 index 0000000000..aade06b32b --- /dev/null +++ b/apple/TableProMobile/TableProMobileTests/Mocks/StubDriverFactory.swift @@ -0,0 +1,48 @@ +import Foundation +import TableProDatabase +import TableProModels + +final class StubDriverFactory: DriverFactory, @unchecked Sendable { + private let lock = NSLock() + private var driversByType: [String: any DatabaseDriver] = [:] + private var queuedDrivers: [any DatabaseDriver] = [] + private var dialedConnections: [DatabaseConnection] = [] + + var drivers: [String: any DatabaseDriver] { + get { lock.withLock { driversByType } } + set { lock.withLock { driversByType = newValue } } + } + + var dialed: [DatabaseConnection] { + lock.withLock { dialedConnections } + } + + // Handed out one per dial, ahead of `drivers`, so each redial gets a driver of its own. + func queue(_ driver: any DatabaseDriver) { + lock.withLock { queuedDrivers.append(driver) } + } + + func createDriver(for connection: DatabaseConnection, password: String?) throws -> any DatabaseDriver { + try lock.withLock { + dialedConnections.append(connection) + if !queuedDrivers.isEmpty { + return queuedDrivers.removeFirst() + } + guard let driver = driversByType[connection.type.rawValue] else { + throw ConnectionError.driverNotFound(connection.type.rawValue) + } + return driver + } + } + + func supportedTypes() -> [DatabaseType] { drivers.keys.map(DatabaseType.init(rawValue:)) } + + // A queued driver answers a dial of any type. + func refusal(for connection: DatabaseConnection) -> ConnectionError? { + lock.withLock { + queuedDrivers.isEmpty && driversByType[connection.type.rawValue] == nil + ? ConnectionError.driverNotFound(connection.type.rawValue) + : nil + } + } +} diff --git a/apple/TableProMobile/TableProMobileTests/Onboarding/AppStateFixture.swift b/apple/TableProMobile/TableProMobileTests/Onboarding/AppStateFixture.swift index 3bbb45d740..ecc4a7db8e 100644 --- a/apple/TableProMobile/TableProMobileTests/Onboarding/AppStateFixture.swift +++ b/apple/TableProMobile/TableProMobileTests/Onboarding/AppStateFixture.swift @@ -49,7 +49,8 @@ struct AppStateFixture { func makeState( syncEnabled: Bool, secureStore: any SecureStore = MockSecureStore(), - libraryPublisher: ConnectionLibraryPublisher? = nil + libraryPublisher: ConnectionLibraryPublisher? = nil, + driverFactory: (any DriverFactory)? = nil ) -> AppState { let coordinator = IOSSyncCoordinator( metadata: metadata, @@ -73,7 +74,8 @@ struct AppStateFixture { searchIndex: RecordingSearchIndex(), writeWidgetItems: { _ in }, refreshShortcutParameters: {} - ) + ), + driverFactory: driverFactory ) } diff --git a/apple/TableProMobile/TableProMobileTests/Platform/SessionTransactionMonitorTests.swift b/apple/TableProMobile/TableProMobileTests/Platform/SessionTransactionMonitorTests.swift new file mode 100644 index 0000000000..d4e18cf25b --- /dev/null +++ b/apple/TableProMobile/TableProMobileTests/Platform/SessionTransactionMonitorTests.swift @@ -0,0 +1,263 @@ +import Foundation +import TableProDatabase +@testable import TableProMobile +import Testing + +@MainActor +private final class LiveSessions { + var drivers: [UUID: any DatabaseDriver] = [:] +} + +@MainActor +@Suite("Session transaction monitor", .timeLimit(.minutes(1))) +struct SessionTransactionMonitorTests { + private let id = UUID() + private let live: LiveSessions + private let monitor: SessionTransactionMonitor + + init() { + let live = LiveSessions() + self.live = live + monitor = SessionTransactionMonitor { live.drivers[$0] } + } + + @discardableResult + private func attach(_ state: DriverTransactionState = .idle) -> MockDatabaseDriver { + let driver = MockDatabaseDriver() + driver.scriptedTransactionState = state + live.drivers[id] = driver + return driver + } + + @Test("A run that leaves a transaction open shows it, explicit or implicit") + func runLeavesTheTransactionOpen() async { + let driver = attach(.explicitTransaction) + await monitor.track(id) {} + #expect(monitor.hold(for: id) == .open) + + driver.scriptedTransactionState = .implicitTransaction + await monitor.track(id) {} + #expect(monitor.hold(for: id) == .open) + } + + @Test("A typed COMMIT clears it on the next settle") + func typedCommitClears() async { + let driver = attach(.explicitTransaction) + await monitor.track(id) {} + + driver.scriptedTransactionState = .idle + await monitor.track(id) {} + + #expect(monitor.hold(for: id) == nil) + } + + @Test("Work that throws still refreshes") + func throwingWorkRefreshes() async { + attach(.abortedTransaction) + + await #expect(throws: MockDatabaseDriver.MockError.self) { + try await monitor.track(id) { throw MockDatabaseDriver.MockError.scripted } + } + + #expect(monitor.hold(for: id) == .aborted) + } + + @Test("Commit and Roll Back run on the live driver and clear the indicator") + func commitAndRollBackHitTheLiveDriver() async throws { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + driver.scriptedTransactionStates = [.explicitTransaction, .idle] + + try await monitor.commit(id, as: #require(monitor.token(for: id, on: driver))) + + #expect(driver.didCommitTransaction) + #expect(monitor.hold(for: id) == nil) + + driver.scriptedTransactionState = .explicitTransaction + await monitor.refresh(id) + driver.scriptedTransactionState = .idle + + try await monitor.rollBack(id, as: #require(monitor.token(for: id, on: driver))) + + #expect(driver.didRollbackTransaction) + #expect(monitor.hold(for: id) == nil) + } + + @Test("An aborted transaction refuses Commit without sending it and still rolls back") + func abortedRefusesCommit() async throws { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + driver.scriptedTransactionState = .abortedTransaction + let commitToken = try #require(monitor.token(for: id, on: driver)) + + await #expect(throws: TransactionActionError.cannotCommit) { + try await monitor.commit(id, as: commitToken) + } + #expect(!driver.didCommitTransaction) + #expect(monitor.hold(for: id) == .aborted) + + driver.scriptedTransactionStates = [.idle] + try await monitor.rollBack(id, as: #require(monitor.token(for: id, on: driver))) + + #expect(driver.didRollbackTransaction) + #expect(monitor.hold(for: id) == nil) + } + + @Test("Commit on a session that replaced the one holding the transaction is refused and clears it") + func replacedSessionRefusesCommit() async throws { + let old = attach(.explicitTransaction) + await monitor.refresh(id) + let token = try #require(monitor.token(for: id, on: old)) + let replacement = attach(.idle) + + await #expect(throws: TransactionActionError.sessionReplaced) { + try await monitor.commit(id, as: token) + } + + #expect(!old.didCommitTransaction) + #expect(!replacement.didCommitTransaction) + #expect(monitor.hold(for: id) == nil) + } + + @Test("A window on a replaced session sees no hold and cannot act on the newer session's transaction") + func staleWindowCannotActOnTheNewerSession() async { + let old = attach(.idle) + let newer = attach(.explicitTransaction) + await monitor.refresh(id) + + #expect(monitor.hold(for: id, on: old) == nil) + #expect(monitor.token(for: id, on: old) == nil) + #expect(monitor.hold(for: id, on: newer) == .open) + await #expect(throws: TransactionActionError.sessionReplaced) { + try await monitor.rollBack(id, as: SessionTransactionMonitor.HoldToken(driver: old, workGeneration: 0)) + } + + #expect(!newer.didRollbackTransaction) + #expect(monitor.hold(for: id) == .open) + } + + @Test("A confirmation goes stale once other work runs on the session") + func workAfterTheTokenRefusesTheAction() async throws { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + let token = try #require(monitor.token(for: id, on: driver)) + + await monitor.track(id) {} + + await #expect(throws: TransactionActionError.transactionChanged) { + try await monitor.rollBack(id, as: token) + } + #expect(!driver.didRollbackTransaction) + #expect(monitor.hold(for: id) == .open) + } + + @Test("Work that runs while Commit reads the session's state refuses the Commit") + func workDuringTheCommitReadRefusesIt() async throws { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + let token = try #require(monitor.token(for: id, on: driver)) + let gate = TestGate() + driver.beforeTransactionState = { await gate.enter() } + + let commit = Task { try await monitor.commit(id, as: token) } + await gate.waitUntilEntered() + driver.beforeTransactionState = nil + await monitor.track(id) {} + await gate.open() + + await #expect(throws: TransactionActionError.transactionChanged) { + try await commit.value + } + #expect(!driver.didCommitTransaction) + } + + @Test("Only a Roll Back that went through counts") + func rollBackCountsOnlySuccess() async throws { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + driver.rollbackError = MockDatabaseDriver.MockError.scripted + + let failing = try #require(monitor.token(for: id, on: driver)) + await #expect(throws: MockDatabaseDriver.MockError.self) { + try await monitor.rollBack(id, as: failing) + } + #expect(monitor.rollBackCount(for: id) == 0) + + driver.rollbackError = nil + let token = try #require(monitor.token(for: id, on: driver)) + driver.scriptedTransactionState = .idle + try await monitor.rollBack(id, as: token) + + #expect(monitor.rollBackCount(for: id) == 1) + } + + @Test("A late answer from a driver that was replaced while it answered is dropped") + func lateAnswerFromReplacedDriverIsDropped() async { + let gate = TestGate() + let old = attach(.explicitTransaction) + old.beforeTransactionState = { await gate.enter() } + + let pending = Task { await monitor.refresh(id) } + await gate.waitUntilEntered() + attach(.unknown) + await gate.open() + await pending.value + + #expect(monitor.hold(for: id) == nil) + } + + @Test("A session's end clears only the entry its own driver holds") + func sessionEndClearsOnlyItsDriver() async { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + + monitor.sessionEnded(id, driver: MockDatabaseDriver()) + #expect(monitor.hold(for: id) == .open) + + monitor.sessionEnded(id, driver: driver) + #expect(monitor.hold(for: id) == nil) + } + + @Test("An unknown answer keeps what the session last reported") + func unknownKeepsTheEntry() async { + let driver = attach(.explicitTransaction) + await monitor.refresh(id) + + driver.scriptedTransactionState = .unknown + await monitor.refresh(id) + + #expect(monitor.hold(for: id) == .open) + } + + @Test("The connection is busy while tracked work runs, and free once it settles") + func busyDuringTrack() async { + attach() + let gate = TestGate() + + let work = Task { await monitor.track(id) { await gate.enter() } } + await gate.waitUntilEntered() + #expect(monitor.isBusy(id)) + + await gate.open() + await work.value + #expect(!monitor.isBusy(id)) + } + + @Test("A real SQLite session shows BEGIN as open and Commit clears it") + func sqliteBeginThenCommit() async throws { + let driver = SQLiteDriver(source: .inMemory) + try await driver.connect() + live.drivers[id] = driver + + _ = try await monitor.track(id) { try await driver.execute(query: "BEGIN") } + let afterBegin = monitor.hold(for: id) + try await monitor.commit(id, as: #require(monitor.token(for: id, on: driver))) + let afterCommit = monitor.hold(for: id) + let serverState = await driver.sessionTransactionState() + try await driver.disconnect() + + #expect(afterBegin == .open) + #expect(afterCommit == nil) + #expect(serverState == .idle) + } +} diff --git a/apple/TableProMobile/TableProMobileTests/QueryEditorViewModelTests.swift b/apple/TableProMobile/TableProMobileTests/QueryEditorViewModelTests.swift index 2114a7970b..135f8de346 100644 --- a/apple/TableProMobile/TableProMobileTests/QueryEditorViewModelTests.swift +++ b/apple/TableProMobile/TableProMobileTests/QueryEditorViewModelTests.swift @@ -247,6 +247,25 @@ struct QueryEditorViewModelTests { #expect(vm.interruption?.standingWork == .openTransaction) } + @Test("a failure that aborted the script's transaction says to roll it back, not to commit") + func failureInsideAbortedTransaction() async { + let driver = MockDatabaseDriver() + driver.scriptedTransactionStates = [.explicitTransaction, .explicitTransaction, .abortedTransaction] + driver.scriptedExecuteResults = [ + .success(QueryResult(columns: [], rows: [], rowsAffected: 0, executionTime: 0)), + .success(QueryResult(columns: [], rows: [], rowsAffected: 1, executionTime: 0)), + .failure(MockDatabaseDriver.MockError.scripted), + ] + + let vm = QueryEditorViewModel(windowCapacity: 100) + await vm.run(driver: driver, script: .statements("BEGIN", "INSERT INTO t VALUES (1)", "SELECT 1/0")) + + #expect(vm.interruption?.standingWork == .abortedTransaction) + #expect( + vm.standingWorkNote == "The transaction on this connection can no longer be committed. Roll it back." + ) + } + @Test("a failing first statement leaves nothing standing behind it") func failureOfTheFirstStatement() async { let driver = MockDatabaseDriver() diff --git a/apple/TableProMobile/TableProMobileTests/Support/TestGate.swift b/apple/TableProMobile/TableProMobileTests/Support/TestGate.swift new file mode 100644 index 0000000000..15a4431237 --- /dev/null +++ b/apple/TableProMobile/TableProMobileTests/Support/TestGate.swift @@ -0,0 +1,28 @@ +import Foundation + +// Holds work at a known point until the test lets it go. +actor TestGate { + private var isOpen = false + private var hasEntered = false + private var blocked: [CheckedContinuation] = [] + private var observers: [CheckedContinuation] = [] + + func enter() async { + hasEntered = true + for observer in observers { observer.resume() } + observers.removeAll() + guard !isOpen else { return } + await withCheckedContinuation { blocked.append($0) } + } + + func open() { + isOpen = true + for waiter in blocked { waiter.resume() } + blocked.removeAll() + } + + func waitUntilEntered() async { + guard !hasEntered else { return } + await withCheckedContinuation { observers.append($0) } + } +} diff --git a/apple/TableProMobile/TableProMobileTests/Views/BottomSafeAreaBarLayoutTests.swift b/apple/TableProMobile/TableProMobileTests/Views/BottomSafeAreaBarLayoutTests.swift index e61dd4db0d..11edfbfb8d 100644 --- a/apple/TableProMobile/TableProMobileTests/Views/BottomSafeAreaBarLayoutTests.swift +++ b/apple/TableProMobile/TableProMobileTests/Views/BottomSafeAreaBarLayoutTests.swift @@ -22,6 +22,27 @@ struct BottomSafeAreaBarLayoutTests { #expect(hit.map { !$0.isDescendant(of: tabBar) } ?? false) } + @Test("A pushed detail screen's bar sits under the screen's own bar and above the tab bar, and both clear its rows") + func pushedScreenBarStacksUnderItsOwnBar() throws { + guard UIDevice.current.userInterfaceIdiom == .phone else { return } + let probe = LayoutProbe() + let host = try HostedTree(probe: probe, variant: .pushedScreenBars) + defer { host.tearDown() } + + let tabBar = try host.settledTabBar() + let marker = probe.markerFrame + let screenBar = probe.screenBarFrame + + #expect(probe.reachedPushedScreen) + #expect(!marker.isEmpty) + #expect(!screenBar.isEmpty) + #expect(!marker.intersects(tabBar.convert(tabBar.bounds, to: host.window))) + #expect(screenBar.maxY <= marker.minY + 0.5) + #expect(probe.listInsets.bottom >= host.window.bounds.maxY - screenBar.minY - 0.5) + let hit = host.window.hitTest(CGPoint(x: marker.midX, y: marker.midY), with: nil) + #expect(hit.map { !$0.isDescendant(of: tabBar) } ?? false) + } + @Test("A hidden bar leaves no blank strip above the tab bar") func emptyBarAddsNoInset() throws { guard UIDevice.current.userInterfaceIdiom == .phone else { return } @@ -42,7 +63,9 @@ struct BottomSafeAreaBarLayoutTests { @MainActor private final class LayoutProbe { var markerFrame: CGRect = .zero + var screenBarFrame: CGRect = .zero var listInsets = EdgeInsets() + var reachedPushedScreen = false } private struct UnsettledLayout: Error, CustomStringConvertible { @@ -55,6 +78,7 @@ private struct HostedTree { case bar case emptyBar case noBar + case pushedScreenBars } private static let layoutTurns = 300 @@ -143,10 +167,7 @@ private struct ProbeTabs: View { var body: some View { TabView { Tab("Tables", systemImage: "tablecells") { - NavigationStack { - content - .navigationTitle("Rows") - } + tables } Tab("Query", systemImage: "terminal") { Text(verbatim: "Query") @@ -155,6 +176,34 @@ private struct ProbeTabs: View { .tabViewStyle(.sidebarAdaptable) } + // The app's Tables tab: a row screen with its own bar is pushed in the detail column and carries the bar too. + @ViewBuilder + private var tables: some View { + if variant == .pushedScreenBars { + NavigationSplitView(preferredCompactColumn: .constant(.detail)) { + List { + Text(verbatim: "albums") + } + } detail: { + NavigationStack(path: .constant([0])) { + Text(verbatim: "Rows") + .navigationDestination(for: Int.self) { _ in + rows + .bottomSafeAreaBar { screenBar } + .bottomSafeAreaBar { marker } + .navigationTitle("Row") + .onAppear { probe.reachedPushedScreen = true } + } + } + } + } else { + NavigationStack { + content + .navigationTitle("Rows") + } + } + } + @ViewBuilder private var content: some View { switch variant { @@ -166,7 +215,7 @@ private struct ProbeTabs: View { marker } } - case .noBar: + case .noBar, .pushedScreenBars: rows } } @@ -192,4 +241,15 @@ private struct ProbeTabs: View { probe.markerFrame = frame } } + + private var screenBar: some View { + Color.blue + .frame(height: 44) + .frame(maxWidth: .infinity) + .onGeometryChange(for: CGRect.self) { proxy in + proxy.frame(in: .global) + } action: { frame in + probe.screenBarFrame = frame + } + } } diff --git a/apple/TableProMobile/TableProMobileTests/Views/TransactionIndicatorContentTests.swift b/apple/TableProMobile/TableProMobileTests/Views/TransactionIndicatorContentTests.swift new file mode 100644 index 0000000000..faf09da866 --- /dev/null +++ b/apple/TableProMobile/TableProMobileTests/Views/TransactionIndicatorContentTests.swift @@ -0,0 +1,139 @@ +import Foundation +@testable import TableProMobile +import TableProModels +import Testing +import UIKit + +struct TransactionIndicatorContentTests { + @Test("An open transaction shows the database it runs in, and VoiceOver also names the connection") + func openNamesDatabaseAndConnection() { + let content = TransactionIndicatorContent( + isAborted: false, + connectionName: "Prod", + activeDatabase: "orders", + switchesDatabases: true, + safeModeLevel: .off + ) + + #expect(content.title == String(localized: "Transaction Open")) + #expect(content.subtitle == "orders") + #expect(content.accessibilityLabel == String( + format: String(localized: "Transaction open on %1$@, %2$@"), + "orders", + "Prod" + )) + #expect(content.symbolName == TransactionIndicatorContent.openSymbolName) + #expect(content.commit == .runs) + } + + @Test("The Sample has no database to name, so the bar and VoiceOver name the connection once") + func sampleNamesTheConnectionOnce() { + let sample = SampleDatabaseInstaller.connectionName + // A SQLite session can restore a stale lastDB, which names nothing on the server. + let content = TransactionIndicatorContent( + isAborted: false, + connectionName: sample, + activeDatabase: SampleDatabaseInstaller.fileName, + switchesDatabases: false, + safeModeLevel: .off + ) + + #expect(content.subtitle == sample) + #expect(content.accessibilityLabel == String(format: String(localized: "Transaction open on %@"), sample)) + #expect(!content.accessibilityLabel.contains(SampleDatabaseInstaller.fileName)) + } + + @Test("An engine that switches databases but has not named one yet falls back to the connection") + func emptyDatabaseFallsBackToConnection() { + let content = TransactionIndicatorContent( + isAborted: false, + connectionName: "Prod", + activeDatabase: "", + switchesDatabases: true, + safeModeLevel: .off + ) + + #expect(content.subtitle == "Prod") + #expect(content.accessibilityLabel == String(format: String(localized: "Transaction open on %@"), "Prod")) + } + + @Test("A failed transaction says to roll it back and offers no Commit") + func abortedOffersOnlyRollBack() { + let content = TransactionIndicatorContent( + isAborted: true, + connectionName: "Prod", + activeDatabase: "orders", + switchesDatabases: true, + safeModeLevel: .off + ) + + #expect(content.title == String(localized: "Transaction Failed")) + #expect(content.subtitle == String(localized: "Roll it back to continue.")) + #expect(content.accessibilityLabel == String( + format: String(localized: "Transaction failed on %1$@, %2$@. Roll it back to continue."), + "orders", + "Prod" + )) + #expect(content.symbolName == TransactionIndicatorContent.abortedSymbolName) + #expect(content.commit == .hidden) + } + + @Test("A failed transaction with no database names the connection once") + func abortedWithoutDatabase() { + let content = TransactionIndicatorContent( + isAborted: true, + connectionName: "Local", + activeDatabase: "", + switchesDatabases: false, + safeModeLevel: .off + ) + + #expect(content.accessibilityLabel == String( + format: String(localized: "Transaction failed on %@. Roll it back to continue."), + "Local" + )) + } + + @Test( + "Commit follows the Safe Mode write permission", + arguments: zip( + [SafeModeLevel.off, .confirmWrites, .readOnly], + [TransactionIndicatorContent.CommitOffer.runs, .asksFirst, .hidden] + ) + ) + func commitFollowsSafeMode(level: SafeModeLevel, offer: TransactionIndicatorContent.CommitOffer) { + let content = TransactionIndicatorContent( + isAborted: false, + connectionName: "Prod", + activeDatabase: "orders", + switchesDatabases: true, + safeModeLevel: level + ) + + #expect(content.commit == offer) + } + + @Test("Only a PostgreSQL or Redshift database switch dials a new session") + func databaseSwitchEndsSession() { + #expect(TransactionIndicatorContent.databaseSwitchEndsSession(for: .postgresql)) + #expect(TransactionIndicatorContent.databaseSwitchEndsSession(for: .redshift)) + #expect(!TransactionIndicatorContent.databaseSwitchEndsSession(for: .mysql)) + #expect(!TransactionIndicatorContent.databaseSwitchEndsSession(for: .mssql)) + #expect(!TransactionIndicatorContent.databaseSwitchEndsSession(for: .sqlite)) + } + + @MainActor + @Test("Every symbol the bar and the connection list draw exists on this system") + func symbolsExist() { + let names = [ + TransactionIndicatorContent.openSymbolName, + TransactionIndicatorContent.abortedSymbolName, + TransactionIndicatorContent.commitSymbolName, + TransactionIndicatorContent.rollBackSymbolName, + ] + + for name in names { + #expect(UIImage(systemName: name) != nil, "\(name)") + } + } +} diff --git a/apple/changelog.d/3688.added.md b/apple/changelog.d/3688.added.md new file mode 100644 index 0000000000..3ce372343c --- /dev/null +++ b/apple/changelog.d/3688.added.md @@ -0,0 +1 @@ +iPhone and iPad show an open transaction on every section of its connection, with Commit and Roll Back. diff --git a/apple/changelog.d/3688.fixed.1.md b/apple/changelog.d/3688.fixed.1.md new file mode 100644 index 0000000000..81ca7edcce --- /dev/null +++ b/apple/changelog.d/3688.fixed.1.md @@ -0,0 +1 @@ +Returning to the app on iPhone and iPad dropped an open PostgreSQL transaction or an in-memory DuckDB database. diff --git a/apple/changelog.d/3688.fixed.2.md b/apple/changelog.d/3688.fixed.2.md new file mode 100644 index 0000000000..d53182a2ab --- /dev/null +++ b/apple/changelog.d/3688.fixed.2.md @@ -0,0 +1 @@ +Saving a row on iPhone and iPad with MySQL autocommit off committed the session's pending changes. diff --git a/apple/changelog.d/3688.fixed.3.md b/apple/changelog.d/3688.fixed.3.md new file mode 100644 index 0000000000..a036abcfe0 --- /dev/null +++ b/apple/changelog.d/3688.fixed.3.md @@ -0,0 +1 @@ +A reconnect on iPhone and iPad went back to the connection's saved database and schema instead of the one in use. diff --git a/apple/changelog.d/3688.fixed.4.md b/apple/changelog.d/3688.fixed.4.md new file mode 100644 index 0000000000..b427e7f1d3 --- /dev/null +++ b/apple/changelog.d/3688.fixed.4.md @@ -0,0 +1 @@ +Oracle DDL such as CREATE TABLE left the connection reporting an open transaction that the DDL had already committed. diff --git a/apple/changelog.d/3688.fixed.md b/apple/changelog.d/3688.fixed.md new file mode 100644 index 0000000000..c0371e18a4 --- /dev/null +++ b/apple/changelog.d/3688.fixed.md @@ -0,0 +1 @@ +A typed COMMIT after a failed PostgreSQL statement on iPhone and iPad reported success while the server kept nothing.