Clean WordPress API cache during account and site removal

Modules/Sources/WordPressCore/ApiCache.swift

@@ -5,63 +5,185 @@ import WordPressApiCache
 import WordPressShared
 
 extension WordPressApiCache {
-    static func bootstrap() -> WordPressApiCache {
-        let instance: WordPressApiCache = .onDiskCache() ?? .memoryCache()
-        instance.startListeningForUpdates()
-        return instance
+    /// The one cache instance shared by every `WordPressClient`.
+    ///
+    /// A single SQLite database backs the cache for the whole app, so every
+    /// `WordPressApiCache` instance opens and migrates the same file.
+    /// Bootstrapping more than one concurrently can fail and destroy a
+    /// database still in use by another; sharing one instance keeps that to a
+    /// single bootstrap. Swift's once-only static initialization guarantees
+    /// it happens once per process.
+    public static let shared = WordPressApiCache.bootstrap()
+
+    /// An isolated in-memory cache for unit tests.
+    ///
+    /// Prefer this over `shared`, which is a process-wide singleton backed by
+    /// an on-disk database and reused across the whole test run, so writing
+    /// through it leaks state between tests.
+    static func forTesting() throws -> WordPressApiCache {
+        let cache = try WordPressApiCache()
+        _ = try cache.performMigrations()
+        return cache
+    }
+
+    /// A failure encountered while opening the on-disk cache. It carries the
+    /// point of failure and the underlying error so the two can be reported
+    /// together for Sentry classification.
+    struct OnDiskCacheFailure: Error {
+        enum Kind: Equatable {
+            case couldNotOpenDatabase
+            case migrationFailed
+            case couldNotRemoveOrphanedSiblings
+            case couldNotDeleteDatabase
+            case couldNotRemoveSiblings
+        }
+
+        let kind: Kind
+        let underlyingError: Error
+    }
+
+    /// The result of opening the on-disk cache.
+    enum OnDiskCacheOutcome {
+        /// Opened an existing or freshly created cache with nothing to report.
+        case opened(WordPressApiCache)
+        /// Opened only after recovering from a broken database. The failure
+        /// that triggered recovery is surfaced for Sentry classification.
+        case recovered(WordPressApiCache, from: OnDiskCacheFailure)
+        /// Could not open an on-disk cache; the caller falls back to memory.
+        case failed(OnDiskCacheFailure)
     }
 
-    private static func onDiskCache() -> WordPressApiCache? {
+    private static func bootstrap() -> WordPressApiCache {
         let cacheURL = URL.libraryDirectory.appending(path: "app.sqlite")
 
-        if let cache = WordPressApiCache.onDiskCache(file: cacheURL) {
-            return cache
+        let cache: WordPressApiCache
+        switch onDiskCache(at: cacheURL) {
+        case .opened(let opened):
+            cache = opened
+        case .recovered(let recovered, let failure):
+            report(failure)
+            cache = recovered
+        case .failed(let failure):
+            report(failure)
+            cache = memoryCache()
         }
 
-        if FileManager.default.fileExists(at: cacheURL) {
-            do {
-                try FileManager.default.removeItem(at: cacheURL)
+        cache.startListeningForUpdates()
+        return cache
+    }
+
+    private static func report(_ failure: OnDiskCacheFailure) {
+        // Report each kind from its own call site. `wpAssertionFailure` derives
+        // its analytics identity and 7-day suppression key from #file/#line
+        // (see AssertionLogger), so a shared call site would let one kind's
+        // report suppress the others and collapse their Sentry grouping.
+        let userInfo = ["error": "\(failure.underlyingError)"]
+        switch failure.kind {
+        case .couldNotOpenDatabase:
+            wpAssertionFailure("Failed to create an instance", userInfo: userInfo)
+        case .migrationFailed:
+            wpAssertionFailure("Failed to migrate database", userInfo: userInfo)
+        case .couldNotRemoveOrphanedSiblings:
+            wpAssertionFailure("Failed to remove orphaned sqlite sibling files", userInfo: userInfo)
+        case .couldNotDeleteDatabase:
+            wpAssertionFailure("Failed to delete sqlite database", userInfo: userInfo)
+        case .couldNotRemoveSiblings:
+            wpAssertionFailure("Failed to remove sqlite sibling files", userInfo: userInfo)
+        }
+    }
 
-                if let cache = WordPressApiCache.onDiskCache(file: cacheURL) {
-                    return cache
-                }
+    /// Opens (or creates) the on-disk cache at the given URL, recovering from a
+    /// broken database by deleting it and starting over. Internal so tests can
+    /// exercise the recovery path against a temporary location.
+    static func onDiskCache(at cacheURL: URL) -> OnDiskCacheOutcome {
+        let fileManager = FileManager.default
+
+        // A previous app version may have deleted the database file without its
+        // sibling files. An orphaned journal left next to a freshly created
+        // database is a documented SQLite corruption vector, so remove any
+        // leftovers first and refuse to continue if that cleanup fails.
+        if !fileManager.fileExists(at: cacheURL) {
+            do {
+                try removeSiblingFiles(of: cacheURL)
             } catch {
-                wpAssertionFailure("Failed to delete sqlite database")
+                return .failed(OnDiskCacheFailure(kind: .couldNotRemoveOrphanedSiblings, underlyingError: error))
             }
         }
 
-        return nil
+        switch openCache(file: cacheURL) {
+        case .success(let cache):
+            return .opened(cache)
+        case .failure(let failure):
+            // Only an existing database can be recovered by recreating it.
+            guard fileManager.fileExists(at: cacheURL) else {
+                return .failed(failure)
+            }
+
+            do {
+                try fileManager.removeItem(at: cacheURL)
+            } catch {
+                return .failed(OnDiskCacheFailure(kind: .couldNotDeleteDatabase, underlyingError: error))
+            }
+
+            do {
+                // Deleting the database but not its journal is a documented
+                // SQLite corruption vector; always remove them together.
+                try removeSiblingFiles(of: cacheURL)
+            } catch {
+                return .failed(OnDiskCacheFailure(kind: .couldNotRemoveSiblings, underlyingError: error))
+            }
+
+            switch openCache(file: cacheURL) {
+            case .success(let cache):
+                return .recovered(cache, from: failure)
+            case .failure(let retryFailure):
+                return .failed(retryFailure)
+            }
+        }
     }
 
-    private static func onDiskCache(file: URL) -> WordPressApiCache? {
+    private static func openCache(file: URL) -> Result<WordPressApiCache, OnDiskCacheFailure> {
         let cache: WordPressApiCache
         do {
             cache = try WordPressApiCache(url: file)
         } catch {
-            wpAssertionFailure("Failed to create an instance")
-            return nil
+            return .failure(OnDiskCacheFailure(kind: .couldNotOpenDatabase, underlyingError: error))
         }
 
         do {
             _ = try cache.performMigrations()
         } catch {
-            wpAssertionFailure("Failed to migrate database")
-            return nil
+            return .failure(OnDiskCacheFailure(kind: .migrationFailed, underlyingError: error))
         }
 
-        do {
-            var url = file
-            var values = URLResourceValues()
-            values.isExcludedFromBackup = true
-            try url.setResourceValues(values)
-        } catch {
-            wpAssertionFailure("Failed exclude the database file from iCloud backup")
-        }
+        // Best-effort: keep the database out of iCloud backups. This can only
+        // fail in exotic filesystem states, and the cache is usable regardless,
+        // so it is not treated as a bootstrap failure.
+        var url = file
+        var values = URLResourceValues()
+        values.isExcludedFromBackup = true
+        try? url.setResourceValues(values)
 
-        return cache
+        return .success(cache)
+    }
+
+    private static func removeSiblingFiles(of cacheURL: URL) throws {
+        let fileManager = FileManager.default
+        for suffix in ["-journal", "-wal", "-shm"] {
+            let sibling = URL(fileURLWithPath: cacheURL.path + suffix)
+            // Absent siblings are fine; only a genuine failure to remove one
+            // that exists is a corruption risk worth surfacing to the caller.
+            if fileManager.fileExists(at: sibling) {
+                try fileManager.removeItem(at: sibling)
+            }
+        }
     }
 
     private static func memoryCache() -> WordPressApiCache {
+        // Fallback when the on-disk cache cannot be opened. Because the cache
+        // is a process-wide singleton, this degradation lasts the whole
+        // session by design; the cache is refetchable, so the cost is a cold
+        // cache until relaunch.
         // Creating an in-memory database should always succeed.
         let cache = try! WordPressApiCache()
         _ = try! cache.performMigrations()

Modules/Sources/WordPressCore/WordPressClient.swift

@@ -81,16 +81,8 @@ public actor WordPressClient {
     /// The underlying API executor used for making network requests.
     public let api: any WordPressClientAPI
 
-    private var _cache: WordPressApiCache?
     public var cache: WordPressApiCache {
-        get {
-            if let _cache {
-                return _cache
-            }
-            let cache = WordPressApiCache.bootstrap()
-            _cache = cache
-            return cache
-        }
+        WordPressApiCache.shared
     }
 
     private var _service: WpService?

Modules/Tests/WordPressCoreTests/ApiCacheBootstrapTests.swift

@@ -0,0 +1,124 @@
+import Foundation
+import Testing
+import WordPressApiCache
+@testable import WordPressCore
+
+@Suite
+struct ApiCacheBootstrapTests {
+    private let directory: URL
+
+    init() throws {
+        directory = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString)
+        try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
+    }
+
+    private var databaseURL: URL {
+        directory.appending(path: "app.sqlite")
+    }
+
+    private var journalURL: URL {
+        URL(fileURLWithPath: databaseURL.path + "-journal")
+    }
+
+    private var walURL: URL {
+        URL(fileURLWithPath: databaseURL.path + "-wal")
+    }
+
+    private var shmURL: URL {
+        URL(fileURLWithPath: databaseURL.path + "-shm")
+    }
+
+    @Test func createsDatabaseOnFirstBootstrap() {
+        let outcome = WordPressApiCache.onDiskCache(at: databaseURL)
+
+        guard case .opened = outcome else {
+            Issue.record("Expected .opened, got \(outcome)")
+            return
+        }
+        #expect(FileManager.default.fileExists(at: databaseURL))
+    }
+
+    @Test func recoversFromCorruptDatabaseAndRemovesSiblings() throws {
+        try Data("not a database".utf8).write(to: databaseURL)
+        try Data("stale journal".utf8).write(to: journalURL)
+        try Data("stale wal".utf8).write(to: walURL)
+        try Data("stale shm".utf8).write(to: shmURL)
+
+        let outcome = WordPressApiCache.onDiskCache(at: databaseURL)
+
+        guard case .recovered(_, let failure) = outcome else {
+            Issue.record("Expected .recovered, got \(outcome)")
+            return
+        }
+        // The corrupt file may fail at open or at migration; either is recovery.
+        #expect(failure.kind == .couldNotOpenDatabase || failure.kind == .migrationFailed)
+        #expect(FileManager.default.fileExists(at: databaseURL))
+        #expect(!FileManager.default.fileExists(at: journalURL))
+        #expect(!FileManager.default.fileExists(at: walURL))
+        #expect(!FileManager.default.fileExists(at: shmURL))
+    }
+
+    @Test func removesOrphanedSiblingsBeforeCreatingDatabase() throws {
+        // No database file, only leftovers a previous delete-on-failure may
+        // have stranded. SQLite ignores an unmatched -wal file in rollback
+        // journal mode, so only the pre-open cleanup removes it.
+        try Data("stale journal".utf8).write(to: journalURL)
+        try Data("stale wal".utf8).write(to: walURL)
+
+        let outcome = WordPressApiCache.onDiskCache(at: databaseURL)
+
+        guard case .opened = outcome else {
+            Issue.record("Expected .opened, got \(outcome)")
+            return
+        }
+        #expect(!FileManager.default.fileExists(at: journalURL))
+        #expect(!FileManager.default.fileExists(at: walURL))
+    }
+
+    @Test func failsWhenOrphanedSiblingCannotBeRemoved() throws {
+        // A stale sibling next to a missing database that cannot be removed
+        // must not be paired with a freshly created database. Make the
+        // directory read-only so removing the journal fails.
+        try Data("stale journal".utf8).write(to: journalURL)
+        let fileManager = FileManager.default
+        try fileManager.setAttributes([.posixPermissions: 0o555], ofItemAtPath: directory.path)
+        defer {
+            try? fileManager.setAttributes([.posixPermissions: 0o755], ofItemAtPath: directory.path)
+        }
+
+        let outcome = WordPressApiCache.onDiskCache(at: databaseURL)
+
+        guard case .failed(let failure) = outcome else {
+            Issue.record("Expected .failed, got \(outcome)")
+            return
+        }
+        #expect(failure.kind == .couldNotRemoveOrphanedSiblings)
+        #expect(!fileManager.fileExists(at: databaseURL))
+    }
+
+    @Test func failsWhenDatabaseCannotBeCreated() {
+        let unwritableURL =
+            directory
+            .appending(path: "missing-subdirectory")
+            .appending(path: "app.sqlite")
+
+        let outcome = WordPressApiCache.onDiskCache(at: unwritableURL)
+
+        guard case .failed(let failure) = outcome else {
+            Issue.record("Expected .failed, got \(outcome)")
+            return
+        }
+        #expect(failure.kind == .couldNotOpenDatabase)
+    }
+
+    @Test func clientsShareSingleCacheInstance() async {
+        let clientA = WordPressClient(api: MockWordPressClientAPI(), siteURL: URL(string: "https://example.com")!)
+        let clientB = WordPressClient(api: MockWordPressClientAPI(), siteURL: URL(string: "https://example.org")!)
+
+        let cacheA = await clientA.cache
+        let cacheB = await clientB.cache
+
+        #expect(cacheA === cacheB)
+        #expect(cacheA === WordPressApiCache.shared)
+    }
+}

Tests/KeystoneTests/Tests/Features/Posts/CustomPostEditorServiceTests.swift

@@ -276,7 +276,7 @@ private func makeService(
         api: api,
         siteURL: URL(string: "https://example.com")!
     )
-    let wpService = try api.createService(cache: .bootstrap())
+    let wpService = try api.createService(cache: .forTesting())
 
     return CustomPostEditorService(
         blog: blog,

Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift

@@ -309,7 +309,7 @@ private func makeServiceDependencies() throws -> (client: WordPressClient, wpSer
         api: api,
         siteURL: URL(string: "https://example.com")!
     )
-    let wpService = try api.createService(cache: .bootstrap())
+    let wpService = try api.createService(cache: .forTesting())
 
     return (client, wpService)
 }