From f60e56e79c73efbf5af1fc712b544810bbca515c Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 7 Sep 2026 23:39:21 -0700 Subject: [PATCH] Import a new generation of the store rather than emptying the old one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The import deleted the live dataset before writing its replacement, in many small committed transactions. Anything that stopped it in that window — a crash, or the system reclaiming the app — left an airport database that was permanently half there, which for an app that cannot work without one is not a state to recover from. It is why nav data was kept off every background path, and why the import had to ask the pilot to leave the app open. Each import now writes the next generation of the store to a file of its own and nothing reads it until it is whole. The switch is one recorded number, made only once the new store has been opened and found to hold airports; before that moment the dataset in use has not been touched at all. So an abandoned import costs a file nobody points at, reclaimed at the next launch, and resetData and its bounded delete loops are gone rather than made safe. Superseded generations are reclaimed only as a process opens its first container — however it opened one, including by discarding a bad one. Reclaiming later would mean deleting a file a container still alive may be reading, which is the very thing numbering generations avoids. A generation is written to a new file rather than over the old one because a store cannot be replaced underneath a reader: SQLite leaves the reader on a deleted inode, and the app is not the only process holding this store — the widget and the App Intents surfaces open it too. Numbers rise rather than alternate, so a generation an extension still has open is never reused beneath it. Cycles are no longer cleared before a download either. They were cleared first so a failed download would read as out-of-date, which a store the download never touches gives for free. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01471376Uhug2TYZN8RDFGtq --- SF50 Shared/Defaults.swift | 14 ++ SF50 Shared/Store/AppStore.swift | 163 ++++++++++++------ SF50 Shared/Store/NavDataStoreInstaller.swift | 86 +++++++++ SF50 Shared/Store/StoreLayout.swift | 88 +++++----- .../NavDataStoreInstallerTests.swift | 95 ++++++++++ SF50 SharedTests/TwoStoreContainerTests.swift | 80 ++++++--- SF50 TOLD/Launch/UITestingHelper.swift | 30 ++-- .../Loaders/BackgroundRefreshScheduler.swift | 28 +-- .../Loaders/NavDataLoader/NavDataLoader.swift | 50 +----- .../NavDataLoaderViewModel.swift | 55 ++++-- SF50 TOLD/SF50_TOLDApp.swift | 52 ++++-- 11 files changed, 515 insertions(+), 226 deletions(-) create mode 100644 SF50 Shared/Store/NavDataStoreInstaller.swift create mode 100644 SF50 SharedTests/NavDataStoreInstallerTests.swift diff --git a/SF50 Shared/Defaults.swift b/SF50 Shared/Defaults.swift index e9db7318..a0510d3f 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 9bc258cd..a9589c94 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 00000000..deb568ca --- /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 871caf8e..23098514 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 00000000..c98ad5e6 --- /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 5cde59dd..8617de1d 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 9b105912..4a22b46c 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 3eaf054d..bb04f41e 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 6baa9110..1b0c6b62 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 a60db50e..34fe0880 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 8b35c92a..4b16697f 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] + } + } }