diff --git a/Modules/Sources/WordPressCore/ApiCache.swift b/Modules/Sources/WordPressCore/ApiCache.swift index c77da9ecdb51..9035893b56dc 100644 --- a/Modules/Sources/WordPressCore/ApiCache.swift +++ b/Modules/Sources/WordPressCore/ApiCache.swift @@ -5,63 +5,174 @@ 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() + + /// 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 + } - if let cache = WordPressApiCache.onDiskCache(file: cacheURL) { - 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) + } + } + + /// 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 { 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() diff --git a/Modules/Sources/WordPressCore/WordPressClient.swift b/Modules/Sources/WordPressCore/WordPressClient.swift index f3c27ded44dd..6b481b45b844 100644 --- a/Modules/Sources/WordPressCore/WordPressClient.swift +++ b/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? diff --git a/Modules/Tests/WordPressCoreTests/ApiCacheBootstrapTests.swift b/Modules/Tests/WordPressCoreTests/ApiCacheBootstrapTests.swift new file mode 100644 index 000000000000..3046cfd97296 --- /dev/null +++ b/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) + } +} diff --git a/Tests/KeystoneTests/Tests/Features/Posts/CustomPostEditorServiceTests.swift b/Tests/KeystoneTests/Tests/Features/Posts/CustomPostEditorServiceTests.swift index df54973fc4a4..def965629ca3 100644 --- a/Tests/KeystoneTests/Tests/Features/Posts/CustomPostEditorServiceTests.swift +++ b/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: .shared) return CustomPostEditorService( blog: blog, diff --git a/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift b/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift index 6d5841baf8b9..a80379b6acac 100644 --- a/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift +++ b/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: .shared) return (client, wpService) } diff --git a/WordPress/Classes/Networking/WordPressApiCacheCleanup.swift b/WordPress/Classes/Networking/WordPressApiCacheCleanup.swift new file mode 100644 index 000000000000..a71ee04b990d --- /dev/null +++ b/WordPress/Classes/Networking/WordPressApiCacheCleanup.swift @@ -0,0 +1,45 @@ +import Foundation +import WordPressAPI +import WordPressAPIInternal +import WordPressApiCache +import WordPressCore +import WordPressData + +extension BlogService { + @objc(removeWordPressApiCachedDataForBlog:) + public func removeWordPressApiCachedData(for blog: Blog) { + WordPressApiCache.shared.removeCachedData(for: blog) + } +} + +extension WordPressApiCache { + func removeCachedData(for blog: Blog) { + removeCachedData(for: [blog]) + } + + func removeCachedData(for blogs: Set) { + removeCachedData(for: Array(blogs)) + } + + private func removeCachedData(for blogs: [Blog]) { + for blog in blogs { + do { + // `restApiRootURL` remains after an Atomic site's application + // password is removed, so it records that direct transport was + // configured without requiring the credential itself. + let isDirect = !blog.isHostedAtWPcom || (blog.isAtomic && blog.restApiRootURL != nil) + if isDirect { + try removeSelfHostedSite(url: blog.getUrl()) + } else if let siteID = blog.dotComID?.intValue { + try removeWordpressComSite(siteId: WpComSiteId(siteID)) + } else { + Loggers.app.error( + "Skipped WordPress API cache cleanup because the site's durable metadata is incomplete" + ) + } + } catch { + Loggers.app.error("Failed to remove WordPress API cached data: \(error)") + } + } + } +} diff --git a/WordPress/Classes/Services/AccountService+Swift.swift b/WordPress/Classes/Services/AccountService+Swift.swift index 57c9356f7672..24cd716b0675 100644 --- a/WordPress/Classes/Services/AccountService+Swift.swift +++ b/WordPress/Classes/Services/AccountService+Swift.swift @@ -3,6 +3,8 @@ import WordPressData import WordPressShared import ShareExtensionCore import WebKit +import WordPressApiCache +import WordPressCore extension AccountService { // MARK: - Current Account @@ -48,6 +50,8 @@ extension AccountService { return } + WordPressApiCache.shared.removeCachedData(for: account.blogs ?? []) + let objectID = TaggedManagedObjectID(account) coreDataStack.performAndSave { context in do { diff --git a/WordPress/Classes/Services/BlogService.m b/WordPress/Classes/Services/BlogService.m index 323d96c64d27..8090217959aa 100644 --- a/WordPress/Classes/Services/BlogService.m +++ b/WordPress/Classes/Services/BlogService.m @@ -344,6 +344,7 @@ - (void)removeBlog:(Blog *)blog DDLogInfo(@" remove", blog.displayURL); [blog.xmlrpcApi invalidateAndCancelTasks]; [self unscheduleBloggingRemindersFor:blog]; + [self removeWordPressApiCachedDataForBlog:blog]; WPAccount *account = blog.account;