diff --git a/SF50 Shared/Defaults.swift b/SF50 Shared/Defaults.swift index e9db731..a0510d3 100644 --- a/SF50 Shared/Defaults.swift +++ b/SF50 Shared/Defaults.swift @@ -10,6 +10,20 @@ public let latestSchemaVersion = 8 extension TerrainRegion: Defaults.Serializable {} extension Defaults.Keys { + /// Which generation of the nav-data store the app is reading. + /// + /// An import writes the next generation to its own file and this is switched to it only once + /// that file has been opened and found sound. Recording a number is the whole of the swap: no + /// store is ever overwritten, so an import that dies part-way leaves a file nobody points at + /// rather than a dataset half-replaced. + /// + /// Lives in the group suite because the widget and the App Intents surfaces open the same store. + public static let activeNavDataGeneration = Key( + "SF50/3/activeNavDataGeneration", + default: 0, + suite: groupDefaults + ) + /// Terrain regions the pilot has asked for, whether or not a payload is on disk right now. /// /// Asset packs are purgeable: the system reclaims one under storage pressure without telling diff --git a/SF50 Shared/Store/AppStore.swift b/SF50 Shared/Store/AppStore.swift index 9bc258c..a9589c9 100644 --- a/SF50 Shared/Store/AppStore.swift +++ b/SF50 Shared/Store/AppStore.swift @@ -1,16 +1,21 @@ public import Foundation -import os public import SwiftData -/// The app's two persistent stores, shared with its extensions through the app group. +import Defaults +import os + +/// The app's stores, shared with its extensions through the app group. /// /// Nav data is opened read-only. It is a downloaded artifact replaced whole every cycle, and a /// store the app cannot write is a store the app cannot leave half-written — which is the failure /// this separation exists to remove. What the pilot authored lives in its own writable store that /// no cycle touches. /// -/// Both configurations are named. An unnamed second configuration was historically the difference -/// between a container that opened both stores and one that silently opened only the first. +/// Every store is opened through the same pair of configurations, and both are named. SwiftData +/// records the whole container's schema in each store it opens, so a store written by a container +/// of a different shape reads back as one needing migration — and a store opened read-only cannot +/// be migrated. An unnamed second configuration was also historically the difference between a +/// container that opened both stores and one that silently opened only the first. public enum AppStore { private static let navConfigurationName = "navData" private static let userConfigurationName = "userData" @@ -20,78 +25,131 @@ public enum AppStore { category: "AppStore" ) - /// The container backing both shared stores. - public static let shared: ModelContainer = { - let layout = StoreLayout.appGroup - do { - return try makeContainer(layout: layout) - } catch { - // Nav data is a downloaded file, so a corrupt one must not be fatal: discard it and open an - // empty store, which the app reads as "no data" and offers to download again. - logger.error("Discarding an unopenable nav-data store: \(error.localizedDescription)") - StoreLayout.removeStore(at: layout.navStoreURL) - do { return try makeContainer(layout: layout) } catch { - fatalError("Couldn’t open the model container: \(error)") - } + private static let lock = OSAllocatedUnfairLock(initialState: nil) + private static let hasSweptStaleGenerations = OSAllocatedUnfairLock(initialState: false) + + /// The container backing the app's stores. + /// + /// Rebuilt by ``reopen()`` when a newly imported generation is switched to, so a running app + /// picks up a new dataset without being relaunched. + public static var shared: ModelContainer { + lock.withLock { container in + if let container { return container } + let opened = openActiveGeneration() + container = opened + return opened } - }() + } - /// Opens both stores as the layout arranges them. + /// Opens both stores, reading the nav-data store of `generation`. /// - /// - Parameter layout: Where the stores live. + /// - Parameters: + /// - layout: Where the stores live. + /// - generation: Which generation of nav data to read. /// - Returns: A container holding a read-only nav store and a writable user store. - public static func makeContainer(layout: StoreLayout) throws -> ModelContainer { + public static func makeContainer(layout: StoreLayout, generation: Int) throws -> ModelContainer { try layout.createDirectories() try LegacyStoreMigration(layout: layout).migrateIfNeeded() - try bootstrapIfAbsent(layout: layout) - return try open(layout: layout, navAllowsSave: false) + try bootstrapIfAbsent(layout: layout, generation: generation) + return try open(layout: layout, generation: generation, navAllowsSave: false) } - private static func open(layout: StoreLayout, navAllowsSave: Bool) throws -> ModelContainer { + /// Opens both stores with a nav-data generation writable, for an importer. + /// + /// The importer writes through its own container so its bulk transactions queue on their own + /// coordinator, leaving the store the rest of the app reads read-only — and it writes a + /// generation nothing is reading yet, so an import that fails costs nothing. + /// + /// - Parameters: + /// - layout: Where the stores live. + /// - generation: The generation to write. + /// - Returns: A container whose nav-data store accepts writes. + public static func makeWritableContainer( + layout: StoreLayout, + generation: Int + ) throws -> ModelContainer { + try layout.createDirectories() + return try open(layout: layout, generation: generation, navAllowsSave: true) + } + + /// Opens throwaway stores held only in memory, for tests, previews and screenshot runs. + /// + /// Nav data is writable here: seeding a test or a preview means inserting airports. + public static func makeInMemoryContainer() throws -> ModelContainer { let navData = ModelConfiguration( navConfigurationName, schema: NavDataSchema.schema, - url: layout.navStoreURL, - allowsSave: navAllowsSave + isStoredInMemoryOnly: true ) let userData = ModelConfiguration( userConfigurationName, schema: UserDataSchema.schema, - url: layout.userStoreURL + isStoredInMemoryOnly: true ) return try ModelContainer(for: AppSchema.schema, configurations: navData, userData) } - /// Opens both stores with the nav-data store writable, for an importer. - /// - /// The importer writes through its own container so its bulk transactions queue on their own - /// coordinator, leaving the store the rest of the app reads read-only. - /// - /// It opens the same pair of configurations rather than the nav store alone. SwiftData records - /// the whole container's schema in every store it opens, so a nav store written by a container - /// of a different shape reads back as one needing migration — and a store opened read-only - /// cannot be migrated. + /// Rebuilds ``shared`` against whichever generation is now current. /// - /// - Parameter layout: Where the stores live, usually with the nav store addressed elsewhere. - /// - Returns: A container whose nav-data store accepts writes. - public static func makeWritableContainer(layout: StoreLayout) throws -> ModelContainer { - try layout.createDirectories() - return try open(layout: layout, navAllowsSave: true) + /// The container holds an open SQLite handle, so a generation is only ever switched to by + /// opening the new file — never by replacing the old one underneath a reader. + public static func reopen() { + lock.withLock { container in + container = nil + container = openActiveGeneration() + } } - /// Opens throwaway stores held only in memory, for tests, previews and screenshot runs. + private static func openActiveGeneration() -> ModelContainer { + let layout = StoreLayout.appGroup, + generation = Defaults[.activeNavDataGeneration] + // Swept however this process first got a container, including by discarding a bad one: a sweep + // that had not happened yet would happen on the next reopen instead, under a container that may + // still be reading what it reclaims. + defer { sweepStaleGenerationsOnce(layout: layout, keeping: generation) } + do { + return try makeContainer(layout: layout, generation: generation) + } catch { + // Nav data is a downloaded file, so a corrupt one must not be fatal: discard it and open an + // empty store, which the app reads as "no data" and offers to download again. + logger.error("Discarding an unopenable nav-data store: \(error.localizedDescription)") + StoreLayout.removeStore(at: layout.navStoreURL(generation: generation)) + do { return try makeContainer(layout: layout, generation: generation) } catch { + fatalError("Couldn’t open the model container: \(error)") + } + } + } + + /// Reclaims superseded generations, but only before this process has opened one. /// - /// Nav data is writable here: seeding a test or a preview means inserting airports. - public static func makeInMemoryContainer() throws -> ModelContainer { + /// A generation is reclaimed at launch and never afterwards. Reopening onto a newer generation + /// leaves the previous file alone, because the container that was reading it may still be alive — + /// deleting it would leave that reader on a file that no longer exists, which is exactly what + /// numbering generations avoids. + private static func sweepStaleGenerationsOnce(layout: StoreLayout, keeping generation: Int) { + let shouldSweep = hasSweptStaleGenerations.withLock { hasSwept in + defer { hasSwept = true } + return !hasSwept + } + guard shouldSweep else { return } + layout.removeNavStores(exceptGeneration: generation) + } + + private static func open( + layout: StoreLayout, + generation: Int, + navAllowsSave: Bool + ) throws -> ModelContainer { let navData = ModelConfiguration( navConfigurationName, schema: NavDataSchema.schema, - isStoredInMemoryOnly: true + url: layout.navStoreURL(generation: generation), + allowsSave: navAllowsSave ) let userData = ModelConfiguration( userConfigurationName, schema: UserDataSchema.schema, - isStoredInMemoryOnly: true + url: layout.userStoreURL ) return try ModelContainer(for: AppSchema.schema, configurations: navData, userData) } @@ -102,12 +160,11 @@ public enum AppStore { /// by this binary matches this binary's schema by construction — which is also why no store needs /// to ship inside the app. /// - /// It is created through the same pair of configurations that will read it. SwiftData records the - /// whole container's schema in each store it opens, so a store stamped by a container of a - /// different shape reads back as one needing migration — and migrating a store opened read-only - /// fails outright. - private static func bootstrapIfAbsent(layout: StoreLayout) throws { - guard !FileManager.default.fileExists(atPath: layout.navStoreURL.path) else { return } - _ = try open(layout: layout, navAllowsSave: true) + /// It is created through the same pair of configurations that will read it, for the reason given + /// on the type. + private static func bootstrapIfAbsent(layout: StoreLayout, generation: Int) throws { + let url = layout.navStoreURL(generation: generation) + guard !FileManager.default.fileExists(atPath: url.path) else { return } + _ = try open(layout: layout, generation: generation, navAllowsSave: true) } } diff --git a/SF50 Shared/Store/NavDataStoreInstaller.swift b/SF50 Shared/Store/NavDataStoreInstaller.swift new file mode 100644 index 0000000..deb568c --- /dev/null +++ b/SF50 Shared/Store/NavDataStoreInstaller.swift @@ -0,0 +1,86 @@ +public import Foundation + +import Defaults +import SwiftData +import os + +/// Switches the app to a newly written generation of the nav-data store. +/// +/// Installing is a single `Defaults` write, and it happens only after the candidate has been opened +/// and found to hold a dataset. Nothing is overwritten and nothing is deleted, so an import that +/// fails — or is killed mid-flight when the pilot swipes the app away — leaves the dataset in use +/// exactly as it was. +public struct NavDataStoreInstaller: Sendable { + private static let logger = Logger( + subsystem: "codes.tim.SF50-TOLD", + category: "NavDataStoreInstaller" + ) + + private let layout: StoreLayout + + /// The generation currently in use. + public var activeGeneration: Int { Defaults[.activeNavDataGeneration] } + + /// Creates an installer for the stores `layout` arranges. + /// + /// - Parameter layout: Where the stores live. + public init(layout: StoreLayout) { + self.layout = layout + } + + /// A generation number no store is using, for an import to write. + /// + /// Numbers rise rather than alternate, so a generation an extension still has open is never + /// reused underneath it. + public func reserveGeneration() -> Int { + let next = max(activeGeneration, layout.navStoreGenerations().max() ?? 0) + 1 + StoreLayout.removeStore(at: layout.navStoreURL(generation: next)) + return next + } + + /// Switches to `generation`, if the store it names holds a usable dataset. + /// + /// - Parameter generation: The generation an import has just written. + /// - Throws: ``Errors/storeIsEmpty`` if the candidate holds no airports, or the error SwiftData + /// raised trying to open it. + public func install(generation: Int) throws { + try validate(generation: generation) + Defaults[.activeNavDataGeneration] = generation + Self.logger.notice("Switched to nav-data generation \(generation, privacy: .public)") + } + + /// Opens a candidate generation and confirms it holds a dataset. + /// + /// Opening it here, through the same configurations the app uses, is what turns a store this + /// binary cannot read into a failed install rather than a broken launch. + /// + /// - Parameter generation: The generation to check. + private func validate(generation: Int) throws { + let container = try AppStore.makeContainer(layout: layout, generation: generation) + let context = ModelContext(container) + guard try context.fetchCount(FetchDescriptor()) > 0 else { + throw Errors.storeIsEmpty + } + } + + /// Reasons a candidate store was refused. + public enum Errors: Swift.Error, LocalizedError { + /// The store held no airports. + case storeIsEmpty + + public var errorDescription: String? { + String(localized: "Couldn’t use the navigation data that was downloaded.") + } + + public var failureReason: String? { + switch self { + case .storeIsEmpty: + String(localized: "The downloaded database contained no airports.") + } + } + + public var recoverySuggestion: String? { + String(localized: "Try downloading the navigation data again.") + } + } +} diff --git a/SF50 Shared/Store/StoreLayout.swift b/SF50 Shared/Store/StoreLayout.swift index 871caf8..2309851 100644 --- a/SF50 Shared/Store/StoreLayout.swift +++ b/SF50 Shared/Store/StoreLayout.swift @@ -1,20 +1,25 @@ public import Foundation -/// Where the app keeps its two persistent stores. +/// Where the app keeps its stores. /// -/// Nav data and user data are separate files because nav data is replaced whole every cycle: a -/// downloaded store is staged beside the live one and installed by replacing it, which is only -/// atomic if nothing the pilot authored lives in the file being replaced. +/// Nav data is replaced whole every cycle, and the replacement is a new file rather than a rewrite +/// of the old one: each import writes the next *generation*, and the app switches to it by +/// recording which generation is current. Nothing ever overwrites a store another process might +/// still have open, and an import that never finishes leaves a file nobody points at. /// -/// Paths are resolved from a base directory rather than assumed, so tests can build a real -/// two-store container in a temporary directory. Only ``appGroup`` reaches for the group -/// container — which a test bundle stripped of the entitlement cannot do. +/// What the pilot authored lives in one store that no cycle touches. +/// +/// Paths are resolved from a base directory rather than assumed, so tests can build real stores in +/// a temporary directory. Only ``appGroup`` reaches for the group container — which a test bundle +/// stripped of the entitlement cannot do. public struct StoreLayout: Sendable { /// The identifier of the app group the app and its extensions are entitled to. public static let groupIdentifier = "group.codes.tim.TOLD" private static let navDataDirectoryName = "NavData" private static let userDataDirectoryName = "UserData" + private static let navStorePrefix = "navdata-" + private static let navStoreSuffix = ".store" /// The layout rooted in the shared app-group container. /// @@ -34,13 +39,6 @@ public struct StoreLayout: Sendable { /// The directory the stores live under. public let baseDirectory: URL - private let navStoreOverride: URL? - - /// The nav-data store currently in use. - public var navStoreURL: URL { - navStoreOverride ?? navDataDirectory.appending(path: "current.store") - } - /// The store holding what the pilot authored. public var userStoreURL: URL { userDataDirectory.appending(path: "user.store") @@ -63,60 +61,56 @@ public struct StoreLayout: Sendable { /// Creates a layout rooted at `baseDirectory`. /// - /// - Parameter baseDirectory: The directory to keep both stores under. + /// - Parameter baseDirectory: The directory to keep the stores under. public init(baseDirectory: URL) { self.baseDirectory = baseDirectory - navStoreOverride = nil - } - - private init(baseDirectory: URL, navStoreOverride: URL?) { - self.baseDirectory = baseDirectory - self.navStoreOverride = navStoreOverride } /// Deletes a store and the write-ahead log and shared-memory files SQLite keeps beside it. /// /// - Parameter url: The store to delete. public static func removeStore(at url: URL) { - for path in sidecars(of: url) { + for path in [url.path, "\(url.path)-wal", "\(url.path)-shm"] { try? FileManager.default.removeItem(atPath: path) } } - /// Puts a store, and the files SQLite keeps beside it, where another one was. - /// - /// Only safe while no container holds either store open: replacing a file SQLite has a handle on - /// leaves the reader on a deleted inode. A downloaded store is installed at launch, before any - /// container opens. + /// The nav-data store holding a given generation of the dataset. /// - /// - Parameters: - /// - source: The store to install. - /// - destination: Where it should end up. - public static func moveStore(from source: URL, to destination: URL) throws { - removeStore(at: destination) - let fileManager = FileManager.default - for (from, to) in zip(sidecars(of: source), sidecars(of: destination)) { - guard fileManager.fileExists(atPath: from) else { continue } - try fileManager.moveItem(atPath: from, toPath: to) - } + /// - Parameter generation: Which generation to address. + /// - Returns: That generation's store, whether or not it exists yet. + public func navStoreURL(generation: Int) -> URL { + navDataDirectory.appending(path: "\(Self.navStorePrefix)\(generation)\(Self.navStoreSuffix)") } - private static func sidecars(of url: URL) -> [String] { - [url.path, "\(url.path)-wal", "\(url.path)-shm"] + /// Every generation with a store on disk, in ascending order. + public func navStoreGenerations() -> [Int] { + let contents = + (try? FileManager.default.contentsOfDirectory(atPath: navDataDirectory.path)) ?? [] + return + contents + .compactMap { name in + guard name.hasPrefix(Self.navStorePrefix), name.hasSuffix(Self.navStoreSuffix) else { + return nil + } + return Int(name.dropFirst(Self.navStorePrefix.count).dropLast(Self.navStoreSuffix.count)) + } + .sorted() } - /// The same layout with its nav-data store at `url`. + /// Deletes every nav-data store except the one in use. /// - /// An importer writes a store somewhere other than the one in use, and has to open it through the - /// same pair of configurations that will later read it. + /// Called at launch, when nothing holds a superseded generation open. An import that failed + /// part-way leaves a store nobody points at, and this is what reclaims it. /// - /// - Parameter url: Where the nav-data store should be. - /// - Returns: A layout naming that store. - public func addressingNavStore(at url: URL) -> Self { - .init(baseDirectory: baseDirectory, navStoreOverride: url) + /// - Parameter generation: The generation to keep. + public func removeNavStores(exceptGeneration generation: Int) { + for stale in navStoreGenerations() where stale != generation { + Self.removeStore(at: navStoreURL(generation: stale)) + } } - /// Creates the directories both stores live in, if they are not there already. + /// Creates the directories the stores live in, if they are not there already. public func createDirectories() throws { for directory in [navDataDirectory, userDataDirectory] { try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) diff --git a/SF50 SharedTests/NavDataStoreInstallerTests.swift b/SF50 SharedTests/NavDataStoreInstallerTests.swift new file mode 100644 index 0000000..c98ad5e --- /dev/null +++ b/SF50 SharedTests/NavDataStoreInstallerTests.swift @@ -0,0 +1,95 @@ +import Defaults +import Foundation +import SwiftData +import Testing + +@testable import SF50_Shared + +/// Switching to a newly imported dataset is a single recorded number, taken only after the new +/// store has been opened and found to hold airports. These are the tests that earn the word +/// "atomic": whatever happens to an import, the dataset in use is either replaced wholly or not +/// at all. +@Suite(.serialized) +struct `Nav Data Store Install` { + private static func airport(recordID: String) -> Airport { + .init( + recordID: recordID, + locationID: "TEST", + ICAO_ID: nil, + name: "Test Airport", + city: nil, + dataSource: .NASR, + latitude: .init(value: 0, unit: .degrees), + longitude: .init(value: 0, unit: .degrees), + elevation: .init(value: 0, unit: .feet), + variation: .init(value: 0, unit: .degrees), + timeZone: nil + ) + } + + private static func temporaryLayout() -> StoreLayout { + .init( + baseDirectory: FileManager.default.temporaryDirectory + .appending(path: "InstallerTests-\(UUID().uuidString)") + ) + } + + private static func write(_ recordID: String, generation: Int, in layout: StoreLayout) throws { + let context = ModelContext( + try AppStore.makeWritableContainer(layout: layout, generation: generation) + ) + context.insert(airport(recordID: recordID)) + try context.save() + } + + @Test("a generation holding a dataset is switched to") + func installsAGoodStore() throws { + let layout = Self.temporaryLayout() + defer { cleanUp(layout) } + let installer = NavDataStoreInstaller(layout: layout) + + let generation = installer.reserveGeneration() + try Self.write("IMPORTED", generation: generation, in: layout) + try installer.install(generation: generation) + + #expect(installer.activeGeneration == generation) + } + + /// An import that produced nothing must not become the dataset the pilot flies on. + @Test("a generation holding no airports is refused, and the dataset in use is kept") + func refusesAnEmptyStore() throws { + let layout = Self.temporaryLayout() + defer { cleanUp(layout) } + let installer = NavDataStoreInstaller(layout: layout) + try Self.write("LIVE", generation: installer.activeGeneration, in: layout) + let live = installer.activeGeneration + + let generation = installer.reserveGeneration() + _ = try AppStore.makeWritableContainer(layout: layout, generation: generation) + + #expect(throws: NavDataStoreInstaller.Errors.storeIsEmpty) { + try installer.install(generation: generation) + } + #expect(installer.activeGeneration == live) + } + + @Test("a reserved generation is one nothing is using") + func reservesAnUnusedGeneration() throws { + let layout = Self.temporaryLayout() + defer { cleanUp(layout) } + let installer = NavDataStoreInstaller(layout: layout) + + let first = installer.reserveGeneration() + try Self.write("FIRST", generation: first, in: layout) + try installer.install(generation: first) + let second = installer.reserveGeneration() + + #expect(second > first) + #expect(!FileManager.default.fileExists(atPath: layout.navStoreURL(generation: second).path)) + } + + private func cleanUp(_ layout: StoreLayout) { + Defaults.reset(.activeNavDataGeneration) + try? FileManager.default.removeItem(at: layout.baseDirectory) + } +} diff --git a/SF50 SharedTests/TwoStoreContainerTests.swift b/SF50 SharedTests/TwoStoreContainerTests.swift index 5cde59d..8617de1 100644 --- a/SF50 SharedTests/TwoStoreContainerTests.swift +++ b/SF50 SharedTests/TwoStoreContainerTests.swift @@ -64,7 +64,7 @@ struct `Two Store Container` { let layout = Self.temporaryLayout() defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } - let context = ModelContext(try AppStore.makeContainer(layout: layout)) + let context = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) #expect(throws: Never.self) { try context.fetchCount(FetchDescriptor()) } #expect(throws: Never.self) { try context.fetchCount(FetchDescriptor()) } @@ -75,9 +75,9 @@ struct `Two Store Container` { let layout = Self.temporaryLayout() defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } - _ = try AppStore.makeContainer(layout: layout) + _ = try AppStore.makeContainer(layout: layout, generation: 0) - #expect(FileManager.default.fileExists(atPath: layout.navStoreURL.path)) + #expect(FileManager.default.fileExists(atPath: layout.navStoreURL(generation: 0).path)) } /// The pilot's own entries are what a cycle must not disturb, so they have to be written through @@ -87,7 +87,7 @@ struct `Two Store Container` { let layout = Self.temporaryLayout() defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } - let context = ModelContext(try AppStore.makeContainer(layout: layout)) + let context = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) context.insert(Scenario(name: "Test", operation: .takeoff)) #expect(throws: Never.self) { try context.save() } @@ -96,35 +96,69 @@ struct `Two Store Container` { /// This is the swap a data cycle performs, and the reason the two stores are separate files. /// - /// The stores are opened in scopes: replacing a file SQLite still has a handle on leaves the - /// reader on a deleted inode, which is why a downloaded store is installed before any container - /// opens rather than underneath one. - @Test("replacing the nav-data store leaves user data alone") + /// The new dataset is written to a generation of its own and switched to by number. Nothing + /// overwrites a file another reader might hold, which is what makes an abandoned import harmless. + @Test("switching to a new nav-data generation leaves user data alone") func swappingNavDataKeepsUserData() throws { let layout = Self.temporaryLayout() defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } + let live = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) + live.insert(Scenario(name: "Carried", operation: .landing)) + try live.save() + do { - let context = ModelContext(try AppStore.makeContainer(layout: layout)) - context.insert(Scenario(name: "Carried", operation: .landing)) - try context.save() + let next = ModelContext(try AppStore.makeWritableContainer(layout: layout, generation: 1)) + next.insert(Self.airport(recordID: "REPLACED")) + try next.save() } - let replacement = layout.baseDirectory.appending(path: "replacement.store") + let reopened = ModelContext(try AppStore.makeContainer(layout: layout, generation: 1)) + + #expect(try reopened.fetch(FetchDescriptor()).map(\.name) == ["Carried"]) + #expect(try reopened.fetch(FetchDescriptor()).map(\.recordID) == ["REPLACED"]) + } + + /// The dataset in use must survive an import that never finishes — the failure that blocked + /// running the import anywhere the system can kill it. + @Test("an abandoned import leaves the dataset in use untouched") + func abandonedImportChangesNothing() throws { + let layout = Self.temporaryLayout() + defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } + do { - let context = ModelContext( - try AppStore.makeWritableContainer(layout: layout.addressingNavStore(at: replacement)) + let live = ModelContext(try AppStore.makeWritableContainer(layout: layout, generation: 0)) + live.insert(Self.airport(recordID: "LIVE")) + try live.save() + } + + // An import that wrote some rows and then stopped. + do { + let abandoned = ModelContext( + try AppStore.makeWritableContainer(layout: layout, generation: 1) ) - context.insert(Self.airport(recordID: "REPLACED")) - try context.save() + abandoned.insert(Self.airport(recordID: "HALF-WRITTEN")) + try abandoned.save() } - try StoreLayout.moveStore(from: replacement, to: layout.navStoreURL) + let reopened = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) - let reopened = ModelContext(try AppStore.makeContainer(layout: layout)) + #expect(try reopened.fetch(FetchDescriptor()).map(\.recordID) == ["LIVE"]) + } - #expect(try reopened.fetch(FetchDescriptor()).map(\.name) == ["Carried"]) - #expect(try reopened.fetch(FetchDescriptor()).map(\.recordID) == ["REPLACED"]) + @Test("superseded generations are reclaimed, and the one in use is not") + func staleGenerationsAreSwept() throws { + let layout = Self.temporaryLayout() + defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } + + for generation in 0...2 { + _ = try AppStore.makeWritableContainer(layout: layout, generation: generation) + } + #expect(layout.navStoreGenerations() == [0, 1, 2]) + + layout.removeNavStores(exceptGeneration: 2) + + #expect(layout.navStoreGenerations() == [2]) } @Test("scenarios are carried out of the store that predated the split") @@ -133,7 +167,7 @@ struct `Two Store Container` { defer { try? FileManager.default.removeItem(at: layout.baseDirectory) } try Self.writeLegacyStore(named: "Mine", to: layout) - let context = ModelContext(try AppStore.makeContainer(layout: layout)) + let context = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) #expect(try context.fetch(FetchDescriptor()).map(\.name) == ["Mine"]) #expect(!FileManager.default.fileExists(atPath: layout.legacyStoreURL.path)) @@ -150,7 +184,7 @@ struct `Two Store Container` { try Self.writeLegacyStore(named: "Mine", to: layout) try Self.writeUserStore(scenarioNames: [], to: layout) - let context = ModelContext(try AppStore.makeContainer(layout: layout)) + let context = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) #expect(try context.fetch(FetchDescriptor()).map(\.name) == ["Mine"]) #expect(!FileManager.default.fileExists(atPath: layout.legacyStoreURL.path)) @@ -165,7 +199,7 @@ struct `Two Store Container` { try Self.writeLegacyStore(named: "Mine", to: layout) try Self.writeUserStore(scenarioNames: ["Mine"], to: layout) - let context = ModelContext(try AppStore.makeContainer(layout: layout)) + let context = ModelContext(try AppStore.makeContainer(layout: layout, generation: 0)) #expect(try context.fetch(FetchDescriptor()).map(\.name) == ["Mine"]) #expect(!FileManager.default.fileExists(atPath: layout.legacyStoreURL.path)) diff --git a/SF50 TOLD/Launch/UITestingHelper.swift b/SF50 TOLD/Launch/UITestingHelper.swift index 9b10591..4a22b46 100644 --- a/SF50 TOLD/Launch/UITestingHelper.swift +++ b/SF50 TOLD/Launch/UITestingHelper.swift @@ -46,7 +46,7 @@ enum UITestingHelper { /// read would hand the picker a different object than its view model is watching. @MainActor static let locationStreamer: (any LocationStreamer)? = scriptedLocationStreamer() - static func setupUITestingEnvironment(container: ModelContainer) { + static func setupUITestingEnvironment() { // Reset all defaults Defaults.removeAll(suite: UserDefaults(suiteName: "group.codes.tim.TOLD")!) @@ -64,28 +64,29 @@ enum UITestingHelper { if ProcessInfo.processInfo.arguments.contains("SKIP-SCENARIO-SEEDING") { Defaults[.defaultScenariosSeeded] = true } - - // Only seed test data for regular UI tests, not screenshot generation - if !isGeneratingScreenshots { - Task { @MainActor in - clearUserData(container: container) - } - } } - /// Seeds the nav-data store before the app opens it. + /// Puts both stores into the state a UI test expects, before the app opens either. /// /// Nav data is read-only once the app holds it, so a test's airports have to be written through - /// a writable container first — the same way a downloaded cycle is. + /// a writable container first — the same way a downloaded cycle is. What a previous run left in + /// the user store is cleared in the same pass, synchronously: doing it afterwards raced the + /// default-scenario seeder and sometimes deleted what it had just written. @MainActor - static func seedNavData() { + static func prepareStores() { guard isUITesting, !isGeneratingScreenshots else { return } + // Always the first generation, so a run never depends on what the last one left behind. + Defaults[.activeNavDataGeneration] = 0 + do { - let context = ModelContext(try AppStore.makeWritableContainer(layout: .appGroup)) + let context = ModelContext( + try AppStore.makeWritableContainer(layout: .appGroup, generation: 0) + ) + clearUserData(in: context) try seedNavData(into: context) } catch { - assertionFailure("Couldn’t seed nav data for UI testing: \(error)") + assertionFailure("Couldn’t prepare the stores for UI testing: \(error)") } } @@ -125,8 +126,7 @@ enum UITestingHelper { /// Clears what a previous run left in the store the pilot writes to. @MainActor - private static func clearUserData(container: ModelContainer) { - let context = container.mainContext + private static func clearUserData(in context: ModelContext) { try? context.delete(model: NOTAM.self) try? context.delete(model: Scenario.self) try? context.save() diff --git a/SF50 TOLD/Loaders/BackgroundRefreshScheduler.swift b/SF50 TOLD/Loaders/BackgroundRefreshScheduler.swift index 3eaf054..bb04f41 100644 --- a/SF50 TOLD/Loaders/BackgroundRefreshScheduler.swift +++ b/SF50 TOLD/Loaders/BackgroundRefreshScheduler.swift @@ -16,12 +16,13 @@ import os /// /// ## Refresh Work /// -/// The refresh action pre-warms `WeatherLoader`'s bulk caches. Nav-data is -/// intentionally **not** refreshed here: ``NavDataLoader`` deletes the live -/// dataset before re-importing over several minutes, far longer than an -/// app-refresh window, so a mid-window force-cancellation could leave the store -/// empty or partially imported. Nav-data staleness is resolved on the next launch -/// instead. +/// The refresh action pre-warms `WeatherLoader`'s bulk caches. Nav-data is not +/// refreshed here: an import runs for several minutes, far longer than an +/// app-refresh window. It is now safe to abandon one — an import writes a new +/// generation of the store and nothing switches to it until it is complete — but +/// an unattended refresh wants a `BGProcessingTask` and a staleness decision made +/// off the launch path, which is its own piece of work. Nav-data staleness is +/// resolved on the next launch instead. @MainActor final class BackgroundRefreshScheduler { /// Shared singleton owning background-refresh scheduling. @@ -101,14 +102,15 @@ final class BackgroundRefreshScheduler { /// Nav-data refresh is deliberately skipped in the app-refresh window. /// - /// ``NavDataLoader/load()`` deletes the live dataset and re-imports over - /// several minutes — longer than an app-refresh window — and its import - /// container shares the live persistent store, so a force-cancellation - /// mid-import could corrupt or empty the store. Refreshing nav data off-launch - /// safely requires either importing into a genuinely separate store and - /// atomically swapping it in, or moving the import to a `BGProcessingTask` + /// ``NavDataLoader/load()`` runs for several minutes, longer than an app-refresh + /// window gives it. Being cut short no longer costs anything — the import writes + /// a generation nothing is reading, and the app only switches to it once it is + /// whole — but running it here would still leave it unfinished every time. + /// + /// Refreshing off-launch wants a `BGProcessingTask` /// (`codes.tim.SF50-TOLD.navdata-processing`, `UIBackgroundModes` `processing`, - /// `requiresNetworkConnectivity = true`) that gets a longer runtime budget. + /// `requiresNetworkConnectivity = true`), and a decision about when data is + /// stale enough to spend a pilot's cellular data on. That is a separate feature. /// Until then, nav-data staleness is resolved on the next launch. private func refreshNavDataIfStale() { logger.debug("Skipping nav-data refresh in app-refresh window (handled on launch).") diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift index 6baa911..1b0c6b6 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift @@ -17,6 +17,12 @@ import SwiftNASR /// 3. **Import**: Populates SwiftData with `Airport`, `Runway`, `Procedure`, /// `ProcedureSegment`, `Leg`, and `Obstacle` models /// +/// It writes into an empty store of its own — the next *generation* of the +/// dataset — and never touches the one in use. Nothing switches to what it wrote +/// until the import finishes and the result is found to hold airports, so an +/// import that fails, or is killed when the pilot swipes the app away, costs the +/// pilot nothing. That is why there is no step here that clears anything first. +/// /// ## Data Source /// /// Navigation data is pre-processed and published as GitHub release assets at: @@ -208,9 +214,6 @@ actor NavDataLoader { state = .extracting(progress: nil) let nasr = try await timing("decode") { try await Self.decompress(fileAt: payload) } - // The replacement data is fully decoded, so the old dataset can go - try await timing("reset") { try await resetData() } - // Load navaids first so they're available for leg relationships try await timing("navaids") { try await loadNavaids(nasr.navaids ?? []) } @@ -269,15 +272,6 @@ actor NavDataLoader { return result } - /// Deletes all persisted `Cycle` records on the loader's background context. - /// - /// Performed off the main thread so it never contends with the main - /// `NSManagedObjectContext` for the persistent store coordinator. - func clearCycles() throws { - try modelContext.delete(model: Cycle.self) - try modelContext.save() - } - private func writeCycles(_ cycles: AirportDataCodable.DataCycles) throws { insertCycle(cycles.nasr, source: .nasr) insertCycle(cycles.cifp, source: .cifp) @@ -400,38 +394,6 @@ actor NavDataLoader { } } - /// Deletes the previous dataset in bounded batches, one entity type at a time. - /// - /// Child entities are deleted before their parents so each delete touches - /// only its own table instead of fanning out through cascade rules. - private func resetData() async throws { - try await deleteAll(SF50_Shared.Leg.self) - try await deleteAll(SF50_Shared.ProcedureSegment.self) - try await deleteAll(SF50_Shared.Procedure.self) - try await deleteAll(SF50_Shared.Runway.self) - try await deleteAll(SF50_Shared.Airport.self) - try await deleteAll(SF50_Shared.Navaid.self) - try await deleteAll(SF50_Shared.Obstacle.self) - } - - /// Deletes every row of `model` in `saveBatchRowLimit`-sized transactions. - /// - /// SwiftData's bulk `delete(model:)` removes all rows in a single transaction - /// that holds the store's write lock for its full duration, stalling - /// concurrent main-context reads long enough to trip an app-hang report. - /// Deleting in bounded transactions with a pause between them keeps each lock - /// hold short so other store users can interleave, mirroring the insert path. - private func deleteAll(_: Model.Type) async throws { - var descriptor = FetchDescriptor() - descriptor.fetchLimit = Self.saveBatchRowLimit - - while case let batch = try modelContext.fetch(descriptor), !batch.isEmpty { - for object in batch { modelContext.delete(object) } - try modelContext.save() - await Task.yield() - } - } - /// Inserts an airport and its runways, procedures, segments, and legs. /// /// - Returns: The number of rows inserted, so callers can bound save batches diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift index a60db50..34fe088 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift @@ -37,6 +37,7 @@ final class NavDataLoaderViewModel: WithIdentifiableError { private(set) var deferred = false private let container: ModelContainer + private let installer = NavDataStoreInstaller(layout: .appGroup) private var cancellables: Set> = [] var showLoader: Bool { @@ -64,27 +65,31 @@ final class NavDataLoaderViewModel: WithIdentifiableError { /// pool rather than on the main thread at the moment the user taps Load. @concurrent nonisolated private static func makeImportLoader( - matching container: ModelContainer + matching container: ModelContainer, + generation: Int ) async throws -> NavDataLoader { - NavDataLoader(modelContainer: try makeImportContainer(matching: container)) + NavDataLoader( + modelContainer: try makeImportContainer(matching: container, generation: generation) + ) } - /// Creates a writable container on the nav-data store for the importer. + /// Creates a writable container for the importer, on the generation it is about to write. /// /// The importer's bulk transactions queue on their own persistent store coordinator, so /// main-context work (`@Query` fetches, model faults, history merges) never waits behind them — /// with WAL journaling, readers on another coordinator are not blocked by an in-flight write. /// - /// It holds nav data alone. The app reads that store read-only, and the pilot's own entries are - /// in a store the importer has no business touching. + /// It writes a generation nothing is reading. The dataset in use is not touched at all, which is + /// what makes an import safe to abandon: a failed one leaves a file nobody points at. nonisolated private static func makeImportContainer( - matching container: ModelContainer + matching container: ModelContainer, + generation: Int ) throws -> ModelContainer { // In-memory stores (used by UI tests) cannot be shared between containers. guard !container.configurations.contains(where: \.isStoredInMemoryOnly) else { return container } - return try AppStore.makeWritableContainer(layout: .appGroup) + return try AppStore.makeWritableContainer(layout: .appGroup, generation: generation) } private func setupObservation() { @@ -151,15 +156,16 @@ final class NavDataLoaderViewModel: WithIdentifiableError { } private func runLoad() async { - guard let loader = await makeLoader() else { return } + let generation = installer.reserveGeneration() + guard let loader = await makeLoader(generation: generation) else { return } let progressTask = await observeProgress(of: loader) addTask(progressTask) - await performLoad(with: loader, progressTask: progressTask) + await performLoad(with: loader, generation: generation, progressTask: progressTask) } - private func makeLoader() async -> NavDataLoader? { + private func makeLoader(generation: Int) async -> NavDataLoader? { do { - return try await Self.makeImportLoader(matching: container) + return try await Self.makeImportLoader(matching: container, generation: generation) } catch { SentrySDK.capture(error: error) { scope in scope.setTag(value: "importContainer", key: "navData.operation") @@ -191,7 +197,11 @@ final class NavDataLoaderViewModel: WithIdentifiableError { } } - private func performLoad(with loader: NavDataLoader, progressTask: Task) async { + private func performLoad( + with loader: NavDataLoader, + generation: Int, + progressTask: Task + ) async { let transaction = SentrySDK.startTransaction( name: "Nav Data Load", operation: "navData.load" @@ -199,10 +209,9 @@ final class NavDataLoaderViewModel: WithIdentifiableError { defer { progressTask.cancel() } do { error = nil - try await loader.clearCycles() Defaults[.ourAirportsLastUpdated] = nil let result = try await loader.load() - try clearNOTAMs() + try install(generation: generation) state = .finished Defaults[.ourAirportsLastUpdated] = result.ourAirportsLastUpdated @@ -222,14 +231,26 @@ final class NavDataLoaderViewModel: WithIdentifiableError { } } + /// Switches to the generation just imported, and reopens the store against it. + /// + /// The switch is a single recorded number, made only after the new store has been opened and + /// found to hold airports. Until that point the dataset in use has not been touched, so a failure + /// here — or a process killed mid-import — costs the pilot nothing. + private func install(generation: Int) throws { + try installer.install(generation: generation) + clearNOTAMs() + // The app watches the active generation and reopens its own store; doing it here as well would + // race that, and leave the container the views hold pointing at the older file. + } + /// Discards the NOTAMs the pilot entered against the dataset just replaced. /// /// A NOTAM carries no effective time, so one written against a previous cycle would otherwise /// keep asserting a contamination or a closure that nothing has re-confirmed. - private func clearNOTAMs() throws { + private func clearNOTAMs() { let context = ModelContext(container) - try NOTAMStore(context: context).removeAll() - try context.save() + try? NOTAMStore(context: context).removeAll() + try? context.save() } private func applyState(_ state: NavDataStateHelper.State) { diff --git a/SF50 TOLD/SF50_TOLDApp.swift b/SF50 TOLD/SF50_TOLDApp.swift index 8b35c92..4b16697 100644 --- a/SF50 TOLD/SF50_TOLDApp.swift +++ b/SF50 TOLD/SF50_TOLDApp.swift @@ -1,5 +1,6 @@ import BackgroundTasks import Combine +import Defaults import SF50_Shared import Sentry import SwiftData @@ -33,19 +34,8 @@ private class WidgetReloadObserver: ObservableObject { @main struct SF50_TOLDApp: App { - var sharedModelContainer: ModelContainer = { - // Screenshot runs hold their data in memory so the generated shots never depend on, or - // disturb, whatever is in the group container. - let isGeneratingScreenshots = ProcessInfo.processInfo.arguments.contains("GENERATE-SCREENSHOTS") - guard isGeneratingScreenshots else { - // Nav data is read-only once the app holds it, so a UI test's airports go in first. - MainActor.assumeIsolated { UITestingHelper.seedNavData() } - return AppStore.shared - } - do { return try AppStore.makeInMemoryContainer() } catch { - fatalError("Could not create ModelContainer: \(error)") - } - }() + @State private var sharedModelContainer = Self.makeContainer() + @State private var navDataGeneration = Defaults[.activeNavDataGeneration] // periphery:ignore - side-effect-only observer; retained for its lifetime, never read @StateObject private var widgetReloadObserver = WidgetReloadObserver() @@ -57,11 +47,13 @@ struct SF50_TOLDApp: App { WindowGroup { ContentView() .modelContainer(sharedModelContainer) + .id(navDataGeneration) .terrainPurgeAlert() .task { await ScenarioSeeder(container: sharedModelContainer).seedDefaultScenariosIfNeeded() _ = TerrainDataLoader.shared } + .task { await adoptNewNavDataGenerations() } } .backgroundTask(.appRefresh(BackgroundRefreshScheduler.appRefreshIdentifier)) { await BackgroundRefreshScheduler.shared.handleAppRefresh() @@ -77,7 +69,7 @@ struct SF50_TOLDApp: App { init() { if ProcessInfo.processInfo.arguments.contains("UI-TESTING") { - UITestingHelper.setupUITestingEnvironment(container: sharedModelContainer) + UITestingHelper.setupUITestingEnvironment() // Skip Sentry under UI tests: its profiling registers a CADisplayLink and // its logging runs on the main thread, which XCTest treats as never-ending // work — stalling wait-for-idle until tests time out (matches FART). @@ -121,4 +113,36 @@ struct SF50_TOLDApp: App { } } } + + /// Opens the app's stores, or an in-memory stand-in for a screenshot run. + private static func makeContainer() -> ModelContainer { + // Screenshot runs hold their data in memory so the generated shots never depend on, or + // disturb, whatever is in the group container. + guard ProcessInfo.processInfo.arguments.contains("GENERATE-SCREENSHOTS") else { + // Nav data is read-only once the app holds it, so a UI test's airports go in first. + MainActor.assumeIsolated { UITestingHelper.prepareStores() } + return AppStore.shared + } + do { return try AppStore.makeInMemoryContainer() } catch { + fatalError("Could not create ModelContainer: \(error)") + } + } + + /// Rebuilds the store when an import switches to a newly downloaded generation. + /// + /// The container holds an open handle on one generation's file, so a new one only reaches the app + /// by opening it. The generation it was reading is left on disk until the next launch, when + /// nothing holds it — which is what makes swapping safe while the app is running. + private func adoptNewNavDataGenerations() async { + var isFirstEmission = true + for await _ in Defaults.updates(.activeNavDataGeneration) { + guard !isFirstEmission else { + isFirstEmission = false + continue + } + AppStore.reopen() + sharedModelContainer = AppStore.shared + navDataGeneration = Defaults[.activeNavDataGeneration] + } + } }