refactor(remote-config): use large item cache for blob store

Sources/Caching/LargeItemCacheType.swift

@@ -25,6 +25,9 @@ protocol LargeItemCacheType {
     /// Check if there is content cached at the url
     func cachedContentExists(at url: URL) -> Bool
 
+    /// Check if there is a regular file cached at the url
+    func cachedFileExists(at url: URL) -> Bool
+
     /// Load data from url
     func loadFile(at url: URL) throws -> Data
 
@@ -151,6 +154,11 @@ extension FileManager: LargeItemCacheType {
         }
     }
 
+    /// Check if a regular file is cached at the given path
+    func cachedFileExists(at url: URL) -> Bool {
+        return (try? url.resourceValues(forKeys: [.isRegularFileKey]).isRegularFile) == true
+    }
+
     /// Creates a directory from a base path in the specified directory type
     /// The `inAppSpecificDirectory` should be set to false only for components
     /// that haven't migrated to the new app specific directory structure yet

Sources/Networking/RemoteConfigBlobStore.swift

@@ -23,7 +23,7 @@ protocol RemoteConfigBlobStoreType: AnyObject {
 /// Content-addressed disk cache for remote config blobs, keyed by 32-character URL-safe base64 refs.
 final class RemoteConfigBlobStore: RemoteConfigBlobStoreType {
 
-    private let fileManager: FileManager
+    private let cache: LargeItemCacheType
     private let directoryURL: URL?
     private let lock = Lock(.nonRecursive)
 
@@ -32,10 +32,10 @@ final class RemoteConfigBlobStore: RemoteConfigBlobStoreType {
     private var knownRefs: Set<String>?
 
     init(
-        fileManager: FileManager = .default,
+        cache: LargeItemCacheType = FileManager.default,
         directoryURL: URL? = RemoteConfigBlobStore.defaultDirectoryURL
     ) {
-        self.fileManager = fileManager
+        self.cache = cache
         self.directoryURL = directoryURL
     }
 
@@ -86,7 +86,7 @@ private extension RemoteConfigBlobStore {
     func containsWithoutLock(ref: String) -> Bool {
         guard self.loadedRefsWithoutLock().contains(ref),
               let fileURL = self.fileURL(for: ref),
-              self.isRegularFile(fileURL) else {
+              self.cache.cachedFileExists(at: fileURL) else {
             self.knownRefs?.remove(ref)
             return false
         }
@@ -96,13 +96,13 @@ private extension RemoteConfigBlobStore {
 
     func readWithoutLock(ref: String) -> Data? {
         guard let fileURL = self.fileURL(for: ref),
-              self.isRegularFile(fileURL) else {
+              self.cache.cachedFileExists(at: fileURL) else {
             self.knownRefs?.remove(ref)
             return nil
         }
 
         do {
-            return try Data(contentsOf: fileURL)
+            return try self.cache.loadFile(at: fileURL)
         } catch {
             Logger.error(Strings.remoteConfig.failedToReadBlob(ref, error))
             return nil
@@ -113,7 +113,7 @@ private extension RemoteConfigBlobStore {
         ref: String,
         bytes: UnsafeRawBufferPointer
     ) -> Bool {
-        guard let directoryURL = self.directoryURL else {
+        guard self.directoryURL != nil else {
             Logger.error(Strings.remoteConfig.cacheURLNotAvailable)
             return false
         }
@@ -124,15 +124,9 @@ private extension RemoteConfigBlobStore {
         }
 
         do {
-            try self.fileManager.createDirectory(
-                at: directoryURL,
-                withIntermediateDirectories: true,
-                attributes: nil
-            )
-
             var data = Data()
             data.append(contentsOf: bytes.bindMemory(to: UInt8.self))
-            try data.write(to: fileURL, options: .atomic)
+            try self.cache.saveData(data, to: fileURL)
             guard var refs = self.knownRefs else {
                 return true
             }
@@ -155,18 +149,15 @@ private extension RemoteConfigBlobStore {
             return
         }
 
-        guard let contents = try? self.fileManager.contentsOfDirectory(
-                at: directoryURL,
-                includingPropertiesForKeys: [.isRegularFileKey],
-                options: []
-              ) else {
+        guard let contents = try? self.cache.contentsOfDirectory(at: directoryURL) else {
             self.knownRefs = nil
             return
         }
 
-        for fileURL in contents where self.isRegularFile(fileURL) && !validRefs.contains(fileURL.lastPathComponent) {
+        for fileURL in contents where self.cache.cachedFileExists(at: fileURL)
+            && !validRefs.contains(fileURL.lastPathComponent) {
             do {
-                try self.fileManager.removeItem(at: fileURL)
+                try self.cache.remove(fileURL)
             } catch {
                 Logger.error(Strings.remoteConfig.failedToDeleteBlob(fileURL.lastPathComponent, error))
             }
@@ -176,16 +167,20 @@ private extension RemoteConfigBlobStore {
     }
 
     func clearWithoutLock() {
-        guard let directoryURL = self.directoryURL,
-              self.fileManager.fileExists(atPath: directoryURL.path) else {
+        guard let directoryURL = self.directoryURL else {
             self.knownRefs = []
             return
         }
 
         do {
-            try self.fileManager.removeItem(at: directoryURL)
+            try self.cache.remove(directoryURL)
             self.knownRefs = []
         } catch {
+            guard !self.isMissingFileError(error) else {
+                self.knownRefs = []
+                return
+            }
+
             Logger.error(Strings.remoteConfig.failedToClearBlobStore(error))
             self.knownRefs = nil
         }
@@ -206,8 +201,9 @@ private extension RemoteConfigBlobStore {
         return self.directoryURL?.appendingPathComponent(ref, isDirectory: false)
     }
 
-    func isRegularFile(_ fileURL: URL) -> Bool {
-        return (try? fileURL.resourceValues(forKeys: [.isRegularFileKey]).isRegularFile) == true
+    func isMissingFileError(_ error: Error) -> Bool {
+        let error = error as NSError
+        return error.domain == NSCocoaErrorDomain && error.code == CocoaError.fileNoSuchFile.rawValue
     }
 
     func loadedRefsWithoutLock() -> Set<String> {
@@ -230,16 +226,12 @@ private extension RemoteConfigBlobStore {
 
     func scannedRefsWithoutLock() -> Set<String>? {
         guard let directoryURL = self.directoryURL,
-              let contents = try? self.fileManager.contentsOfDirectory(
-                at: directoryURL,
-                includingPropertiesForKeys: [.isRegularFileKey],
-                options: []
-              ) else {
+              let contents = try? self.cache.contentsOfDirectory(at: directoryURL) else {
             return nil
         }
 
         let scannedRefs = contents.reduce(into: Set<String>()) { refs, fileURL in
-            guard self.isRegularFile(fileURL),
+            guard self.cache.cachedFileExists(at: fileURL),
                   RemoteConfigBlobRefHelpers.isValid(fileURL.lastPathComponent) else {
                 return
             }

Tests/UnitTests/Caching/LargeItemCacheTypeTests.swift

@@ -216,6 +216,46 @@ final class LargeItemCacheTypeTests: TestCase {
 
     }
 
+    // MARK: - cachedFileExists Tests
+
+    func testCachedFileExistsReturnsTrueForNonEmptyFile() throws {
+        let url = self.testDirectory.appendingPathComponent("cached_file_nonempty.txt")
+        try "A".write(to: url, atomically: true, encoding: .utf8)
+
+        let exists = self.fileManager.cachedFileExists(at: url)
+
+        expect(exists) == true
+        try fileManager.removeItem(at: url)
+    }
+
+    func testCachedFileExistsReturnsTrueForEmptyFile() throws {
+        let url = self.testDirectory.appendingPathComponent("cached_file_empty.txt")
+        try Data().write(to: url)
+
+        let exists = self.fileManager.cachedFileExists(at: url)
+
+        expect(exists) == true
+        try fileManager.removeItem(at: url)
+    }
+
+    func testCachedFileExistsReturnsFalseForDirectory() throws {
+        let url = self.testDirectory.appendingPathComponent("cached_file_directory", isDirectory: true)
+        try self.fileManager.createDirectory(at: url, withIntermediateDirectories: true, attributes: nil)
+
+        let exists = self.fileManager.cachedFileExists(at: url)
+
+        expect(exists) == false
+        try fileManager.removeItem(at: url)
+    }
+
+    func testCachedFileExistsReturnsFalseForMissingFile() {
+        let url = self.testDirectory.appendingPathComponent("cached_file_missing.txt")
+
+        let exists = self.fileManager.cachedFileExists(at: url)
+
+        expect(exists) == false
+    }
+
     // MARK: - loadFile Tests
 
     func testLoadFileReturnsCorrectData() throws {

Tests/UnitTests/Mocks/MockLargeItemCache.swift

@@ -55,6 +55,13 @@ final class MockLargeItemCache: LargeItemCacheType {
         return storage[url] != nil
     }
 
+    func cachedFileExists(at url: URL) -> Bool {
+        lock.lock()
+        defer { lock.unlock() }
+
+        return storage[url] != nil
+    }
+
     func loadFile(at url: URL) throws -> Data {
         lock.lock()
         defer { lock.unlock() }

Tests/UnitTests/Mocks/MockSimpleCache.swift

@@ -84,6 +84,10 @@ class MockSimpleCache: LargeItemCacheType, @unchecked Sendable {
         }
     }
 
+    func cachedFileExists(at url: URL) -> Bool {
+        self.cachedContentExists(at: url)
+    }
+
     func stubSaveData(at index: Int = 0, with result: Result<SaveData, Error>) {
         lock.withLock {
             saveDataResponses.insert(result, at: index)

Tests/UnitTests/Mocks/MockUnderlyingSynchronizedFileCache.swift

@@ -85,6 +85,10 @@ extension SynchronizedLargeItemCache {
             }
         }
 
+        func cachedFileExists(at url: URL) -> Bool {
+            self.cachedContentExists(at: url)
+        }
+
         func loadFile(at url: URL) throws -> Data {
             try lock.withLock {
                 loadFileInvocations.append(url)

Tests/UnitTests/Networking/RemoteConfig/RemoteConfigBlobFetcherTests.swift

@@ -337,17 +337,22 @@ final class RemoteConfigBlobFetcherTests: TestCase {
         expect(self.blobStore.invokedWriteCount) == 1
     }
 
-    func testDifferentRefsRunConcurrentlyUpToWorkerLimit() async {
+    func testDifferentRefsRunConcurrentlyUpToWorkerLimit() async throws {
         let refs = (0..<6).map { Self.ref(for: "blob-\($0)".asData) }
 
         self.fetcher.prefetch(refs: refs)
 
         await self.downloader.waitForRequestCount(4)
         expect(self.downloader.activeRequestCount) == 4
+        let firstBatch = Array(self.downloader.requestedRefs.prefix(4))
+        expect(Set(firstBatch).isSubset(of: Set(refs))) == true
+        expect(Set(firstBatch)).to(haveCount(4))
 
-        self.downloader.complete(ref: refs[0], with: .success("blob-0".asData))
+        let activeRef = try XCTUnwrap(firstBatch.first)
+        let activeIndex = try XCTUnwrap(refs.firstIndex(of: activeRef))
+        self.downloader.complete(ref: activeRef, with: .success("blob-\(activeIndex)".asData))
         await self.downloader.waitForRequestCount(5)
-        expect(Array(self.downloader.requestedRefs.prefix(4))) == Array(refs.prefix(4))
+        expect(self.downloader.activeRequestCount) == 4
     }
 
     func testPrefetchSchedulesLowPriorityRefs() async {
@@ -359,17 +364,19 @@ final class RemoteConfigBlobFetcherTests: TestCase {
         expect(Set(self.downloader.requestedRefs)) == Set(refs)
     }
 
-    func testOnDemandRequestRunsBeforeQueuedPrefetches() async {
+    func testOnDemandRequestRunsBeforeQueuedPrefetches() async throws {
         let prefetchRefs = (0..<5).map { Self.ref(for: "prefetch-\($0)".asData) }
         let onDemandPayload = "on demand".asData
         let onDemandRef = Self.ref(for: onDemandPayload)
 
         self.fetcher.prefetch(refs: prefetchRefs)
         await self.downloader.waitForRequestCount(4)
+        let activePrefetchRef = try XCTUnwrap(self.downloader.requestedRefs.prefix(4).first)
+        let activePrefetchIndex = try XCTUnwrap(prefetchRefs.firstIndex(of: activePrefetchRef))
 
         let onDemand = Task { await self.fetcher.ensureDownloaded(ref: onDemandRef) }
         await self.waitForScheduledTaskToReachFetcher()
-        self.downloader.complete(ref: prefetchRefs[0], with: .success("prefetch-0".asData))
+        self.downloader.complete(ref: activePrefetchRef, with: .success("prefetch-\(activePrefetchIndex)".asData))
 
         await self.downloader.waitForRequestCount(5)
         expect(self.downloader.requestedRefs[4]) == onDemandRef
@@ -379,17 +386,19 @@ final class RemoteConfigBlobFetcherTests: TestCase {
         expect(result) == true
     }
 
-    func testOnDemandRequestBoostsAndJoinsQueuedPrefetch() async {
+    func testOnDemandRequestBoostsAndJoinsQueuedPrefetch() async throws {
         let prefetchRefs = (0..<5).map { Self.ref(for: "prefetch-\($0)".asData) }
         let boostedPayload = "boosted".asData
         let boostedRef = Self.ref(for: boostedPayload)
 
         self.fetcher.prefetch(refs: prefetchRefs + [boostedRef])
         await self.downloader.waitForRequestCount(4)
+        let activePrefetchRef = try XCTUnwrap(self.downloader.requestedRefs.prefix(4).first)
+        let activePrefetchIndex = try XCTUnwrap(prefetchRefs.firstIndex(of: activePrefetchRef))
 
         let boosted = Task { await self.fetcher.ensureDownloaded(ref: boostedRef) }
         await self.waitForScheduledTaskToReachFetcher()
-        self.downloader.complete(ref: prefetchRefs[0], with: .success("prefetch-0".asData))
+        self.downloader.complete(ref: activePrefetchRef, with: .success("prefetch-\(activePrefetchIndex)".asData))
 
         await self.downloader.waitForRequestCount(5)
         expect(self.downloader.requestedRefs[4]) == boostedRef