diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f1c46614..6625f2e86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Cancel now stops a connection attempt right away instead of letting it run on in the background. A cancelled connection is also dropped from the last session, so restarting no longer reconnects to a host you gave up on, and it can no longer interrupt a later successful connection. (#1358) - Fixed a crash when clicking a column header on a table whose columns have comments. Sorting such a table quit the app immediately. (#1869) ## [0.57.0] - 2026-07-14 diff --git a/CLAUDE.md b/CLAUDE.md index 1c168b89d..d6e64d560 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -164,6 +164,8 @@ Missing a case produces a wrong "{Language} Query" title on the first frame. **Selection indices are display positions**: `GridSelectionState.indices` come from `NSTableView.selectedRowIndexes` and are display-row positions, not indices into `TableRows.rows`. They match array indices only when `displayIDs` (`valueFilteredIDs ?? sortedIDs`) is nil; a per-column value filter makes them diverge. Resolve any selected index through `DisplayRowMapping` (or `TableViewCoordinator.displayRow(at:)` / `tableRowsIndex(forDisplayRow:)`) before reading or mutating a row; never index `TableRows.rows` with a display position. The row details inspector shipped this bug (#1837). +**Cancelling a connect does not stop the driver**: `Task.cancel()` is cooperative, so it cannot interrupt a driver blocked in a C call. A cancelled attempt keeps running and completes late. Two rules follow. First, a driver that blocks on connect must expose its own abort path and poll it (the PostgreSQL driver uses `PQconnectStart`/`PQconnectPoll` with an app-owned deadline and a cancel flag flipped from `withTaskCancellationHandler`; a blocking `PQconnectdb` cannot be cancelled at all). Second, never assume the losing attempt is gone: every attempt validates its `ConnectionAttemptRegistry` generation before adopting a driver into `activeSessions` or tearing session state down, so a late attempt discards its own driver instead of clobbering the winner. Cancelling also drops the connection from `LastOpenConnections.json` (via `MainContentCoordinator.syncRecoveryList()`, activated windows only) so "Reopen Last Session" never replays a connect the user cancelled. This area shipped the same bug four times (#1185, #1358, #1369). + ### Main Coordinator Pattern `MainContentCoordinator` is the central coordinator, split across 7+ extension files in `Views/Main/Extensions/` (e.g., `+Alerts`, `+Filtering`, `+Pagination`, `+RowOperations`). When adding coordinator functionality, add a new extension file rather than growing the main file. diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift index 82e3df1e9..a2b0fe023 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift @@ -25,6 +25,8 @@ struct LibPQPluginError: Error { message: String(localized: "Not connected to database"), sqlState: nil, detail: nil) static let connectionFailed = LibPQPluginError( message: String(localized: "Failed to establish connection"), sqlState: nil, detail: nil) + static let connectionTimedOut = LibPQPluginError( + message: String(localized: "Timed out while connecting to the server"), sqlState: nil, detail: nil) } // MARK: - Query Result @@ -84,6 +86,9 @@ private func pgOidToTypeName(_ oid: UInt32) -> String { // MARK: - Connection Class final class LibPQPluginConnection: @unchecked Sendable { + private static let connectTimeoutMicroseconds: Int64 = 10_000_000 + private static let pollSliceMicroseconds: Int64 = 100_000 + private var conn: OpaquePointer? private let queue = DispatchQueue(label: "com.TablePro.libpq.plugin", qos: .userInitiated) @@ -101,6 +106,7 @@ final class LibPQPluginConnection: @unchecked Sendable { private var _cachedServerVersion: String? private var _cachedServerVersionNumber: Int32 = 0 private var _isCancelled: Bool = false + private var _isConnectCancelled: Bool = false private var _postgisOidMap: [UInt32: String] = [:] var isConnected: Bool { @@ -154,79 +160,160 @@ final class LibPQPluginConnection: @unchecked Sendable { // MARK: - Connection Management func connect() async throws { - try await pluginDispatchAsyncCancellable(on: queue) { [self] in - func escapeConnParam(_ value: String) -> String { - value.replacingOccurrences(of: "\\", with: "\\\\") - .replacingOccurrences(of: "'", with: "\\'") + stateLock.lock() + _isConnectCancelled = false + stateLock.unlock() + + try await withTaskCancellationHandler { + try await pluginDispatchAsyncCancellable( + on: queue, + cancellationCheck: { [weak self] in self?.isConnectCancelled ?? true } + ) { [self] in + try performConnect() } + } onCancel: { + cancelConnect() + } + } - var connStr = "host='\(escapeConnParam(host))' port='\(port)' dbname='\(escapeConnParam(database))' connect_timeout='10'" + func cancelConnect() { + stateLock.lock() + _isConnectCancelled = true + stateLock.unlock() + } - if !user.isEmpty { - connStr += " user='\(escapeConnParam(user))'" - } + private var isConnectCancelled: Bool { + stateLock.lock() + defer { stateLock.unlock() } + return _isConnectCancelled + } - if let password = password, !password.isEmpty { - connStr += " password='\(escapeConnParam(password))'" - } + private func performConnect() throws { + guard let connection = buildConnectionString().withCString({ PQconnectStart($0) }) else { + throw LibPQPluginError.connectionFailed + } - connStr += " sslmode='\(LibPQSSLMapping.sslmode(for: sslConfig.mode))'" + var adopted = false + defer { + if !adopted { PQfinish(connection) } + } - if sslConfig.verifiesCertificate, !sslConfig.caCertificatePath.isEmpty { - connStr += " sslrootcert='\(escapeConnParam(sslConfig.caCertificatePath))'" - } - if !sslConfig.clientCertificatePath.isEmpty { - connStr += " sslcert='\(escapeConnParam(sslConfig.clientCertificatePath))'" - } - if !sslConfig.clientKeyPath.isEmpty { - connStr += " sslkey='\(escapeConnParam(sslConfig.clientKeyPath))'" - } + guard PQstatus(connection) != CONNECTION_BAD else { + throw connectionError(from: connection) + } - if let options, !options.isEmpty { - connStr += " options='\(escapeConnParam(options))'" - } + try pollUntilConnected(connection) + configureEstablishedConnection(connection) - let connection = connStr.withCString { cStr in - PQconnectdb(cStr) - } + stateLock.lock() + conn = connection + _isConnected = true + stateLock.unlock() + adopted = true + } - guard let connection = connection else { - throw LibPQPluginError.connectionFailed + private func pollUntilConnected(_ connection: OpaquePointer) throws { + let deadline = PQgetCurrentTimeUSec() + Self.connectTimeoutMicroseconds + var status = PGRES_POLLING_WRITING + + while true { + try checkConnectCancellation() + + switch status { + case PGRES_POLLING_OK: + return + case PGRES_POLLING_FAILED: + throw connectionError(from: connection) + case PGRES_POLLING_READING, PGRES_POLLING_WRITING: + let socket = PQsocket(connection) + guard socket >= 0 else { throw connectionError(from: connection) } + + let now = PQgetCurrentTimeUSec() + guard now < deadline else { throw LibPQPluginError.connectionTimedOut } + + let ready = PQsocketPoll( + socket, + status == PGRES_POLLING_READING ? 1 : 0, + status == PGRES_POLLING_WRITING ? 1 : 0, + min(deadline, now + Self.pollSliceMicroseconds) + ) + guard ready >= 0 else { throw LibPQPluginError.connectionFailed } + guard ready > 0 else { continue } + + status = PQconnectPoll(connection) + default: + status = PQconnectPoll(connection) } + } + } - if PQstatus(connection) != CONNECTION_OK { - let error = self.getError(from: connection) - PQfinish(connection) - if let sslError = LibPQSSLClassifier.classifySSLError(error.message) { - throw sslError - } - throw error - } + private func checkConnectCancellation() throws { + guard isConnectCancelled else { return } + throw CancellationError() + } - "SET client_encoding TO 'UTF8'".withCString { cStr in - let result = PQexec(connection, cStr) - PQclear(result) - } + private func connectionError(from connection: OpaquePointer) -> Error { + let error = getError(from: connection) + if let sslError = LibPQSSLClassifier.classifySSLError(error.message) { + return sslError + } + return error + } - let version = PQserverVersion(connection) - if version > 0 { - self._cachedServerVersionNumber = version - let major = version / 10_000 - if major >= 10 { - let minor = version % 10_000 - self._cachedServerVersion = "\(major).\(minor)" - } else { - let minor = (version / 100) % 100 - let revision = version % 100 - self._cachedServerVersion = "\(major).\(minor).\(revision)" - } - } + private func configureEstablishedConnection(_ connection: OpaquePointer) { + "SET client_encoding TO 'UTF8'".withCString { cStr in + let result = PQexec(connection, cStr) + PQclear(result) + } + + let version = PQserverVersion(connection) + guard version > 0 else { return } + + _cachedServerVersionNumber = version + let major = version / 10_000 + if major >= 10 { + let minor = version % 10_000 + _cachedServerVersion = "\(major).\(minor)" + } else { + let minor = (version / 100) % 100 + let revision = version % 100 + _cachedServerVersion = "\(major).\(minor).\(revision)" + } + } + + private func buildConnectionString() -> String { + func escapeConnParam(_ value: String) -> String { + value.replacingOccurrences(of: "\\", with: "\\\\") + .replacingOccurrences(of: "'", with: "\\'") + } + + var connStr = "host='\(escapeConnParam(host))' port='\(port)' dbname='\(escapeConnParam(database))'" + + if !user.isEmpty { + connStr += " user='\(escapeConnParam(user))'" + } + + if let password, !password.isEmpty { + connStr += " password='\(escapeConnParam(password))'" + } + + connStr += " sslmode='\(LibPQSSLMapping.sslmode(for: sslConfig.mode))'" - self.stateLock.lock() - self.conn = connection - self._isConnected = true - self.stateLock.unlock() + if sslConfig.verifiesCertificate, !sslConfig.caCertificatePath.isEmpty { + connStr += " sslrootcert='\(escapeConnParam(sslConfig.caCertificatePath))'" + } + if !sslConfig.clientCertificatePath.isEmpty { + connStr += " sslcert='\(escapeConnParam(sslConfig.clientCertificatePath))'" + } + if !sslConfig.clientKeyPath.isEmpty { + connStr += " sslkey='\(escapeConnParam(sslConfig.clientKeyPath))'" } + + if let options, !options.isEmpty { + connStr += " options='\(escapeConnParam(options))'" + } + + return connStr } func disconnect() { @@ -234,6 +321,7 @@ final class LibPQPluginConnection: @unchecked Sendable { stateLock.lock() _isConnected = false + _isConnectCancelled = true let handle = conn conn = nil stateLock.unlock() diff --git a/TablePro/AppDelegate.swift b/TablePro/AppDelegate.swift index 77dba2c79..9dd33a442 100644 --- a/TablePro/AppDelegate.swift +++ b/TablePro/AppDelegate.swift @@ -155,11 +155,7 @@ class AppDelegate: NSObject, NSApplicationDelegate { } private func persistOpenConnectionsForRecovery() { - var seen = Set() - let connectionIds = MainContentCoordinator.activeCoordinators.values - .map(\.connectionId) - .filter { seen.insert($0).inserted } - LastOpenConnectionsStorage.shared.save(connectionIds: connectionIds) + LastOpenConnectionsStorage.shared.save(connectionIds: SessionRecoveryTracker.connectionIds()) } @objc func handleSystemDidWake(_ notification: Notification) { diff --git a/TablePro/Core/Database/ConnectionAttemptRegistry.swift b/TablePro/Core/Database/ConnectionAttemptRegistry.swift new file mode 100644 index 000000000..15d9dad59 --- /dev/null +++ b/TablePro/Core/Database/ConnectionAttemptRegistry.swift @@ -0,0 +1,30 @@ +// +// ConnectionAttemptRegistry.swift +// TablePro +// + +import Foundation + +struct ConnectionAttemptRegistry { + private var generations: [UUID: Int] = [:] + private var lastGeneration: Int = 0 + + mutating func begin(for connectionId: UUID) -> Int { + lastGeneration += 1 + generations[connectionId] = lastGeneration + return lastGeneration + } + + func isCurrent(_ generation: Int, for connectionId: UUID) -> Bool { + generations[connectionId] == generation + } + + mutating func invalidate(for connectionId: UUID) { + generations.removeValue(forKey: connectionId) + } + + mutating func finish(_ generation: Int, for connectionId: UUID) { + guard generations[connectionId] == generation else { return } + generations.removeValue(forKey: connectionId) + } +} diff --git a/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift b/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift index 394ec1cbf..55b7cabb1 100644 --- a/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift +++ b/TablePro/Core/Database/DatabaseManager+EnsureConnected.swift @@ -23,6 +23,7 @@ extension DatabaseManager { } func cancelEnsureConnected(_ connectionId: UUID) async { + connectionAttempts.invalidate(for: connectionId) await ensureConnectedDedup.cancel(key: connectionId) if let session = activeSessions[connectionId], session.driver == nil { if let tunnelManager = activeTunnelManager(for: session.connection) { diff --git a/TablePro/Core/Database/DatabaseManager+Sessions.swift b/TablePro/Core/Database/DatabaseManager+Sessions.swift index 0782191e5..b3acf68a2 100644 --- a/TablePro/Core/Database/DatabaseManager+Sessions.swift +++ b/TablePro/Core/Database/DatabaseManager+Sessions.swift @@ -28,6 +28,8 @@ extension DatabaseManager { MacAnalyticsProvider.shared.markConnectionAttempted() + let attempt = connectionAttempts.begin(for: connection.id) + let resolvedConnection: DatabaseConnection if LicenseManager.shared.isFeatureAvailable(.envVarReferences) { resolvedConnection = EnvVarResolver.resolveConnection(connection) @@ -49,7 +51,10 @@ extension DatabaseManager { sshPasswordOverride: sshPasswordOverride ) } catch { - finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) + finalizeConnectionFailure( + for: connection.id, + cancelled: isAttemptCancelled(attempt, for: connection.id) + ) throw error } @@ -59,7 +64,10 @@ extension DatabaseManager { do { try await PreConnectHookRunner.run(script: script) } catch { - finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) + finalizeConnectionFailure( + for: connection.id, + cancelled: isAttemptCancelled(attempt, for: connection.id) + ) throw error } } @@ -75,7 +83,10 @@ extension DatabaseManager { isAPIToken: isApiOnly, window: NSApp.keyWindow ) else { - finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) + finalizeConnectionFailure( + for: connection.id, + cancelled: isAttemptCancelled(attempt, for: connection.id) + ) throw CancellationError() } passwordOverride = prompted @@ -90,16 +101,18 @@ extension DatabaseManager { awaitPlugins: true ) } catch { - if !Task.isCancelled { + let cancelled = isAttemptCancelled(attempt, for: connection.id) + if !cancelled { closeActiveTunnel(for: connection) } - finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) + finalizeConnectionFailure(for: connection.id, cancelled: cancelled) throw error } do { try await driver.connect() try Task.checkCancellation() + try ensureAttemptIsCurrent(attempt, for: connection.id, driver: driver) await applyTimeoutAndStartupCommands( on: driver, @@ -116,6 +129,7 @@ extension DatabaseManager { ) try Task.checkCancellation() + try ensureAttemptIsCurrent(attempt, for: connection.id, driver: driver) // Batch all session mutations into a single write to fire objectWillChange once. if var session = activeSessions[connection.id] { @@ -128,6 +142,8 @@ extension DatabaseManager { setSession(session, for: connection.id) } + connectionAttempts.finish(attempt, for: connection.id) + MacAnalyticsProvider.shared.markConnectionSucceeded() AppEvents.shared.databaseDidConnect.send(DatabaseDidConnect(connectionId: connection.id)) @@ -139,7 +155,7 @@ extension DatabaseManager { await startHealthMonitor(for: connection.id) } } catch { - let cancelled = Task.isCancelled + let cancelled = isAttemptCancelled(attempt, for: connection.id) if cancelled { driver.disconnect() } else { @@ -151,6 +167,21 @@ extension DatabaseManager { } } + private func isAttemptCancelled(_ attempt: Int, for connectionId: UUID) -> Bool { + Task.isCancelled || !connectionAttempts.isCurrent(attempt, for: connectionId) + } + + private func ensureAttemptIsCurrent( + _ attempt: Int, + for connectionId: UUID, + driver: DatabaseDriver + ) throws { + guard !isAttemptCancelled(attempt, for: connectionId) else { + driver.disconnect() + throw CancellationError() + } + } + internal func resolvedConnectionDefinition(for connection: DatabaseConnection) -> DatabaseConnection { guard let stored = connectionStorage.loadConnection(id: connection.id) else { return connection } var resolved = connection diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 09c6cba59..abc147a15 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -60,6 +60,11 @@ final class DatabaseManager { @ObservationIgnored internal let ensureConnectedDedup = OnceTask() + /// Generation token per connection. A cancelled or superseded attempt keeps running + /// when its driver blocks in a C call, so every attempt validates its generation + /// before touching shared session state and discards its driver when it lost. + @ObservationIgnored internal var connectionAttempts = ConnectionAttemptRegistry() + /// Current session (computed from currentSessionId) var currentSession: ConnectionSession? { guard let sessionId = currentSessionId else { return nil } diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index 625783227..683bc25a6 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -431,6 +431,9 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi } private func cancelConnectionAttempt() { + if let connectionId = payload?.connectionId { + Task { await DatabaseManager.shared.cancelEnsureConnected(connectionId) } + } view.window?.performClose(nil) } diff --git a/TablePro/Core/Services/Infrastructure/TabWindowController.swift b/TablePro/Core/Services/Infrastructure/TabWindowController.swift index 389c196ce..50b984cf9 100644 --- a/TablePro/Core/Services/Infrastructure/TabWindowController.swift +++ b/TablePro/Core/Services/Infrastructure/TabWindowController.swift @@ -173,6 +173,7 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { let connectionId = payload.connectionId let session = DatabaseManager.shared.activeSessions[connectionId] guard session?.driver == nil else { return } + SessionRecoveryTracker.sync() Task { await DatabaseManager.shared.cancelEnsureConnected(connectionId) } diff --git a/TablePro/Core/Storage/RecoveryConnectionList.swift b/TablePro/Core/Storage/RecoveryConnectionList.swift new file mode 100644 index 000000000..caef9d6f4 --- /dev/null +++ b/TablePro/Core/Storage/RecoveryConnectionList.swift @@ -0,0 +1,21 @@ +// +// RecoveryConnectionList.swift +// TablePro +// + +import Foundation + +struct RecoveryCandidate { + let connectionId: UUID + let isActivated: Bool +} + +enum RecoveryConnectionList { + static func connectionIds(from candidates: [RecoveryCandidate]) -> [UUID] { + var seen = Set() + return candidates + .filter(\.isActivated) + .map(\.connectionId) + .filter { seen.insert($0).inserted } + } +} diff --git a/TablePro/Core/Storage/SessionRecoveryTracker.swift b/TablePro/Core/Storage/SessionRecoveryTracker.swift new file mode 100644 index 000000000..eccd8e7c8 --- /dev/null +++ b/TablePro/Core/Storage/SessionRecoveryTracker.swift @@ -0,0 +1,28 @@ +// +// SessionRecoveryTracker.swift +// TablePro +// + +import Foundation + +@MainActor +enum SessionRecoveryTracker { + /// Connections eligible for "Reopen Last Session". A window that never finished + /// connecting was never activated, so a cancelled or still-connecting attempt is + /// excluded and cannot be replayed on the next launch. + static func connectionIds() -> [UUID] { + RecoveryConnectionList.connectionIds( + from: MainContentCoordinator.activeCoordinators.values.map { + RecoveryCandidate(connectionId: $0.connectionId, isActivated: $0.isActivated) + } + ) + } + + /// Rewrite the recovery list from the live window set. Called whenever that set + /// changes so the file stays correct after a crash or a force quit, neither of + /// which runs `applicationWillTerminate`. + static func sync() { + guard !MainContentCoordinator.isAppTerminating else { return } + LastOpenConnectionsStorage.shared.save(connectionIds: connectionIds()) + } +} diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 4431bb91b..78b546332 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -282,6 +282,10 @@ final class MainContentCoordinator { Self.activeCoordinators.removeValue(forKey: instanceId) } + var isActivated: Bool { + _didActivate.withLock { $0 } + } + /// Collect tabs across all of a connection's windows for persistence, tagged with /// the index of the native window group they belong to so tab order restores intact. static func aggregatedTabs(for connectionId: UUID) -> [(tab: QueryTab, windowGroupIndex: Int)] { @@ -533,6 +537,7 @@ final class MainContentCoordinator { services.schemaProviderRegistry.retain(for: connection.id) } registerForPersistence() + SessionRecoveryTracker.sync() startPeriodicSave() setupPluginDriver() startFileWatcherIfNeeded() @@ -724,6 +729,7 @@ final class MainContentCoordinator { _didTeardown.withLock { $0 = true } unregisterFromPersistence() + SessionRecoveryTracker.sync() if let observer = terminationObserver { NotificationCenter.default.removeObserver(observer) terminationObserver = nil @@ -787,6 +793,7 @@ final class MainContentCoordinator { let id = instanceId Task { @MainActor in Self.activeCoordinators.removeValue(forKey: id) + SessionRecoveryTracker.sync() } return } diff --git a/TableProTests/Core/Concurrency/OnceTaskTests.swift b/TableProTests/Core/Concurrency/OnceTaskTests.swift index d22cb23d9..5ae856290 100644 --- a/TableProTests/Core/Concurrency/OnceTaskTests.swift +++ b/TableProTests/Core/Concurrency/OnceTaskTests.swift @@ -21,6 +21,50 @@ final class OnceTaskTests: XCTestCase { let tag: String } + /// Work that ignores cancellation, standing in for a driver blocked in a C call. + actor Gate { + private var continuation: CheckedContinuation? + private var isOpen = false + + func wait() async { + guard !isOpen else { return } + await withCheckedContinuation { continuation in + self.continuation = continuation + } + } + + func open() { + isOpen = true + continuation?.resume() + continuation = nil + } + } + + func testRerunDoesNotAdoptAbandonedNonCooperativeWork() async throws { + let dedup = OnceTask() + let gate = Gate() + let started = expectation(description: "abandoned work started") + started.assertForOverFulfill = false + + let abandoned = Task { + try await dedup.execute(key: "k") { + started.fulfill() + await gate.wait() + return 1 + } + } + + await fulfillment(of: [started], timeout: 2.0) + await dedup.cancel(key: "k") + + let rerunValue = try await dedup.execute(key: "k") { 2 } + XCTAssertEqual(rerunValue, 2, "A rerun must run fresh work, not adopt the abandoned attempt") + + await gate.open() + let abandonedValue = try await abandoned.value + XCTAssertEqual(abandonedValue, 1, "The abandoned attempt still completes; its result belongs to no one") + } + func testConcurrentSameKeyRunsWorkOnce() async throws { let dedup = OnceTask() let counter = Counter() diff --git a/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift b/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift index ee8d1f886..e95693199 100644 --- a/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift +++ b/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift @@ -78,6 +78,22 @@ struct CancelledConnectionCleanupTests { #expect(DatabaseManager.shared.currentSessionId != id) } + @Test("Cancelling a pending connection invalidates the attempt still in flight") + func cancelInvalidatesInFlightAttempt() async { + let id = UUID() + let attempt = DatabaseManager.shared.connectionAttempts.begin(for: id) + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Pending")), + for: id + ) + defer { DatabaseManager.shared.removeSession(for: id) } + + await DatabaseManager.shared.cancelEnsureConnected(id) + + #expect(!DatabaseManager.shared.connectionAttempts.isCurrent(attempt, for: id)) + #expect(DatabaseManager.shared.activeSessions[id] == nil) + } + @Test("Genuine failure moves currentSessionId to a remaining session") func genuineFailureSwitchesToRemainingSession() { let failedId = UUID() diff --git a/TableProTests/Core/Database/ConnectionAttemptRegistryTests.swift b/TableProTests/Core/Database/ConnectionAttemptRegistryTests.swift new file mode 100644 index 000000000..a6f414e60 --- /dev/null +++ b/TableProTests/Core/Database/ConnectionAttemptRegistryTests.swift @@ -0,0 +1,98 @@ +// +// ConnectionAttemptRegistryTests.swift +// TableProTests +// +// Pins #1358: a cancelled or superseded connection attempt keeps running when its +// driver blocks in a C call, so it must never adopt into or tear down the session +// that a newer attempt owns. +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Connection attempt registry") +struct ConnectionAttemptRegistryTests { + @Test("The only attempt for a connection is current") + func singleAttemptIsCurrent() { + var registry = ConnectionAttemptRegistry() + let id = UUID() + + let attempt = registry.begin(for: id) + + #expect(registry.isCurrent(attempt, for: id)) + } + + @Test("A newer attempt supersedes the one in flight") + func newerAttemptSupersedesOlder() { + var registry = ConnectionAttemptRegistry() + let id = UUID() + + let first = registry.begin(for: id) + let second = registry.begin(for: id) + + #expect(!registry.isCurrent(first, for: id)) + #expect(registry.isCurrent(second, for: id)) + } + + @Test("Cancelling invalidates the attempt in flight") + func cancelInvalidatesInFlightAttempt() { + var registry = ConnectionAttemptRegistry() + let id = UUID() + let attempt = registry.begin(for: id) + + registry.invalidate(for: id) + + #expect(!registry.isCurrent(attempt, for: id)) + } + + @Test("A cancelled attempt completing late cannot claim the retry that replaced it") + func lateCancelledAttemptCannotClaimRetry() { + var registry = ConnectionAttemptRegistry() + let id = UUID() + + let cancelled = registry.begin(for: id) + registry.invalidate(for: id) + let retry = registry.begin(for: id) + + #expect(!registry.isCurrent(cancelled, for: id)) + #expect(registry.isCurrent(retry, for: id)) + } + + @Test("Finishing an attempt does not invalidate the attempt that superseded it") + func finishingStaleAttemptLeavesCurrentIntact() { + var registry = ConnectionAttemptRegistry() + let id = UUID() + + let stale = registry.begin(for: id) + let current = registry.begin(for: id) + registry.finish(stale, for: id) + + #expect(registry.isCurrent(current, for: id)) + } + + @Test("Attempts are tracked per connection") + func attemptsAreScopedPerConnection() { + var registry = ConnectionAttemptRegistry() + let first = UUID() + let second = UUID() + + let firstAttempt = registry.begin(for: first) + let secondAttempt = registry.begin(for: second) + registry.invalidate(for: second) + + #expect(registry.isCurrent(firstAttempt, for: first)) + #expect(!registry.isCurrent(secondAttempt, for: second)) + } + + @Test("A finished attempt is no longer current") + func finishedAttemptIsNotCurrent() { + var registry = ConnectionAttemptRegistry() + let id = UUID() + let attempt = registry.begin(for: id) + + registry.finish(attempt, for: id) + + #expect(!registry.isCurrent(attempt, for: id)) + } +} diff --git a/TableProTests/Core/Storage/RecoveryConnectionListTests.swift b/TableProTests/Core/Storage/RecoveryConnectionListTests.swift new file mode 100644 index 000000000..39adb4102 --- /dev/null +++ b/TableProTests/Core/Storage/RecoveryConnectionListTests.swift @@ -0,0 +1,77 @@ +// +// RecoveryConnectionListTests.swift +// TableProTests +// +// Pins #1358: "Reopen Last Session" must not replay a connection whose attempt the +// user cancelled. A window that never finished connecting was never activated, so +// it is not part of the session to restore. +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Recovery connection list") +struct RecoveryConnectionListTests { + @Test("A connected window is restored") + func activatedWindowIsRestored() { + let id = UUID() + + let ids = RecoveryConnectionList.connectionIds( + from: [RecoveryCandidate(connectionId: id, isActivated: true)] + ) + + #expect(ids == [id]) + } + + @Test("A cancelled connection attempt is not restored") + func cancelledAttemptIsNotRestored() { + let ids = RecoveryConnectionList.connectionIds( + from: [RecoveryCandidate(connectionId: UUID(), isActivated: false)] + ) + + #expect(ids.isEmpty) + } + + @Test("Cancelling one connection leaves the other connected windows restorable") + func cancelledAttemptDoesNotDropConnectedWindows() { + let connected = UUID() + let cancelled = UUID() + + let ids = RecoveryConnectionList.connectionIds(from: [ + RecoveryCandidate(connectionId: connected, isActivated: true), + RecoveryCandidate(connectionId: cancelled, isActivated: false), + ]) + + #expect(ids == [connected]) + } + + @Test("A connection with several windows is listed once") + func duplicateConnectionIsListedOnce() { + let id = UUID() + + let ids = RecoveryConnectionList.connectionIds(from: [ + RecoveryCandidate(connectionId: id, isActivated: true), + RecoveryCandidate(connectionId: id, isActivated: true), + ]) + + #expect(ids == [id]) + } + + @Test("A connection stays restorable while any of its windows is connected") + func connectionSurvivesWhenOneWindowIsStillConnecting() { + let id = UUID() + + let ids = RecoveryConnectionList.connectionIds(from: [ + RecoveryCandidate(connectionId: id, isActivated: false), + RecoveryCandidate(connectionId: id, isActivated: true), + ]) + + #expect(ids == [id]) + } + + @Test("No windows means nothing to restore") + func emptyCandidatesRestoreNothing() { + #expect(RecoveryConnectionList.connectionIds(from: []).isEmpty) + } +}