diff --git a/Modules/Sources/WordPressCore/DiskCache.swift b/Modules/Sources/WordPressCore/DiskCache.swift index 37b7268fc02d..82acfa0b2357 100644 --- a/Modules/Sources/WordPressCore/DiskCache.swift +++ b/Modules/Sources/WordPressCore/DiskCache.swift @@ -7,9 +7,11 @@ public actor DiskCache: DiskCacheProtocol { public static let shared = DiskCache() - private let cacheRoot = URL.cachesDirectory + private let cacheRoot: URL - public init() {} + public init(cacheRoot: URL = .cachesDirectory) { + self.cacheRoot = cacheRoot + } public func read( _ type: T.Type, @@ -23,15 +25,19 @@ public actor DiskCache: DiskCacheProtocol { } if let interval { - let attributes = try FileManager.default.attributesOfItem(atPath: path.path()) + // Read the date off the URL rather than a path string: `path()` percent-encodes, so a + // key needing encoding would name a file that doesn't exist. + let creationDate = try path.resourceValues(forKeys: [.creationDateKey]).creationDate // If we can't find the creation date, assume the cache object is invalid because we can't guarantee // the developer's intent will be respected. - guard let creationDate = attributes[.creationDate] as? Date else { + guard let creationDate else { return nil } - if creationDate.addingTimeInterval(interval) > Date.now { + // The entry expires once `interval` has elapsed since it was written, so treat it as a + // miss only once that moment has passed. + if creationDate.addingTimeInterval(interval) < Date.now { return nil } } diff --git a/Modules/Tests/WordPressCoreTests/DiskCacheTests.swift b/Modules/Tests/WordPressCoreTests/DiskCacheTests.swift new file mode 100644 index 000000000000..6ed4d4e646fe --- /dev/null +++ b/Modules/Tests/WordPressCoreTests/DiskCacheTests.swift @@ -0,0 +1,101 @@ +import Foundation +import Testing + +@testable import WordPressCore + +struct DiskCacheTests { + + /// Each test gets its own root so a run never reads, writes, or deletes anything in the real + /// caches directory. + private func makeCache() throws -> (DiskCache, URL) { + let root = URL.temporaryDirectory.appending(path: "DiskCacheTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + return (DiskCache(cacheRoot: root), root) + } + + @Test func storesAndReadsBackAValue() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store(["a", "b"], forKey: "key") + + #expect(try await cache.read([String].self, forKey: "key") == ["a", "b"]) + } + + /// A freshly-written entry is inside any sane window, so it has to come back. Before the fix + /// the comparison was inverted and this returned `nil`. + @Test func readsBackAnEntryThatIsStillFresh() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store("value", forKey: "key") + + #expect(try await cache.read(String.self, forKey: "key", notOlderThan: 3600) == "value") + } + + /// The counterpart: a zero-length window expires the entry immediately. + @Test func treatsAnExpiredEntryAsAMiss() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store("value", forKey: "key") + // The entry was written in the past, however narrowly, so any elapsed window excludes it. + try await Task.sleep(for: .milliseconds(50)) + + #expect(try await cache.read(String.self, forKey: "key", notOlderThan: 0) == nil) + } + + /// Passing no interval skips the age check entirely. + @Test func readsBackAnEntryWhenNoIntervalIsGiven() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store("value", forKey: "key") + + #expect(try await cache.read(String.self, forKey: "key") == "value") + } + + @Test func returnsNilForAKeyThatWasNeverStored() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + #expect(try await cache.read(String.self, forKey: "missing") == nil) + #expect(try await cache.read(String.self, forKey: "missing", notOlderThan: 3600) == nil) + } + + /// The age check used to resolve the file through a percent-encoded path string, which named a + /// file that doesn't exist whenever the key needed encoding. + @Test func handlesAKeyThatNeedsPercentEncoding() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store("value", forKey: "conversation 1 of 100%") + + #expect(try await cache.read(String.self, forKey: "conversation 1 of 100%", notOlderThan: 3600) == "value") + } + + @Test func removesAStoredEntry() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store("value", forKey: "key") + try await cache.remove(key: "key") + + #expect(try await cache.read(String.self, forKey: "key") == nil) + } + + /// `removeAll` only touches the injected root, which is what keeps these tests from clearing a + /// developer's real cache. + @Test func removeAllClearsOnlyTheInjectedRoot() async throws { + let (cache, root) = try makeCache() + defer { try? FileManager.default.removeItem(at: root) } + + try await cache.store("a", forKey: "one") + try await cache.store("b", forKey: "two") + #expect(try await cache.count() == 2) + + try await cache.removeAll() + + #expect(try await cache.count() == 0) + } +}