From c4ed544fd1076d7ba4e4111e4d40450714c7d736 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 28 Jun 2026 22:37:39 -0400 Subject: [PATCH 01/19] Scaffold StorageKit module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the Shared/StorageKit library product + target in Package.swift and the StorageKitTests hosted unit-test bundle + scheme in Project.swift, plus the StorageMode enum so the new target builds. Pure groundwork — no behavior yet. (plan step: scaffold) Co-authored-by: Cursor --- Package.swift | 5 +++++ Project.swift | 7 +++++++ Shared/StorageKit/Sources/StorageMode.swift | 17 +++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 Shared/StorageKit/Sources/StorageMode.swift diff --git a/Package.swift b/Package.swift index f14f07a3..579224fc 100644 --- a/Package.swift +++ b/Package.swift @@ -13,6 +13,7 @@ let package = Package( .library(name: "LogKit", targets: ["LogKit"]), .library(name: "LogViewerUI", targets: ["LogViewerUI"]), .library(name: "SwiftDataInspector", targets: ["SwiftDataInspector"]), + .library(name: "StorageKit", targets: ["StorageKit"]), .library(name: "WhereCore", targets: ["WhereCore"]), .library(name: "WhereUI", targets: ["WhereUI"]), .library(name: "WhereTesting", targets: ["WhereTesting"]), @@ -47,6 +48,10 @@ let package = Package( name: "SwiftDataInspector", path: "Shared/SwiftDataInspector/Sources", ), + .target( + name: "StorageKit", + path: "Shared/StorageKit/Sources", + ), .target( name: "WhereCore", dependencies: [ diff --git a/Project.swift b/Project.swift index 846a2c5d..0d72dc96 100644 --- a/Project.swift +++ b/Project.swift @@ -235,6 +235,12 @@ let project = Project( productDependency: "SwiftDataInspector", sources: ["Shared/SwiftDataInspector/Tests/**"], ), + unitTests( + name: "StorageKitTests", + bundleIdSuffix: "storagekit", + productDependency: "StorageKit", + sources: ["Shared/StorageKit/Tests/**"], + ), unitTests( name: "WhereCoreTests", bundleIdSuffix: "wherecore", @@ -269,6 +275,7 @@ let project = Project( testScheme(name: "LogKitTests"), testScheme(name: "LogViewerUITests"), testScheme(name: "SwiftDataInspectorTests"), + testScheme(name: "StorageKitTests"), testScheme(name: "WhereCoreTests"), testScheme(name: "WhereTests"), testScheme(name: "WhereUITests"), diff --git a/Shared/StorageKit/Sources/StorageMode.swift b/Shared/StorageKit/Sources/StorageMode.swift new file mode 100644 index 00000000..4cfedfc4 --- /dev/null +++ b/Shared/StorageKit/Sources/StorageMode.swift @@ -0,0 +1,17 @@ +/// How a `StorageSystem` — and the whole container tree below it — persists. +/// +/// The mode is set once on the `StorageSystem` and inherited by every container, +/// so test and preview code can flip a single switch and nothing below has to +/// know it is running against ephemeral storage. The vending methods +/// (`StorageContainer.keyValueStore()` / `modelContainer(for:)`) do "the right +/// thing" for each mode automatically. +public enum StorageMode: Sendable { + /// Real files under a base directory; UserDefaults suites and SwiftData + /// stores persist across launches. + case persistent + + /// Ephemeral: files live in a temp directory removed by `deleteAll()`, + /// key-value stores are in-memory, and SwiftData stores are + /// `isStoredInMemoryOnly`. No app or model code needs to know. + case inMemory +} From b08ac4f8763c02e30af5478b72759c566fa1fa93 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 28 Jun 2026 22:54:23 -0400 Subject: [PATCH 02/19] Implement StorageKit container tree, vends, and teardown Add the mode-aware container tree and its full behavior: - StorageKey (sanitized path component), BaseDirectory (standard / custom roots + subdirectory namespacing), StorageError, and the StorageSystem root actor that vends top-level StorageContainers. - StorageContainer: recursive, cached/idempotent child vending, fileURL, a namespaced KeyValueStore (UserDefaults suite vs. in-memory), and an isolated SwiftData ModelContainer per node (its own child dir; isStoredInMemoryOnly + CloudKit-off in .inMemory). - Two children-first teardown paths: reversible deactivate() and destructive deleteContainer() (prepareForDeletion -> onDeactivate -> delete + suite purge -> afterDeletion), park-safe before the delete, plus deleteContents(). Sources and the StorageKitTests suite land together because StorageContainer depends on every source type and the hosted test bundle needs its sources. 41 Swift Testing cases cover the tree, modes, vends, sanitization, and teardown ordering/park-safety. (plan steps: core-tree, keyvalue, modelcontainer, teardown, tests) Co-authored-by: Cursor --- Shared/StorageKit/Sources/BaseDirectory.swift | 67 +++ .../StorageKit/Sources/CloudKitOption.swift | 19 + Shared/StorageKit/Sources/KeyValueStore.swift | 164 ++++++++ .../StorageKit/Sources/StorageContainer.swift | 392 ++++++++++++++++++ Shared/StorageKit/Sources/StorageError.swift | 7 + Shared/StorageKit/Sources/StorageKey.swift | 53 +++ Shared/StorageKit/Sources/StorageSystem.swift | 74 ++++ .../StorageKit/Tests/BaseDirectoryTests.swift | 41 ++ .../StorageKit/Tests/KeyValueStoreTests.swift | 67 +++ .../Tests/StorageContainerTests.swift | 367 ++++++++++++++++ Shared/StorageKit/Tests/StorageKeyTests.swift | 46 ++ .../Tests/StorageKitTestSupport.swift | 51 +++ .../StorageKit/Tests/StorageSystemTests.swift | 65 +++ 13 files changed, 1413 insertions(+) create mode 100644 Shared/StorageKit/Sources/BaseDirectory.swift create mode 100644 Shared/StorageKit/Sources/CloudKitOption.swift create mode 100644 Shared/StorageKit/Sources/KeyValueStore.swift create mode 100644 Shared/StorageKit/Sources/StorageContainer.swift create mode 100644 Shared/StorageKit/Sources/StorageError.swift create mode 100644 Shared/StorageKit/Sources/StorageKey.swift create mode 100644 Shared/StorageKit/Sources/StorageSystem.swift create mode 100644 Shared/StorageKit/Tests/BaseDirectoryTests.swift create mode 100644 Shared/StorageKit/Tests/KeyValueStoreTests.swift create mode 100644 Shared/StorageKit/Tests/StorageContainerTests.swift create mode 100644 Shared/StorageKit/Tests/StorageKeyTests.swift create mode 100644 Shared/StorageKit/Tests/StorageKitTestSupport.swift create mode 100644 Shared/StorageKit/Tests/StorageSystemTests.swift diff --git a/Shared/StorageKit/Sources/BaseDirectory.swift b/Shared/StorageKit/Sources/BaseDirectory.swift new file mode 100644 index 00000000..ce4f853c --- /dev/null +++ b/Shared/StorageKit/Sources/BaseDirectory.swift @@ -0,0 +1,67 @@ +import Foundation + +/// Where a `.persistent` `StorageSystem` is rooted on disk. +/// +/// `subdirectory` (optional) namespaces under the standard directory so a system +/// isn't created at the very top of Application Support / Caches; `custom(_:)` is +/// for tests and relocations. `resolvedURL(using:)` is public so a caller can +/// resolve a standard directory and build a `.custom` base from it. +/// +/// Ignored entirely in `.inMemory` mode — there the system roots itself in a +/// temporary directory it owns and removes on `deleteAll()`. +public struct BaseDirectory: Sendable { + private enum Root { + case applicationSupport + case caches + case custom(URL) + } + + private let root: Root + private let subdirectory: String? + + /// The app's Application Support directory, optionally namespaced by + /// `subdirectory`. + public static func applicationSupport(subdirectory: String? = nil) -> BaseDirectory { + BaseDirectory(root: .applicationSupport, subdirectory: subdirectory) + } + + /// The app's Caches directory, optionally namespaced by `subdirectory`. + public static func caches(subdirectory: String? = nil) -> BaseDirectory { + BaseDirectory(root: .caches, subdirectory: subdirectory) + } + + /// An explicit directory URL (tests, relocations). Build one from a resolved + /// standard directory via `resolvedURL(using:)`. + public static func custom(_ url: URL) -> BaseDirectory { + BaseDirectory(root: .custom(url), subdirectory: nil) + } + + /// Resolve to a concrete directory URL, creating the standard search-path + /// directory if needed. + /// + /// The optional `subdirectory` is appended but not created here — the + /// `StorageSystem` creates its namespace directory (and any intermediates) + /// underneath it. + public func resolvedURL(using fileManager: FileManager = .default) throws -> URL { + let base: URL = switch root { + case .applicationSupport: + try fileManager.url( + for: .applicationSupportDirectory, + in: .userDomainMask, + appropriateFor: nil, + create: true, + ) + case .caches: + try fileManager.url( + for: .cachesDirectory, + in: .userDomainMask, + appropriateFor: nil, + create: true, + ) + case let .custom(url): + url + } + guard let subdirectory, !subdirectory.isEmpty else { return base } + return base.appending(path: subdirectory, directoryHint: .isDirectory) + } +} diff --git a/Shared/StorageKit/Sources/CloudKitOption.swift b/Shared/StorageKit/Sources/CloudKitOption.swift new file mode 100644 index 00000000..ec2f23b6 --- /dev/null +++ b/Shared/StorageKit/Sources/CloudKitOption.swift @@ -0,0 +1,19 @@ +import SwiftData + +/// Whether a vended SwiftData store syncs through CloudKit. A thin pass-through +/// to `ModelConfiguration.CloudKitDatabase`; forced to `.none` in `.inMemory` +/// mode (an in-memory store can't sync). Richer CloudKit configuration is out of +/// scope for now. +public enum CloudKitOption: Sendable { + /// Local only — no CloudKit sync. + case none + /// Sync through the automatic (default) CloudKit container. + case automatic + + var cloudKitDatabase: ModelConfiguration.CloudKitDatabase { + switch self { + case .none: .none + case .automatic: .automatic + } + } +} diff --git a/Shared/StorageKit/Sources/KeyValueStore.swift b/Shared/StorageKit/Sources/KeyValueStore.swift new file mode 100644 index 00000000..bc5e9084 --- /dev/null +++ b/Shared/StorageKit/Sources/KeyValueStore.swift @@ -0,0 +1,164 @@ +import Foundation + +/// A minimal, `Sendable`-clean key-value store: typed accessors only (no +/// `Any?`), so a value can't silently fail to round-trip the way an arbitrary +/// `set(_: Any?)` can. `StorageContainer.keyValueStore()` vends a namespaced one +/// per node — a real `UserDefaults` suite in `.persistent` mode, an in-memory +/// dictionary in `.inMemory` mode — behind this same protocol, so app code +/// doesn't know or care which it got. +/// +/// Reads of an absent key mirror `UserDefaults`: `bool` → `false`, +/// `integer`/`double` → `0`, `string`/`data` → `nil`. +public protocol KeyValueStore: AnyObject, Sendable { + func bool(forKey key: String) -> Bool + func integer(forKey key: String) -> Int + func double(forKey key: String) -> Double + func string(forKey key: String) -> String? + func data(forKey key: String) -> Data? + + func set(_ value: Bool, forKey key: String) + func set(_ value: Int, forKey key: String) + func set(_ value: Double, forKey key: String) + func set(_ value: String?, forKey key: String) + func set(_ value: Data?, forKey key: String) + + func removeObject(forKey key: String) +} + +/// `KeyValueStore` backed by a `UserDefaults` suite. `UserDefaults` is internally +/// thread-safe, so wrapping it as `@unchecked Sendable` is sound. Setting a `nil` +/// `String`/`Data` removes the key, matching the in-memory store. +final class UserDefaultsKeyValueStore: KeyValueStore, @unchecked Sendable { + private let defaults: UserDefaults + + init(_ defaults: UserDefaults) { + self.defaults = defaults + } + + func bool(forKey key: String) -> Bool { + defaults.bool(forKey: key) + } + + func integer(forKey key: String) -> Int { + defaults.integer(forKey: key) + } + + func double(forKey key: String) -> Double { + defaults.double(forKey: key) + } + + func string(forKey key: String) -> String? { + defaults.string(forKey: key) + } + + func data(forKey key: String) -> Data? { + defaults.data(forKey: key) + } + + func set(_ value: Bool, forKey key: String) { + defaults.set(value, forKey: key) + } + + func set(_ value: Int, forKey key: String) { + defaults.set(value, forKey: key) + } + + func set(_ value: Double, forKey key: String) { + defaults.set(value, forKey: key) + } + + func set(_ value: String?, forKey key: String) { + if let value { + defaults.set(value, forKey: key) + } else { + defaults.removeObject(forKey: key) + } + } + + func set(_ value: Data?, forKey key: String) { + if let value { + defaults.set(value, forKey: key) + } else { + defaults.removeObject(forKey: key) + } + } + + func removeObject(forKey key: String) { + defaults.removeObject(forKey: key) + } +} + +/// In-memory `KeyValueStore` used in `.inMemory` mode (and handy for tests). +/// Lock-guarded so it is safely `Sendable`, with one typed slot per key so a +/// value read back as the wrong type reads as absent rather than crashing. +public final class InMemoryKeyValueStore: KeyValueStore, @unchecked Sendable { + private enum Value { + case bool(Bool) + case integer(Int) + case double(Double) + case string(String) + case data(Data) + } + + private let lock = NSLock() + private var storage: [String: Value] = [:] + + public init() {} + + private func value(forKey key: String) -> Value? { + lock.withLock { storage[key] } + } + + private func store(_ value: Value?, forKey key: String) { + lock.withLock { storage[key] = value } + } + + public func bool(forKey key: String) -> Bool { + if case let .bool(value) = value(forKey: key) { return value } + return false + } + + public func integer(forKey key: String) -> Int { + if case let .integer(value) = value(forKey: key) { return value } + return 0 + } + + public func double(forKey key: String) -> Double { + if case let .double(value) = value(forKey: key) { return value } + return 0 + } + + public func string(forKey key: String) -> String? { + if case let .string(value) = value(forKey: key) { return value } + return nil + } + + public func data(forKey key: String) -> Data? { + if case let .data(value) = value(forKey: key) { return value } + return nil + } + + public func set(_ value: Bool, forKey key: String) { + store(.bool(value), forKey: key) + } + + public func set(_ value: Int, forKey key: String) { + store(.integer(value), forKey: key) + } + + public func set(_ value: Double, forKey key: String) { + store(.double(value), forKey: key) + } + + public func set(_ value: String?, forKey key: String) { + store(value.map(Value.string), forKey: key) + } + + public func set(_ value: Data?, forKey key: String) { + store(value.map(Value.data), forKey: key) + } + + public func removeObject(forKey key: String) { + store(nil, forKey: key) + } +} diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift new file mode 100644 index 00000000..e7089829 --- /dev/null +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -0,0 +1,392 @@ +import Foundation +import SwiftData + +/// A handler registered for one of a container's teardown phases. Awaited during +/// `deactivate()` / `deleteContainer()`; a throw aborts the operation (see +/// `StorageContainer` for the park-safe guarantees). +public typealias TeardownHandler = @Sendable () async throws -> Void + +/// A node in a `StorageSystem`'s tree, and the place where storage actually +/// happens. Each container owns one directory and can vend child containers +/// (subdirectories), raw file URLs, a namespaced key-value store, and a SwiftData +/// `ModelContainer`. Children are `StorageContainer`s too, so the tree is +/// recursive below the root. +/// +/// An `actor`: it guards a mutable child registry and teardown registrations, and +/// serializes the file I/O behind vending and deletion. `key` / `url` / `mode` +/// are `nonisolated let` for cheap synchronous reads. +/// +/// ## Deactivate vs. delete +/// +/// Two registration-based, children-first teardown paths, never a boolean flag: +/// +/// - ``deactivate()`` — reversible. Runs every `onDeactivate` handler and drops +/// the framework's own cached vends, but leaves files and registrations in +/// place; the node goes `inactive` and the next vend reactivates it. (account +/// switch / keep-data logout) +/// - ``deleteContainer()`` — irreversible. Runs, children-first across the +/// subtree: every `prepareForDeletion` handler (resources still live), then +/// every `onDeactivate` handler (+ drop cached vends), then deletes the +/// directories and key-value suites and deregisters, then every `afterDeletion` +/// handler. Everything before the delete is **park-safe** — a throw aborts with +/// nothing deleted, so a retry is safe; an `afterDeletion` throw is post-commit +/// (the data is already gone, so it retries only that step). (delete account / +/// wipe-on-logout) +public actor StorageContainer { + /// Lifecycle of a node, modeled as one enum (not loose flags) per the repo's + /// "make invalid states unrepresentable" rule. + enum State { + /// Live: the directory exists and vends work. + case active + /// `deactivate()`d: cached vends dropped and `onDeactivate` handlers run, + /// but the directory and registrations remain. The next vend reactivates. + case inactive + /// `deleteContainer()`d: the directory is gone and the node is detached. + /// Any further vend throws (or, for the non-throwing `keyValueStore()`, + /// traps as the programmer error it is). + case deleted + } + + /// An opaque handle to a registered teardown handler. Returned by + /// `onDeactivate` / `prepareForDeletion` / `afterDeletion` and passed back to + /// `deregister(_:)`. + public struct Token: Hashable, Sendable { + fileprivate enum Phase { + case onDeactivate + case prepareForDeletion + case afterDeletion + } + + fileprivate let id: UInt64 + fileprivate let phase: Phase + } + + /// This node's key (a single, sanitized path component). + public nonisolated let key: StorageKey + /// This node's directory on disk. + public nonisolated let url: URL + /// Inherited from the owning `StorageSystem`. + public nonisolated let mode: StorageMode + + /// Hierarchical suite-name for this node's `.persistent` key-value store, e.g. + /// `"Where.user-1.logs"`. Unused in `.inMemory` mode. + nonisolated let suiteName: String + + private weak var parent: StorageContainer? + /// Internal (not `private`) so `@testable` tests can assert lifecycle + /// transitions; only this type mutates it. + private(set) var state: State = .active + private var children: [StorageKey: StorageContainer] = [:] + + private var keyValueStoreCache: (any KeyValueStore)? + private var modelContainerCache: [StorageKey: ModelContainer] = [:] + + private var nextTokenID: UInt64 = 0 + private var onDeactivateHandlers: [UInt64: TeardownHandler] = [:] + private var prepareForDeletionHandlers: [UInt64: TeardownHandler] = [:] + private var afterDeletionHandlers: [UInt64: TeardownHandler] = [:] + + init( + key: StorageKey, + url: URL, + mode: StorageMode, + suiteName: String, + parent: StorageContainer?, + ) { + self.key = key + self.url = url + self.mode = mode + self.suiteName = suiteName + self.parent = parent + } + + // MARK: - Child containers + + /// Vend the child container named `childKey`, creating its directory the first + /// time. Cached and idempotent: the same `childKey` always returns the same + /// instance. + public func container(_ childKey: StorageKey) throws -> StorageContainer { + try activate() + if let existing = children[childKey] { + return existing + } + let childURL = url.appending(path: childKey.name, directoryHint: .isDirectory) + try FileManager.default.createDirectory(at: childURL, withIntermediateDirectories: true) + let child = StorageContainer( + key: childKey, + url: childURL, + mode: mode, + suiteName: "\(suiteName).\(childKey.name)", + parent: self, + ) + children[childKey] = child + return child + } + + /// Vend (creating as needed) the container at a path of nested keys, e.g. + /// `container(path: ["user-1", "logs"])`. + public func container(path keys: [StorageKey]) async throws -> StorageContainer { + var node = self + for childKey in keys { + node = try await node.container(childKey) + } + return node + } + + // MARK: - Files + + /// A URL for a raw file named `name` directly inside this container's + /// directory. The directory already exists for a live container; writing the + /// file is the caller's job. + public nonisolated func fileURL(_ name: String) -> URL { + url.appending(path: name, directoryHint: .notDirectory) + } + + // MARK: - Key-value store + + /// Vend this node's namespaced key-value store: a `UserDefaults` suite in + /// `.persistent` mode, an in-memory store in `.inMemory` mode. Cached per node + /// (repeated calls return the same instance) and dropped by `deactivate()` / + /// `deleteContainer()`. + public func keyValueStore() -> any KeyValueStore { + switch state { + case .active: + break + case .inactive: + state = .active + case .deleted: + preconditionFailure( + "StorageKit: keyValueStore() on a deleted container \"\(key)\"", + ) + } + if let cached = keyValueStoreCache { + return cached + } + let store: any KeyValueStore + switch mode { + case .persistent: + guard let suite = UserDefaults(suiteName: suiteName) else { + preconditionFailure( + "StorageKit: invalid UserDefaults suite name \"\(suiteName)\"", + ) + } + store = UserDefaultsKeyValueStore(suite) + case .inMemory: + store = InMemoryKeyValueStore() + } + keyValueStoreCache = store + return store + } + + // MARK: - SwiftData + + /// Vend a SwiftData `ModelContainer` whose store lives in a dedicated child + /// container, so the `.store` file and its `-wal` / `-shm` sidecars and any + /// external-storage blobs are isolated in one directory (deleting that child — + /// or this container — deletes exactly that store's files). Cached per `named` + /// key. In `.inMemory` mode the store is `isStoredInMemoryOnly` and CloudKit + /// is forced off. + public func modelContainer( + for types: [any PersistentModel.Type], + named name: StorageKey = "store", + cloudKit: CloudKitOption = .none, + ) throws -> ModelContainer { + try activate() + if let cached = modelContainerCache[name] { + return cached + } + let storeContainer = try container(name) + let schema = Schema(types) + let configuration: ModelConfiguration + switch mode { + case .inMemory: + configuration = ModelConfiguration( + schema: schema, + isStoredInMemoryOnly: true, + cloudKitDatabase: .none, + ) + case .persistent: + let storeURL = storeContainer.url.appending( + path: "\(name.name).store", + directoryHint: .notDirectory, + ) + configuration = ModelConfiguration( + schema: schema, + url: storeURL, + cloudKitDatabase: cloudKit.cloudKitDatabase, + ) + } + let modelContainer = try ModelContainer(for: schema, configurations: [configuration]) + modelContainerCache[name] = modelContainer + return modelContainer + } + + // MARK: - Teardown registration + + /// Register a handler run on **both** teardown paths: "stop using your handle + /// and release it." Idempotent (it can run again after a parked retry). The + /// returned `Token` deregisters it. + @discardableResult + public func onDeactivate(_ handler: @escaping TeardownHandler) -> Token { + let id = takeTokenID() + onDeactivateHandlers[id] = handler + return Token(id: id, phase: .onDeactivate) + } + + /// Register a handler run **only** on deletion, first, while resources are + /// still live — reversible prep / final reads. A throw parks the deletion with + /// nothing deleted, so don't commit irreversible external work here. + @discardableResult + public func prepareForDeletion(_ handler: @escaping TeardownHandler) -> Token { + let id = takeTokenID() + prepareForDeletionHandlers[id] = handler + return Token(id: id, phase: .prepareForDeletion) + } + + /// Register a handler run **only** on deletion, after the files are gone — the + /// irreversible external commit ("it's gone now"). A throw leaves the deletion + /// done and retries only this step, so handlers must be idempotent and must + /// not assume the data still exists. + @discardableResult + public func afterDeletion(_ handler: @escaping TeardownHandler) -> Token { + let id = takeTokenID() + afterDeletionHandlers[id] = handler + return Token(id: id, phase: .afterDeletion) + } + + /// Remove a previously registered handler. + public func deregister(_ token: Token) { + switch token.phase { + case .onDeactivate: onDeactivateHandlers[token.id] = nil + case .prepareForDeletion: prepareForDeletionHandlers[token.id] = nil + case .afterDeletion: afterDeletionHandlers[token.id] = nil + } + } + + // MARK: - Teardown + + /// Reversible teardown: run `onDeactivate` handlers and drop cached vends + /// across the subtree, children-first, leaving files and registrations in + /// place. A no-op on an already-inactive node. Re-vending reactivates. + public func deactivate() async throws { + guard state == .active else { return } + for child in children.values { + try await child.deactivate() + } + for handler in onDeactivateHandlers.values { + try await handler() + } + dropCachedVends() + state = .inactive + } + + /// Irreversible teardown of this container and its whole subtree. See the type + /// doc for the phase order and the park-safe guarantee. + public func deleteContainer() async throws { + try await runPrepareForDeletion() + try await deactivate() + try removeDirectoryTree() + await purgeAndMarkDeleted() + try await runAfterDeletion() + await detachFromParent() + } + + /// Delete every child container and clear this container's own files, but keep + /// this (now empty) node live. The node's key-value suite is left intact. + public func deleteContents() async throws { + try activate() + for child in Array(children.values) { + try await child.deleteContainer() + } + try clearLooseFiles() + } + + // MARK: - Internals + + private func takeTokenID() -> UInt64 { + defer { nextTokenID += 1 } + return nextTokenID + } + + private func activate() throws { + switch state { + case .active: + break + case .inactive: + try FileManager.default.createDirectory( + at: url, + withIntermediateDirectories: true, + ) + state = .active + case .deleted: + throw StorageError.containerDeleted(key) + } + } + + private func dropCachedVends() { + keyValueStoreCache = nil + modelContainerCache.removeAll() + } + + private func runPrepareForDeletion() async throws { + guard state != .deleted else { return } + for child in children.values { + try await child.runPrepareForDeletion() + } + for handler in prepareForDeletionHandlers.values { + try await handler() + } + } + + private func removeDirectoryTree() throws { + guard FileManager.default.fileExists(atPath: url.path) else { return } + try FileManager.default.removeItem(at: url) + } + + /// Children-first across the subtree: remove the (persistent) key-value suite, + /// drop cached vends, and mark the node `deleted`. Disk directories are + /// already gone via `removeDirectoryTree()`; suites live outside the directory + /// so they're cleared here. + private func purgeAndMarkDeleted() async { + for child in children.values { + await child.purgeAndMarkDeleted() + } + purgeKeyValueSuite() + dropCachedVends() + state = .deleted + } + + private func purgeKeyValueSuite() { + guard mode == .persistent else { return } + UserDefaults().removePersistentDomain(forName: suiteName) + } + + private func runAfterDeletion() async throws { + for child in children.values { + try await child.runAfterDeletion() + } + for handler in afterDeletionHandlers.values { + try await handler() + } + } + + private func detachFromParent() async { + await parent?.removeChild(key) + parent = nil + children.removeAll() + } + + private func removeChild(_ childKey: StorageKey) { + children[childKey] = nil + } + + private func clearLooseFiles() throws { + let contents = try FileManager.default.contentsOfDirectory( + at: url, + includingPropertiesForKeys: nil, + ) + for item in contents { + try FileManager.default.removeItem(at: item) + } + } +} diff --git a/Shared/StorageKit/Sources/StorageError.swift b/Shared/StorageKit/Sources/StorageError.swift new file mode 100644 index 00000000..9f5500cc --- /dev/null +++ b/Shared/StorageKit/Sources/StorageError.swift @@ -0,0 +1,7 @@ +/// Errors thrown by StorageKit operations. +public enum StorageError: Error, Sendable, Equatable { + /// A container method was used after the container (or one of its ancestors) + /// was destroyed via `deleteContainer()` / `deleteAll()`. The node is gone; + /// vend a fresh one from a live `StorageSystem`. + case containerDeleted(StorageKey) +} diff --git a/Shared/StorageKit/Sources/StorageKey.swift b/Shared/StorageKit/Sources/StorageKey.swift new file mode 100644 index 00000000..220211e8 --- /dev/null +++ b/Shared/StorageKit/Sources/StorageKey.swift @@ -0,0 +1,53 @@ +import Foundation + +/// A filesystem-safe, single path component used to name a `StorageSystem` and +/// every container in its tree. +/// +/// Honors the repo's "typed keys, not raw `String`" rule: prefer building one +/// from a typed enum (`StorageKey(MyKeys.logs)`); the string-literal convenience +/// (`"logs"`) is for call-site brevity. The raw input is sanitized into a safe +/// single path component — path separators (`/`, `\`), `:`, and control +/// characters become `_`, and an empty / `.` / `..` input gets a leading `_` — +/// so a key can never traverse out of its parent directory. +/// +/// Sanitization is normalization, not validation: two different raw inputs can +/// sanitize to the same `name` (e.g. `"a/b"` and `"a_b"`), so keep your keys +/// distinct. +public struct StorageKey: Hashable, Sendable, ExpressibleByStringLiteral, CustomStringConvertible { + /// The sanitized path component, used both on disk and as this node's + /// key-value suite-name segment. + public let name: String + + public init(_ raw: String) { + name = Self.sanitized(raw) + } + + public init(_ key: some RawRepresentable) { + self.init(key.rawValue) + } + + public init(stringLiteral value: String) { + self.init(value) + } + + public var description: String { + name + } + + private static func sanitized(_ raw: String) -> String { + let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) + var result = "" + result.unicodeScalars.reserveCapacity(trimmed.unicodeScalars.count) + for scalar in trimmed.unicodeScalars { + if scalar == "/" || scalar == "\\" || scalar == ":" || scalar.value < 0x20 { + result.unicodeScalars.append("_") + } else { + result.unicodeScalars.append(scalar) + } + } + if result.isEmpty || result == "." || result == ".." { + return "_" + result + } + return result + } +} diff --git a/Shared/StorageKit/Sources/StorageSystem.swift b/Shared/StorageKit/Sources/StorageSystem.swift new file mode 100644 index 00000000..9878610e --- /dev/null +++ b/Shared/StorageKit/Sources/StorageSystem.swift @@ -0,0 +1,74 @@ +import Foundation + +/// The root of a StorageKit tree: pure configuration plus a vending point. It +/// owns the `mode`, the resolved namespace directory, and whole-tree teardown, +/// and vends the first level of containers (e.g. one per user id). All actual +/// storage operations live on the `StorageContainer`s it vends — this type does +/// no file work of its own beyond creating its namespace directory. +/// +/// The split costs one extra directory level on disk (the layout is an invisible +/// implementation detail) in exchange for keeping "what do I configure" separate +/// from "what do I store". +public actor StorageSystem { + /// How this system and everything below it persists. + public nonisolated let mode: StorageMode + /// The resolved namespace directory (a temp directory in `.inMemory` mode). + public nonisolated let url: URL + + private let root: StorageContainer + + /// Create a storage system named `name`. + /// + /// - `.persistent`: rooted at `/`. + /// - `.inMemory`: rooted at a unique temporary directory removed by + /// `deleteAll()`; `base` is ignored. + /// + /// `fileManager` is injectable so tests can resolve a `.custom` base + /// deterministically. + public init( + _ name: StorageKey, + mode: StorageMode, + base: BaseDirectory = .applicationSupport(), + fileManager: FileManager = .default, + ) throws { + self.mode = mode + let rootURL: URL + switch mode { + case .persistent: + let baseURL = try base.resolvedURL(using: fileManager) + rootURL = baseURL.appending(path: name.name, directoryHint: .isDirectory) + case .inMemory: + rootURL = fileManager.temporaryDirectory.appending( + path: "StorageKit-\(name.name)-\(UUID().uuidString)", + directoryHint: .isDirectory, + ) + } + try fileManager.createDirectory(at: rootURL, withIntermediateDirectories: true) + url = rootURL + root = StorageContainer( + key: name, + url: rootURL, + mode: mode, + suiteName: name.name, + parent: nil, + ) + } + + /// Vend a top-level container (e.g. one per user id). Cached and idempotent. + public func container(_ key: StorageKey) async throws -> StorageContainer { + try await root.container(key) + } + + /// Reversibly release the whole tree's handles, keeping all data. The next + /// vend reactivates. + public func deactivate() async throws { + try await root.deactivate() + } + + /// Delete every container and the namespace directory itself (in `.inMemory` + /// mode, the temp directory). The system is spent afterwards — build a new one + /// to start over. + public func deleteAll() async throws { + try await root.deleteContainer() + } +} diff --git a/Shared/StorageKit/Tests/BaseDirectoryTests.swift b/Shared/StorageKit/Tests/BaseDirectoryTests.swift new file mode 100644 index 00000000..4f4a26d5 --- /dev/null +++ b/Shared/StorageKit/Tests/BaseDirectoryTests.swift @@ -0,0 +1,41 @@ +import Foundation +import StorageKit +import Testing + +struct BaseDirectoryTests { + @Test + func customResolvesToTheGivenURL() throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + + #expect(try BaseDirectory.custom(temp).resolvedURL() == temp) + } + + @Test + func appendsSubdirectoryUnderAStandardDirectory() throws { + let resolved = try BaseDirectory + .applicationSupport(subdirectory: "StorageKitTests-namespace") + .resolvedURL() + #expect(resolved.lastPathComponent == "StorageKitTests-namespace") + } + + @Test + func omittingSubdirectoryReturnsTheStandardDirectory() throws { + let without = try BaseDirectory.applicationSupport().resolvedURL() + let with = try BaseDirectory + .applicationSupport(subdirectory: "child") + .resolvedURL() + #expect(with.deletingLastPathComponent().path == without.path) + } + + @Test + func doesNotCreateTheSubdirectory() throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + + let resolved = try BaseDirectory.custom(temp).resolvedURL() + let child = resolved.appending(path: "not-created", directoryHint: .isDirectory) + // `resolvedURL` only resolves; the system creates the namespace dir. + #expect(!FileManager.default.fileExists(atPath: child.path)) + } +} diff --git a/Shared/StorageKit/Tests/KeyValueStoreTests.swift b/Shared/StorageKit/Tests/KeyValueStoreTests.swift new file mode 100644 index 00000000..7bd79a2f --- /dev/null +++ b/Shared/StorageKit/Tests/KeyValueStoreTests.swift @@ -0,0 +1,67 @@ +import Foundation +@testable import StorageKit +import Testing + +struct KeyValueStoreTests { + @Test + func inMemoryRoundTripsEachType() { + let store = InMemoryKeyValueStore() + store.set(true, forKey: "flag") + store.set(42, forKey: "count") + store.set(3.5, forKey: "ratio") + store.set("hi", forKey: "label") + store.set(Data([1, 2, 3]), forKey: "blob") + + #expect(store.bool(forKey: "flag")) + #expect(store.integer(forKey: "count") == 42) + #expect(store.double(forKey: "ratio") == 3.5) + #expect(store.string(forKey: "label") == "hi") + #expect(store.data(forKey: "blob") == Data([1, 2, 3])) + } + + @Test + func inMemoryReturnsDefaultsForAbsentKeys() { + let store = InMemoryKeyValueStore() + #expect(!store.bool(forKey: "missing")) + #expect(store.integer(forKey: "missing") == 0) + #expect(store.double(forKey: "missing") == 0) + #expect(store.string(forKey: "missing") == nil) + #expect(store.data(forKey: "missing") == nil) + } + + @Test + func inMemoryReadingTheWrongTypeReturnsTheDefault() { + let store = InMemoryKeyValueStore() + store.set(true, forKey: "flag") + #expect(store.integer(forKey: "flag") == 0) + #expect(store.string(forKey: "flag") == nil) + } + + @Test + func inMemorySettingNilOrRemovingClearsTheKey() { + let store = InMemoryKeyValueStore() + store.set("hi", forKey: "label") + store.set(nil as String?, forKey: "label") + #expect(store.string(forKey: "label") == nil) + + store.set(Data([9]), forKey: "blob") + store.removeObject(forKey: "blob") + #expect(store.data(forKey: "blob") == nil) + } + + @Test + func userDefaultsWrapperRoundTripsAndRemoves() throws { + let suiteName = "StorageKitTests-\(UUID().uuidString)" + let suite = try #require(UserDefaults(suiteName: suiteName)) + defer { suite.removePersistentDomain(forName: suiteName) } + let store = UserDefaultsKeyValueStore(suite) + + store.set(7, forKey: "count") + store.set("hi", forKey: "label") + #expect(store.integer(forKey: "count") == 7) + #expect(store.string(forKey: "label") == "hi") + + store.set(nil as String?, forKey: "label") + #expect(store.string(forKey: "label") == nil) + } +} diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift new file mode 100644 index 00000000..6605d276 --- /dev/null +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -0,0 +1,367 @@ +import Foundation +@testable import StorageKit +import SwiftData +import Testing + +struct StorageContainerTests { + // MARK: - Tree + + @Test + func vendsAndCachesChildContainers() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let logs = try await user.container("logs") + let logsAgain = try await user.container("logs") + + #expect(logs === logsAgain) + #expect(logs.url == user.url.appending(path: "logs", directoryHint: .isDirectory)) + #expect(FileManager.default.fileExists(atPath: logs.url.path)) + } + + @Test + func vendsANestedPath() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let leaf = try await user.container(path: ["logs", "today"]) + + #expect(leaf.key == "today") + #expect(leaf.url == user.url + .appending(path: "logs", directoryHint: .isDirectory) + .appending(path: "today", directoryHint: .isDirectory)) + #expect(FileManager.default.fileExists(atPath: leaf.url.path)) + } + + @Test + func fileURLPointsIntoTheContainerDirectory() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let fileURL = user.fileURL("note.txt") + try Data("hello".utf8).write(to: fileURL) + + #expect(fileURL == user.url.appending(path: "note.txt", directoryHint: .notDirectory)) + #expect(try String(contentsOf: fileURL, encoding: .utf8) == "hello") + } + + // MARK: - Key-value store + + @Test + func inMemoryKeyValueStoresAreNamespacedPerContainerAndCached() async throws { + let system = try StorageSystem("Where", mode: .inMemory) + let a = try await system.container("a") + let b = try await system.container("b") + + let aStore = await a.keyValueStore() + let bStore = await b.keyValueStore() + aStore.set(true, forKey: "flag") + + #expect(aStore.bool(forKey: "flag")) + #expect(!bStore.bool(forKey: "flag")) + #expect(await a.keyValueStore() === aStore) + } + + @Test + func persistentKeyValueStoreRoundTripsAndIsPurgedOnDelete() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + await user.keyValueStore().set(99, forKey: "count") + #expect(await user.keyValueStore().integer(forKey: "count") == 99) + + try await user.deleteContainer() + + let revived = try await system.container("user-1") + #expect(await revived.keyValueStore().integer(forKey: "count") == 0) + + try await system.deleteAll() + } + + // MARK: - SwiftData + + @Test + func persistentModelContainerIsolatesItsStoreAndCaches() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let container = try await user.modelContainer(for: [Note.self]) + let again = try await user.modelContainer(for: [Note.self]) + #expect(container === again) + + let context = ModelContext(container) + context.insert(Note(text: "hi")) + try context.save() + #expect(try context.fetchCount(FetchDescriptor()) == 1) + + let storeDir = user.url.appending(path: "store", directoryHint: .isDirectory) + let storeFile = storeDir.appending(path: "store.store", directoryHint: .notDirectory) + #expect(FileManager.default.fileExists(atPath: storeFile.path)) + } + + @Test + func inMemoryModelContainerWritesNoStoreFile() async throws { + let system = try StorageSystem("Where", mode: .inMemory) + let user = try await system.container("user-1") + let container = try await user.modelContainer(for: [Note.self]) + + let context = ModelContext(container) + context.insert(Note(text: "hi")) + try context.save() + #expect(try context.fetchCount(FetchDescriptor()) == 1) + + let storeFile = user.url + .appending(path: "store", directoryHint: .isDirectory) + .appending(path: "store.store", directoryHint: .notDirectory) + #expect(!FileManager.default.fileExists(atPath: storeFile.path)) + + try await system.deleteAll() + } + + // MARK: - Deactivate + + @Test + func deactivateRunsHandlersChildrenFirstAndKeepsData() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let logs = try await user.container("logs") + let file = logs.fileURL("a.txt") + try Data("x".utf8).write(to: file) + + let log = CallLog() + await user.onDeactivate { await log.record("user") } + await logs.onDeactivate { await log.record("logs") } + + try await user.deactivate() + + #expect(await log.entries == ["logs", "user"]) + #expect(await user.state == .inactive) + #expect(await logs.state == .inactive) + #expect(FileManager.default.fileExists(atPath: file.path)) + } + + @Test + func reVendingReactivatesAnInactiveContainer() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + _ = try await user.container("logs") + try await user.deactivate() + #expect(await user.state == .inactive) + + let logs = try await user.container("logs") + #expect(await user.state == .active) + #expect(FileManager.default.fileExists(atPath: logs.url.path)) + } + + @Test + func deactivateIsANoOpWhenAlreadyInactive() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let log = CallLog() + await user.onDeactivate { await log.record("user") } + + try await user.deactivate() + try await user.deactivate() + + #expect(await log.entries == ["user"]) + } + + @Test + func deregisterStopsAHandlerFromFiring() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let log = CallLog() + let token = await user.onDeactivate { await log.record("user") } + await user.deregister(token) + + try await user.deactivate() + #expect(await log.entries.isEmpty) + } + + // MARK: - Delete + + @Test + func deleteContainerRunsPhasesInOrderChildrenFirst() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let logs = try await user.container("logs") + let log = CallLog() + for (container, label) in [(user, "user"), (logs, "logs")] { + await container.prepareForDeletion { await log.record("prepare:\(label)") } + await container.onDeactivate { await log.record("deactivate:\(label)") } + await container.afterDeletion { await log.record("after:\(label)") } + } + + try await user.deleteContainer() + + #expect(await log.entries == [ + "prepare:logs", + "prepare:user", + "deactivate:logs", + "deactivate:user", + "after:logs", + "after:user", + ]) + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + } + + @Test + func deleteContainerLeavesSiblingsAndDeregisters() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user1 = try await system.container("user-1") + let user2 = try await system.container("user-2") + let logs1 = try await user1.container("logs") + + try await user1.deleteContainer() + + #expect(!FileManager.default.fileExists(atPath: user1.url.path)) + #expect(FileManager.default.fileExists(atPath: user2.url.path)) + #expect(await user1.state == .deleted) + #expect(await logs1.state == .deleted) + + await #expect(throws: StorageError.self) { _ = try await user1.container("x") } + await #expect(throws: StorageError.self) { _ = try await logs1.container("x") } + + let user1Revived = try await system.container("user-1") + #expect(user1Revived !== user1) + #expect(FileManager.default.fileExists(atPath: user1Revived.url.path)) + } + + // MARK: - Park-safe retries + + @Test + func throwingPrepareForDeletionParksWithNothingDeleted() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let throwOnce = ThrowOnce() + await user.prepareForDeletion { try await throwOnce.fireIfArmed() } + + await #expect(throws: StorageTestError.self) { try await user.deleteContainer() } + #expect(FileManager.default.fileExists(atPath: user.url.path)) + + try await user.deleteContainer() + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + } + + @Test + func throwingOnDeactivateParksWithNothingDeleted() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let throwOnce = ThrowOnce() + await user.onDeactivate { try await throwOnce.fireIfArmed() } + + await #expect(throws: StorageTestError.self) { try await user.deleteContainer() } + #expect(FileManager.default.fileExists(atPath: user.url.path)) + + try await user.deleteContainer() + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + } + + @Test + func throwingAfterDeletionIsPostCommitAndRetriesOnlyThatStep() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let throwOnce = ThrowOnce() + await user.afterDeletion { try await throwOnce.fireIfArmed() } + + await #expect(throws: StorageTestError.self) { try await user.deleteContainer() } + // Deletion already stands — the files are gone despite the throw. + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + #expect(await user.state == .deleted) + + // Retry re-runs only the post-commit step; it no longer throws. + try await user.deleteContainer() + } + + // MARK: - deleteContents + + @Test + func deleteContentsClearsDescendantsAndFilesButKeepsTheNode() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let logs = try await user.container("logs") + let loose = user.fileURL("note.txt") + try Data("x".utf8).write(to: loose) + + try await user.deleteContents() + + #expect(!FileManager.default.fileExists(atPath: logs.url.path)) + #expect(!FileManager.default.fileExists(atPath: loose.path)) + #expect(FileManager.default.fileExists(atPath: user.url.path)) + #expect(await user.state == .active) + + let logsRevived = try await user.container("logs") + #expect(FileManager.default.fileExists(atPath: logsRevived.url.path)) + } + + // MARK: - Error surfacing + + @Test + func deletionErrorsSurfaceAndLeaveDataInPlace() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let parent = system.url + // A read-only parent directory makes removing `user` fail. + try FileManager.default.setAttributes( + [.posixPermissions: 0o500], + ofItemAtPath: parent.path, + ) + defer { + try? FileManager.default.setAttributes( + [.posixPermissions: 0o700], + ofItemAtPath: parent.path, + ) + } + + await #expect(throws: (any Error).self) { try await user.deleteContainer() } + + try FileManager.default.setAttributes( + [.posixPermissions: 0o700], + ofItemAtPath: parent.path, + ) + #expect(FileManager.default.fileExists(atPath: user.url.path)) + } +} diff --git a/Shared/StorageKit/Tests/StorageKeyTests.swift b/Shared/StorageKit/Tests/StorageKeyTests.swift new file mode 100644 index 00000000..2beb6dcd --- /dev/null +++ b/Shared/StorageKit/Tests/StorageKeyTests.swift @@ -0,0 +1,46 @@ +import StorageKit +import Testing + +struct StorageKeyTests { + @Test + func passesThroughASafeComponent() { + #expect(StorageKey("logs").name == "logs") + } + + @Test + func trimsSurroundingWhitespace() { + #expect(StorageKey(" logs \n").name == "logs") + } + + @Test + func replacesPathSeparatorsAndColons() { + #expect(StorageKey("a/b").name == "a_b") + #expect(StorageKey("a\\b").name == "a_b") + #expect(StorageKey("a:b").name == "a_b") + } + + @Test + func neutralizesTraversalAndEmpty() { + #expect(StorageKey("").name == "_") + #expect(StorageKey(".").name == "_.") + #expect(StorageKey("..").name == "_..") + } + + @Test + func buildsFromARawRepresentableEnum() { + enum Keys: String { case logs } + #expect(StorageKey(Keys.logs).name == "logs") + } + + @Test + func acceptsAStringLiteral() { + let key: StorageKey = "logs" + #expect(key.name == "logs") + } + + @Test + func isValueEquatableByName() { + #expect(StorageKey("a/b") == StorageKey("a_b")) + #expect(StorageKey("logs") != StorageKey("notes")) + } +} diff --git a/Shared/StorageKit/Tests/StorageKitTestSupport.swift b/Shared/StorageKit/Tests/StorageKitTestSupport.swift new file mode 100644 index 00000000..64676211 --- /dev/null +++ b/Shared/StorageKit/Tests/StorageKitTestSupport.swift @@ -0,0 +1,51 @@ +import Foundation +import SwiftData + +/// Error a test injects into a teardown handler to exercise the park-safe paths. +enum StorageTestError: Error, Equatable { + case injected +} + +/// A `@Model` fixture for `modelContainer(for:)` tests. +@Model +final class Note { + var text: String + + init(text: String) { + self.text = text + } +} + +/// Records the order teardown handlers fire in, so a test can assert phase +/// ordering and children-first traversal. +actor CallLog { + private(set) var entries: [String] = [] + + func record(_ entry: String) { + entries.append(entry) + } +} + +/// Throws `StorageTestError.injected` the first time it's fired and succeeds +/// afterwards — used to verify a parked teardown completes on retry. +actor ThrowOnce { + private var armed = true + + func fireIfArmed() throws { + if armed { + armed = false + throw StorageTestError.injected + } + } +} + +/// A fresh, unique temporary directory for a `.custom` base, isolating each test +/// from the others and from the host's real Application Support. +func makeTemporaryDirectory() throws -> URL { + let url = FileManager.default.temporaryDirectory.appending( + path: "StorageKitTests-\(UUID().uuidString)", + directoryHint: .isDirectory, + ) + try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + return url +} diff --git a/Shared/StorageKit/Tests/StorageSystemTests.swift b/Shared/StorageKit/Tests/StorageSystemTests.swift new file mode 100644 index 00000000..99b0730f --- /dev/null +++ b/Shared/StorageKit/Tests/StorageSystemTests.swift @@ -0,0 +1,65 @@ +import Foundation +import StorageKit +import Testing + +struct StorageSystemTests { + @Test + func persistentSystemCreatesItsNamespaceDirectoryUnderTheBase() throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + #expect(system.url == temp.appending(path: "Where", directoryHint: .isDirectory)) + #expect(FileManager.default.fileExists(atPath: system.url.path)) + } + + @Test + func vendsTopLevelContainersIdempotently() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let first = try await system.container("user-1") + let second = try await system.container("user-1") + #expect(first === second) + #expect(FileManager.default.fileExists(atPath: first.url.path)) + } + + @Test + func inMemorySystemRootsInATempDirectoryAndDeleteAllRemovesIt() async throws { + let system = try StorageSystem("Where", mode: .inMemory) + let root = system.url + #expect(FileManager.default.fileExists(atPath: root.path)) + #expect(root.path.hasPrefix(FileManager.default.temporaryDirectory.path)) + + try await system.deleteAll() + #expect(!FileManager.default.fileExists(atPath: root.path)) + } + + @Test + func deleteAllRemovesThePersistentNamespaceDirectory() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + _ = try await system.container("user-1") + #expect(FileManager.default.fileExists(atPath: system.url.path)) + + try await system.deleteAll() + #expect(!FileManager.default.fileExists(atPath: system.url.path)) + } + + @Test + func subdirectoryNamespacesTheSystemUnderTheBase() throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + + let system = try StorageSystem( + "Where", + mode: .persistent, + base: .custom(temp.appending(path: "ns", directoryHint: .isDirectory)), + ) + #expect(system.url.deletingLastPathComponent().lastPathComponent == "ns") + #expect(FileManager.default.fileExists(atPath: system.url.path)) + } +} From 3e6c8ec455f727c2a0f1bc90109c84c37380340e Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 28 Jun 2026 22:57:09 -0400 Subject: [PATCH 03/19] Add StorageKit README + AGENTS docs Document the module: the root/node split, mode-aware vends, isolated SwiftData stores, and the deactivate vs. deleteContainer teardown paths with their park-safe guarantee. AGENTS.md captures the invariants to preserve (cached idempotent vends, in-memory parity, deletion ordering, suites purged on delete) and the test patterns. CLAUDE.md is generated by ./sync-agents and gitignored. (plan step: docs-generate) Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 127 ++++++++++++++++++++++++ Shared/StorageKit/README.md | 187 ++++++++++++++++++++++++++++++++++++ 2 files changed, 314 insertions(+) create mode 100644 Shared/StorageKit/AGENTS.md create mode 100644 Shared/StorageKit/README.md diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md new file mode 100644 index 00000000..a2c005ba --- /dev/null +++ b/Shared/StorageKit/AGENTS.md @@ -0,0 +1,127 @@ +# StorageKit – Module Shape + +StorageKit is an app-agnostic library for **containerized, mode-aware storage**: a +`StorageSystem` vends a tree of `StorageContainer`s, each owning one directory and +vending raw files, a namespaced key-value store, and an isolated SwiftData +`ModelContainer`. One `mode` switch makes the whole tree persistent or in-memory, +and two registration-based, children-first teardown paths cleanly stop or destroy +any subtree. See [`README.md`](README.md) for the narrative and usage. + +This file complements the root [`AGENTS.md`](../../AGENTS.md), which owns build +system, formatting, and global conventions. Read that first. + +## Scope & dependencies + +- Pure **Foundation + SwiftData**. It must **not** import UIKit/SwiftUI, app code + (WhereCore/WhereUI), or `LifecycleKit` — a consumer that wants a teardown step + wraps a `deactivate()` / `deleteContainer()` call in its own `LifecycleStep`, + keeping the two decoupled. +- Library target only ([`Package.swift`](../../Package.swift), + `Shared/StorageKit/Sources`); the hosted test bundle `StorageKitTests` is wired + in [`Project.swift`](../../Project.swift) via the `unitTests` helper (host: + `StuffTestHost`). +- Not yet adopted by the Where app — this is a standalone module. Migrating + Where's ad-hoc storage (`LocationOutbox`, `WidgetSnapshotStore`, + `SwiftDataStore`, `WherePreferences`) onto it is a future pass. + +## Key types + +- [`StorageSystem`](Sources/StorageSystem.swift) – the **root**: an `actor` that + is pure configuration (`mode`, resolved namespace `url`, injected + `FileManager`) plus a vending point. It owns no storage logic of its own — it + creates its namespace directory and delegates `container` / `deactivate` / + `deleteAll` to a private root `StorageContainer`. +- [`StorageContainer`](Sources/StorageContainer.swift) – the **node**, and where + everything happens: an `actor` holding a child registry, cached vends, and + teardown registrations. `key` / `url` / `mode` / `suiteName` are + `nonisolated let`. Children are `StorageContainer`s (recursive); the parent + back-reference is `weak` so a deleted subtree releases. +- [`StorageKey`](Sources/StorageKey.swift) – a sanitized single path component + (`Hashable`, `ExpressibleByStringLiteral`, `init(some RawRepresentable)`). +- [`BaseDirectory`](Sources/BaseDirectory.swift) – where a persistent system + roots (`applicationSupport` / `caches` with optional `subdirectory`, or + `custom`); `resolvedURL(using:)` is public so a `.custom` base can be built from + a standard one. +- [`StorageMode`](Sources/StorageMode.swift) / [`CloudKitOption`](Sources/CloudKitOption.swift) + / [`StorageError`](Sources/StorageError.swift) – the mode switch, the CloudKit + pass-through (`.none` / `.automatic`), and the typed error + (`containerDeleted`). +- [`KeyValueStore`](Sources/KeyValueStore.swift) – a minimal, `Sendable`-clean + protocol (typed `bool/int/double/string/data`, no `Any?`) with two conformers: + `UserDefaultsKeyValueStore` (`@unchecked Sendable` wrapper around a suite) and + the public `InMemoryKeyValueStore` (lock-guarded, typed slots). + +## Behaviors to preserve + +- **Root vs. node split.** Keep `StorageSystem` config-only; do not add storage + operations there. New per-node behavior goes on `StorageContainer`. +- **Vends are cached and idempotent.** `container(_:)`, `keyValueStore()`, and + `modelContainer(for:)` return the same instance per node until the cache is + dropped by teardown. This is load-bearing for `ModelContainer` (never two on one + file) — don't make any vend construct a fresh instance each call. +- **In-memory does the right thing automatically.** `.inMemory` mode roots in a + temp directory, vends `InMemoryKeyValueStore`, and builds the SwiftData store + `isStoredInMemoryOnly` with **CloudKit forced off**. Callers use identical code + in both modes — keep that invariant when touching the vends. +- **Each SwiftData store gets its own child directory.** `modelContainer(for:)` + vends a dedicated child (`named`, default `"store"`) and places the store at + `/.store`, so the db + `-wal`/`-shm` + external blobs are isolated + and deletable as a unit. Don't put a store directly in a shared directory. +- **Two teardown paths, never a flag.** `deactivate()` (reversible: run + `onDeactivate` + drop cached vends, keep files, go `inactive`, reactivate on + re-vend) and `deleteContainer()` (destructive). Don't collapse them into a + `tearDown(delete:)` boolean. +- **Deletion order and park-safety.** `deleteContainer()` runs children-first: + `prepareForDeletion` (resources live) → `deactivate` (`onDeactivate` + drop + vends) → remove directories **and** key-value suites + deregister → + `afterDeletion`. Everything before the remove must be reversible so a throw + parks with the subtree intact (retry-safe); only `afterDeletion` may run after + the point of no return, and a throw there retries just that step. Preserve this + ordering and the "nothing deleted on an early throw" guarantee. +- **State is one enum.** `State` is `active` / `inactive` / `deleted` (no loose + flags); vending reactivates an `inactive` node and throws/traps on a `deleted` + one; `deactivate()` is a no-op when already `inactive`. Keep invalid states + unrepresentable per the root rules. +- **Errors surface.** Vending/teardown throw `StorageError` or rethrow the + underlying `FileManager`/SwiftData error — never swallow into an empty default. + The only trap is the non-throwing `keyValueStore()` on a `deleted` node, which + is a genuine programmer error. +- **Key-value suites live outside the directory.** Because a `UserDefaults` suite + isn't under the container's directory, deletion must `removePersistentDomain` + per node (it does, in the purge phase). If you change suite naming, keep it + hierarchical, stable, and purged on delete. + +## Conventions + +- Follow the root rules: exhaustive `switch` over enums (no bare `default:`), + small named structs over tuples, typed keys not raw `String`s, `@_spi`/internal + for testing-only access, and "never silently swallow errors." +- `FileManager` is injected into `StorageSystem` (mirroring + `FileLocationOutbox.applicationSupport(fileManager:)`) for resolving the base + and creating the namespace directory; containers use `FileManager.default` for + their own I/O. Test determinism comes from `base: .custom(tempDir)`, not from + threading a mock `FileManager` through every actor (which would fight Sendable). +- Concurrency: both types are `actor`s; cross-actor reads use `nonisolated let`. + Teardown handlers are `@Sendable () async throws -> Void` (`TeardownHandler`). + +## Testing + +Tests live in [`Tests/`](Tests) (Swift Testing only, never XCTest), 1:1 with +sources, hosted in `StuffTestHost`. Shared fixtures (the `Note` `@Model`, a +`CallLog` actor, a `ThrowOnce` actor, `makeTemporaryDirectory()`) live in +[`StorageKitTestSupport.swift`](Tests/StorageKitTestSupport.swift). Patterns: + +- Always use `base: .custom(makeTemporaryDirectory())` (+ `defer` cleanup) so a + test never touches the host's real Application Support or Caches. +- Assert tree shape via the on-disk directories (`FileManager.fileExists`) and + identity via `===` on the vended actors / objects. +- Drive teardown ordering with a `CallLog` actor recording `phase:node` strings + and assert the exact children-first sequence. +- Drive park-safety with a `ThrowOnce` actor in a handler: assert the first call + throws and leaves data intact (or, for `afterDeletion`, deletes it), and the + retry completes. +- Read lifecycle transitions through `@testable import StorageKit` and the + internal `state` property; the `UserDefaultsKeyValueStore` wrapper is internal, + so its test also needs `@testable`. + +Consider these rules if they affect your changes. diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md new file mode 100644 index 00000000..921e4796 --- /dev/null +++ b/Shared/StorageKit/README.md @@ -0,0 +1,187 @@ +# StorageKit + +A small, app-agnostic library for **containerized, mode-aware storage**. You ask +a `StorageSystem` for a container ("give me a container for this user id"), ask +that container for sub-containers ("…and one for logs"), and each container vends +the things you actually store — raw files, a namespaced key-value store, and a +SwiftData `ModelContainer` — all rooted in one directory it owns. A single `mode` +switch makes the whole tree persistent or in-memory, and two registration-based +teardown paths let you cleanly stop or delete any subtree. + +StorageKit depends only on **Foundation + SwiftData** — no app code, no UI, and +not even `LifecycleKit` (wrap a teardown call in a `LifecycleStep` yourself if you +want one). + +## What you get + +- **A tree of containers.** A `StorageSystem` is the root (configuration + a + vending point); every `StorageContainer` below it owns one directory and can + vend child containers recursively. The on-disk layout mirrors the tree. +- **Mode-aware vends.** In `.persistent` mode you get real files, a + `UserDefaults` suite, and an on-disk SwiftData store; flip to `.inMemory` and + the same calls give you a temp directory, an in-memory key-value store, and an + `isStoredInMemoryOnly` SwiftData store with CloudKit forced off — **app and + model code never has to know**. +- **Isolated SwiftData stores.** `modelContainer(for:)` puts each store in its + own child directory, so the `.store` file, its `-wal` / `-shm` sidecars, and any + external-storage blobs stay together and delete together. +- **Idempotent, cached vends.** Asking for the same child container, key-value + store, or model container twice returns the same instance (critical for + `ModelContainer`, which must be unique per file). +- **Two teardown paths, never a boolean.** `deactivate()` reversibly releases + handles but keeps data (account switch); `deleteContainer()` destroys a subtree + for good (delete account). Both are children-first and registration-based, and + deletion is **park-safe** (a failure before the point of no return leaves + everything intact for a retry). + +## Installation + +`StorageKit` is a local SPM library in this repo (`Shared/StorageKit`). Add it to +a target's dependencies in [`Package.swift`](../../Package.swift): + +```swift +.target(name: "YourCore", dependencies: [.target(name: "StorageKit")]) +``` + +## Quick start + +```swift +import StorageKit + +// One system per app (or per test). Persistent in production… +let storage = try StorageSystem("Where", mode: .persistent) +// …or ephemeral in tests/previews — nothing below changes: +// let storage = try StorageSystem("Where", mode: .inMemory) + +let user = try await storage.container("user-1") // a directory per user +let logs = try await user.container("logs") // a subdirectory + +// Raw file +let url = logs.fileURL("today.json") +try Data(...).write(to: url) + +// Key-value (a UserDefaults suite, or in-memory) +let prefs = await user.keyValueStore() +prefs.set(true, forKey: "onboarded") + +// SwiftData (its store isolated in its own child directory) +let container = try await user.modelContainer(for: [Note.self]) +``` + +### Tearing down + +```swift +// Switching accounts: release handles, keep everything on disk. +try await user.deactivate() // re-vending later reactivates + +// Deleting an account: destroy this user's whole subtree. +try await user.deleteContainer() // siblings untouched + +// Wipe the whole system (and its namespace directory). +try await storage.deleteAll() +``` + +## Public API + +```swift +public enum StorageMode { case persistent, inMemory } + +public struct BaseDirectory { + public static func applicationSupport(subdirectory: String? = nil) -> BaseDirectory + public static func caches(subdirectory: String? = nil) -> BaseDirectory + public static func custom(_ url: URL) -> BaseDirectory + public func resolvedURL(using: FileManager = .default) throws -> URL +} + +public struct StorageKey: Hashable, Sendable, ExpressibleByStringLiteral { + public init(_ raw: String) + public init(_ key: some RawRepresentable) + public let name: String +} + +public actor StorageSystem { + public init(_ name: StorageKey, mode: StorageMode, + base: BaseDirectory = .applicationSupport(), + fileManager: FileManager = .default) throws + public nonisolated let mode: StorageMode + public nonisolated let url: URL + public func container(_ key: StorageKey) async throws -> StorageContainer + public func deactivate() async throws + public func deleteAll() async throws +} + +public actor StorageContainer { + public nonisolated let key: StorageKey + public nonisolated let url: URL + public nonisolated let mode: StorageMode + + public func container(_ key: StorageKey) async throws -> StorageContainer + public func container(path keys: [StorageKey]) async throws -> StorageContainer + public nonisolated func fileURL(_ name: String) -> URL + public func keyValueStore() async -> any KeyValueStore + public func modelContainer(for types: [any PersistentModel.Type], + named: StorageKey = "store", + cloudKit: CloudKitOption = .none) async throws -> ModelContainer + + @discardableResult public func onDeactivate(_ handler: @escaping TeardownHandler) async -> Token + @discardableResult public func prepareForDeletion(_ handler: @escaping TeardownHandler) async -> Token + @discardableResult public func afterDeletion(_ handler: @escaping TeardownHandler) async -> Token + public func deregister(_ token: Token) async + + public func deactivate() async throws + public func deleteContainer() async throws + public func deleteContents() async throws +} + +public protocol KeyValueStore: AnyObject, Sendable { /* typed bool/int/double/string/data */ } +public final class InMemoryKeyValueStore: KeyValueStore { public init() } +``` + +## How it works + +- **Root vs. node.** `StorageSystem` is pure configuration (mode, base directory, + injected `FileManager`) plus a vending point; it does no storage work beyond + creating its namespace directory and delegating to a hidden root container. All + operations live on `StorageContainer`. The split costs one extra directory level + on disk, which is invisible to users. +- **Where things live.** Persistent systems root at `/`; in-memory + systems root at a unique temporary directory removed by `deleteAll()`. A + container's `url` is `/`. A key-value store is a `UserDefaults` + suite named hierarchically (e.g. `Where.user-1.logs`) — those live in the app's + preferences, not the container directory, so deletion clears them explicitly. +- **Teardown order.** `deactivate()` runs `onDeactivate` handlers and drops cached + vends across the subtree, children-first, leaving files in place; + `deleteContainer()` runs, children-first: every `prepareForDeletion` handler + (resources still live) → every `onDeactivate` handler (+ drop vends) → delete + directories and key-value suites + deregister → every `afterDeletion` handler. + Everything before the delete is reversible, so a throw there parks the operation + with the subtree fully intact and a retry is safe; an `afterDeletion` throw is + post-commit (the data is already gone, so it retries only that step — keep those + handlers idempotent). + +## Contracts & limitations + +- **Vends are cached and idempotent**; the same key always returns the same + instance until the node is deactivated (drops the cache) or deleted. +- **`StorageKey` sanitizes, it doesn't validate.** Path separators, `:`, and + control characters become `_`, and empty / `.` / `..` get a leading `_`, so a + key can't escape its directory — but two different raw strings can collapse to + the same name, so keep keys distinct (prefer typed enums). +- **Errors surface.** Vending and teardown throw on real failures; nothing is + swallowed into a benign default. Using a deleted container throws + `StorageError.containerDeleted` (the non-throwing `keyValueStore()` traps, since + that's a programmer error). +- **CloudKit is a pass-through** (`.none` / `.automatic`), and is forced off in + `.inMemory` mode. Richer CloudKit configuration is out of scope. +- **`deleteAll()` spends the system** — its namespace directory is gone + afterwards; build a new `StorageSystem` to start over. + +## Testing + +Exercised with Swift Testing in a hosted bundle. Tests use `base: .custom(temp)` +so they never touch the real Application Support directory, and cover the tree, +both modes, cached/idempotent vends, key sanitization, key-value and SwiftData +vends, and — in detail — the two teardown paths: children-first ordering, the +`prepareForDeletion → onDeactivate → delete → afterDeletion` sequence, park-safe +retries on a thrown handler, and that deletion errors surface while leaving data +in place. From 139a02667b95ce7b677ab6db86529fd1e7758a12 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 13:48:14 -0400 Subject: [PATCH 04/19] Drop parent's cached ModelContainer when its child is removed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `deleteContents()` (and any direct child deletion) deletes a model store's child directory but left the parent's `modelContainerCache` entry in place, so a re-vend handed back a `ModelContainer` pointing at deleted files — silent data loss. Clear `modelContainerCache[childKey]` in `removeChild`, the single choke point every detach flows through. Fixes review finding F1. Co-authored-by: Cursor --- .../StorageKit/Sources/StorageContainer.swift | 6 +++++ .../Tests/StorageContainerTests.swift | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index e7089829..f4608fb4 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -378,6 +378,12 @@ public actor StorageContainer { private func removeChild(_ childKey: StorageKey) { children[childKey] = nil + // A model store lives in the child keyed by `childKey`, so its cached + // `ModelContainer` is held here on the parent. Drop it when the child is + // detached (e.g. by `deleteContents()` deleting that child) so a re-vend + // rebuilds against fresh files instead of returning a container backed by + // a just-deleted store. + modelContainerCache[childKey] = nil } private func clearLooseFiles() throws { diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index 6605d276..03e4c385 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -128,6 +128,29 @@ struct StorageContainerTests { try await system.deleteAll() } + @Test + func modelContainerIsRecreatedAfterDeleteContents() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let first = try await user.modelContainer(for: [Note.self]) + let writeContext = ModelContext(first) + writeContext.insert(Note(text: "hi")) + try writeContext.save() + #expect(try writeContext.fetchCount(FetchDescriptor()) == 1) + + try await user.deleteContents() + + // The store's files were deleted; re-vending must rebuild a fresh container + // rather than hand back the stale cached one pointing at deleted files. + let second = try await user.modelContainer(for: [Note.self]) + #expect(second !== first) + let readContext = ModelContext(second) + #expect(try readContext.fetchCount(FetchDescriptor()) == 0) + } + // MARK: - Deactivate @Test From 661fc46eb48d18a3241483b24ecccd75021849d1 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 13:49:25 -0400 Subject: [PATCH 05/19] Namespace key-value suites by resolved base directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The root UserDefaults suite name was derived from the system name alone, so two systems with the same name but different base directories shared one global suite — clobbering each other in production and making parallel tests flaky (one test's purge wiping another's data). Fold a stable (FNV-1a, base-36) hash of the resolved root URL into the root suite name; children inherit it. Persistent because Hasher is per-process-seeded and would break persistence across launches. Fixes review finding F2. Co-authored-by: Cursor --- .../StorageKit/Sources/StorageContainer.swift | 5 +++- Shared/StorageKit/Sources/StorageSystem.swift | 24 ++++++++++++++++++- .../StorageKit/Tests/StorageSystemTests.swift | 24 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index f4608fb4..59f06ef1 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -69,7 +69,10 @@ public actor StorageContainer { public nonisolated let mode: StorageMode /// Hierarchical suite-name for this node's `.persistent` key-value store, e.g. - /// `"Where.user-1.logs"`. Unused in `.inMemory` mode. + /// `"Where..user-1.logs"`. The root segment folds in a hash of the + /// system's resolved base directory (see `StorageSystem.rootSuiteName`) so + /// distinct systems don't share a global suite; children append their keys. + /// Unused in `.inMemory` mode. nonisolated let suiteName: String private weak var parent: StorageContainer? diff --git a/Shared/StorageKit/Sources/StorageSystem.swift b/Shared/StorageKit/Sources/StorageSystem.swift index 9878610e..a1ef5de2 100644 --- a/Shared/StorageKit/Sources/StorageSystem.swift +++ b/Shared/StorageKit/Sources/StorageSystem.swift @@ -49,11 +49,33 @@ public actor StorageSystem { key: name, url: rootURL, mode: mode, - suiteName: name.name, + suiteName: Self.rootSuiteName(name: name, url: rootURL), parent: nil, ) } + /// The root `UserDefaults` suite name for the tree. It folds a stable hash of + /// the resolved root URL into `name`, so two systems with the same `name` but + /// different base directories (parallel tests, or genuinely distinct on-disk + /// locations) get independent global suites instead of clobbering one shared + /// `""` domain. Child suites append their keys to this. Unused in + /// `.inMemory` mode, where key-value stores never touch `UserDefaults`. + private static func rootSuiteName(name: StorageKey, url: URL) -> String { + "\(name.name).\(stableHash(url.path))" + } + + /// A deterministic FNV-1a hash, rendered base-36. `Hasher`/`hashValue` is + /// seeded per process, so it can't be used here — the suite name must stay + /// stable across launches for persistence to survive. + private static func stableHash(_ string: String) -> String { + var hash: UInt64 = 0xCBF2_9CE4_8422_2325 + for byte in string.utf8 { + hash ^= UInt64(byte) + hash = hash &* 0x0000_0100_0000_01B3 + } + return String(hash, radix: 36) + } + /// Vend a top-level container (e.g. one per user id). Cached and idempotent. public func container(_ key: StorageKey) async throws -> StorageContainer { try await root.container(key) diff --git a/Shared/StorageKit/Tests/StorageSystemTests.swift b/Shared/StorageKit/Tests/StorageSystemTests.swift index 99b0730f..7f776dfd 100644 --- a/Shared/StorageKit/Tests/StorageSystemTests.swift +++ b/Shared/StorageKit/Tests/StorageSystemTests.swift @@ -49,6 +49,30 @@ struct StorageSystemTests { #expect(!FileManager.default.fileExists(atPath: system.url.path)) } + @Test + func sameNamedSystemsWithDifferentBasesHaveIndependentKeyValueStores() async throws { + let tempA = try makeTemporaryDirectory() + let tempB = try makeTemporaryDirectory() + defer { + try? FileManager.default.removeItem(at: tempA) + try? FileManager.default.removeItem(at: tempB) + } + + let systemA = try StorageSystem("Where", mode: .persistent, base: .custom(tempA)) + let systemB = try StorageSystem("Where", mode: .persistent, base: .custom(tempB)) + let userA = try await systemA.container("user-1") + let userB = try await systemB.container("user-1") + + await userA.keyValueStore().set(99, forKey: "count") + + // Same logical key path, different base: the suites must not collide. + #expect(await userA.keyValueStore().integer(forKey: "count") == 99) + #expect(await userB.keyValueStore().integer(forKey: "count") == 0) + + try await systemA.deleteAll() + try await systemB.deleteAll() + } + @Test func subdirectoryNamespacesTheSystemUnderTheBase() throws { let temp = try makeTemporaryDirectory() From 3efa1ea7ad09ae1295050ff3c0781ac30ba77682 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 13:51:18 -0400 Subject: [PATCH 06/19] Make teardown reentrancy-safe with a deleting state deleteContainer() / deleteContents() suspend at await points between removing a directory and marking the node deleted. A concurrent vend landing in that window would call activate() and recreate the very directory being torn down, leaving an orphaned tree on disk. Add a `deleting` state and mark the whole subtree (self before recursing, so no new child slips past the snapshot) before running any handler. While deleting, activate() throws and keyValueStore() traps, so racing vends are rejected instead of resurrecting the directory. A pre-commit throw reverts the freeze back to active (still park-safe); committing the delete advances to deleted. Fixes review finding F3. Co-authored-by: Cursor --- .../StorageKit/Sources/StorageContainer.swift | 106 +++++++++++++++--- .../Tests/StorageContainerTests.swift | 27 +++++ .../Tests/StorageKitTestSupport.swift | 10 ++ 3 files changed, 128 insertions(+), 15 deletions(-) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index 59f06ef1..e8c12a65 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -30,8 +30,9 @@ public typealias TeardownHandler = @Sendable () async throws -> Void /// directories and key-value suites and deregisters, then every `afterDeletion` /// handler. Everything before the delete is **park-safe** — a throw aborts with /// nothing deleted, so a retry is safe; an `afterDeletion` throw is post-commit -/// (the data is already gone, so it retries only that step). (delete account / -/// wipe-on-logout) +/// (the data is already gone, so it retries only that step). The subtree is +/// marked `deleting` up front, so a vend racing the teardown is rejected rather +/// than resurrecting a directory mid-delete. (delete account / wipe-on-logout) public actor StorageContainer { /// Lifecycle of a node, modeled as one enum (not loose flags) per the repo's /// "make invalid states unrepresentable" rule. @@ -41,6 +42,11 @@ public actor StorageContainer { /// `deactivate()`d: cached vends dropped and `onDeactivate` handlers run, /// but the directory and registrations remain. The next vend reactivates. case inactive + /// `deleteContainer()` / `deleteContents()` is mid-flight on this subtree. + /// Vends are rejected so a concurrent caller can't resurrect a directory + /// that is being torn down. A parked (pre-commit) throw reverts this to + /// `active`; committing the delete advances it to `deleted`. + case deleting /// `deleteContainer()`d: the directory is gone and the node is detached. /// Any further vend throws (or, for the non-throwing `keyValueStore()`, /// traps as the programmer error it is). @@ -157,7 +163,7 @@ public actor StorageContainer { break case .inactive: state = .active - case .deleted: + case .deleting, .deleted: preconditionFailure( "StorageKit: keyValueStore() on a deleted container \"\(key)\"", ) @@ -276,19 +282,34 @@ public actor StorageContainer { for child in children.values { try await child.deactivate() } - for handler in onDeactivateHandlers.values { - try await handler() - } - dropCachedVends() + try await fireOnDeactivate() state = .inactive } /// Irreversible teardown of this container and its whole subtree. See the type /// doc for the phase order and the park-safe guarantee. public func deleteContainer() async throws { - try await runPrepareForDeletion() - try await deactivate() - try removeDirectoryTree() + if state == .deleted { + // Retry after a post-commit (`afterDeletion`) throw: the data is + // already gone, so only the post-commit tail remains to re-run. + try await runAfterDeletion() + await detachFromParent() + return + } + + // Mark the whole subtree `deleting` first, so a vend racing the teardown + // is rejected instead of recreating a directory we're about to remove. + await beginDeleting() + do { + try await runPrepareForDeletion() + try await runOnDeactivate() + try removeDirectoryTree() + } catch { + // Park-safe: nothing is committed yet, so undo the freeze and leave + // the subtree usable for a later retry. + await revertDeleting() + throw error + } await purgeAndMarkDeleted() try await runAfterDeletion() await detachFromParent() @@ -298,10 +319,20 @@ public actor StorageContainer { /// this (now empty) node live. The node's key-value suite is left intact. public func deleteContents() async throws { try activate() - for child in Array(children.values) { - try await child.deleteContainer() + let existingChildren = Array(children.values) + // Freeze this node while clearing it so a concurrent vend can't slip a new + // child past the snapshot and have `clearLooseFiles()` delete it underfoot. + state = .deleting + do { + for child in existingChildren { + try await child.deleteContainer() + } + try clearLooseFiles() + } catch { + state = .active + throw error } - try clearLooseFiles() + state = .active } // MARK: - Internals @@ -321,7 +352,7 @@ public actor StorageContainer { withIntermediateDirectories: true, ) state = .active - case .deleted: + case .deleting, .deleted: throw StorageError.containerDeleted(key) } } @@ -331,8 +362,53 @@ public actor StorageContainer { modelContainerCache.removeAll() } + /// Run this node's `onDeactivate` handlers and drop its cached vends. Shared by + /// the reversible `deactivate()` and the deletion path; neither touches state + /// here so each can set its own (`inactive` vs. keep `deleting`). + private func fireOnDeactivate() async throws { + for handler in onDeactivateHandlers.values { + try await handler() + } + dropCachedVends() + } + + /// Mark the subtree `deleting` so vends are rejected for the duration of a + /// teardown. Marks `self` before recursing: once `self` is `deleting` no new + /// child can be vended onto it, so the child set it then snapshots is stable. + private func beginDeleting() async { + switch state { + case .active, .inactive: + state = .deleting + case .deleting, .deleted: + return + } + for child in children.values { + await child.beginDeleting() + } + } + + /// Undo `beginDeleting()` after a parked (pre-commit) throw, returning the + /// still-intact subtree to `active`. + private func revertDeleting() async { + for child in children.values { + await child.revertDeleting() + } + if state == .deleting { + state = .active + } + } + + /// The deletion-path counterpart to `deactivate()`: run `onDeactivate` + + /// drop cached vends across the subtree, children-first, but keep the + /// `deleting` state rather than going `inactive`. + private func runOnDeactivate() async throws { + for child in children.values { + try await child.runOnDeactivate() + } + try await fireOnDeactivate() + } + private func runPrepareForDeletion() async throws { - guard state != .deleted else { return } for child in children.values { try await child.runPrepareForDeletion() } diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index 03e4c385..aec4e991 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -278,6 +278,33 @@ struct StorageContainerTests { #expect(FileManager.default.fileExists(atPath: user1Revived.url.path)) } + @Test + func vendingIsRejectedWhileTeardownIsInFlight() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let outcome = VendOutcome() + // A handler running mid-teardown tries to vend onto the node being deleted; + // it must be rejected rather than resurrecting the directory. + await user.prepareForDeletion { + do { + _ = try await user.container("sneaky") + await outcome.record(rejected: false) + } catch { + await outcome.record(rejected: true) + } + } + + try await user.deleteContainer() + + #expect(await outcome.wasRejected == true) + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + let sneaky = user.url.appending(path: "sneaky", directoryHint: .isDirectory) + #expect(!FileManager.default.fileExists(atPath: sneaky.path)) + } + // MARK: - Park-safe retries @Test diff --git a/Shared/StorageKit/Tests/StorageKitTestSupport.swift b/Shared/StorageKit/Tests/StorageKitTestSupport.swift index 64676211..91b9d154 100644 --- a/Shared/StorageKit/Tests/StorageKitTestSupport.swift +++ b/Shared/StorageKit/Tests/StorageKitTestSupport.swift @@ -39,6 +39,16 @@ actor ThrowOnce { } } +/// Records whether a vend attempted from inside a teardown handler was rejected, +/// so a test can prove the subtree refuses vends while it is being deleted. +actor VendOutcome { + private(set) var wasRejected: Bool? + + func record(rejected: Bool) { + wasRejected = rejected + } +} + /// A fresh, unique temporary directory for a `.custom` base, isolating each test /// from the others and from the host's real Application Support. func makeTemporaryDirectory() throws -> URL { From 7dc0c6380c59c666d5b0093c2826e82cbe32b9b7 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 13:52:43 -0400 Subject: [PATCH 07/19] Document keyValueStore()'s non-throwing trap contract keyValueStore() intentionally stays non-throwing so reads don't need a try, at the cost of trapping when vended on a deleting/deleted container. That's a deliberate trade-off (F4): rather than make every access try/catch, spell out that the trap signals a lifecycle bug, that callers should hold the store through a live handle and release it on teardown, and that container(_:)/modelContainer(for:) are the throwing alternatives. Doc comment + README + AGENTS only; no behavior change. Addresses review finding F4 by documentation, as agreed. Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 19 +++++++++++++------ Shared/StorageKit/README.md | 12 ++++++++++-- .../StorageKit/Sources/StorageContainer.swift | 11 +++++++++++ 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md index a2c005ba..1009508f 100644 --- a/Shared/StorageKit/AGENTS.md +++ b/Shared/StorageKit/AGENTS.md @@ -78,14 +78,21 @@ system, formatting, and global conventions. Read that first. parks with the subtree intact (retry-safe); only `afterDeletion` may run after the point of no return, and a throw there retries just that step. Preserve this ordering and the "nothing deleted on an early throw" guarantee. -- **State is one enum.** `State` is `active` / `inactive` / `deleted` (no loose - flags); vending reactivates an `inactive` node and throws/traps on a `deleted` - one; `deactivate()` is a no-op when already `inactive`. Keep invalid states - unrepresentable per the root rules. +- **State is one enum.** `State` is `active` / `inactive` / `deleting` / + `deleted` (no loose flags); vending reactivates an `inactive` node and + throws/traps on a `deleting` or `deleted` one; `deactivate()` is a no-op when + already `inactive`. `deleting` is the in-flight teardown state that makes the + subtree reject vends so a race can't resurrect a directory mid-delete — a + pre-commit throw reverts it to `active`, committing advances it to `deleted`. + Keep invalid states unrepresentable per the root rules. - **Errors surface.** Vending/teardown throw `StorageError` or rethrow the underlying `FileManager`/SwiftData error — never swallow into an empty default. - The only trap is the non-throwing `keyValueStore()` on a `deleted` node, which - is a genuine programmer error. + The one deliberate exception is the **non-throwing `keyValueStore()`**, which + *traps* (not throws) when called on a `deleting`/`deleted` node. This keeps + reads terse; it's a genuine programmer error (vending while/after a delete is a + lifecycle bug), so don't make callers `try`/`catch` it — preserve the + non-throwing signature and the trap, and route through the throwing + `container(_:)` / `modelContainer(for:)` when a recoverable failure is wanted. - **Key-value suites live outside the directory.** Because a `UserDefaults` suite isn't under the container's directory, deletion must `removePersistentDomain` per node (it does, in the purge phase). If you change suite naming, keep it diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md index 921e4796..2781e990 100644 --- a/Shared/StorageKit/README.md +++ b/Shared/StorageKit/README.md @@ -169,8 +169,16 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } the same name, so keep keys distinct (prefer typed enums). - **Errors surface.** Vending and teardown throw on real failures; nothing is swallowed into a benign default. Using a deleted container throws - `StorageError.containerDeleted` (the non-throwing `keyValueStore()` traps, since - that's a programmer error). + `StorageError.containerDeleted`. +- **`keyValueStore()` is intentionally non-throwing — don't `try`/`catch` it.** + So reads stay terse (`store.bool(forKey:)`), it traps (rather than throws) when + called on a container that's being deleted or already deleted. That's a + *programmer error*, not a runtime condition to defend against on every access: + hold the store through a live handle, and release it when you tear the container + down (e.g. in an `onDeactivate` handler). The only way to hit the trap is to + vend while a concurrent delete is in flight — a lifecycle bug to fix at the call + site. If you need a throwing failure mode, use `container(_:)` / + `modelContainer(for:)` instead. - **CloudKit is a pass-through** (`.none` / `.automatic`), and is forced off in `.inMemory` mode. Richer CloudKit configuration is out of scope. - **`deleteAll()` spends the system** — its namespace directory is gone diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index e8c12a65..f7c04808 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -157,6 +157,17 @@ public actor StorageContainer { /// `.persistent` mode, an in-memory store in `.inMemory` mode. Cached per node /// (repeated calls return the same instance) and dropped by `deactivate()` / /// `deleteContainer()`. + /// + /// - Important: This is deliberately **non-throwing** so the common case reads + /// like `store.bool(forKey:)` without a `try`. The price is that calling it + /// on a container that is being deleted or is already deleted is a + /// **programmer error and traps** — don't wrap every access in + /// `try`/`catch`. Reach for the store through a live handle and stop using it + /// once you've torn its container down (e.g. release it in an `onDeactivate` + /// handler); a vend racing a concurrent delete is the one case where the + /// trap can fire, and that's a lifecycle bug to fix at the call site, not to + /// defend against on each call. Use `container(_:)` / `modelContainer(for:)` + /// instead if you need a throwing failure mode. public func keyValueStore() -> any KeyValueStore { switch state { case .active: From e39d9f34e85a328eb899695523627ca055d7968f Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 13:54:10 -0400 Subject: [PATCH 08/19] Deregister a deleted container before its afterDeletion step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an afterDeletion handler threw, detachFromParent() was skipped, so the node lingered in its parent's children map marked deleted — and a re-vend of that key returned the dead node, which threw on every use. Move the parent deregistration into the delete commit (before afterDeletion, per the documented phase order) and split out releaseChildren() so the post-commit step can still recurse children before they're dropped. A failed afterDeletion now leaves the key free to re-vend a fresh node. Fixes review finding F5. Co-authored-by: Cursor --- .../StorageKit/Sources/StorageContainer.swift | 21 ++++++++++++++---- .../Tests/StorageContainerTests.swift | 22 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index f7c04808..f4cb23d9 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -301,10 +301,11 @@ public actor StorageContainer { /// doc for the phase order and the park-safe guarantee. public func deleteContainer() async throws { if state == .deleted { - // Retry after a post-commit (`afterDeletion`) throw: the data is - // already gone, so only the post-commit tail remains to re-run. + // Retry after a post-commit (`afterDeletion`) throw: the data is gone + // and the node is already detached, so only the post-commit tail + // remains to re-run. try await runAfterDeletion() - await detachFromParent() + releaseChildren() return } @@ -322,8 +323,14 @@ public actor StorageContainer { throw error } await purgeAndMarkDeleted() - try await runAfterDeletion() + // Deregister from the parent as part of the commit, *before* the + // post-commit step — so that even if an `afterDeletion` handler throws, + // re-vending this key builds a fresh node instead of handing back this + // deleted one. `runAfterDeletion` still recurses the (not-yet-released) + // children, which are freed once it succeeds. await detachFromParent() + try await runAfterDeletion() + releaseChildren() } /// Delete every child container and clear this container's own files, but keep @@ -463,6 +470,12 @@ public actor StorageContainer { private func detachFromParent() async { await parent?.removeChild(key) parent = nil + } + + /// Release the subtree's child registry once `afterDeletion` has run. Kept + /// separate from `detachFromParent()` so the post-commit step can still + /// recurse the children before they're dropped. + private func releaseChildren() { children.removeAll() } diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index aec4e991..9f5ea07d 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -360,6 +360,28 @@ struct StorageContainerTests { try await user.deleteContainer() } + @Test + func reVendingAfterAFailedAfterDeletionGivesAFreshContainer() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let throwOnce = ThrowOnce() + await user.afterDeletion { try await throwOnce.fireIfArmed() } + + await #expect(throws: StorageTestError.self) { try await user.deleteContainer() } + #expect(await user.state == .deleted) + + // Despite the afterDeletion throw, the node was deregistered from its + // parent, so re-vending the key builds a fresh, usable container rather + // than handing back the deleted one. + let fresh = try await system.container("user-1") + #expect(fresh !== user) + #expect(FileManager.default.fileExists(atPath: fresh.url.path)) + _ = try await fresh.container("logs") + } + // MARK: - deleteContents @Test From b65ad16f5b334d7d593d7db5df2152b18e3a13b4 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:33:27 -0400 Subject: [PATCH 09/19] Run onDeactivate once across deactivate-then-delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The F3 freeze collapsed active and inactive into one deleting state, losing the "already deactivated?" bit. So deleting a node that had been deactivate()d re-fired its onDeactivate handlers (idempotent, but surprising), and a parked delete of an inactive node reverted to active rather than inactive. Carry the pre-freeze state in the case itself — deleting(wasActive:) — so onDeactivate fires exactly once (skipped when the node was already inactive) and a pre-commit throw reverts to the exact prior resting state. prepareForDeletion/afterDeletion are unchanged: they're delete- only and still always run at their points. State gains an explicit Equatable since the associated value drops the implicit conformance. Refines review finding F3. Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 18 ++++---- .../StorageKit/Sources/StorageContainer.swift | 42 ++++++++++++------- .../Tests/StorageContainerTests.swift | 42 +++++++++++++++++++ 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md index 1009508f..121154eb 100644 --- a/Shared/StorageKit/AGENTS.md +++ b/Shared/StorageKit/AGENTS.md @@ -78,13 +78,17 @@ system, formatting, and global conventions. Read that first. parks with the subtree intact (retry-safe); only `afterDeletion` may run after the point of no return, and a throw there retries just that step. Preserve this ordering and the "nothing deleted on an early throw" guarantee. -- **State is one enum.** `State` is `active` / `inactive` / `deleting` / - `deleted` (no loose flags); vending reactivates an `inactive` node and - throws/traps on a `deleting` or `deleted` one; `deactivate()` is a no-op when - already `inactive`. `deleting` is the in-flight teardown state that makes the - subtree reject vends so a race can't resurrect a directory mid-delete — a - pre-commit throw reverts it to `active`, committing advances it to `deleted`. - Keep invalid states unrepresentable per the root rules. +- **State is one enum.** `State` is `active` / `inactive` / + `deleting(wasActive:)` / `deleted` (no loose flags); vending reactivates an + `inactive` node and throws/traps on a `deleting` or `deleted` one; + `deactivate()` is a no-op when already `inactive`. `deleting` is the in-flight + teardown state that makes the subtree reject vends so a race can't resurrect a + directory mid-delete. Its `wasActive` payload records the pre-freeze state so + `onDeactivate` fires exactly once (skipped when the node was already + `inactive`, since it ran during that `deactivate()`) and a pre-commit throw + reverts to the *exact* prior resting state (`active` or `inactive`); + committing advances it to `deleted`. Keep invalid states unrepresentable per + the root rules. - **Errors surface.** Vending/teardown throw `StorageError` or rethrow the underlying `FileManager`/SwiftData error — never swallow into an empty default. The one deliberate exception is the **non-throwing `keyValueStore()`**, which diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index f4cb23d9..2c4e8345 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -36,7 +36,7 @@ public typealias TeardownHandler = @Sendable () async throws -> Void public actor StorageContainer { /// Lifecycle of a node, modeled as one enum (not loose flags) per the repo's /// "make invalid states unrepresentable" rule. - enum State { + enum State: Equatable { /// Live: the directory exists and vends work. case active /// `deactivate()`d: cached vends dropped and `onDeactivate` handlers run, @@ -44,9 +44,12 @@ public actor StorageContainer { case inactive /// `deleteContainer()` / `deleteContents()` is mid-flight on this subtree. /// Vends are rejected so a concurrent caller can't resurrect a directory - /// that is being torn down. A parked (pre-commit) throw reverts this to - /// `active`; committing the delete advances it to `deleted`. - case deleting + /// that is being torn down. `wasActive` remembers the pre-freeze state so + /// `onDeactivate` runs exactly once (skipped if the node was already + /// `inactive`) and a parked (pre-commit) throw reverts to the right resting + /// state (`active` or `inactive`); committing the delete advances it to + /// `deleted`. + case deleting(wasActive: Bool) /// `deleteContainer()`d: the directory is gone and the node is detached. /// Any further vend throws (or, for the non-throwing `keyValueStore()`, /// traps as the programmer error it is). @@ -340,7 +343,9 @@ public actor StorageContainer { let existingChildren = Array(children.values) // Freeze this node while clearing it so a concurrent vend can't slip a new // child past the snapshot and have `clearLooseFiles()` delete it underfoot. - state = .deleting + // `activate()` above left it `active`, and the node itself survives, so the + // `wasActive` payload is moot here. + state = .deleting(wasActive: true) do { for child in existingChildren { try await child.deleteContainer() @@ -391,12 +396,15 @@ public actor StorageContainer { } /// Mark the subtree `deleting` so vends are rejected for the duration of a - /// teardown. Marks `self` before recursing: once `self` is `deleting` no new - /// child can be vended onto it, so the child set it then snapshots is stable. + /// teardown, recording each node's pre-freeze state in `wasActive`. Marks + /// `self` before recursing: once `self` is `deleting` no new child can be + /// vended onto it, so the child set it then snapshots is stable. private func beginDeleting() async { switch state { - case .active, .inactive: - state = .deleting + case .active: + state = .deleting(wasActive: true) + case .inactive: + state = .deleting(wasActive: false) case .deleting, .deleted: return } @@ -405,25 +413,29 @@ public actor StorageContainer { } } - /// Undo `beginDeleting()` after a parked (pre-commit) throw, returning the - /// still-intact subtree to `active`. + /// Undo `beginDeleting()` after a parked (pre-commit) throw, returning each + /// still-intact node to the exact resting state it came from. private func revertDeleting() async { for child in children.values { await child.revertDeleting() } - if state == .deleting { - state = .active + if case let .deleting(wasActive) = state { + state = wasActive ? .active : .inactive } } /// The deletion-path counterpart to `deactivate()`: run `onDeactivate` + /// drop cached vends across the subtree, children-first, but keep the - /// `deleting` state rather than going `inactive`. + /// `deleting` state rather than going `inactive`. Skips a node that was + /// already `inactive` before the delete, so its `onDeactivate` handlers (which + /// ran during that earlier `deactivate()`) don't fire a second time. private func runOnDeactivate() async throws { for child in children.values { try await child.runOnDeactivate() } - try await fireOnDeactivate() + if case .deleting(wasActive: true) = state { + try await fireOnDeactivate() + } } private func runPrepareForDeletion() async throws { diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index 9f5ea07d..57ce1176 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -278,6 +278,28 @@ struct StorageContainerTests { #expect(FileManager.default.fileExists(atPath: user1Revived.url.path)) } + @Test + func deletingAnAlreadyDeactivatedNodeRunsOnDeactivateOnce() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let logs = try await user.container("logs") + let log = CallLog() + await user.onDeactivate { await log.record("user") } + await logs.onDeactivate { await log.record("logs") } + + try await user.deactivate() + #expect(await log.entries == ["logs", "user"]) + + try await user.deleteContainer() + + // The delete must not re-fire onDeactivate — it already ran on deactivate. + #expect(await log.entries == ["logs", "user"]) + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + } + @Test func vendingIsRejectedWhileTeardownIsInFlight() async throws { let temp = try makeTemporaryDirectory() @@ -341,6 +363,26 @@ struct StorageContainerTests { #expect(!FileManager.default.fileExists(atPath: user.url.path)) } + @Test + func parkedDeleteOfAnInactiveNodeRevertsToInactive() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + try await user.deactivate() + #expect(await user.state == .inactive) + + let throwOnce = ThrowOnce() + await user.prepareForDeletion { try await throwOnce.fireIfArmed() } + + await #expect(throws: StorageTestError.self) { try await user.deleteContainer() } + + // A parked delete restores the exact prior resting state, not just `active`. + #expect(await user.state == .inactive) + #expect(FileManager.default.fileExists(atPath: user.url.path)) + } + @Test func throwingAfterDeletionIsPostCommitAndRetriesOnlyThatStep() async throws { let temp = try makeTemporaryDirectory() From d81fb2300cc67dc882427f3e0fdce7463d935786 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:43:20 -0400 Subject: [PATCH 10/19] Reject re-vending a model store with a different schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit modelContainer(for:named:) cached only by name, so a second call under the same store name with a different type set silently returned the first container — a schema mismatch the caller never sees. Cache the type identities alongside the container and throw a new StorageError.modelStoreSchemaMismatch on a mismatched re-vend; an identical type set still returns the cached instance. Addresses review minor note (model cache ignores types). Co-authored-by: Cursor --- .../StorageKit/Sources/StorageContainer.swift | 23 ++++++++++++++++--- Shared/StorageKit/Sources/StorageError.swift | 7 ++++++ .../Tests/StorageContainerTests.swift | 19 +++++++++++++++ .../Tests/StorageKitTestSupport.swift | 10 ++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index 2c4e8345..71fa8c5d 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -70,6 +70,14 @@ public actor StorageContainer { fileprivate let phase: Phase } + /// A cached `ModelContainer` plus the identity of the model types it was built + /// from, so a re-vend under the same store name with a different type set is + /// caught instead of silently returning a container with the wrong schema. + private struct CachedModelStore { + let container: ModelContainer + let typeIDs: Set + } + /// This node's key (a single, sanitized path component). public nonisolated let key: StorageKey /// This node's directory on disk. @@ -91,7 +99,7 @@ public actor StorageContainer { private var children: [StorageKey: StorageContainer] = [:] private var keyValueStoreCache: (any KeyValueStore)? - private var modelContainerCache: [StorageKey: ModelContainer] = [:] + private var modelContainerCache: [StorageKey: CachedModelStore] = [:] private var nextTokenID: UInt64 = 0 private var onDeactivateHandlers: [UInt64: TeardownHandler] = [:] @@ -209,14 +217,23 @@ public actor StorageContainer { /// or this container — deletes exactly that store's files). Cached per `named` /// key. In `.inMemory` mode the store is `isStoredInMemoryOnly` and CloudKit /// is forced off. + /// + /// A store name identifies exactly one schema: calling this again under the + /// same `named` key with a different set of `types` throws + /// `StorageError.modelStoreSchemaMismatch` rather than handing back the first + /// container with a mismatched schema. Use distinct names for distinct stores. public func modelContainer( for types: [any PersistentModel.Type], named name: StorageKey = "store", cloudKit: CloudKitOption = .none, ) throws -> ModelContainer { try activate() + let typeIDs = Set(types.map { ObjectIdentifier($0) }) if let cached = modelContainerCache[name] { - return cached + guard cached.typeIDs == typeIDs else { + throw StorageError.modelStoreSchemaMismatch(name) + } + return cached.container } let storeContainer = try container(name) let schema = Schema(types) @@ -240,7 +257,7 @@ public actor StorageContainer { ) } let modelContainer = try ModelContainer(for: schema, configurations: [configuration]) - modelContainerCache[name] = modelContainer + modelContainerCache[name] = CachedModelStore(container: modelContainer, typeIDs: typeIDs) return modelContainer } diff --git a/Shared/StorageKit/Sources/StorageError.swift b/Shared/StorageKit/Sources/StorageError.swift index 9f5500cc..f770395b 100644 --- a/Shared/StorageKit/Sources/StorageError.swift +++ b/Shared/StorageKit/Sources/StorageError.swift @@ -4,4 +4,11 @@ public enum StorageError: Error, Sendable, Equatable { /// was destroyed via `deleteContainer()` / `deleteAll()`. The node is gone; /// vend a fresh one from a live `StorageSystem`. case containerDeleted(StorageKey) + + /// `modelContainer(for:named:)` was called again under the same `named` store + /// (the associated key) but with a different set of model types than the + /// already-open store was built with. A store has exactly one schema — vend + /// distinct schemas under distinct names rather than silently reusing the + /// first one. + case modelStoreSchemaMismatch(StorageKey) } diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index 57ce1176..2732c7cb 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -151,6 +151,25 @@ struct StorageContainerTests { #expect(try readContext.fetchCount(FetchDescriptor()) == 0) } + @Test + func reVendingAStoreWithDifferentTypesThrows() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let first = try await user.modelContainer(for: [Note.self]) + + // Same name, different schema → caught, not silently the wrong container. + await #expect(throws: StorageError.modelStoreSchemaMismatch("store")) { + _ = try await user.modelContainer(for: [Note.self, Tag.self]) + } + + // The same type set still returns the cached container. + let again = try await user.modelContainer(for: [Note.self]) + #expect(again === first) + } + // MARK: - Deactivate @Test diff --git a/Shared/StorageKit/Tests/StorageKitTestSupport.swift b/Shared/StorageKit/Tests/StorageKitTestSupport.swift index 91b9d154..81fa709e 100644 --- a/Shared/StorageKit/Tests/StorageKitTestSupport.swift +++ b/Shared/StorageKit/Tests/StorageKitTestSupport.swift @@ -16,6 +16,16 @@ final class Note { } } +/// A second `@Model` fixture, used to vary a store's schema across calls. +@Model +final class Tag { + var label: String + + init(label: String) { + self.label = label + } +} + /// Records the order teardown handlers fire in, so a test can assert phase /// ordering and children-first traversal. actor CallLog { From 417dff25006ff570ff09772a982f0e3f1c5942b0 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:44:15 -0400 Subject: [PATCH 11/19] Keep in-memory modelContainer off disk modelContainer() vended the store's child container before the mode switch, so even .inMemory mode created an empty store directory on disk. Move the child vend into the .persistent branch; in-memory builds its isStoredInMemoryOnly configuration without touching the filesystem. Test now also asserts the store directory itself is absent. Addresses review minor note (in-memory still creates a store directory). Co-authored-by: Cursor --- Shared/StorageKit/Sources/StorageContainer.swift | 3 ++- Shared/StorageKit/Tests/StorageContainerTests.swift | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index 71fa8c5d..c6e5b61a 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -235,17 +235,18 @@ public actor StorageContainer { } return cached.container } - let storeContainer = try container(name) let schema = Schema(types) let configuration: ModelConfiguration switch mode { case .inMemory: + // No backing files, so don't vend a store child / touch disk. configuration = ModelConfiguration( schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none, ) case .persistent: + let storeContainer = try container(name) let storeURL = storeContainer.url.appending( path: "\(name.name).store", directoryHint: .notDirectory, diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index 2732c7cb..be281982 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -120,10 +120,11 @@ struct StorageContainerTests { try context.save() #expect(try context.fetchCount(FetchDescriptor()) == 1) - let storeFile = user.url - .appending(path: "store", directoryHint: .isDirectory) - .appending(path: "store.store", directoryHint: .notDirectory) + let storeDir = user.url.appending(path: "store", directoryHint: .isDirectory) + let storeFile = storeDir.appending(path: "store.store", directoryHint: .notDirectory) #expect(!FileManager.default.fileExists(atPath: storeFile.path)) + // In-memory mode must not touch disk at all — not even the store directory. + #expect(!FileManager.default.fileExists(atPath: storeDir.path)) try await system.deleteAll() } From 632ccdea800509b9f8136e1099d05042ead5efa5 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:45:21 -0400 Subject: [PATCH 12/19] Scope teardown Tokens to their issuing container Token ids start at 0 per node, so deregister(_:) keyed on id+phase alone could remove an unrelated handler when handed a token from a different container (same id, same phase). Stamp each Token with its issuing node's ObjectIdentifier and ignore a foreign token in deregister(_:). Addresses review minor note (Token is not node-scoped). Co-authored-by: Cursor --- .../StorageKit/Sources/StorageContainer.swift | 15 ++++++++++---- .../Tests/StorageContainerTests.swift | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index c6e5b61a..ff32507e 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -66,6 +66,11 @@ public actor StorageContainer { case afterDeletion } + /// Identity of the container that issued the token. Per-node token ids + /// start at 0, so without this a token from one container could match (and + /// deregister) an unrelated handler on another. `deregister(_:)` ignores a + /// token whose `node` isn't this container. + fileprivate let node: ObjectIdentifier fileprivate let id: UInt64 fileprivate let phase: Phase } @@ -271,7 +276,7 @@ public actor StorageContainer { public func onDeactivate(_ handler: @escaping TeardownHandler) -> Token { let id = takeTokenID() onDeactivateHandlers[id] = handler - return Token(id: id, phase: .onDeactivate) + return Token(node: ObjectIdentifier(self), id: id, phase: .onDeactivate) } /// Register a handler run **only** on deletion, first, while resources are @@ -281,7 +286,7 @@ public actor StorageContainer { public func prepareForDeletion(_ handler: @escaping TeardownHandler) -> Token { let id = takeTokenID() prepareForDeletionHandlers[id] = handler - return Token(id: id, phase: .prepareForDeletion) + return Token(node: ObjectIdentifier(self), id: id, phase: .prepareForDeletion) } /// Register a handler run **only** on deletion, after the files are gone — the @@ -292,11 +297,13 @@ public actor StorageContainer { public func afterDeletion(_ handler: @escaping TeardownHandler) -> Token { let id = takeTokenID() afterDeletionHandlers[id] = handler - return Token(id: id, phase: .afterDeletion) + return Token(node: ObjectIdentifier(self), id: id, phase: .afterDeletion) } - /// Remove a previously registered handler. + /// Remove a previously registered handler. A token issued by a different + /// container is ignored (it identifies no handler here). public func deregister(_ token: Token) { + guard token.node == ObjectIdentifier(self) else { return } switch token.phase { case .onDeactivate: onDeactivateHandlers[token.id] = nil case .prepareForDeletion: prepareForDeletionHandlers[token.id] = nil diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index be281982..b8bcd46e 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -243,6 +243,26 @@ struct StorageContainerTests { #expect(await log.entries.isEmpty) } + @Test + func deregisterIgnoresATokenFromAnotherContainer() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let a = try await system.container("a") + let b = try await system.container("b") + let log = CallLog() + // Both first tokens share id 0; only the issuing node may deregister them. + let tokenA = await a.onDeactivate { await log.record("a") } + await b.onDeactivate { await log.record("b") } + + await b.deregister(tokenA) + + try await a.deactivate() + try await b.deactivate() + #expect(await log.entries.sorted() == ["a", "b"]) + } + // MARK: - Delete @Test From 589d4b379bc8fb0010d91ab4eb14c93ac1b850dd Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:46:44 -0400 Subject: [PATCH 13/19] Document the model store / child-key namespace collision The default model store name "store" lives in the same key namespace as container(_:), so container("store") and the default model store would share a directory. Spell out the collision (and the distinct-named: guidance) in the modelContainer doc comment, README contracts, and AGENTS. Doc only. Addresses review minor note (default "store" name collides with a child). Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 18 ++++++++++++------ Shared/StorageKit/README.md | 6 ++++++ .../StorageKit/Sources/StorageContainer.swift | 7 +++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md index 121154eb..ab11a6a5 100644 --- a/Shared/StorageKit/AGENTS.md +++ b/Shared/StorageKit/AGENTS.md @@ -61,12 +61,18 @@ system, formatting, and global conventions. Read that first. file) — don't make any vend construct a fresh instance each call. - **In-memory does the right thing automatically.** `.inMemory` mode roots in a temp directory, vends `InMemoryKeyValueStore`, and builds the SwiftData store - `isStoredInMemoryOnly` with **CloudKit forced off**. Callers use identical code - in both modes — keep that invariant when touching the vends. -- **Each SwiftData store gets its own child directory.** `modelContainer(for:)` - vends a dedicated child (`named`, default `"store"`) and places the store at - `/.store`, so the db + `-wal`/`-shm` + external blobs are isolated - and deletable as a unit. Don't put a store directly in a shared directory. + `isStoredInMemoryOnly` with **CloudKit forced off** and **no disk access** (it + skips vending the store child, so no directory is created). Callers use + identical code in both modes — keep that invariant when touching the vends. +- **Each SwiftData store gets its own child directory (persistent only).** + `modelContainer(for:)` vends a dedicated child (`named`, default `"store"`) and + places the store at `/.store`, so the db + `-wal`/`-shm` + + external blobs are isolated and deletable as a unit. The store child shares the + `container(_:)` key namespace, so the default `"store"` collides with a + user-vended `container("store")` — documented; use distinct `named:` keys. A + store name pins one schema: re-vending the same name with different `types` + throws `StorageError.modelStoreSchemaMismatch` (cache keys on the type set). + Don't put a store directly in a shared directory. - **Two teardown paths, never a flag.** `deactivate()` (reversible: run `onDeactivate` + drop cached vends, keep files, go `inactive`, reactivate on re-vend) and `deleteContainer()` (destructive). Don't collapse them into a diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md index 2781e990..19393f52 100644 --- a/Shared/StorageKit/README.md +++ b/Shared/StorageKit/README.md @@ -179,6 +179,12 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } vend while a concurrent delete is in flight — a lifecycle bug to fix at the call site. If you need a throwing failure mode, use `container(_:)` / `modelContainer(for:)` instead. +- **A model store is a child container named `named` (default `"store"`).** In + `.persistent` mode it lives in the same key namespace as `container(_:)`, so + don't also vend a plain child under that key — `container("store")` and the + default model store would share one directory. Pass a distinct `named:` for + each store, and avoid colliding child keys. Re-vending a store under the same + name with a different `types` set throws `StorageError.modelStoreSchemaMismatch`. - **CloudKit is a pass-through** (`.none` / `.automatic`), and is forced off in `.inMemory` mode. Richer CloudKit configuration is out of scope. - **`deleteAll()` spends the system** — its namespace directory is gone diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index ff32507e..ec0505ac 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -227,6 +227,13 @@ public actor StorageContainer { /// same `named` key with a different set of `types` throws /// `StorageError.modelStoreSchemaMismatch` rather than handing back the first /// container with a mismatched schema. Use distinct names for distinct stores. + /// + /// - Important: In `.persistent` mode the store occupies a child container + /// named `named` (default `"store"`), in the **same** key namespace as + /// `container(_:)`. Don't also vend a plain child under that key — e.g. + /// `container("store")` and the default model store would share one + /// directory and stomp each other. Pass a distinct `named:` (or avoid that + /// key for your own children) to keep them apart. public func modelContainer( for types: [any PersistentModel.Type], named name: StorageKey = "store", From 19c46ecaaf4e4b9d9c28dbaa55641b292e74c582 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:47:21 -0400 Subject: [PATCH 14/19] Document fileURL() as pure, state-unaware path construction fileURL(_:) is nonisolated and can't see the node's state, so on an inactive or deleted node it hands back a URL into a missing directory. Call out the sharp edge in its doc comment and the README contracts so callers only use the URL while the container is live. Doc only. Addresses review minor note (fileURL on inactive/deleted nodes). Co-authored-by: Cursor --- Shared/StorageKit/README.md | 4 ++++ Shared/StorageKit/Sources/StorageContainer.swift | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md index 19393f52..f94bbdf3 100644 --- a/Shared/StorageKit/README.md +++ b/Shared/StorageKit/README.md @@ -179,6 +179,10 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } vend while a concurrent delete is in flight — a lifecycle bug to fix at the call site. If you need a throwing failure mode, use `container(_:)` / `modelContainer(for:)` instead. +- **`fileURL(_:)` is pure path construction.** It's `nonisolated`, so it doesn't + check the node's state or create the directory: on an `inactive` node the + directory is absent until the next vend, and on a `deleted` one it's gone, so + the URL points at a missing directory. Use it only while the container is live. - **A model store is a child container named `named` (default `"store"`).** In `.persistent` mode it lives in the same key namespace as `container(_:)`, so don't also vend a plain child under that key — `container("store")` and the diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index ec0505ac..0c945c3f 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -163,6 +163,13 @@ public actor StorageContainer { /// A URL for a raw file named `name` directly inside this container's /// directory. The directory already exists for a live container; writing the /// file is the caller's job. + /// + /// - Warning: This is `nonisolated` pure path construction — it can't read the + /// node's state, so it neither checks liveness nor ensures the directory + /// exists. On an `inactive` node the directory is absent until the next vend + /// reactivates it, and on a `deleted` one it's gone for good; in both cases + /// the returned URL points into a missing directory and writes will fail. + /// Only use the URL while the container is live. public nonisolated func fileURL(_ name: String) -> URL { url.appending(path: name, directoryHint: .notDirectory) } From 335d8f5ce622ad8a9c691af753c63be1988b6c80 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 29 Jun 2026 16:51:28 -0400 Subject: [PATCH 15/19] Add deferred teardown/concurrency test coverage Fill the remaining gaps from the review (the F1 and F2 gaps were already covered by their fixes' tests): - deactivate() drops the vend caches, and reactivation re-vends a fresh ModelContainer over the surviving on-disk store while the KV suite round-trips. - deleteContents() runs each child's full teardown hook sequence while the node stays live. - concurrent vends racing deleteContainer() never resurrect the deleted directory (regression guard for the F3 deleting-state freeze). Co-authored-by: Cursor --- .../Tests/StorageContainerTests.swift | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index b8bcd46e..2451b5d4 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -212,6 +212,35 @@ struct StorageContainerTests { #expect(FileManager.default.fileExists(atPath: logs.url.path)) } + @Test + func deactivateDropsVendCachesAndReVendReopensPersistedData() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let firstModel = try await user.modelContainer(for: [Note.self]) + let writeContext = ModelContext(firstModel) + writeContext.insert(Note(text: "hi")) + try writeContext.save() + await user.keyValueStore().set(7, forKey: "count") + + try await user.deactivate() + #expect(await user.state == .inactive) + + // Reactivation re-vends a *fresh* ModelContainer (cache was dropped) over + // the surviving on-disk store. + let secondModel = try await user.modelContainer(for: [Note.self]) + #expect(secondModel !== firstModel) + let readContext = ModelContext(secondModel) + #expect(try readContext.fetchCount(FetchDescriptor()) == 1) + + // The KV cache was dropped too, but the persisted suite round-trips. + #expect(await user.keyValueStore().integer(forKey: "count") == 7) + + try await system.deleteAll() + } + @Test func deactivateIsANoOpWhenAlreadyInactive() async throws { let temp = try makeTemporaryDirectory() @@ -488,6 +517,29 @@ struct StorageContainerTests { #expect(FileManager.default.fileExists(atPath: logsRevived.url.path)) } + @Test + func deleteContentsRunsChildTeardownHooks() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + let logs = try await user.container("logs") + let log = CallLog() + await logs.prepareForDeletion { await log.record("prepare") } + await logs.onDeactivate { await log.record("deactivate") } + await logs.afterDeletion { await log.record("after") } + + try await user.deleteContents() + + // Each child is fully deleted (all three hooks fire) while the node lives. + #expect(await log.entries == ["prepare", "deactivate", "after"]) + #expect(await user.state == .active) + #expect(await logs.state == .deleted) + #expect(!FileManager.default.fileExists(atPath: logs.url.path)) + #expect(FileManager.default.fileExists(atPath: user.url.path)) + } + // MARK: - Error surfacing @Test @@ -518,4 +570,32 @@ struct StorageContainerTests { ) #expect(FileManager.default.fileExists(atPath: user.url.path)) } + + // MARK: - Concurrency + + @Test + func concurrentVendsDoNotResurrectADeletedContainer() async throws { + let temp = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: temp) } + let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + + let user = try await system.container("user-1") + + // Hammer the node with vends while it's torn down. Each vend either lands + // before the freeze (and is deleted with the tree) or after it (and + // throws) — never recreating the directory mid-delete. + await withTaskGroup(of: Void.self) { group in + for index in 0 ..< 50 { + group.addTask { + _ = try? await user.container(StorageKey("child-\(index)")) + } + } + group.addTask { + try? await user.deleteContainer() + } + } + + #expect(await user.state == .deleted) + #expect(!FileManager.default.fileExists(atPath: user.url.path)) + } } From b36a43150ad940d75eecc7cc2e7e0eca853de0ae Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Tue, 30 Jun 2026 21:09:44 -0400 Subject: [PATCH 16/19] Add documents/library BaseDirectory cases Broaden the standard search-path options so `custom(_:)` is reserved for genuinely exceptional bases (tests, relocations, App Group / security-scoped URLs) rather than standing in for common directories like Documents. Addresses PR review: "there's more directory types than just application support and caches (eg documents), can we include those too so custom is really for exceptional cases?" Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 7 +- Shared/StorageKit/README.md | 4 +- Shared/StorageKit/Sources/BaseDirectory.swift | 68 +++++++++++++------ .../StorageKit/Tests/BaseDirectoryTests.swift | 13 ++++ 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md index ab11a6a5..31dc3b50 100644 --- a/Shared/StorageKit/AGENTS.md +++ b/Shared/StorageKit/AGENTS.md @@ -39,9 +39,10 @@ system, formatting, and global conventions. Read that first. - [`StorageKey`](Sources/StorageKey.swift) – a sanitized single path component (`Hashable`, `ExpressibleByStringLiteral`, `init(some RawRepresentable)`). - [`BaseDirectory`](Sources/BaseDirectory.swift) – where a persistent system - roots (`applicationSupport` / `caches` with optional `subdirectory`, or - `custom`); `resolvedURL(using:)` is public so a `.custom` base can be built from - a standard one. + roots (`applicationSupport` / `caches` / `documents` / `library` with optional + `subdirectory`, or `custom` for the exceptional cases — tests, relocations, an + App Group / security-scoped URL); `resolvedURL(using:)` is public so a `.custom` + base can be built from a standard one. - [`StorageMode`](Sources/StorageMode.swift) / [`CloudKitOption`](Sources/CloudKitOption.swift) / [`StorageError`](Sources/StorageError.swift) – the mode switch, the CloudKit pass-through (`.none` / `.automatic`), and the typed error diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md index f94bbdf3..e4bc7bb7 100644 --- a/Shared/StorageKit/README.md +++ b/Shared/StorageKit/README.md @@ -89,7 +89,9 @@ public enum StorageMode { case persistent, inMemory } public struct BaseDirectory { public static func applicationSupport(subdirectory: String? = nil) -> BaseDirectory public static func caches(subdirectory: String? = nil) -> BaseDirectory - public static func custom(_ url: URL) -> BaseDirectory + public static func documents(subdirectory: String? = nil) -> BaseDirectory + public static func library(subdirectory: String? = nil) -> BaseDirectory + public static func custom(_ url: URL) -> BaseDirectory // tests, relocations, App Group / security-scoped URLs public func resolvedURL(using: FileManager = .default) throws -> URL } diff --git a/Shared/StorageKit/Sources/BaseDirectory.swift b/Shared/StorageKit/Sources/BaseDirectory.swift index ce4f853c..93775d2f 100644 --- a/Shared/StorageKit/Sources/BaseDirectory.swift +++ b/Shared/StorageKit/Sources/BaseDirectory.swift @@ -2,10 +2,14 @@ import Foundation /// Where a `.persistent` `StorageSystem` is rooted on disk. /// -/// `subdirectory` (optional) namespaces under the standard directory so a system -/// isn't created at the very top of Application Support / Caches; `custom(_:)` is -/// for tests and relocations. `resolvedURL(using:)` is public so a caller can -/// resolve a standard directory and build a `.custom` base from it. +/// Pick the standard search-path directory that fits the data: `applicationSupport` +/// (the default home for app-managed data), `caches` (purgeable), `documents` +/// (user-facing, backed up and visible in the Files app when the app opts in), or +/// `library`. `subdirectory` (optional) namespaces under the chosen directory so a +/// system isn't created at the very top of it. `custom(_:)` is the escape hatch for +/// the exceptional cases the standard directories don't cover — tests, relocations, +/// an App Group / security-scoped URL — and `resolvedURL(using:)` is public so a +/// caller can resolve a standard directory and build a `.custom` base from it. /// /// Ignored entirely in `.inMemory` mode — there the system roots itself in a /// temporary directory it owns and removes on `deleteAll()`. @@ -13,6 +17,8 @@ public struct BaseDirectory: Sendable { private enum Root { case applicationSupport case caches + case documents + case library case custom(URL) } @@ -20,18 +26,34 @@ public struct BaseDirectory: Sendable { private let subdirectory: String? /// The app's Application Support directory, optionally namespaced by - /// `subdirectory`. + /// `subdirectory`. The default home for data your app creates and manages. public static func applicationSupport(subdirectory: String? = nil) -> BaseDirectory { BaseDirectory(root: .applicationSupport, subdirectory: subdirectory) } - /// The app's Caches directory, optionally namespaced by `subdirectory`. + /// The app's Caches directory, optionally namespaced by `subdirectory`. The OS + /// may purge it under storage pressure, so only put regenerable data here. public static func caches(subdirectory: String? = nil) -> BaseDirectory { BaseDirectory(root: .caches, subdirectory: subdirectory) } - /// An explicit directory URL (tests, relocations). Build one from a resolved - /// standard directory via `resolvedURL(using:)`. + /// The app's Documents directory, optionally namespaced by `subdirectory`. + /// User-facing: it's included in backups and shows up in the Files app when the + /// app opts in (`UISupportsDocumentBrowser` / `LSSupportsOpeningDocumentsInPlace`). + public static func documents(subdirectory: String? = nil) -> BaseDirectory { + BaseDirectory(root: .documents, subdirectory: subdirectory) + } + + /// The app's Library directory (the parent of Application Support and Caches), + /// optionally namespaced by `subdirectory`. Prefer `applicationSupport` / + /// `caches` unless you specifically need to root beside them. + public static func library(subdirectory: String? = nil) -> BaseDirectory { + BaseDirectory(root: .library, subdirectory: subdirectory) + } + + /// An explicit directory URL for the exceptional cases the standard directories + /// don't cover (tests, relocations, an App Group / security-scoped URL). Build + /// one from a resolved standard directory via `resolvedURL(using:)`. public static func custom(_ url: URL) -> BaseDirectory { BaseDirectory(root: .custom(url), subdirectory: nil) } @@ -45,23 +67,29 @@ public struct BaseDirectory: Sendable { public func resolvedURL(using fileManager: FileManager = .default) throws -> URL { let base: URL = switch root { case .applicationSupport: - try fileManager.url( - for: .applicationSupportDirectory, - in: .userDomainMask, - appropriateFor: nil, - create: true, - ) + try standardURL(.applicationSupportDirectory, using: fileManager) case .caches: - try fileManager.url( - for: .cachesDirectory, - in: .userDomainMask, - appropriateFor: nil, - create: true, - ) + try standardURL(.cachesDirectory, using: fileManager) + case .documents: + try standardURL(.documentDirectory, using: fileManager) + case .library: + try standardURL(.libraryDirectory, using: fileManager) case let .custom(url): url } guard let subdirectory, !subdirectory.isEmpty else { return base } return base.appending(path: subdirectory, directoryHint: .isDirectory) } + + private func standardURL( + _ directory: FileManager.SearchPathDirectory, + using fileManager: FileManager, + ) throws -> URL { + try fileManager.url( + for: directory, + in: .userDomainMask, + appropriateFor: nil, + create: true, + ) + } } diff --git a/Shared/StorageKit/Tests/BaseDirectoryTests.swift b/Shared/StorageKit/Tests/BaseDirectoryTests.swift index 4f4a26d5..7d86cb83 100644 --- a/Shared/StorageKit/Tests/BaseDirectoryTests.swift +++ b/Shared/StorageKit/Tests/BaseDirectoryTests.swift @@ -19,6 +19,19 @@ struct BaseDirectoryTests { #expect(resolved.lastPathComponent == "StorageKitTests-namespace") } + @Test + func standardDirectoriesResolveToDistinctLocations() throws { + let appSupport = try BaseDirectory.applicationSupport().resolvedURL() + let caches = try BaseDirectory.caches().resolvedURL() + let documents = try BaseDirectory.documents().resolvedURL() + let library = try BaseDirectory.library().resolvedURL() + + // Each standard directory maps to its own location — no two collapse. + #expect(Set([appSupport.path, caches.path, documents.path, library.path]).count == 4) + #expect(documents.lastPathComponent == "Documents") + #expect(library.lastPathComponent == "Library") + } + @Test func omittingSubdirectoryReturnsTheStandardDirectory() throws { let without = try BaseDirectory.applicationSupport().resolvedURL() From fe0530c6e20fa9eda9d78d45a43d559e25f04364 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Tue, 30 Jun 2026 21:12:20 -0400 Subject: [PATCH 17/19] Fold the base directory into StorageMode.persistent Make the persistence base an associated value of `.persistent(base:)` instead of a defaulted `StorageSystem.init` parameter. On-disk storage now can't be requested without saying where it lives, `.inMemory` can't carry a base it would ignore, and the base is a required, explicit choice at the call site. Addresses PR review: "Remove default, should need to be specified explicitly." Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 17 +++--- Shared/StorageKit/README.md | 15 +++--- .../StorageKit/Sources/StorageContainer.swift | 2 +- Shared/StorageKit/Sources/StorageMode.swift | 12 +++-- Shared/StorageKit/Sources/StorageSystem.swift | 7 ++- .../Tests/StorageContainerTests.swift | 52 +++++++++---------- .../StorageKit/Tests/StorageSystemTests.swift | 15 +++--- 7 files changed, 65 insertions(+), 55 deletions(-) diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md index 31dc3b50..671937ba 100644 --- a/Shared/StorageKit/AGENTS.md +++ b/Shared/StorageKit/AGENTS.md @@ -44,9 +44,11 @@ system, formatting, and global conventions. Read that first. App Group / security-scoped URL); `resolvedURL(using:)` is public so a `.custom` base can be built from a standard one. - [`StorageMode`](Sources/StorageMode.swift) / [`CloudKitOption`](Sources/CloudKitOption.swift) - / [`StorageError`](Sources/StorageError.swift) – the mode switch, the CloudKit - pass-through (`.none` / `.automatic`), and the typed error - (`containerDeleted`). + / [`StorageError`](Sources/StorageError.swift) – the mode switch + (`.persistent(base:)` / `.inMemory`; the base rides inside `.persistent` so + on-disk storage can't be asked for without saying where, and is consumed only at + the system root), the CloudKit pass-through (`.none` / `.automatic`), and the + typed error (`containerDeleted` / `modelStoreSchemaMismatch`). - [`KeyValueStore`](Sources/KeyValueStore.swift) – a minimal, `Sendable`-clean protocol (typed `bool/int/double/string/data`, no `Any?`) with two conformers: `UserDefaultsKeyValueStore` (`@unchecked Sendable` wrapper around a suite) and @@ -117,8 +119,9 @@ system, formatting, and global conventions. Read that first. - `FileManager` is injected into `StorageSystem` (mirroring `FileLocationOutbox.applicationSupport(fileManager:)`) for resolving the base and creating the namespace directory; containers use `FileManager.default` for - their own I/O. Test determinism comes from `base: .custom(tempDir)`, not from - threading a mock `FileManager` through every actor (which would fight Sendable). + their own I/O. Test determinism comes from `.persistent(base: .custom(tempDir))`, + not from threading a mock `FileManager` through every actor (which would fight + Sendable). - Concurrency: both types are `actor`s; cross-actor reads use `nonisolated let`. Teardown handlers are `@Sendable () async throws -> Void` (`TeardownHandler`). @@ -129,8 +132,8 @@ sources, hosted in `StuffTestHost`. Shared fixtures (the `Note` `@Model`, a `CallLog` actor, a `ThrowOnce` actor, `makeTemporaryDirectory()`) live in [`StorageKitTestSupport.swift`](Tests/StorageKitTestSupport.swift). Patterns: -- Always use `base: .custom(makeTemporaryDirectory())` (+ `defer` cleanup) so a - test never touches the host's real Application Support or Caches. +- Always use `.persistent(base: .custom(makeTemporaryDirectory()))` (+ `defer` + cleanup) so a test never touches the host's real Application Support or Caches. - Assert tree shape via the on-disk directories (`FileManager.fileExists`) and identity via `===` on the vended actors / objects. - Drive teardown ordering with a `CallLog` actor recording `phase:node` strings diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md index e4bc7bb7..c6ec129e 100644 --- a/Shared/StorageKit/README.md +++ b/Shared/StorageKit/README.md @@ -49,7 +49,7 @@ a target's dependencies in [`Package.swift`](../../Package.swift): import StorageKit // One system per app (or per test). Persistent in production… -let storage = try StorageSystem("Where", mode: .persistent) +let storage = try StorageSystem("Where", mode: .persistent(base: .applicationSupport())) // …or ephemeral in tests/previews — nothing below changes: // let storage = try StorageSystem("Where", mode: .inMemory) @@ -84,7 +84,7 @@ try await storage.deleteAll() ## Public API ```swift -public enum StorageMode { case persistent, inMemory } +public enum StorageMode { case persistent(base: BaseDirectory), inMemory } public struct BaseDirectory { public static func applicationSupport(subdirectory: String? = nil) -> BaseDirectory @@ -103,7 +103,6 @@ public struct StorageKey: Hashable, Sendable, ExpressibleByStringLiteral { public actor StorageSystem { public init(_ name: StorageKey, mode: StorageMode, - base: BaseDirectory = .applicationSupport(), fileManager: FileManager = .default) throws public nonisolated let mode: StorageMode public nonisolated let url: URL @@ -141,8 +140,9 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } ## How it works -- **Root vs. node.** `StorageSystem` is pure configuration (mode, base directory, - injected `FileManager`) plus a vending point; it does no storage work beyond +- **Root vs. node.** `StorageSystem` is pure configuration (the `mode` — which + carries the base directory in `.persistent` — plus an injected `FileManager`) + and a vending point; it does no storage work beyond creating its namespace directory and delegating to a hidden root container. All operations live on `StorageContainer`. The split costs one extra directory level on disk, which is invisible to users. @@ -198,8 +198,9 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } ## Testing -Exercised with Swift Testing in a hosted bundle. Tests use `base: .custom(temp)` -so they never touch the real Application Support directory, and cover the tree, +Exercised with Swift Testing in a hosted bundle. Tests use +`.persistent(base: .custom(temp))` so they never touch the real Application +Support directory, and cover the tree, both modes, cached/idempotent vends, key sanitization, key-value and SwiftData vends, and — in detail — the two teardown paths: children-first ordering, the `prepareForDeletion → onDeactivate → delete → afterDeletion` sequence, park-safe diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index 0c945c3f..8d8e64bd 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -505,7 +505,7 @@ public actor StorageContainer { } private func purgeKeyValueSuite() { - guard mode == .persistent else { return } + guard case .persistent = mode else { return } UserDefaults().removePersistentDomain(forName: suiteName) } diff --git a/Shared/StorageKit/Sources/StorageMode.swift b/Shared/StorageKit/Sources/StorageMode.swift index 4cfedfc4..626500c3 100644 --- a/Shared/StorageKit/Sources/StorageMode.swift +++ b/Shared/StorageKit/Sources/StorageMode.swift @@ -5,10 +5,16 @@ /// know it is running against ephemeral storage. The vending methods /// (`StorageContainer.keyValueStore()` / `modelContainer(for:)`) do "the right /// thing" for each mode automatically. +/// +/// `.persistent` carries its `base` so the two are inseparable — you can't ask for +/// on-disk storage without saying *where*, and `.inMemory` can't carry a base it +/// would ignore. The base is consumed once, at `StorageSystem` init, to resolve +/// the root directory; containers below the root only ever branch on +/// persistent-vs-in-memory. public enum StorageMode: Sendable { - /// Real files under a base directory; UserDefaults suites and SwiftData - /// stores persist across launches. - case persistent + /// Real files under `base`; UserDefaults suites and SwiftData stores persist + /// across launches. + case persistent(base: BaseDirectory) /// Ephemeral: files live in a temp directory removed by `deleteAll()`, /// key-value stores are in-memory, and SwiftData stores are diff --git a/Shared/StorageKit/Sources/StorageSystem.swift b/Shared/StorageKit/Sources/StorageSystem.swift index a1ef5de2..4b468add 100644 --- a/Shared/StorageKit/Sources/StorageSystem.swift +++ b/Shared/StorageKit/Sources/StorageSystem.swift @@ -19,22 +19,21 @@ public actor StorageSystem { /// Create a storage system named `name`. /// - /// - `.persistent`: rooted at `/`. + /// - `.persistent(base:)`: rooted at `/`. /// - `.inMemory`: rooted at a unique temporary directory removed by - /// `deleteAll()`; `base` is ignored. + /// `deleteAll()`. /// /// `fileManager` is injectable so tests can resolve a `.custom` base /// deterministically. public init( _ name: StorageKey, mode: StorageMode, - base: BaseDirectory = .applicationSupport(), fileManager: FileManager = .default, ) throws { self.mode = mode let rootURL: URL switch mode { - case .persistent: + case let .persistent(base): let baseURL = try base.resolvedURL(using: fileManager) rootURL = baseURL.appending(path: name.name, directoryHint: .isDirectory) case .inMemory: diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index 2451b5d4..fe817e0c 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -10,7 +10,7 @@ struct StorageContainerTests { func vendsAndCachesChildContainers() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let logs = try await user.container("logs") @@ -25,7 +25,7 @@ struct StorageContainerTests { func vendsANestedPath() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let leaf = try await user.container(path: ["logs", "today"]) @@ -41,7 +41,7 @@ struct StorageContainerTests { func fileURLPointsIntoTheContainerDirectory() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let fileURL = user.fileURL("note.txt") @@ -72,7 +72,7 @@ struct StorageContainerTests { func persistentKeyValueStoreRoundTripsAndIsPurgedOnDelete() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") await user.keyValueStore().set(99, forKey: "count") @@ -92,7 +92,7 @@ struct StorageContainerTests { func persistentModelContainerIsolatesItsStoreAndCaches() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let container = try await user.modelContainer(for: [Note.self]) @@ -133,7 +133,7 @@ struct StorageContainerTests { func modelContainerIsRecreatedAfterDeleteContents() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let first = try await user.modelContainer(for: [Note.self]) @@ -156,7 +156,7 @@ struct StorageContainerTests { func reVendingAStoreWithDifferentTypesThrows() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let first = try await user.modelContainer(for: [Note.self]) @@ -177,7 +177,7 @@ struct StorageContainerTests { func deactivateRunsHandlersChildrenFirstAndKeepsData() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let logs = try await user.container("logs") @@ -200,7 +200,7 @@ struct StorageContainerTests { func reVendingReactivatesAnInactiveContainer() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") _ = try await user.container("logs") @@ -216,7 +216,7 @@ struct StorageContainerTests { func deactivateDropsVendCachesAndReVendReopensPersistedData() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let firstModel = try await user.modelContainer(for: [Note.self]) @@ -245,7 +245,7 @@ struct StorageContainerTests { func deactivateIsANoOpWhenAlreadyInactive() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let log = CallLog() @@ -261,7 +261,7 @@ struct StorageContainerTests { func deregisterStopsAHandlerFromFiring() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let log = CallLog() @@ -276,7 +276,7 @@ struct StorageContainerTests { func deregisterIgnoresATokenFromAnotherContainer() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let a = try await system.container("a") let b = try await system.container("b") @@ -298,7 +298,7 @@ struct StorageContainerTests { func deleteContainerRunsPhasesInOrderChildrenFirst() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let logs = try await user.container("logs") @@ -326,7 +326,7 @@ struct StorageContainerTests { func deleteContainerLeavesSiblingsAndDeregisters() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user1 = try await system.container("user-1") let user2 = try await system.container("user-2") @@ -351,7 +351,7 @@ struct StorageContainerTests { func deletingAnAlreadyDeactivatedNodeRunsOnDeactivateOnce() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let logs = try await user.container("logs") @@ -373,7 +373,7 @@ struct StorageContainerTests { func vendingIsRejectedWhileTeardownIsInFlight() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let outcome = VendOutcome() @@ -402,7 +402,7 @@ struct StorageContainerTests { func throwingPrepareForDeletionParksWithNothingDeleted() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let throwOnce = ThrowOnce() @@ -419,7 +419,7 @@ struct StorageContainerTests { func throwingOnDeactivateParksWithNothingDeleted() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let throwOnce = ThrowOnce() @@ -436,7 +436,7 @@ struct StorageContainerTests { func parkedDeleteOfAnInactiveNodeRevertsToInactive() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") try await user.deactivate() @@ -456,7 +456,7 @@ struct StorageContainerTests { func throwingAfterDeletionIsPostCommitAndRetriesOnlyThatStep() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let throwOnce = ThrowOnce() @@ -475,7 +475,7 @@ struct StorageContainerTests { func reVendingAfterAFailedAfterDeletionGivesAFreshContainer() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let throwOnce = ThrowOnce() @@ -499,7 +499,7 @@ struct StorageContainerTests { func deleteContentsClearsDescendantsAndFilesButKeepsTheNode() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let logs = try await user.container("logs") @@ -521,7 +521,7 @@ struct StorageContainerTests { func deleteContentsRunsChildTeardownHooks() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let logs = try await user.container("logs") @@ -546,7 +546,7 @@ struct StorageContainerTests { func deletionErrorsSurfaceAndLeaveDataInPlace() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") let parent = system.url @@ -577,7 +577,7 @@ struct StorageContainerTests { func concurrentVendsDoNotResurrectADeletedContainer() async throws { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") diff --git a/Shared/StorageKit/Tests/StorageSystemTests.swift b/Shared/StorageKit/Tests/StorageSystemTests.swift index 7f776dfd..a12d1505 100644 --- a/Shared/StorageKit/Tests/StorageSystemTests.swift +++ b/Shared/StorageKit/Tests/StorageSystemTests.swift @@ -8,7 +8,7 @@ struct StorageSystemTests { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) #expect(system.url == temp.appending(path: "Where", directoryHint: .isDirectory)) #expect(FileManager.default.fileExists(atPath: system.url.path)) } @@ -18,7 +18,7 @@ struct StorageSystemTests { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let first = try await system.container("user-1") let second = try await system.container("user-1") #expect(first === second) @@ -41,7 +41,7 @@ struct StorageSystemTests { let temp = try makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: temp) } - let system = try StorageSystem("Where", mode: .persistent, base: .custom(temp)) + let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) _ = try await system.container("user-1") #expect(FileManager.default.fileExists(atPath: system.url.path)) @@ -58,8 +58,8 @@ struct StorageSystemTests { try? FileManager.default.removeItem(at: tempB) } - let systemA = try StorageSystem("Where", mode: .persistent, base: .custom(tempA)) - let systemB = try StorageSystem("Where", mode: .persistent, base: .custom(tempB)) + let systemA = try StorageSystem("Where", mode: .persistent(base: .custom(tempA))) + let systemB = try StorageSystem("Where", mode: .persistent(base: .custom(tempB))) let userA = try await systemA.container("user-1") let userB = try await systemB.container("user-1") @@ -80,8 +80,9 @@ struct StorageSystemTests { let system = try StorageSystem( "Where", - mode: .persistent, - base: .custom(temp.appending(path: "ns", directoryHint: .isDirectory)), + mode: .persistent( + base: .custom(temp.appending(path: "ns", directoryHint: .isDirectory)), + ), ) #expect(system.url.deletingLastPathComponent().lastPathComponent == "ns") #expect(FileManager.default.fileExists(atPath: system.url.path)) From 69540dc6ebe3d2039f4367da7c938115c7f5f5d6 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Tue, 30 Jun 2026 21:12:53 -0400 Subject: [PATCH 18/19] Document the key sanitizer and suite-name hash rationale Comment-only: explain that Foundation has no built-in safe-path-component API (so StorageKey sanitizes by hand), and why the root UserDefaults suite name hashes the base URL rather than embedding the raw path (a suite name is a plist filename, so the absolute path can't be used directly). Addresses PR review questions on StorageKey.sanitized and the suite-name hash. Co-authored-by: Cursor --- Shared/StorageKit/Sources/StorageKey.swift | 5 +++++ Shared/StorageKit/Sources/StorageSystem.swift | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/Shared/StorageKit/Sources/StorageKey.swift b/Shared/StorageKit/Sources/StorageKey.swift index 220211e8..7ba7af00 100644 --- a/Shared/StorageKit/Sources/StorageKey.swift +++ b/Shared/StorageKit/Sources/StorageKey.swift @@ -34,6 +34,11 @@ public struct StorageKey: Hashable, Sendable, ExpressibleByStringLiteral, Custom name } + // Foundation has no built-in "make a safe single path component" API — the + // near-misses all fall short: `addingPercentEncoding` is URL-oriented (it + // leaves `.`/`..` intact and `%`-mangles otherwise-fine names), + // `URL(fileURLWithPath:).lastPathComponent` doesn't neutralize traversal, and + // `FileManager` offers no name sanitizer. So we sanitize scalar-by-scalar here. private static func sanitized(_ raw: String) -> String { let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) var result = "" diff --git a/Shared/StorageKit/Sources/StorageSystem.swift b/Shared/StorageKit/Sources/StorageSystem.swift index 4b468add..1570ecf6 100644 --- a/Shared/StorageKit/Sources/StorageSystem.swift +++ b/Shared/StorageKit/Sources/StorageSystem.swift @@ -59,6 +59,13 @@ public actor StorageSystem { /// locations) get independent global suites instead of clobbering one shared /// `""` domain. Child suites append their keys to this. Unused in /// `.inMemory` mode, where key-value stores never touch `UserDefaults`. + /// + /// Why a *hash* of the URL rather than the URL string itself: a suite name + /// becomes a plist filename (`~/Library/Preferences/.plist`), so the + /// raw path can't be used — it contains `/`, is absolute, and would leak the + /// user's home directory into a global preferences filename. Sanitizing the + /// full path would sidestep the slashes but stays long and leaky; the hash is + /// a compact, stable disambiguator that keeps the readable `name` up front. private static func rootSuiteName(name: StorageKey, url: URL) -> String { "\(name.name).\(stableHash(url.path))" } From dba67ec016c69914ab38d582bd29e818bd8e6b92 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Tue, 30 Jun 2026 21:21:28 -0400 Subject: [PATCH 19/19] Split StorageContainer storage into typed namespaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the raw-file, key-value, and SwiftData vends off the StorageContainer base type onto small Sendable facade namespaces — `container.files.url(_:)`, `container.keyValue.store()`, `container.swiftData.modelContainer(for:)` — each in its own `StorageContainer+…` file with its actor-isolated backing. The core file now carries only lifecycle, the child registry, and teardown, and new storage concerns can be added the same way (an extension returning a facade). The facades forward into actor-isolated `make…` methods whose caches stay on the actor (an opaque type-keyed registry would fight actor isolation), so behavior and the cached/idempotent vends are unchanged. Addresses PR review: StorageContainer.swift was long and mixed many concerns. Co-authored-by: Cursor --- Shared/StorageKit/AGENTS.md | 52 +++-- Shared/StorageKit/README.md | 51 +++-- Shared/StorageKit/Sources/KeyValueStore.swift | 2 +- .../Sources/StorageContainer+Files.swift | 30 +++ .../Sources/StorageContainer+KeyValue.swift | 62 ++++++ .../Sources/StorageContainer+SwiftData.swift | 87 +++++++++ .../StorageKit/Sources/StorageContainer.swift | 179 +++++------------- Shared/StorageKit/Sources/StorageError.swift | 2 +- Shared/StorageKit/Sources/StorageMode.swift | 6 +- .../Tests/StorageContainerTests.swift | 42 ++-- .../Tests/StorageKitTestSupport.swift | 2 +- .../StorageKit/Tests/StorageSystemTests.swift | 6 +- 12 files changed, 328 insertions(+), 193 deletions(-) create mode 100644 Shared/StorageKit/Sources/StorageContainer+Files.swift create mode 100644 Shared/StorageKit/Sources/StorageContainer+KeyValue.swift create mode 100644 Shared/StorageKit/Sources/StorageContainer+SwiftData.swift diff --git a/Shared/StorageKit/AGENTS.md b/Shared/StorageKit/AGENTS.md index 671937ba..9351976d 100644 --- a/Shared/StorageKit/AGENTS.md +++ b/Shared/StorageKit/AGENTS.md @@ -33,9 +33,21 @@ system, formatting, and global conventions. Read that first. `deleteAll` to a private root `StorageContainer`. - [`StorageContainer`](Sources/StorageContainer.swift) – the **node**, and where everything happens: an `actor` holding a child registry, cached vends, and - teardown registrations. `key` / `url` / `mode` / `suiteName` are - `nonisolated let`. Children are `StorageContainer`s (recursive); the parent - back-reference is `weak` so a deleted subtree releases. + teardown registrations. Kept deliberately lean — lifecycle, children, teardown. + `key` / `url` / `mode` / `suiteName` are `nonisolated let`. Children are + `StorageContainer`s (recursive); the parent back-reference is `weak` so a + deleted subtree releases. +- **Storage namespaces** – each storage concern is a `public extension + StorageContainer` in its own file, exposing a `nonisolated` accessor that returns + a tiny `Sendable` facade: [`+Files`](Sources/StorageContainer+Files.swift) + (`container.files.url(_:)`), [`+KeyValue`](Sources/StorageContainer+KeyValue.swift) + (`container.keyValue.store()`), and + [`+SwiftData`](Sources/StorageContainer+SwiftData.swift) + (`container.swiftData.modelContainer(for:)`). Each facade holds only the + container and forwards into an actor-isolated `make…` method that lives in the + same file; the cache state it touches (`keyValueStoreCache` / + `modelContainerCache` / `CachedModelStore` / `activate` / + `requireLiveForKeyValueVend`) is `internal` on the actor for exactly this reason. - [`StorageKey`](Sources/StorageKey.swift) – a sanitized single path component (`Hashable`, `ExpressibleByStringLiteral`, `init(some RawRepresentable)`). - [`BaseDirectory`](Sources/BaseDirectory.swift) – where a persistent system @@ -58,17 +70,28 @@ system, formatting, and global conventions. Read that first. - **Root vs. node split.** Keep `StorageSystem` config-only; do not add storage operations there. New per-node behavior goes on `StorageContainer`. -- **Vends are cached and idempotent.** `container(_:)`, `keyValueStore()`, and - `modelContainer(for:)` return the same instance per node until the cache is - dropped by teardown. This is load-bearing for `ModelContainer` (never two on one - file) — don't make any vend construct a fresh instance each call. +- **Vends are cached and idempotent.** `container(_:)`, `keyValue.store()`, and + `swiftData.modelContainer(for:)` return the same instance per node until the + cache is dropped by teardown. This is load-bearing for `ModelContainer` (never + two on one file) — don't make any vend construct a fresh instance each call. The + namespace accessors (`files` / `keyValue` / `swiftData`) themselves are cheap + `nonisolated` factories — a fresh facade struct each call is fine; only the + vended *store* / *container* is cached, on the actor. +- **Namespaces are facades over actor-held state, not a dynamic registry.** The + "opaque per-container storage keyed by type" idea fights actor isolation (a + separate extension object can't share the actor's isolation domain to touch its + caches synchronously), so each built-in namespace is a `Sendable` facade whose + `make…` backing is an actor-isolated method and whose caches live on the actor. + Add a new namespace the same way: a `nonisolated var` returning a facade that + composes the public primitives (child containers, `files`, teardown hooks); only + namespaces that need private cache slots (like the built-ins) live in-module. - **In-memory does the right thing automatically.** `.inMemory` mode roots in a temp directory, vends `InMemoryKeyValueStore`, and builds the SwiftData store `isStoredInMemoryOnly` with **CloudKit forced off** and **no disk access** (it skips vending the store child, so no directory is created). Callers use identical code in both modes — keep that invariant when touching the vends. - **Each SwiftData store gets its own child directory (persistent only).** - `modelContainer(for:)` vends a dedicated child (`named`, default `"store"`) and + `swiftData.modelContainer(for:)` vends a dedicated child (`named`, default `"store"`) and places the store at `/.store`, so the db + `-wal`/`-shm` + external blobs are isolated and deletable as a unit. The store child shares the `container(_:)` key namespace, so the default `"store"` collides with a @@ -100,12 +123,13 @@ system, formatting, and global conventions. Read that first. the root rules. - **Errors surface.** Vending/teardown throw `StorageError` or rethrow the underlying `FileManager`/SwiftData error — never swallow into an empty default. - The one deliberate exception is the **non-throwing `keyValueStore()`**, which - *traps* (not throws) when called on a `deleting`/`deleted` node. This keeps - reads terse; it's a genuine programmer error (vending while/after a delete is a - lifecycle bug), so don't make callers `try`/`catch` it — preserve the - non-throwing signature and the trap, and route through the throwing - `container(_:)` / `modelContainer(for:)` when a recoverable failure is wanted. + The one deliberate exception is the **non-throwing `keyValue.store()`**, which + *traps* (not throws) when called on a `deleting`/`deleted` node (via the actor's + `requireLiveForKeyValueVend()`). This keeps reads terse; it's a genuine + programmer error (vending while/after a delete is a lifecycle bug), so don't make + callers `try`/`catch` it — preserve the non-throwing signature and the trap, and + route through the throwing `container(_:)` / `swiftData.modelContainer(for:)` + when a recoverable failure is wanted. - **Key-value suites live outside the directory.** Because a `UserDefaults` suite isn't under the container's directory, deletion must `removePersistentDomain` per node (it does, in the purge phase). If you change suite naming, keep it diff --git a/Shared/StorageKit/README.md b/Shared/StorageKit/README.md index c6ec129e..7d8ccd9c 100644 --- a/Shared/StorageKit/README.md +++ b/Shared/StorageKit/README.md @@ -17,14 +17,18 @@ want one). - **A tree of containers.** A `StorageSystem` is the root (configuration + a vending point); every `StorageContainer` below it owns one directory and can vend child containers recursively. The on-disk layout mirrors the tree. +- **Storage lives on typed namespaces.** The container base type stays small; + what you store hangs off `container.files` (raw file URLs), `container.keyValue` + (a key-value store), and `container.swiftData` (isolated `ModelContainer`s), so + each concern is separable and you can add your own namespace by extension. - **Mode-aware vends.** In `.persistent` mode you get real files, a `UserDefaults` suite, and an on-disk SwiftData store; flip to `.inMemory` and the same calls give you a temp directory, an in-memory key-value store, and an `isStoredInMemoryOnly` SwiftData store with CloudKit forced off — **app and model code never has to know**. -- **Isolated SwiftData stores.** `modelContainer(for:)` puts each store in its - own child directory, so the `.store` file, its `-wal` / `-shm` sidecars, and any - external-storage blobs stay together and delete together. +- **Isolated SwiftData stores.** `swiftData.modelContainer(for:)` puts each store + in its own child directory, so the `.store` file, its `-wal` / `-shm` sidecars, + and any external-storage blobs stay together and delete together. - **Idempotent, cached vends.** Asking for the same child container, key-value store, or model container twice returns the same instance (critical for `ModelContainer`, which must be unique per file). @@ -57,15 +61,15 @@ let user = try await storage.container("user-1") // a directory per user let logs = try await user.container("logs") // a subdirectory // Raw file -let url = logs.fileURL("today.json") +let url = logs.files.url("today.json") try Data(...).write(to: url) // Key-value (a UserDefaults suite, or in-memory) -let prefs = await user.keyValueStore() +let prefs = await user.keyValue.store() prefs.set(true, forKey: "onboarded") // SwiftData (its store isolated in its own child directory) -let container = try await user.modelContainer(for: [Note.self]) +let container = try await user.swiftData.modelContainer(for: [Note.self]) ``` ### Tearing down @@ -118,11 +122,11 @@ public actor StorageContainer { public func container(_ key: StorageKey) async throws -> StorageContainer public func container(path keys: [StorageKey]) async throws -> StorageContainer - public nonisolated func fileURL(_ name: String) -> URL - public func keyValueStore() async -> any KeyValueStore - public func modelContainer(for types: [any PersistentModel.Type], - named: StorageKey = "store", - cloudKit: CloudKitOption = .none) async throws -> ModelContainer + + // Storage namespaces (see below) + public nonisolated var files: FileStorage + public nonisolated var keyValue: KeyValueStorage + public nonisolated var swiftData: SwiftDataStorage @discardableResult public func onDeactivate(_ handler: @escaping TeardownHandler) async -> Token @discardableResult public func prepareForDeletion(_ handler: @escaping TeardownHandler) async -> Token @@ -134,6 +138,19 @@ public actor StorageContainer { public func deleteContents() async throws } +// Storage namespaces — tiny Sendable facades vended by the accessors above. +public struct FileStorage: Sendable { + public func url(_ name: String) -> URL +} +public struct KeyValueStorage: Sendable { + public func store() async -> any KeyValueStore +} +public struct SwiftDataStorage: Sendable { + public func modelContainer(for types: [any PersistentModel.Type], + named: StorageKey = "store", + cloudKit: CloudKitOption = .none) async throws -> ModelContainer +} + public protocol KeyValueStore: AnyObject, Sendable { /* typed bool/int/double/string/data */ } public final class InMemoryKeyValueStore: KeyValueStore { public init() } ``` @@ -172,7 +189,7 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } - **Errors surface.** Vending and teardown throw on real failures; nothing is swallowed into a benign default. Using a deleted container throws `StorageError.containerDeleted`. -- **`keyValueStore()` is intentionally non-throwing — don't `try`/`catch` it.** +- **`keyValue.store()` is intentionally non-throwing — don't `try`/`catch` it.** So reads stay terse (`store.bool(forKey:)`), it traps (rather than throws) when called on a container that's being deleted or already deleted. That's a *programmer error*, not a runtime condition to defend against on every access: @@ -180,11 +197,11 @@ public final class InMemoryKeyValueStore: KeyValueStore { public init() } down (e.g. in an `onDeactivate` handler). The only way to hit the trap is to vend while a concurrent delete is in flight — a lifecycle bug to fix at the call site. If you need a throwing failure mode, use `container(_:)` / - `modelContainer(for:)` instead. -- **`fileURL(_:)` is pure path construction.** It's `nonisolated`, so it doesn't - check the node's state or create the directory: on an `inactive` node the - directory is absent until the next vend, and on a `deleted` one it's gone, so - the URL points at a missing directory. Use it only while the container is live. + `swiftData.modelContainer(for:)` instead. +- **`files.url(_:)` is pure path construction.** It doesn't check the node's state + or create the directory: on an `inactive` node the directory is absent until the + next vend, and on a `deleted` one it's gone, so the URL points at a missing + directory. Use it only while the container is live. - **A model store is a child container named `named` (default `"store"`).** In `.persistent` mode it lives in the same key namespace as `container(_:)`, so don't also vend a plain child under that key — `container("store")` and the diff --git a/Shared/StorageKit/Sources/KeyValueStore.swift b/Shared/StorageKit/Sources/KeyValueStore.swift index bc5e9084..b97b3cf1 100644 --- a/Shared/StorageKit/Sources/KeyValueStore.swift +++ b/Shared/StorageKit/Sources/KeyValueStore.swift @@ -2,7 +2,7 @@ import Foundation /// A minimal, `Sendable`-clean key-value store: typed accessors only (no /// `Any?`), so a value can't silently fail to round-trip the way an arbitrary -/// `set(_: Any?)` can. `StorageContainer.keyValueStore()` vends a namespaced one +/// `set(_: Any?)` can. `StorageContainer.keyValue.store()` vends a namespaced one /// per node — a real `UserDefaults` suite in `.persistent` mode, an in-memory /// dictionary in `.inMemory` mode — behind this same protocol, so app code /// doesn't know or care which it got. diff --git a/Shared/StorageKit/Sources/StorageContainer+Files.swift b/Shared/StorageKit/Sources/StorageContainer+Files.swift new file mode 100644 index 00000000..2c48edf2 --- /dev/null +++ b/Shared/StorageKit/Sources/StorageContainer+Files.swift @@ -0,0 +1,30 @@ +import Foundation + +extension StorageContainer { + /// The raw-file namespace for this container: URLs for files kept directly in + /// the container's own directory. See ``FileStorage``. + public nonisolated var files: FileStorage { + FileStorage(container: self) + } +} + +/// The `files` namespace of a ``StorageContainer`` — a tiny `Sendable` facade for +/// raw files stored directly in the container's directory. Vend it with +/// `container.files`. +public struct FileStorage: Sendable { + let container: StorageContainer + + /// A URL for a raw file named `name` directly inside the container's directory. + /// The directory already exists for a live container; writing the file is the + /// caller's job. + /// + /// - Warning: This is pure path construction — it can't read the node's state, + /// so it neither checks liveness nor ensures the directory exists. On an + /// `inactive` node the directory is absent until the next vend reactivates + /// it, and on a `deleted` one it's gone for good; in both cases the returned + /// URL points into a missing directory and writes will fail. Only use the URL + /// while the container is live. + public func url(_ name: String) -> URL { + container.url.appending(path: name, directoryHint: .notDirectory) + } +} diff --git a/Shared/StorageKit/Sources/StorageContainer+KeyValue.swift b/Shared/StorageKit/Sources/StorageContainer+KeyValue.swift new file mode 100644 index 00000000..3f371636 --- /dev/null +++ b/Shared/StorageKit/Sources/StorageContainer+KeyValue.swift @@ -0,0 +1,62 @@ +import Foundation + +extension StorageContainer { + /// The key-value namespace for this container: a namespaced `UserDefaults` + /// suite (`.persistent`) or an in-memory store (`.inMemory`). See + /// ``KeyValueStorage``. + public nonisolated var keyValue: KeyValueStorage { + KeyValueStorage(container: self) + } +} + +/// The `keyValue` namespace of a ``StorageContainer`` — a tiny `Sendable` facade +/// vending the node's namespaced key-value store. Vend it with +/// `container.keyValue`. +public struct KeyValueStorage: Sendable { + let container: StorageContainer + + /// Vend this node's namespaced key-value store: a `UserDefaults` suite in + /// `.persistent` mode, an in-memory store in `.inMemory` mode. Cached per node + /// (repeated calls return the same instance) and dropped by `deactivate()` / + /// `deleteContainer()`. + /// + /// - Important: This is deliberately **non-throwing** so the common case reads + /// like `store.bool(forKey:)` without a `try`. The price is that calling it + /// on a container that is being deleted or is already deleted is a + /// **programmer error and traps** — don't wrap every access in + /// `try`/`catch`. Reach for the store through a live handle and stop using it + /// once you've torn its container down (e.g. release it in an `onDeactivate` + /// handler); a vend racing a concurrent delete is the one case where the + /// trap can fire, and that's a lifecycle bug to fix at the call site, not to + /// defend against on each call. Use `container(_:)` / + /// `swiftData.modelContainer(for:)` instead if you need a throwing failure + /// mode. + public func store() async -> any KeyValueStore { + await container.makeKeyValueStore() + } +} + +extension StorageContainer { + /// Actor-isolated backing for `keyValue.store()`. Lives here beside its facade + /// so the key-value concern stays out of the core `StorageContainer` file. + func makeKeyValueStore() -> any KeyValueStore { + requireLiveForKeyValueVend() + if let cached = keyValueStoreCache { + return cached + } + let store: any KeyValueStore + switch mode { + case .persistent: + guard let suite = UserDefaults(suiteName: suiteName) else { + preconditionFailure( + "StorageKit: invalid UserDefaults suite name \"\(suiteName)\"", + ) + } + store = UserDefaultsKeyValueStore(suite) + case .inMemory: + store = InMemoryKeyValueStore() + } + keyValueStoreCache = store + return store + } +} diff --git a/Shared/StorageKit/Sources/StorageContainer+SwiftData.swift b/Shared/StorageKit/Sources/StorageContainer+SwiftData.swift new file mode 100644 index 00000000..2eabc529 --- /dev/null +++ b/Shared/StorageKit/Sources/StorageContainer+SwiftData.swift @@ -0,0 +1,87 @@ +import Foundation +import SwiftData + +extension StorageContainer { + /// The SwiftData namespace for this container: isolated `ModelContainer`s whose + /// stores live in dedicated child directories. See ``SwiftDataStorage``. + public nonisolated var swiftData: SwiftDataStorage { + SwiftDataStorage(container: self) + } +} + +/// The `swiftData` namespace of a ``StorageContainer`` — a tiny `Sendable` facade +/// vending isolated SwiftData `ModelContainer`s. Vend it with `container.swiftData`. +public struct SwiftDataStorage: Sendable { + let container: StorageContainer + + /// Vend a SwiftData `ModelContainer` whose store lives in a dedicated child + /// container, so the `.store` file and its `-wal` / `-shm` sidecars and any + /// external-storage blobs are isolated in one directory (deleting that child — + /// or this container — deletes exactly that store's files). Cached per `named` + /// key. In `.inMemory` mode the store is `isStoredInMemoryOnly` and CloudKit + /// is forced off. + /// + /// A store name identifies exactly one schema: calling this again under the + /// same `named` key with a different set of `types` throws + /// `StorageError.modelStoreSchemaMismatch` rather than handing back the first + /// container with a mismatched schema. Use distinct names for distinct stores. + /// + /// - Important: In `.persistent` mode the store occupies a child container + /// named `named` (default `"store"`), in the **same** key namespace as + /// `container(_:)`. Don't also vend a plain child under that key — e.g. + /// `container("store")` and the default model store would share one + /// directory and stomp each other. Pass a distinct `named:` (or avoid that + /// key for your own children) to keep them apart. + public func modelContainer( + for types: [any PersistentModel.Type], + named name: StorageKey = "store", + cloudKit: CloudKitOption = .none, + ) async throws -> ModelContainer { + try await container.makeModelContainer(for: types, named: name, cloudKit: cloudKit) + } +} + +extension StorageContainer { + /// Actor-isolated backing for `swiftData.modelContainer(for:)`. Lives here + /// beside its facade so the SwiftData concern stays out of the core + /// `StorageContainer` file. + func makeModelContainer( + for types: [any PersistentModel.Type], + named name: StorageKey, + cloudKit: CloudKitOption, + ) throws -> ModelContainer { + try activate() + let typeIDs = Set(types.map { ObjectIdentifier($0) }) + if let cached = modelContainerCache[name] { + guard cached.typeIDs == typeIDs else { + throw StorageError.modelStoreSchemaMismatch(name) + } + return cached.container + } + let schema = Schema(types) + let configuration: ModelConfiguration + switch mode { + case .inMemory: + // No backing files, so don't vend a store child / touch disk. + configuration = ModelConfiguration( + schema: schema, + isStoredInMemoryOnly: true, + cloudKitDatabase: .none, + ) + case .persistent: + let storeContainer = try container(name) + let storeURL = storeContainer.url.appending( + path: "\(name.name).store", + directoryHint: .notDirectory, + ) + configuration = ModelConfiguration( + schema: schema, + url: storeURL, + cloudKitDatabase: cloudKit.cloudKitDatabase, + ) + } + let modelContainer = try ModelContainer(for: schema, configurations: [configuration]) + modelContainerCache[name] = CachedModelStore(container: modelContainer, typeIDs: typeIDs) + return modelContainer + } +} diff --git a/Shared/StorageKit/Sources/StorageContainer.swift b/Shared/StorageKit/Sources/StorageContainer.swift index 8d8e64bd..29b77866 100644 --- a/Shared/StorageKit/Sources/StorageContainer.swift +++ b/Shared/StorageKit/Sources/StorageContainer.swift @@ -7,10 +7,19 @@ import SwiftData public typealias TeardownHandler = @Sendable () async throws -> Void /// A node in a `StorageSystem`'s tree, and the place where storage actually -/// happens. Each container owns one directory and can vend child containers -/// (subdirectories), raw file URLs, a namespaced key-value store, and a SwiftData -/// `ModelContainer`. Children are `StorageContainer`s too, so the tree is -/// recursive below the root. +/// happens. Each container owns one directory and vends child containers +/// (subdirectories) directly; the things you store inside it hang off typed +/// namespaces — ``files`` (raw file URLs), ``keyValue`` (a namespaced key-value +/// store), and ``swiftData`` (an isolated SwiftData `ModelContainer`). Children +/// are `StorageContainer`s too, so the tree is recursive below the root. +/// +/// The base type stays deliberately small — lifecycle, the child registry, and +/// teardown — and each storage concern lives in its own namespace file +/// (`StorageContainer+Files`, `+KeyValue`, `+SwiftData`) that vends through a tiny +/// `Sendable` facade (`container.swiftData.modelContainer(for:)`). Add your own +/// namespace the same way: extend `StorageContainer` with a `nonisolated` accessor +/// returning a facade that composes the public primitives (child containers, +/// `files`, the teardown hooks). /// /// An `actor`: it guards a mutable child registry and teardown registrations, and /// serializes the file I/O behind vending and deletion. `key` / `url` / `mode` @@ -51,7 +60,7 @@ public actor StorageContainer { /// `deleted`. case deleting(wasActive: Bool) /// `deleteContainer()`d: the directory is gone and the node is detached. - /// Any further vend throws (or, for the non-throwing `keyValueStore()`, + /// Any further vend throws (or, for the non-throwing `keyValue.store()`, /// traps as the programmer error it is). case deleted } @@ -78,7 +87,9 @@ public actor StorageContainer { /// A cached `ModelContainer` plus the identity of the model types it was built /// from, so a re-vend under the same store name with a different type set is /// caught instead of silently returning a container with the wrong schema. - private struct CachedModelStore { + /// Internal (not `private`) so the `swiftData` namespace extension in a sibling + /// file can build and read it. + struct CachedModelStore { let container: ModelContainer let typeIDs: Set } @@ -103,8 +114,12 @@ public actor StorageContainer { private(set) var state: State = .active private var children: [StorageKey: StorageContainer] = [:] - private var keyValueStoreCache: (any KeyValueStore)? - private var modelContainerCache: [StorageKey: CachedModelStore] = [:] + /// Cached vends. Internal (not `private`) so the `keyValue` / `swiftData` + /// namespace extensions in sibling files can read and refresh them; only this + /// type touches them, always under the actor. Dropped by teardown via + /// `dropCachedVends()`. + var keyValueStoreCache: (any KeyValueStore)? + var modelContainerCache: [StorageKey: CachedModelStore] = [:] private var nextTokenID: UInt64 = 0 private var onDeactivateHandlers: [UInt64: TeardownHandler] = [:] @@ -158,129 +173,6 @@ public actor StorageContainer { return node } - // MARK: - Files - - /// A URL for a raw file named `name` directly inside this container's - /// directory. The directory already exists for a live container; writing the - /// file is the caller's job. - /// - /// - Warning: This is `nonisolated` pure path construction — it can't read the - /// node's state, so it neither checks liveness nor ensures the directory - /// exists. On an `inactive` node the directory is absent until the next vend - /// reactivates it, and on a `deleted` one it's gone for good; in both cases - /// the returned URL points into a missing directory and writes will fail. - /// Only use the URL while the container is live. - public nonisolated func fileURL(_ name: String) -> URL { - url.appending(path: name, directoryHint: .notDirectory) - } - - // MARK: - Key-value store - - /// Vend this node's namespaced key-value store: a `UserDefaults` suite in - /// `.persistent` mode, an in-memory store in `.inMemory` mode. Cached per node - /// (repeated calls return the same instance) and dropped by `deactivate()` / - /// `deleteContainer()`. - /// - /// - Important: This is deliberately **non-throwing** so the common case reads - /// like `store.bool(forKey:)` without a `try`. The price is that calling it - /// on a container that is being deleted or is already deleted is a - /// **programmer error and traps** — don't wrap every access in - /// `try`/`catch`. Reach for the store through a live handle and stop using it - /// once you've torn its container down (e.g. release it in an `onDeactivate` - /// handler); a vend racing a concurrent delete is the one case where the - /// trap can fire, and that's a lifecycle bug to fix at the call site, not to - /// defend against on each call. Use `container(_:)` / `modelContainer(for:)` - /// instead if you need a throwing failure mode. - public func keyValueStore() -> any KeyValueStore { - switch state { - case .active: - break - case .inactive: - state = .active - case .deleting, .deleted: - preconditionFailure( - "StorageKit: keyValueStore() on a deleted container \"\(key)\"", - ) - } - if let cached = keyValueStoreCache { - return cached - } - let store: any KeyValueStore - switch mode { - case .persistent: - guard let suite = UserDefaults(suiteName: suiteName) else { - preconditionFailure( - "StorageKit: invalid UserDefaults suite name \"\(suiteName)\"", - ) - } - store = UserDefaultsKeyValueStore(suite) - case .inMemory: - store = InMemoryKeyValueStore() - } - keyValueStoreCache = store - return store - } - - // MARK: - SwiftData - - /// Vend a SwiftData `ModelContainer` whose store lives in a dedicated child - /// container, so the `.store` file and its `-wal` / `-shm` sidecars and any - /// external-storage blobs are isolated in one directory (deleting that child — - /// or this container — deletes exactly that store's files). Cached per `named` - /// key. In `.inMemory` mode the store is `isStoredInMemoryOnly` and CloudKit - /// is forced off. - /// - /// A store name identifies exactly one schema: calling this again under the - /// same `named` key with a different set of `types` throws - /// `StorageError.modelStoreSchemaMismatch` rather than handing back the first - /// container with a mismatched schema. Use distinct names for distinct stores. - /// - /// - Important: In `.persistent` mode the store occupies a child container - /// named `named` (default `"store"`), in the **same** key namespace as - /// `container(_:)`. Don't also vend a plain child under that key — e.g. - /// `container("store")` and the default model store would share one - /// directory and stomp each other. Pass a distinct `named:` (or avoid that - /// key for your own children) to keep them apart. - public func modelContainer( - for types: [any PersistentModel.Type], - named name: StorageKey = "store", - cloudKit: CloudKitOption = .none, - ) throws -> ModelContainer { - try activate() - let typeIDs = Set(types.map { ObjectIdentifier($0) }) - if let cached = modelContainerCache[name] { - guard cached.typeIDs == typeIDs else { - throw StorageError.modelStoreSchemaMismatch(name) - } - return cached.container - } - let schema = Schema(types) - let configuration: ModelConfiguration - switch mode { - case .inMemory: - // No backing files, so don't vend a store child / touch disk. - configuration = ModelConfiguration( - schema: schema, - isStoredInMemoryOnly: true, - cloudKitDatabase: .none, - ) - case .persistent: - let storeContainer = try container(name) - let storeURL = storeContainer.url.appending( - path: "\(name.name).store", - directoryHint: .notDirectory, - ) - configuration = ModelConfiguration( - schema: schema, - url: storeURL, - cloudKitDatabase: cloudKit.cloudKitDatabase, - ) - } - let modelContainer = try ModelContainer(for: schema, configurations: [configuration]) - modelContainerCache[name] = CachedModelStore(container: modelContainer, typeIDs: typeIDs) - return modelContainer - } - // MARK: - Teardown registration /// Register a handler run on **both** teardown paths: "stop using your handle @@ -404,7 +296,11 @@ public actor StorageContainer { return nextTokenID } - private func activate() throws { + /// Ensure the node is live before a throwing vend, recreating the directory for + /// an `inactive` node. Internal so the `swiftData` namespace extension can + /// gate its vend on it. Throws `StorageError.containerDeleted` on a torn-down + /// node. + func activate() throws { switch state { case .active: break @@ -419,6 +315,25 @@ public actor StorageContainer { } } + /// Ensure the node is live enough to vend its **non-throwing** key-value store, + /// reactivating an `inactive` node in place (the suite needs no directory, so + /// this doesn't recreate one). Traps on a `deleting`/`deleted` node — see + /// `KeyValueStorage.store()` for why that's a programmer error, not a throw. + /// Internal so the `keyValue` namespace extension can gate its vend on it while + /// the `state` setter stays private to this file. + func requireLiveForKeyValueVend() { + switch state { + case .active: + break + case .inactive: + state = .active + case .deleting, .deleted: + preconditionFailure( + "StorageKit: keyValue.store() on a deleted container \"\(key)\"", + ) + } + } + private func dropCachedVends() { keyValueStoreCache = nil modelContainerCache.removeAll() diff --git a/Shared/StorageKit/Sources/StorageError.swift b/Shared/StorageKit/Sources/StorageError.swift index f770395b..cd6ed4ec 100644 --- a/Shared/StorageKit/Sources/StorageError.swift +++ b/Shared/StorageKit/Sources/StorageError.swift @@ -5,7 +5,7 @@ public enum StorageError: Error, Sendable, Equatable { /// vend a fresh one from a live `StorageSystem`. case containerDeleted(StorageKey) - /// `modelContainer(for:named:)` was called again under the same `named` store + /// `swiftData.modelContainer(for:named:)` was called again under the same `named` store /// (the associated key) but with a different set of model types than the /// already-open store was built with. A store has exactly one schema — vend /// distinct schemas under distinct names rather than silently reusing the diff --git a/Shared/StorageKit/Sources/StorageMode.swift b/Shared/StorageKit/Sources/StorageMode.swift index 626500c3..d8a598a7 100644 --- a/Shared/StorageKit/Sources/StorageMode.swift +++ b/Shared/StorageKit/Sources/StorageMode.swift @@ -2,9 +2,9 @@ /// /// The mode is set once on the `StorageSystem` and inherited by every container, /// so test and preview code can flip a single switch and nothing below has to -/// know it is running against ephemeral storage. The vending methods -/// (`StorageContainer.keyValueStore()` / `modelContainer(for:)`) do "the right -/// thing" for each mode automatically. +/// know it is running against ephemeral storage. The vending namespaces +/// (`StorageContainer.keyValue.store()` / `.swiftData.modelContainer(for:)`) do +/// "the right thing" for each mode automatically. /// /// `.persistent` carries its `base` so the two are inseparable — you can't ask for /// on-disk storage without saying *where*, and `.inMemory` can't carry a base it diff --git a/Shared/StorageKit/Tests/StorageContainerTests.swift b/Shared/StorageKit/Tests/StorageContainerTests.swift index fe817e0c..41ca10a3 100644 --- a/Shared/StorageKit/Tests/StorageContainerTests.swift +++ b/Shared/StorageKit/Tests/StorageContainerTests.swift @@ -44,7 +44,7 @@ struct StorageContainerTests { let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") - let fileURL = user.fileURL("note.txt") + let fileURL = user.files.url("note.txt") try Data("hello".utf8).write(to: fileURL) #expect(fileURL == user.url.appending(path: "note.txt", directoryHint: .notDirectory)) @@ -59,13 +59,13 @@ struct StorageContainerTests { let a = try await system.container("a") let b = try await system.container("b") - let aStore = await a.keyValueStore() - let bStore = await b.keyValueStore() + let aStore = await a.keyValue.store() + let bStore = await b.keyValue.store() aStore.set(true, forKey: "flag") #expect(aStore.bool(forKey: "flag")) #expect(!bStore.bool(forKey: "flag")) - #expect(await a.keyValueStore() === aStore) + #expect(await a.keyValue.store() === aStore) } @Test @@ -75,13 +75,13 @@ struct StorageContainerTests { let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") - await user.keyValueStore().set(99, forKey: "count") - #expect(await user.keyValueStore().integer(forKey: "count") == 99) + await user.keyValue.store().set(99, forKey: "count") + #expect(await user.keyValue.store().integer(forKey: "count") == 99) try await user.deleteContainer() let revived = try await system.container("user-1") - #expect(await revived.keyValueStore().integer(forKey: "count") == 0) + #expect(await revived.keyValue.store().integer(forKey: "count") == 0) try await system.deleteAll() } @@ -95,8 +95,8 @@ struct StorageContainerTests { let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") - let container = try await user.modelContainer(for: [Note.self]) - let again = try await user.modelContainer(for: [Note.self]) + let container = try await user.swiftData.modelContainer(for: [Note.self]) + let again = try await user.swiftData.modelContainer(for: [Note.self]) #expect(container === again) let context = ModelContext(container) @@ -113,7 +113,7 @@ struct StorageContainerTests { func inMemoryModelContainerWritesNoStoreFile() async throws { let system = try StorageSystem("Where", mode: .inMemory) let user = try await system.container("user-1") - let container = try await user.modelContainer(for: [Note.self]) + let container = try await user.swiftData.modelContainer(for: [Note.self]) let context = ModelContext(container) context.insert(Note(text: "hi")) @@ -136,7 +136,7 @@ struct StorageContainerTests { let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") - let first = try await user.modelContainer(for: [Note.self]) + let first = try await user.swiftData.modelContainer(for: [Note.self]) let writeContext = ModelContext(first) writeContext.insert(Note(text: "hi")) try writeContext.save() @@ -146,7 +146,7 @@ struct StorageContainerTests { // The store's files were deleted; re-vending must rebuild a fresh container // rather than hand back the stale cached one pointing at deleted files. - let second = try await user.modelContainer(for: [Note.self]) + let second = try await user.swiftData.modelContainer(for: [Note.self]) #expect(second !== first) let readContext = ModelContext(second) #expect(try readContext.fetchCount(FetchDescriptor()) == 0) @@ -159,15 +159,15 @@ struct StorageContainerTests { let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") - let first = try await user.modelContainer(for: [Note.self]) + let first = try await user.swiftData.modelContainer(for: [Note.self]) // Same name, different schema → caught, not silently the wrong container. await #expect(throws: StorageError.modelStoreSchemaMismatch("store")) { - _ = try await user.modelContainer(for: [Note.self, Tag.self]) + _ = try await user.swiftData.modelContainer(for: [Note.self, Tag.self]) } // The same type set still returns the cached container. - let again = try await user.modelContainer(for: [Note.self]) + let again = try await user.swiftData.modelContainer(for: [Note.self]) #expect(again === first) } @@ -181,7 +181,7 @@ struct StorageContainerTests { let user = try await system.container("user-1") let logs = try await user.container("logs") - let file = logs.fileURL("a.txt") + let file = logs.files.url("a.txt") try Data("x".utf8).write(to: file) let log = CallLog() @@ -219,24 +219,24 @@ struct StorageContainerTests { let system = try StorageSystem("Where", mode: .persistent(base: .custom(temp))) let user = try await system.container("user-1") - let firstModel = try await user.modelContainer(for: [Note.self]) + let firstModel = try await user.swiftData.modelContainer(for: [Note.self]) let writeContext = ModelContext(firstModel) writeContext.insert(Note(text: "hi")) try writeContext.save() - await user.keyValueStore().set(7, forKey: "count") + await user.keyValue.store().set(7, forKey: "count") try await user.deactivate() #expect(await user.state == .inactive) // Reactivation re-vends a *fresh* ModelContainer (cache was dropped) over // the surviving on-disk store. - let secondModel = try await user.modelContainer(for: [Note.self]) + let secondModel = try await user.swiftData.modelContainer(for: [Note.self]) #expect(secondModel !== firstModel) let readContext = ModelContext(secondModel) #expect(try readContext.fetchCount(FetchDescriptor()) == 1) // The KV cache was dropped too, but the persisted suite round-trips. - #expect(await user.keyValueStore().integer(forKey: "count") == 7) + #expect(await user.keyValue.store().integer(forKey: "count") == 7) try await system.deleteAll() } @@ -503,7 +503,7 @@ struct StorageContainerTests { let user = try await system.container("user-1") let logs = try await user.container("logs") - let loose = user.fileURL("note.txt") + let loose = user.files.url("note.txt") try Data("x".utf8).write(to: loose) try await user.deleteContents() diff --git a/Shared/StorageKit/Tests/StorageKitTestSupport.swift b/Shared/StorageKit/Tests/StorageKitTestSupport.swift index 81fa709e..0c05d12a 100644 --- a/Shared/StorageKit/Tests/StorageKitTestSupport.swift +++ b/Shared/StorageKit/Tests/StorageKitTestSupport.swift @@ -6,7 +6,7 @@ enum StorageTestError: Error, Equatable { case injected } -/// A `@Model` fixture for `modelContainer(for:)` tests. +/// A `@Model` fixture for `swiftData.modelContainer(for:)` tests. @Model final class Note { var text: String diff --git a/Shared/StorageKit/Tests/StorageSystemTests.swift b/Shared/StorageKit/Tests/StorageSystemTests.swift index a12d1505..bb6dc182 100644 --- a/Shared/StorageKit/Tests/StorageSystemTests.swift +++ b/Shared/StorageKit/Tests/StorageSystemTests.swift @@ -63,11 +63,11 @@ struct StorageSystemTests { let userA = try await systemA.container("user-1") let userB = try await systemB.container("user-1") - await userA.keyValueStore().set(99, forKey: "count") + await userA.keyValue.store().set(99, forKey: "count") // Same logical key path, different base: the suites must not collide. - #expect(await userA.keyValueStore().integer(forKey: "count") == 99) - #expect(await userB.keyValueStore().integer(forKey: "count") == 0) + #expect(await userA.keyValue.store().integer(forKey: "count") == 99) + #expect(await userB.keyValue.store().integer(forKey: "count") == 0) try await systemA.deleteAll() try await systemB.deleteAll()