diff --git a/FrameworkTests/HistoryRenderingSnapshotTests.swift b/FrameworkTests/HistoryRenderingSnapshotTests.swift new file mode 100644 index 00000000..b55933a1 --- /dev/null +++ b/FrameworkTests/HistoryRenderingSnapshotTests.swift @@ -0,0 +1,172 @@ +import Foundation +import Observation +import TermindApplication +import TermindDomain +import Testing + +@Suite @MainActor +struct HistoryRenderingSnapshotTests { + @Test func failedInitialTrimDoesNotHideReadableHistory() throws { + let repository = HistoryRepository() + let entry = ConnectionHistoryEntry(hostID: UUID(), kind: .ssh) + try repository.insert(entry, limit: 10) + repository.failsTrim = true + + let model = ConnectionHistoryModel(repository: repository) + + #expect(model.entries().map(\.id) == [entry.id]) + #expect(model.storageError != nil) + model.reload() + #expect(model.entries().map(\.id) == [entry.id]) + #expect(model.storageError == nil) + } + + @Test func failedInitialRegistrationDoesNotHideReadableDeviceNames() throws { + let repository = DeviceRepository() + let remote = DeviceRecord(id: UUID(), displayName: "Remote") + try repository.upsert(remote) + repository.failsWrites = true + + let model = DeviceDirectoryModel(repository: repository, currentDeviceName: "Local") + + #expect(model.name(for: remote.id) == "Remote") + #expect(model.name(for: model.currentDeviceID) == "Local") + #expect(model.storageError != nil) + model.reload() + #expect(model.name(for: remote.id) == "Remote") + #expect(model.storageError == nil) + } + + @Test(arguments: [false, true]) + func historyReadsDoNotFetchOrPublishDuringRendering(fails: Bool) { + let repository = HistoryRepository() + repository.fails = fails + let model = ConnectionHistoryModel(repository: repository) + let hostID = UUID() + let reads = repository.reads + withObservationTracking { + _ = model.storageError + _ = model.entries() + } onChange: { + Issue.record("Reading history must not publish observable changes") + } + + for _ in 0..<2 { + _ = model.entries() + _ = model.entries(for: hostID) + _ = model.mostRecentDate(for: hostID) + _ = model.localEntries() + } + #expect(repository.reads == reads) + #expect((model.storageError != nil) == fails) + } + + @Test func historyReloadPublishesCommittedRowsAndRecoversFromReadFailure() throws { + let repository = HistoryRepository() + let model = ConnectionHistoryModel(repository: repository) + let entry = ConnectionHistoryEntry(hostID: UUID(), kind: .ssh) + try repository.insert(entry, limit: 10) + model.reload() + #expect(model.entries().map(\.id) == [entry.id]) + + repository.fails = true + model.reload() + let failedReads = repository.reads + #expect(model.entries().isEmpty) + #expect(model.storageError != nil) + #expect(repository.reads == failedReads) + + repository.fails = false + model.reload() + #expect(model.mostRecentDate(for: entry.hostID) == entry.connectedAt) + #expect(model.storageError == nil) + model.delete(entry.id) + #expect(model.entries().isEmpty) + } + + @Test(arguments: [false, true]) + func deviceReadsDoNotFetchOrPublishDuringRendering(fails: Bool) { + let repository = DeviceRepository() + repository.fails = fails + let model = DeviceDirectoryModel(repository: repository, currentDeviceName: "Local") + let reads = repository.reads + withObservationTracking { + _ = model.storageError + _ = model.devices() + } onChange: { + Issue.record("Reading device names must not publish observable changes") + } + + for _ in 0..<2 { + _ = model.devices() + #expect(model.name(for: model.currentDeviceID) == "Local") + #expect(model.name(for: UUID()) == nil) + } + #expect(repository.reads == reads) + #expect((model.storageError != nil) == fails) + } + + @Test func deviceReloadPublishesCommittedNamesAndRecoversFromReadFailure() throws { + let repository = DeviceRepository() + let model = DeviceDirectoryModel(repository: repository, currentDeviceName: "Local") + let remote = DeviceRecord(id: UUID(), displayName: "Remote") + try repository.upsert(remote) + model.reload() + #expect(model.name(for: remote.id) == "Remote") + + repository.fails = true + model.reload() + let failedReads = repository.reads + #expect(model.devices().isEmpty) + #expect(model.storageError != nil) + #expect(repository.reads == failedReads) + + repository.fails = false + model.reload() + #expect(model.name(for: remote.id) == "Remote") + #expect(model.storageError == nil) + model.updateCurrentDeviceName("Renamed") + #expect(model.name(for: model.currentDeviceID) == "Renamed") + } + + private enum ReadFailure: Error { case unavailable } + + private final class HistoryRepository: ConnectionHistoryRepository { + let storage = InMemoryConnectionHistoryRepository() + var reads = 0 + var fails = false + var failsTrim = false + + func entries() throws -> [ConnectionHistoryEntry] { + reads += 1 + if fails { throw ReadFailure.unavailable } + return try storage.entries() + } + func trim(to limit: Int) throws { + if failsTrim { throw ReadFailure.unavailable } + try storage.trim(to: limit) + } + func insert(_ entry: ConnectionHistoryEntry, limit: Int) throws { try storage.insert(entry, limit: limit) } + func end(_ id: UUID, at date: Date) throws { try storage.end(id, at: date) } + func delete(_ id: UUID) throws { try storage.delete(id) } + func clear(for hostID: UUID) throws { try storage.clear(for: hostID) } + func clearAll() throws { try storage.clearAll() } + } + + private final class DeviceRepository: DeviceDirectoryRepository { + let storage = InMemoryDeviceDirectoryRepository() + var reads = 0 + var fails = false + var failsWrites = false + + func devices() throws -> [DeviceRecord] { + reads += 1 + if fails { throw ReadFailure.unavailable } + return try storage.devices() + } + func upsert(_ device: DeviceRecord) throws { + if failsWrites { throw ReadFailure.unavailable } + try storage.upsert(device) + } + } +} diff --git a/FrameworkTests/SSHConnectionWorkerLifecycleTests.swift b/FrameworkTests/SSHConnectionWorkerLifecycleTests.swift new file mode 100644 index 00000000..6f6a5d31 --- /dev/null +++ b/FrameworkTests/SSHConnectionWorkerLifecycleTests.swift @@ -0,0 +1,34 @@ +import Dispatch +import Logging +import Testing + +@testable import TermindRemoteInfrastructure + +@Suite +struct SSHConnectionWorkerLifecycleTests { + @Test func lastReferenceCanBeReleasedOnTheConnectionQueue() async { + let queue = DispatchQueue(label: "termind.tests.ssh.connection") + await withCheckedContinuation { continuation in + queue.async { + var worker: SSHConnectionWorker? = SSHConnectionWorker( + logger: Logger(label: "termind.tests"), maxSessionsPolicy: SSHMaxSessionsPolicy(), queue: queue) + weak let released = worker + #expect(released != nil) + worker = nil + #expect(released == nil) + continuation.resume() + } + } + } + + @Test func lastReferenceCanBeReleasedOutsideTheConnectionQueue() { + let queue = DispatchQueue(label: "termind.tests.ssh.connection") + dispatchPrecondition(condition: .notOnQueue(queue)) + var worker: SSHConnectionWorker? = SSHConnectionWorker( + logger: Logger(label: "termind.tests"), maxSessionsPolicy: SSHMaxSessionsPolicy(), queue: queue) + weak let released = worker + #expect(released != nil) + worker = nil + #expect(released == nil) + } +} diff --git a/Frameworks/TermindApplication/FeatureModels/ConnectionHistoryModel.swift b/Frameworks/TermindApplication/FeatureModels/ConnectionHistoryModel.swift index 45feeb50..6842397b 100644 --- a/Frameworks/TermindApplication/FeatureModels/ConnectionHistoryModel.swift +++ b/Frameworks/TermindApplication/FeatureModels/ConnectionHistoryModel.swift @@ -18,7 +18,7 @@ public final class ConnectionHistoryModel { private let repository: any ConnectionHistoryRepository public let sourceDeviceID: UUID - private var cachedEntries: [ConnectionHistoryEntry]? + private var cachedEntries: [ConnectionHistoryEntry] = [] public private(set) var revision: UInt64 = 0 public private(set) var storageError: String? @@ -34,20 +34,17 @@ public final class ConnectionHistoryModel { } catch { storageError = error.localizedDescription } + // A failed maintenance write must not hide readable existing history. + do { + cachedEntries = try self.repository.entries() + } catch { + storageError = error.localizedDescription + } } public func entries() -> [ConnectionHistoryEntry] { _ = revision - if let cachedEntries { return cachedEntries } - do { - let values = try repository.entries() - cachedEntries = values - storageError = nil - return values - } catch { - storageError = error.localizedDescription - return [] - } + return cachedEntries } public func entries(for hostID: Host.ID) -> [ConnectionHistoryEntry] { @@ -95,9 +92,7 @@ public final class ConnectionHistoryModel { ) do { try repository.insert(entry, limit: Self.limit) - cachedEntries = nil - storageError = nil - revision &+= 1 + reload() return entry } catch { storageError = error.localizedDescription @@ -108,35 +103,34 @@ public final class ConnectionHistoryModel { public func end(_ id: ConnectionHistoryEntry.ID, at date: Date = Date()) { do { try repository.end(id, at: date) - cachedEntries = nil - storageError = nil - revision &+= 1 + reload() } catch { storageError = error.localizedDescription } } - /// The sync coordinator persists remote events through its own transaction. - /// Drop the presentation cache so the next read observes those committed rows. + /// Refresh outside rendering, after a local write or a committed sync transaction. + /// View reads must never publish cache or error changes into an active graph update. public func reload() { - cachedEntries = nil - storageError = nil + do { + cachedEntries = try repository.entries() + storageError = nil + } catch { + cachedEntries = [] + storageError = error.localizedDescription + } revision &+= 1 } public func delete(_ id: ConnectionHistoryEntry.ID) { do { try repository.delete(id) - cachedEntries = nil - storageError = nil - revision &+= 1 + reload() } catch { storageError = error.localizedDescription } } public func clear(for hostID: Host.ID) { do { try repository.clear(for: hostID) - cachedEntries = nil - storageError = nil - revision &+= 1 + reload() } catch { storageError = error.localizedDescription } } diff --git a/Frameworks/TermindApplication/FeatureModels/DeviceDirectoryModel.swift b/Frameworks/TermindApplication/FeatureModels/DeviceDirectoryModel.swift index 9f6e1a22..ce2915f0 100644 --- a/Frameworks/TermindApplication/FeatureModels/DeviceDirectoryModel.swift +++ b/Frameworks/TermindApplication/FeatureModels/DeviceDirectoryModel.swift @@ -8,7 +8,7 @@ public final class DeviceDirectoryModel { private let repository: any DeviceDirectoryRepository public let currentDeviceID: UUID public private(set) var currentDeviceName: String - private var cachedDevices: [UUID: DeviceRecord]? + private var cachedDevices: [UUID: DeviceRecord] = [:] public private(set) var revision: UInt64 = 0 public private(set) var storageError: String? @@ -25,28 +25,25 @@ public final class DeviceDirectoryModel { public func devices() -> [DeviceRecord] { _ = revision - if let cachedDevices { return Array(cachedDevices.values) } - do { - let values = try repository.devices() - cachedDevices = Dictionary(uniqueKeysWithValues: values.map { ($0.id, $0) }) - storageError = nil - return values - } catch { - storageError = error.localizedDescription - return [] - } + return Array(cachedDevices.values) } public func name(for deviceID: UUID?) -> String? { guard let deviceID else { return nil } - if cachedDevices == nil { _ = devices() } - return cachedDevices?[deviceID]?.displayName + _ = revision + return cachedDevices[deviceID]?.displayName ?? (deviceID == currentDeviceID ? currentDeviceName : nil) } public func reload() { - cachedDevices = nil - storageError = nil + do { + let values = try repository.devices() + cachedDevices = Dictionary(uniqueKeysWithValues: values.map { ($0.id, $0) }) + storageError = nil + } catch { + cachedDevices = [:] + storageError = error.localizedDescription + } revision &+= 1 } @@ -54,9 +51,7 @@ public final class DeviceDirectoryModel { do { try repository.upsert(DeviceRecord(id: currentDeviceID, displayName: name)) currentDeviceName = name - cachedDevices = nil - storageError = nil - revision &+= 1 + reload() } catch { storageError = error.localizedDescription } @@ -66,8 +61,13 @@ public final class DeviceDirectoryModel { do { try repository.registerIfNeeded( DeviceRecord(id: currentDeviceID, displayName: currentDeviceName)) - cachedDevices = nil - storageError = nil + } catch { + storageError = error.localizedDescription + } + // Registration can fail while previously saved device names remain readable. + do { + let values = try repository.devices() + cachedDevices = Dictionary(uniqueKeysWithValues: values.map { ($0.id, $0) }) } catch { storageError = error.localizedDescription } diff --git a/Frameworks/TermindRemoteInfrastructure/SSH/SSHConnectionWorker.swift b/Frameworks/TermindRemoteInfrastructure/SSH/SSHConnectionWorker.swift index 9dbbd11e..b7258b7d 100644 --- a/Frameworks/TermindRemoteInfrastructure/SSH/SSHConnectionWorker.swift +++ b/Frameworks/TermindRemoteInfrastructure/SSH/SSHConnectionWorker.swift @@ -90,7 +90,8 @@ nonisolated final class SSHConnectionWorker: @unchecked Sendable { } } - private let queue = DispatchQueue(label: "com.akinokaede.termind.ssh.connection") + private let queue: DispatchQueue + private let queueKey = DispatchSpecificKey() private let logger: Logger private let maxSessionsPolicy: SSHMaxSessionsPolicy private var connection: SSHAuthenticatedConnection? @@ -114,9 +115,14 @@ nonisolated final class SSHConnectionWorker: @unchecked Sendable { private var connectionFailure: (@Sendable (String) -> Void)? private var didReportConnectionFailure = false - init(logger: Logger, maxSessionsPolicy: SSHMaxSessionsPolicy) { + init( + logger: Logger, maxSessionsPolicy: SSHMaxSessionsPolicy, + queue: DispatchQueue = DispatchQueue(label: "com.akinokaede.termind.ssh.connection") + ) { + self.queue = queue self.logger = logger self.maxSessionsPolicy = maxSessionsPolicy + queue.setSpecific(key: queueKey, value: ()) sftpChannelOwner = SSHMultiplexedSFTPChannelOwner( queue: queue, context: { [weak self] in @@ -134,7 +140,13 @@ nonisolated final class SSHConnectionWorker: @unchecked Sendable { } deinit { - queue.sync { cleanup(error: CancellationError()) } + // A queued callback can release the last reference to this worker. + // Cleanup must stay serialized without synchronously reentering its queue. + if DispatchQueue.getSpecific(key: queueKey) != nil { + cleanup(error: CancellationError()) + } else { + queue.sync { cleanup(error: CancellationError()) } + } } func connect( diff --git a/TFI/App/MobileStartupView.swift b/TFI/App/MobileStartupView.swift new file mode 100644 index 00000000..b90468da --- /dev/null +++ b/TFI/App/MobileStartupView.swift @@ -0,0 +1,47 @@ +import SwiftData +import SwiftUI +import TermindPresentation + +struct MobileStartupView: View { + let startup: Result + @AppStorage("mobile.appearance") private var appearance = "system" + + nonisolated init(startup: Result) { + self.startup = startup + } + + // SwiftUI's LazyView can invoke WindowGroup content on AsyncRenderer. + // Only carry the startup result here; access app state in the View body, + // where SwiftUI tracks the main-actor requirement. + nonisolated static func sceneContent( + for startup: Result + ) -> @Sendable () -> MobileStartupView { + { MobileStartupView(startup: startup) } + } + + var body: some View { + switch startup { + case .success(let app): + MobileQuickActionSceneRoot(app: app) + .modelContainer(app.container) + .preferredColorScheme(appearance == "dark" ? .dark : appearance == "light" ? .light : nil) + case .failure: + ContentUnavailableView( + AppStrings.Common.unableToOpenTermind.swiftUIKey, systemImage: "exclamationmark.triangle", + description: Text(AppStrings.Common.openDataStoreFailure)) + } + } +} + +private struct MobileQuickActionSceneRoot: View { + let app: MobileAppModel + @EnvironmentObject private var sceneDelegate: MobileSceneDelegate + + var body: some View { + MobileRootView( + app: app, + pendingQuickAction: sceneDelegate.pendingQuickAction, + consumePendingQuickAction: sceneDelegate.consumePendingQuickAction + ) + } +} diff --git a/TFI/App/TermindIOSApp.swift b/TFI/App/TermindIOSApp.swift index edc41a43..2de68f01 100644 --- a/TFI/App/TermindIOSApp.swift +++ b/TFI/App/TermindIOSApp.swift @@ -1,12 +1,9 @@ -import SwiftData import SwiftUI import TermindApplication -import TermindPresentation @main struct TermindIOSApp: App { @UIApplicationDelegateAdaptor(MobileAppDelegate.self) private var appDelegate - @AppStorage("mobile.appearance") private var appearance = "system" private let startup: Result init() { @@ -18,31 +15,7 @@ struct TermindIOSApp: App { } var body: some Scene { - WindowGroup { - switch startup { - case .success(let app): - MobileQuickActionSceneRoot(app: app) - .modelContainer(app.container) - .preferredColorScheme(appearance == "dark" ? .dark : appearance == "light" ? .light : nil) - case .failure: - ContentUnavailableView( - AppStrings.Common.unableToOpenTermind.swiftUIKey, systemImage: "exclamationmark.triangle", - description: Text(AppStrings.Common.openDataStoreFailure)) - } - } - .commands { MobileCommands() } - } -} - -private struct MobileQuickActionSceneRoot: View { - let app: MobileAppModel - @EnvironmentObject private var sceneDelegate: MobileSceneDelegate - - var body: some View { - MobileRootView( - app: app, - pendingQuickAction: sceneDelegate.pendingQuickAction, - consumePendingQuickAction: sceneDelegate.consumePendingQuickAction - ) + WindowGroup(makeContent: MobileStartupView.sceneContent(for: startup)) + .commands { MobileCommands() } } } diff --git a/TFITests/MobileStartupViewTests.swift b/TFITests/MobileStartupViewTests.swift new file mode 100644 index 00000000..f8dbfd98 --- /dev/null +++ b/TFITests/MobileStartupViewTests.swift @@ -0,0 +1,37 @@ +import Foundation +import Testing + +@testable import TermindIOS + +@Suite @MainActor +struct MobileStartupViewTests { + private enum StartupFailure: Error { case unavailableStore } + + @Test func successfulSceneContentCanBeCreatedOffMainThread() async throws { + let app = try MobileAppModel(inMemory: true) + defer { app.outbox.stop() } + let makeContent = MobileStartupView.sceneContent(for: .success(app)) + + let content = await Task.detached { + dispatchPrecondition(condition: .notOnQueue(.main)) + return makeContent() + }.value + + #expect(try content.startup.get() === app) + } + + @Test func failedSceneContentCanBeCreatedOffMainThread() async { + let makeContent = MobileStartupView.sceneContent(for: .failure(StartupFailure.unavailableStore)) + + let content = await Task.detached { + dispatchPrecondition(condition: .notOnQueue(.main)) + return makeContent() + }.value + + guard case .failure(let error) = content.startup else { + Issue.record("Failed startup must remain a failure when creating scene content") + return + } + #expect(error as? StartupFailure == .unavailableStore) + } +}