From d90537354aa6b2d0d62538fbbfc16e3f49e322d2 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 11 Sep 2026 16:18:25 -0400 Subject: [PATCH 1/2] fix(avatars): cache profile pictures on disk and reload them when they change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profile pictures re-downloaded on every launch and, once downloaded, could show the wrong person's photo until the process restarted. Four separate causes, all in how the bytes were held: `RemoteImageCache.install()` puts `.cacheMemoryOnly` into Kingfisher's `defaultOptions`, so `RemoteImageLoader` — the signed-in user's picture — never wrote to the disk cache and its `retrieveImage(forKey:)` could not hit after a relaunch. `URLCache` cannot stand in for that here: `blobDownloadURL` mints a fresh signed URL per fetch, so the same picture arrives under a new key every time. Other users' pictures went through `TipAvatarStore`, which held raw bytes in memory only and was gone on relaunch. `TipAvatarStore.load` guarded on `dataByUser[userID] == nil`, so a user who changed their picture — new blob id, same user id — never reloaded. `ContactAvatarCache` had the same shape of bug one layer down: it keyed decoded images by contact id and ignored the `data` argument on a hit, so the old photo stayed on screen on every avatar surface in the app. `UserProfileScreen` read the store at body-eval time and fed it into a `@State` model built once, so a picture that arrived after the screen opened never reached it. The fix is one byte cache both paths read: `ProfilePictureCache`, keyed by blob id in the App Group container. A blob is immutable, so a hit needs no revalidation and the notification extension can warm an avatar the app then draws at first paint. It is LRU-bounded at 24 MB and cleared on logout, next to `NotificationPreviewCache` — these are other people's photographs sitting unencrypted in a container the next account on the device reads from. `TipAvatarStore` becomes `ProfileAvatarStore` under `Core/Controllers/`: it serves every surface, not just tips, and it now tracks which blob each entry came from so a changed picture reloads. `TipConversationsScreen` warms the whole list rather than letting each row start its download when it scrolls into view. `RemoteImageLoader` is deleted; its one consumer reads the byte cache directly. `RemoteImage`/`URLCache` is untouched — it backs token and currency logos, whose URLs are stable and public, and is correct as-is. --- .../Core/Controllers/ProfileAvatarStore.swift | 115 +++++++++++++++ .../Conversation/ConversationScreen.swift | 16 +- .../Screens/Main/Home/ActivityAvatar.swift | 4 +- .../Main/Profile/ProfilePictureLoader.swift | 17 ++- .../Screens/Main/Tips/TipAvatarStore.swift | 64 -------- .../Main/Tips/TipConversationsScreen.swift | 16 +- Flipcash/Core/Screens/Main/Tips/TipFlow.swift | 4 +- .../Screens/Profile/UserProfileScreen.swift | 23 ++- .../Core/Session/SessionAuthenticator.swift | 8 +- .../Images/ProfilePictureCache.swift | 139 ++++++++++++++++++ .../ProfilePictureCacheTests.swift | 122 +++++++++++++++ FlipcashTests/ContactAvatarCacheTests.swift | 61 ++++++++ .../FlipcashUI/Images/RemoteImageLoader.swift | 29 ---- .../Views/Contacts/ContactAvatarView.swift | 39 ++++- 14 files changed, 531 insertions(+), 126 deletions(-) create mode 100644 Flipcash/Core/Controllers/ProfileAvatarStore.swift delete mode 100644 Flipcash/Core/Screens/Main/Tips/TipAvatarStore.swift create mode 100644 FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift create mode 100644 FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift create mode 100644 FlipcashTests/ContactAvatarCacheTests.swift delete mode 100644 FlipcashUI/Sources/FlipcashUI/Images/RemoteImageLoader.swift diff --git a/Flipcash/Core/Controllers/ProfileAvatarStore.swift b/Flipcash/Core/Controllers/ProfileAvatarStore.swift new file mode 100644 index 000000000..b232f4b62 --- /dev/null +++ b/Flipcash/Core/Controllers/ProfileAvatarStore.swift @@ -0,0 +1,115 @@ +// +// ProfileAvatarStore.swift +// Flipcash +// + +import Foundation +import FlipcashCore + +private let logger = Logger(label: "flipcash.profile-avatars") + +/// Other users' profile-picture thumbnails, as raw bytes so every surface renders them through the +/// same `ContactAvatarView(imageData:)` path address-book avatars use. +/// +/// Backed by ``ProfilePictureCache``, so bytes fetched in one launch are on screen at first paint in +/// the next. The in-memory map is what `@Observable` watches — the disk cache alone cannot invalidate +/// a view — and it is populated from disk without a round trip. +@Observable +final class ProfileAvatarStore { + + private var dataByUser: [UserID: Data] = [:] + + /// Which blob each cached entry came from. A user who changes their picture keeps their user id + /// and gets a new blob id, so this is what tells a stale entry from a current one. + @ObservationIgnored private var blobByUser: [UserID: BlobID] = [:] + + @ObservationIgnored private var inFlight: Set = [] + @ObservationIgnored private let cache: ProfilePictureCache + @ObservationIgnored private let mintURL: (BlobID, UserID) async throws -> URL? + @ObservationIgnored private let fetch: (URL) async throws -> Data + + /// - Parameters: + /// - mintURL: mints a download URL for a blob. Injected because `FlipClient` is a concrete + /// class with a live gRPC channel, so there is no fake to pass a test. + /// - fetch: downloads the bytes at a URL. + init( + cache: ProfilePictureCache = .shared, + mintURL: @escaping (BlobID, UserID) async throws -> URL?, + fetch: @escaping (URL) async throws -> Data = { url in + try await URLSession.shared.data(from: url).0 + } + ) { + self.cache = cache + self.mintURL = mintURL + self.fetch = fetch + } + + convenience init(flipClient: FlipClient, owner: KeyPair, cache: ProfilePictureCache = .shared) { + self.init( + cache: cache, + mintURL: { blobID, userID in + try await flipClient.blobDownloadURL( + blobID: blobID, + owner: owner, + accessContext: .profile(userID) + ) + } + ) + } + + /// The cached thumbnail bytes for a user, or nil until ``load(userID:picture:)`` has them. + func data(for userID: UserID?) -> Data? { + userID.flatMap { dataByUser[$0] } + } + + /// Makes a member's current thumbnail available to ``data(for:)``. + /// + /// Returns without a round trip when the bytes are already in memory or on disk for that exact + /// blob. Download URLs expire, so one is minted per fetch and never stored. + func load(userID: UserID?, picture: ProfilePicture?) async { + guard let userID, let blobID = picture?.thumbnailBlobID else { return } + + // A different blob under the same user id is a changed picture, not a cache hit. + if blobByUser[userID] == blobID, dataByUser[userID] != nil { return } + guard !inFlight.contains(userID) else { return } + + if let cached = cache.data(for: blobID) { + store(cached, userID: userID, blobID: blobID) + return + } + + inFlight.insert(userID) + defer { inFlight.remove(userID) } + + do { + guard let url = try await mintURL(blobID, userID) else { return } + + let data = try await fetch(url) + cache.write(data, for: blobID) + store(data, userID: userID, blobID: blobID) + } catch { + guard !Task.isCancelled else { return } + // The surfaces render the monogram placeholder without it. + logger.info("Failed to load a profile avatar", metadata: [ + "userId": "\(userID)", + "error": "\(error)", + ]) + } + } + + /// Warms several members' thumbnails at once, for a list that is about to draw all of them — + /// including the rows below the fold, so scrolling never lands on a blurhash. + /// + /// Returns immediately. Each fetch lands in the store on its own and invalidates only the rows + /// that read it; a caller that awaited the set would hold the whole list behind the slowest one. + func preload(_ members: [(userID: UserID?, picture: ProfilePicture?)]) { + for member in members { + Task { await load(userID: member.userID, picture: member.picture) } + } + } + + private func store(_ data: Data, userID: UserID, blobID: BlobID) { + dataByUser[userID] = data + blobByUser[userID] = blobID + } +} diff --git a/Flipcash/Core/Screens/Conversation/ConversationScreen.swift b/Flipcash/Core/Screens/Conversation/ConversationScreen.swift index f58f227eb..f67bcdcfb 100644 --- a/Flipcash/Core/Screens/Conversation/ConversationScreen.swift +++ b/Flipcash/Core/Screens/Conversation/ConversationScreen.swift @@ -259,7 +259,7 @@ struct ConversationScreen: View { title: title, contact: contact, conversationID: conversationID, - imageData: contact?.imageData ?? sessionContainer.tipAvatars.data(for: tipCounterpart?.userID), + imageData: contact?.imageData ?? sessionContainer.profileAvatars.data(for: tipCounterpart?.userID), blurhash: tipCounterpart?.profilePicture?.thumbnailBlurhash, width: max(navBarWidth - Self.titleSideInset * 2, 0), onTap: titleTapAction, @@ -270,7 +270,7 @@ struct ConversationScreen: View { } // Fetch the tip counterpart's avatar for the title and profile card. .task(id: tipCounterpart?.userID) { - await sessionContainer.tipAvatars.load( + await sessionContainer.profileAvatars.load( userID: tipCounterpart?.userID, picture: tipCounterpart?.profilePicture ) @@ -530,15 +530,15 @@ struct ConversationScreen: View { conversationID: id, controller: conversationController, session: session, - // `tipAvatars` is captured directly so the coordinator retains + // `profileAvatars` is captured directly so the coordinator retains // one small store, not the whole session container. - profileCard: { [context, contactSyncController, conversationController, session, counterpartUserID, tipAvatars = sessionContainer.tipAvatars] in + profileCard: { [context, contactSyncController, conversationController, session, counterpartUserID, profileAvatars = sessionContainer.profileAvatars] in Self.profileCard( context: context, conversationID: id, directory: contactSyncController.resolvedContacts.onFlipcash, controller: conversationController, - tipAvatars: tipAvatars, + profileAvatars: profileAvatars, // Resolved inside the closure, not captured: the card // must pick up the conversation the first tip creates. fallbackCounterpart: Self.cachedCounterpart(counterpartUserID, session: session) @@ -586,7 +586,7 @@ struct ConversationScreen: View { conversationID: ConversationID, directory: [ResolvedContact], controller: ConversationController, - tipAvatars: TipAvatarStore, + profileAvatars: ProfileAvatarStore, fallbackCounterpart: ConversationMember? = nil ) -> ChatProfileCard { if let conversation = controller.conversation(withID: conversationID), @@ -595,7 +595,7 @@ struct ConversationScreen: View { return ChatProfileCard( name: controller.displayName(for: conversation), avatarID: counterpart?.userID?.uuidString ?? conversationID.description, - imageData: tipAvatars.data(for: counterpart?.userID), + imageData: profileAvatars.data(for: counterpart?.userID), blurhash: counterpart?.profilePicture?.thumbnailBlurhash, counterpart: Self.tipDMCounterpart(counterpart) ) @@ -605,7 +605,7 @@ struct ConversationScreen: View { return ChatProfileCard( name: counterpart.displayName, avatarID: counterpart.userID?.uuidString ?? conversationID.description, - imageData: tipAvatars.data(for: counterpart.userID), + imageData: profileAvatars.data(for: counterpart.userID), blurhash: counterpart.profilePicture?.thumbnailBlurhash, counterpart: Self.tipDMCounterpart(counterpart) ) diff --git a/Flipcash/Core/Screens/Main/Home/ActivityAvatar.swift b/Flipcash/Core/Screens/Main/Home/ActivityAvatar.swift index f479d994d..f8e8ecf7a 100644 --- a/Flipcash/Core/Screens/Main/Home/ActivityAvatar.swift +++ b/Flipcash/Core/Screens/Main/Home/ActivityAvatar.swift @@ -249,8 +249,8 @@ final class ActivityResolution { avatarBlurhash = picture?.thumbnailBlurhash guard let picture else { return } - await sessionContainer.tipAvatars.load(userID: userID, picture: picture) - avatarData = sessionContainer.tipAvatars.data(for: userID) + await sessionContainer.profileAvatars.load(userID: userID, picture: picture) + avatarData = sessionContainer.profileAvatars.data(for: userID) } private func resolveMintMetadata(_ mint: PublicKey, session: Session) async -> StoredMintMetadata? { diff --git a/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift b/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift index 706e5b7f0..30aafe579 100644 --- a/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift +++ b/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift @@ -21,15 +21,16 @@ enum ProfilePictureLoader { static func thumbnail( for picture: ProfilePicture?, using client: FlipClient, - owner: KeyPair + owner: KeyPair, + cache: ProfilePictureCache = .shared ) async -> UIImage? { guard let blobID = picture?.thumbnailBlobID else { return nil } - // Answered from the cache before the round trip below, so a relaunch draws - // the picture at first paint instead of after a download URL comes back. - // Safe to trust: a blob is immutable, so a new picture is a new key. - if let cached = await RemoteImageLoader.cachedImage(cacheKey: blobID.description) { - return cached + // Answered from disk before the round trip below, so a relaunch draws the + // picture at first paint instead of after a download URL comes back. Safe + // to trust: a blob is immutable, so a new picture is a new key. + if let cached = cache.data(for: blobID), let image = UIImage(data: cached) { + return image } do { @@ -39,7 +40,9 @@ enum ProfilePictureLoader { return nil } - return try await RemoteImageLoader.image(at: url, cacheKey: blobID.description) + let (data, _) = try await URLSession.shared.data(from: url) + cache.write(data, for: blobID) + return UIImage(data: data) } catch { guard !Task.isCancelled else { return nil } logger.info("Failed to load a profile picture", metadata: ["error": "\(error)"]) diff --git a/Flipcash/Core/Screens/Main/Tips/TipAvatarStore.swift b/Flipcash/Core/Screens/Main/Tips/TipAvatarStore.swift deleted file mode 100644 index 8b1acec1d..000000000 --- a/Flipcash/Core/Screens/Main/Tips/TipAvatarStore.swift +++ /dev/null @@ -1,64 +0,0 @@ -// -// TipAvatarStore.swift -// Flipcash -// - -import Foundation -import FlipcashCore - -private let logger = Logger(label: "flipcash.tip-avatars") - -/// Session-scoped cache of tip counterparts' profile-picture thumbnails, as -/// raw bytes so every surface renders them through the same -/// `ContactAvatarView(imageData:)` path address-book avatars use. -@Observable -final class TipAvatarStore { - - private var dataByUser: [UserID: Data] = [:] - - @ObservationIgnored private var inFlight: Set = [] - @ObservationIgnored private let flipClient: FlipClient - @ObservationIgnored private let owner: KeyPair - - init(flipClient: FlipClient, owner: KeyPair) { - self.flipClient = flipClient - self.owner = owner - } - - /// The cached thumbnail bytes for a user, or nil until ``load(userID:picture:)`` - /// has fetched them. - func data(for userID: UserID?) -> Data? { - userID.flatMap { dataByUser[$0] } - } - - /// Fetches and caches the thumbnail for a member's current profile - /// picture. No-ops when already cached, already fetching, or the member - /// has no picture. Download URLs expire, so one is minted per fetch. - func load(userID: UserID?, picture: ProfilePicture?) async { - guard let userID, - let blobID = picture?.thumbnailBlobID, - dataByUser[userID] == nil, - !inFlight.contains(userID) else { return } - - inFlight.insert(userID) - defer { inFlight.remove(userID) } - - do { - guard let url = try await flipClient.blobDownloadURL( - blobID: blobID, - owner: owner, - accessContext: .profile(userID) - ) else { return } - - let (data, _) = try await URLSession.shared.data(from: url) - dataByUser[userID] = data - } catch { - guard !Task.isCancelled else { return } - // The surfaces render the monogram placeholder without it. - logger.info("Failed to load tip avatar", metadata: [ - "userId": "\(userID)", - "error": "\(error)", - ]) - } - } -} diff --git a/Flipcash/Core/Screens/Main/Tips/TipConversationsScreen.swift b/Flipcash/Core/Screens/Main/Tips/TipConversationsScreen.swift index ee231e63b..8da56d4f1 100644 --- a/Flipcash/Core/Screens/Main/Tips/TipConversationsScreen.swift +++ b/Flipcash/Core/Screens/Main/Tips/TipConversationsScreen.swift @@ -11,6 +11,7 @@ import FlipcashUI struct TipConversationsScreen: View { @Environment(ConversationController.self) private var conversationController + @Environment(SessionContainer.self) private var sessionContainer @Environment(AppRouter.self) private var router var body: some View { @@ -41,6 +42,17 @@ struct TipConversationsScreen: View { NewChatButton() } } + // Every counterpart, not just the rows on screen. A row's own `.task` fires when the row is + // built, which in a `List` is when it scrolls into view — so without this the avatar below + // the fold starts downloading at the moment the user is looking at its blurhash. + .task(id: conversations.count) { + sessionContainer.profileAvatars.preload( + conversations.map { + let counterpart = $0.counterpart(excluding: conversationController.selfUserID) + return (counterpart?.userID, counterpart?.profilePicture) + } + ) + } } } @@ -131,7 +143,7 @@ private struct TipConversationRow: View { avatarID: counterpart?.userID?.uuidString ?? conversation.id.description, title: title, subtitle: subtitle, - imageData: sessionContainer.tipAvatars.data(for: counterpart?.userID), + imageData: sessionContainer.profileAvatars.data(for: counterpart?.userID), blurhash: counterpart?.profilePicture?.thumbnailBlurhash, accessoryPlacement: .titleLine, accessibilityLabel: hasUnread ? "\(title), unread messages" : title, @@ -144,7 +156,7 @@ private struct TipConversationRow: View { ) } .task(id: counterpart?.userID) { - await sessionContainer.tipAvatars.load( + await sessionContainer.profileAvatars.load( userID: counterpart?.userID, picture: counterpart?.profilePicture ) diff --git a/Flipcash/Core/Screens/Main/Tips/TipFlow.swift b/Flipcash/Core/Screens/Main/Tips/TipFlow.swift index 65be97891..5fefe600b 100644 --- a/Flipcash/Core/Screens/Main/Tips/TipFlow.swift +++ b/Flipcash/Core/Screens/Main/Tips/TipFlow.swift @@ -367,12 +367,12 @@ final class TipFlow { } } - /// Fetches the recipient's avatar through the shared tip-avatar store — + /// Fetches the recipient's avatar through the shared profile-avatar store — /// warming the same cache the conversation surfaces read — and re-renders /// the card with it. The card is already up, so a failure just leaves the /// placeholder. private func loadAvatar(for recipient: TipRecipient, picture: ProfilePicture?) async { - let store = sessionContainer.tipAvatars + let store = sessionContainer.profileAvatars await store.load(userID: recipient.userID, picture: picture) guard let data = store.data(for: recipient.userID), let avatar = UIImage(data: data), diff --git a/Flipcash/Core/Screens/Profile/UserProfileScreen.swift b/Flipcash/Core/Screens/Profile/UserProfileScreen.swift index 33194bb9b..8c830d2a3 100644 --- a/Flipcash/Core/Screens/Profile/UserProfileScreen.swift +++ b/Flipcash/Core/Screens/Profile/UserProfileScreen.swift @@ -19,7 +19,6 @@ struct UserProfileScreen: View { var body: some View { let seed = sessionContainer.conversationController.counterpartSeed(forUserID: userID) - let avatarData = sessionContainer.tipAvatars.data(for: userID) UserProfileContent( model: UserProfileViewModel( userID: userID, @@ -28,8 +27,8 @@ struct UserProfileScreen: View { blocklistController: blocklistController, router: router, session: sessionContainer.session, - seed: seed, - avatarData: avatarData + profileAvatars: sessionContainer.profileAvatars, + seed: seed ) ) } @@ -124,7 +123,6 @@ final class UserProfileViewModel { private(set) var name: String? private(set) var username: Username? - private(set) var imageData: Data? private(set) var blurhash: String? private(set) var joinedText: String? @@ -147,17 +145,28 @@ final class UserProfileViewModel { @ObservationIgnored private let blocklistController: BlocklistController @ObservationIgnored private let router: AppRouter @ObservationIgnored private let session: Session + @ObservationIgnored private let profileAvatars: ProfileAvatarStore + @ObservationIgnored private let seedImageData: Data? + + /// The avatar bytes to draw, read through to the store on every access rather than captured at + /// init: the store fills in after a round trip, and a counterpart who changes their picture + /// replaces what it holds. A screen holding a copy from init shows the blurhash until it is + /// dismissed and reopened. + var imageData: Data? { + profileAvatars.data(for: userID) ?? seedImageData + } - init(userID: UserID, flipClient: FlipClient, owner: KeyPair, blocklistController: BlocklistController, router: AppRouter, session: Session, seed: CounterpartSeed, avatarData: Data?) { + init(userID: UserID, flipClient: FlipClient, owner: KeyPair, blocklistController: BlocklistController, router: AppRouter, session: Session, profileAvatars: ProfileAvatarStore, seed: CounterpartSeed) { self.userID = userID self.flipClient = flipClient self.owner = owner self.blocklistController = blocklistController self.router = router self.session = session + self.profileAvatars = profileAvatars self.name = seed.name self.username = seed.username - self.imageData = avatarData ?? seed.imageData + self.seedImageData = seed.imageData self.blurhash = seed.blurhash } @@ -167,10 +176,12 @@ final class UserProfileViewModel { func loadProfile() async { if let cached = session.cachedUserProfile(for: userID) { apply(cached) + await profileAvatars.load(userID: userID, picture: cached.profilePicture) } guard let profile = try? await flipClient.fetchProfile(userID: userID, owner: owner) else { return } session.cacheUserProfile(profile, for: userID) apply(profile) + await profileAvatars.load(userID: userID, picture: profile.profilePicture) } private func apply(_ profile: Profile) { diff --git a/Flipcash/Core/Session/SessionAuthenticator.swift b/Flipcash/Core/Session/SessionAuthenticator.swift index 364fef6f6..b4542ed58 100644 --- a/Flipcash/Core/Session/SessionAuthenticator.swift +++ b/Flipcash/Core/Session/SessionAuthenticator.swift @@ -425,6 +425,10 @@ final class SessionAuthenticator { // Drop cached notification previews — they hold chat text in cleartext in the App Group. NotificationPreviewCache.clear() + // Drop cached avatars for the same reason: other people's photographs, unencrypted, in a + // container the next account on the device reads from. + ProfilePictureCache.shared.clear() + // Drop the locally collected onramp email — it must not leak into // another account's Coinbase orders. CoinbaseOrderEmail.unverifiedEmail = nil @@ -459,7 +463,7 @@ final class SessionContainer { let conversationController: ConversationController let blocklistController: BlocklistController let chatSpotlightIndexer: ChatSpotlightIndexer - let tipAvatars: TipAvatarStore + let profileAvatars: ProfileAvatarStore /// Lazy so it can capture the container it reads its dependencies from; /// observation-ignored because `TipFlow` is itself observable and the @@ -582,7 +586,7 @@ final class SessionContainer { chatSpotlightIndexer.start() self.chatSpotlightIndexer = chatSpotlightIndexer - self.tipAvatars = TipAvatarStore(flipClient: flipClient, owner: session.ownerKeyPair) + self.profileAvatars = ProfileAvatarStore(flipClient: flipClient, owner: session.ownerKeyPair) } fileprivate func injectingEnvironment(into view: SomeView) -> some View where SomeView: View { diff --git a/FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift b/FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift new file mode 100644 index 000000000..ebc19500d --- /dev/null +++ b/FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift @@ -0,0 +1,139 @@ +// +// ProfilePictureCache.swift +// FlipcashCore +// +// Copyright © 2026 Code Inc. All rights reserved. +// + +import Foundation + +/// Profile-picture bytes on disk, keyed by blob id. +/// +/// A blob is immutable — a user who changes their picture gets a new id — so a hit never needs +/// revalidating, and the entry never goes stale. That is what the URL-keyed HTTP cache cannot do +/// here: `blobDownloadURL` mints a fresh signed URL on every fetch, so the same picture arrives +/// under a different key each time and every launch re-downloads it. +/// +/// It lives in the App Group container so the notification extension and the app are looking at the +/// same bytes: the extension can warm an avatar on push arrival and the app draws it at first paint. +/// +/// Best-effort throughout. A failed read or write costs a re-download, never a broken screen. +public final class ProfilePictureCache: @unchecked Sendable { + + /// The App Group shared by the app and both notification extensions. + public static let appGroup = NotificationPreviewCache.appGroup + + /// The process-wide instance, in the App Group container. + public static let shared = ProfilePictureCache() + + /// Roughly 300 avatar thumbnails. Avatars accumulate for every counterparty the user ever sees, + /// so the directory needs a ceiling; the coldest entries go first. + public static let defaultLimitBytes = 24 * 1024 * 1024 + + private let directory: URL + private let limitBytes: Int + private let fileManager = FileManager.default + + /// Serializes eviction against concurrent writes. Avatars are fetched from several screens at + /// once, and two overlapping evictions would each size the directory before the other's delete. + private let lock = NSLock() + + /// - Parameters: + /// - directory: where the bytes go. Injected so tests run against a temporary directory — + /// `containerURL(forSecurityApplicationGroupIdentifier:)` resolves differently on macOS, + /// where this package's tests run, than it does on a device. + /// - limitBytes: the ceiling for the whole directory. + public init(directory: URL, limitBytes: Int = ProfilePictureCache.defaultLimitBytes) { + self.directory = directory + self.limitBytes = limitBytes + } + + private convenience init() { + let container = FileManager.default + .containerURL(forSecurityApplicationGroupIdentifier: ProfilePictureCache.appGroup) + ?? URL.cachesDirectory + self.init(directory: container.appendingPathComponent("Avatars", isDirectory: true)) + } + + // MARK: - Reading - + + /// The cached bytes for a blob, or nil on a miss. + /// + /// A hit touches the file's modification date, which is what eviction orders by — so the + /// pictures on the screen the user just left survive a round of eviction. + public func data(for blobID: BlobID) -> Data? { + let url = fileURL(for: blobID) + guard let data = try? Data(contentsOf: url) else { return nil } + try? fileManager.setAttributes([.modificationDate: Date()], ofItemAtPath: url.path) + return data + } + + /// Whether the blob's bytes are already on disk, without reading them. + public func holds(_ blobID: BlobID) -> Bool { + fileManager.fileExists(atPath: fileURL(for: blobID).path) + } + + // MARK: - Writing - + + /// Stores the bytes for a blob, evicting the coldest entries if that puts the directory over + /// its limit. Best-effort. + /// + /// A blob larger than the whole budget is not stored at all: writing it would evict everything + /// else and then itself, leaving the caller to re-download it on every read. + public func write(_ data: Data, for blobID: BlobID) { + guard data.count <= limitBytes else { return } + + lock.lock() + defer { lock.unlock() } + + do { + try fileManager.createDirectory(at: directory, withIntermediateDirectories: true) + try data.write(to: fileURL(for: blobID), options: .atomic) + } catch { + return + } + + evictDownToLimit() + } + + /// Deletes every cached picture. Call on logout: the bytes are other users' photographs sitting + /// unencrypted in a container the next account on the device would read from. + public func clear() { + lock.lock() + defer { lock.unlock() } + try? fileManager.removeItem(at: directory) + } + + // MARK: - Internal - + + /// `ID.description` is the blob's 32 hex characters, which is already a valid file name. + private func fileURL(for blobID: BlobID) -> URL { + directory.appendingPathComponent(blobID.description) + } + + /// Deletes least-recently-used entries until the directory fits. Caller holds `lock`. + private func evictDownToLimit() { + let keys: [URLResourceKey] = [.contentAccessDateKey, .contentModificationDateKey, .fileSizeKey] + guard let contents = try? fileManager.contentsOfDirectory( + at: directory, includingPropertiesForKeys: keys, options: .skipsHiddenFiles + ) else { return } + + var entries: [(url: URL, date: Date, size: Int)] = contents.compactMap { url in + guard let values = try? url.resourceValues(forKeys: Set(keys)), + let size = values.fileSize + else { return nil } + return (url, values.contentModificationDate ?? .distantPast, size) + } + + var total = entries.reduce(0) { $0 + $1.size } + guard total > limitBytes else { return } + + // Coldest first. + entries.sort { $0.date < $1.date } + + for entry in entries where total > limitBytes { + try? fileManager.removeItem(at: entry.url) + total -= entry.size + } + } +} diff --git a/FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift b/FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift new file mode 100644 index 000000000..7ad7c988c --- /dev/null +++ b/FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift @@ -0,0 +1,122 @@ +// +// ProfilePictureCacheTests.swift +// FlipcashCoreTests +// +// Copyright © 2026 Code Inc. All rights reserved. +// + +import Testing +import Foundation +@testable import FlipcashCore + +@Suite("Profile picture cache") +struct ProfilePictureCacheTests { + + private func cache(limitBytes: Int = 1 << 20) -> ProfilePictureCache { + ProfilePictureCache( + directory: FileManager.default.temporaryDirectory + .appendingPathComponent("avatars-\(UUID().uuidString)", isDirectory: true), + limitBytes: limitBytes + ) + } + + private func blob(_ byte: UInt8) -> BlobID { + BlobID(data: Data(repeating: byte, count: 16)) + } + + // MARK: - Round trip - + + @Test func writtenBytesReadBack() { + let cache = cache() + cache.write(Data([1, 2, 3]), for: blob(1)) + + #expect(cache.data(for: blob(1)) == Data([1, 2, 3])) + } + + @Test func missReturnsNil() { + #expect(cache().data(for: blob(1)) == nil) + } + + @Test func blobsDoNotCollide() { + let cache = cache() + cache.write(Data([1]), for: blob(1)) + cache.write(Data([2]), for: blob(2)) + + #expect(cache.data(for: blob(1)) == Data([1])) + #expect(cache.data(for: blob(2)) == Data([2])) + } + + /// The app and the notification extension are separate processes holding separate instances over + /// the same App Group directory. A write is only useful if the other side can read it, so the + /// bytes have to live on disk under a name derived from the blob id alone — no in-process index. + @Test func anotherInstanceOverTheSameDirectoryReadsTheSameBytes() { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("avatars-\(UUID().uuidString)", isDirectory: true) + let writer = ProfilePictureCache(directory: directory, limitBytes: 1 << 20) + let reader = ProfilePictureCache(directory: directory, limitBytes: 1 << 20) + + writer.write(Data([9, 9, 9]), for: blob(4)) + + #expect(reader.data(for: blob(4)) == Data([9, 9, 9])) + } + + // MARK: - Clearing - + + @Test func clearDropsEverything() { + let cache = cache() + cache.write(Data([1]), for: blob(1)) + cache.clear() + + #expect(cache.data(for: blob(1)) == nil) + } + + /// Logout clears the cache and the account stays usable, so a write after one has to recreate the + /// directory rather than silently failing for the rest of the process. + @Test func writingAfterClearRecreatesTheCache() { + let cache = cache() + cache.write(Data([1]), for: blob(1)) + cache.clear() + cache.write(Data([2]), for: blob(2)) + + #expect(cache.data(for: blob(2)) == Data([2])) + } + + // MARK: - Eviction - + + /// Avatars accumulate for every counterparty the user ever sees, so the directory needs a ceiling. + @Test func writingPastTheLimitEvicts() { + let cache = cache(limitBytes: 300) + for byte in UInt8(1)...UInt8(6) { + cache.write(Data(repeating: byte, count: 100), for: blob(byte)) + } + + let remaining = (UInt8(1)...UInt8(6)).filter { cache.data(for: blob($0)) != nil } + #expect(remaining.count < 6) + } + + /// Eviction takes the coldest entries, so the picture drawn on the screen the user just left is + /// still there when they come back to it. + @Test func evictionKeepsTheMostRecentlyUsed() { + let cache = cache(limitBytes: 300) + cache.write(Data(repeating: 1, count: 100), for: blob(1)) + cache.write(Data(repeating: 2, count: 100), for: blob(2)) + + // Reading blob 1 makes blob 2 the coldest entry. + _ = cache.data(for: blob(1)) + cache.write(Data(repeating: 3, count: 100), for: blob(3)) + cache.write(Data(repeating: 4, count: 100), for: blob(4)) + + #expect(cache.data(for: blob(4)) == Data(repeating: 4, count: 100)) + #expect(cache.data(for: blob(1)) != nil) + } + + /// A single blob larger than the whole budget must not evict itself into a permanent miss, or the + /// caller re-downloads it on every read. + @Test func anOversizedBlobIsNotCached() { + let cache = cache(limitBytes: 100) + cache.write(Data(repeating: 1, count: 500), for: blob(1)) + cache.write(Data(repeating: 2, count: 50), for: blob(2)) + + #expect(cache.data(for: blob(2)) == Data(repeating: 2, count: 50)) + } +} diff --git a/FlipcashTests/ContactAvatarCacheTests.swift b/FlipcashTests/ContactAvatarCacheTests.swift new file mode 100644 index 000000000..f6d6fb16c --- /dev/null +++ b/FlipcashTests/ContactAvatarCacheTests.swift @@ -0,0 +1,61 @@ +// +// ContactAvatarCacheTests.swift +// FlipcashTests +// + +import Testing +import UIKit +import FlipcashUI + +@Suite("ContactAvatarCache") +struct ContactAvatarCacheTests { + + /// A PNG exactly `side` pixels square. `scale = 1` so the decoded image's `size` is the number + /// asked for: the renderer otherwise draws at the screen's scale, and `UIImage(data:)` reads a + /// PNG back at scale 1, which turns an 8pt request into a 24pt image on a 3x device. + private func png(side: Int) -> Data { + let format = UIGraphicsImageRendererFormat.default() + format.scale = 1 + let size = CGSize(width: side, height: side) + return UIGraphicsImageRenderer(size: size, format: format).image { context in + UIColor.red.setFill() + context.fill(CGRect(origin: .zero, size: size)) + }.pngData()! + } + + @Test("Decodes and caches the bytes it is given") + func decodesBytes() { + let image = ContactAvatarCache.shared.image(forKey: UUID().uuidString, data: png(side: 4)) + + #expect(image?.size == CGSize(width: 4, height: 4)) + } + + @Test("Returns nil for bytes that are not an image") + func rejectsGarbage() { + #expect(ContactAvatarCache.shared.image(forKey: UUID().uuidString, data: Data([0, 1, 2])) == nil) + } + + /// The key is the contact's id, which survives the contact changing their picture. Serving the + /// entry cached under it without checking the bytes leaves the old photo on screen for the rest + /// of the process — every avatar surface in the app reads through this cache. + @Test("New bytes under an existing key replace the cached image") + func newBytesUnderSameKeyReplaceTheImage() { + let key = UUID().uuidString + + _ = ContactAvatarCache.shared.image(forKey: key, data: png(side: 4)) + let updated = ContactAvatarCache.shared.image(forKey: key, data: png(side: 8)) + + #expect(updated?.size == CGSize(width: 8, height: 8)) + } + + @Test("Unchanged bytes under an existing key reuse the decoded image") + func unchangedBytesReuseTheDecodedImage() { + let key = UUID().uuidString + let data = png(side: 4) + + let first = ContactAvatarCache.shared.image(forKey: key, data: data) + let second = ContactAvatarCache.shared.image(forKey: key, data: data) + + #expect(first === second) + } +} diff --git a/FlipcashUI/Sources/FlipcashUI/Images/RemoteImageLoader.swift b/FlipcashUI/Sources/FlipcashUI/Images/RemoteImageLoader.swift deleted file mode 100644 index 0d4db4de2..000000000 --- a/FlipcashUI/Sources/FlipcashUI/Images/RemoteImageLoader.swift +++ /dev/null @@ -1,29 +0,0 @@ -// -// RemoteImageLoader.swift -// FlipcashUI -// - -import UIKit -import Kingfisher - -/// Loads a remote image eagerly, for callers that need the bytes rather than a -/// view — rendering to an image, or compositing into a graphic. -public enum RemoteImageLoader { - - /// Returns the image at `url`, cached under `cacheKey`. - /// - /// Pass a durable key for a signed URL: those are re-minted on every fetch, - /// so a URL-derived key would re-download identical bytes each time. - public static func image(at url: URL, cacheKey: String) async throws -> UIImage { - let resource = KF.ImageResource(downloadURL: url, cacheKey: cacheKey) - return try await KingfisherManager.shared.retrieveImage(with: resource).image - } - - /// The image already cached under `cacheKey`, or nil when it isn't cached. - /// - /// Never touches the network, so a caller holding a durable key can draw a - /// previously-fetched image before it mints a signed URL to re-fetch it. - public static func cachedImage(cacheKey: String) async -> UIImage? { - try? await KingfisherManager.shared.cache.retrieveImage(forKey: cacheKey).image - } -} diff --git a/FlipcashUI/Sources/FlipcashUI/Views/Contacts/ContactAvatarView.swift b/FlipcashUI/Sources/FlipcashUI/Views/Contacts/ContactAvatarView.swift index ecf25f2c3..397a60a76 100644 --- a/FlipcashUI/Sources/FlipcashUI/Views/Contacts/ContactAvatarView.swift +++ b/FlipcashUI/Sources/FlipcashUI/Views/Contacts/ContactAvatarView.swift @@ -97,7 +97,18 @@ nonisolated public final class ContactAvatarCache: @unchecked Sendable { public static let shared = ContactAvatarCache() - private let cache = NSCache() + /// A decoded image and a fingerprint of the bytes it was decoded from. + private final class Entry { + let image: UIImage + let fingerprint: Int + + init(image: UIImage, fingerprint: Int) { + self.image = image + self.fingerprint = fingerprint + } + } + + private let cache = NSCache() private init() { cache.countLimit = 200 @@ -106,14 +117,34 @@ nonisolated public final class ContactAvatarCache: @unchecked Sendable { /// Returns the cached `UIImage` for `key`. Decodes `data` and caches the /// result on a miss; returns `nil` when `data` doesn't yield a valid /// image. + /// + /// The key is a contact or user id, which survives that person changing + /// their picture — so a hit is only a hit when the bytes match too. + /// Returning the entry on the id alone leaves the old photo on screen for + /// the rest of the process, on every surface, since they all read through + /// here. public func image(forKey key: String, data: Data) -> UIImage? { - if let cached = cache.object(forKey: key as NSString) { - return cached + let fingerprint = Self.fingerprint(of: data) + + if let cached = cache.object(forKey: key as NSString), cached.fingerprint == fingerprint { + return cached.image } + guard let image = UIImage(data: data) else { return nil } - cache.setObject(image, forKey: key as NSString) + cache.setObject(Entry(image: image, fingerprint: fingerprint), forKey: key as NSString) return image } + + /// Identifies the bytes without retaining them — holding the source `Data` + /// alongside every decoded image would roughly double what the cache costs. + /// `Data`'s hash is seeded per process, which is the same lifetime as the + /// cache, and the length guards the collision. + private static func fingerprint(of data: Data) -> Int { + var hasher = Hasher() + hasher.combine(data.count) + hasher.combine(data) + return hasher.finalize() + } } /// The shared "people" silhouette glyph. The avatar placeholder and the From cca535c3b765f46a3e01ab9603d40dede8619304 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 11 Sep 2026 16:52:37 -0400 Subject: [PATCH 2/2] refactor(avatars): generalize the byte cache to any blob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chat images are blobs too, and everything that makes this cache work for avatars is true of them: keyed by blob id, immutable so a hit never needs revalidating, LRU-bounded, in the App Group container. The only avatar-specific parts were the subdirectory name and the budget. Renaming it before a second call site exists costs nothing; renaming it after does. Each kind gets its own directory and its own ceiling rather than sharing a pool. An avatar thumbnail is tens of kilobytes and a full-size chat image is single-digit megabytes, so one shared 24 MB budget would let a scroll through an image-heavy conversation evict every avatar on the device — back to blurhashes on the next launch, which is the bug this cache exists to fix. `oneKindOverItsLimitDoesNotEvictAnother` covers it. The kinds sit under a common `Blobs/` parent so `clearAll()` can drop the lot on logout without a registry of kinds to keep in sync. A kind missing from such a registry would leave other people's photographs on disk through a logout with nothing to surface the omission. The fetch path is deliberately left alone. `BlobAccessContext` has one case, `.profile(UserID)`, so there is nothing to authorize a chat-image download against yet; a generic fetcher today would be guessing at a proto that does not exist. --- .../Core/Controllers/ProfileAvatarStore.swift | 8 +-- .../Main/Profile/ProfilePictureLoader.swift | 2 +- .../Core/Session/SessionAuthenticator.swift | 4 +- ...filePictureCache.swift => BlobCache.swift} | 69 +++++++++++++------ ...eCacheTests.swift => BlobCacheTests.swift} | 37 +++++++--- 5 files changed, 82 insertions(+), 38 deletions(-) rename FlipcashCore/Sources/FlipcashCore/Images/{ProfilePictureCache.swift => BlobCache.swift} (62%) rename FlipcashCore/Tests/FlipcashCoreTests/{ProfilePictureCacheTests.swift => BlobCacheTests.swift} (75%) diff --git a/Flipcash/Core/Controllers/ProfileAvatarStore.swift b/Flipcash/Core/Controllers/ProfileAvatarStore.swift index b232f4b62..c77687bd9 100644 --- a/Flipcash/Core/Controllers/ProfileAvatarStore.swift +++ b/Flipcash/Core/Controllers/ProfileAvatarStore.swift @@ -11,7 +11,7 @@ private let logger = Logger(label: "flipcash.profile-avatars") /// Other users' profile-picture thumbnails, as raw bytes so every surface renders them through the /// same `ContactAvatarView(imageData:)` path address-book avatars use. /// -/// Backed by ``ProfilePictureCache``, so bytes fetched in one launch are on screen at first paint in +/// Backed by ``BlobCache``, so bytes fetched in one launch are on screen at first paint in /// the next. The in-memory map is what `@Observable` watches — the disk cache alone cannot invalidate /// a view — and it is populated from disk without a round trip. @Observable @@ -24,7 +24,7 @@ final class ProfileAvatarStore { @ObservationIgnored private var blobByUser: [UserID: BlobID] = [:] @ObservationIgnored private var inFlight: Set = [] - @ObservationIgnored private let cache: ProfilePictureCache + @ObservationIgnored private let cache: BlobCache @ObservationIgnored private let mintURL: (BlobID, UserID) async throws -> URL? @ObservationIgnored private let fetch: (URL) async throws -> Data @@ -33,7 +33,7 @@ final class ProfileAvatarStore { /// class with a live gRPC channel, so there is no fake to pass a test. /// - fetch: downloads the bytes at a URL. init( - cache: ProfilePictureCache = .shared, + cache: BlobCache = .profilePictures, mintURL: @escaping (BlobID, UserID) async throws -> URL?, fetch: @escaping (URL) async throws -> Data = { url in try await URLSession.shared.data(from: url).0 @@ -44,7 +44,7 @@ final class ProfileAvatarStore { self.fetch = fetch } - convenience init(flipClient: FlipClient, owner: KeyPair, cache: ProfilePictureCache = .shared) { + convenience init(flipClient: FlipClient, owner: KeyPair, cache: BlobCache = .profilePictures) { self.init( cache: cache, mintURL: { blobID, userID in diff --git a/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift b/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift index 30aafe579..7fa6f4106 100644 --- a/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift +++ b/Flipcash/Core/Screens/Main/Profile/ProfilePictureLoader.swift @@ -22,7 +22,7 @@ enum ProfilePictureLoader { for picture: ProfilePicture?, using client: FlipClient, owner: KeyPair, - cache: ProfilePictureCache = .shared + cache: BlobCache = .profilePictures ) async -> UIImage? { guard let blobID = picture?.thumbnailBlobID else { return nil } diff --git a/Flipcash/Core/Session/SessionAuthenticator.swift b/Flipcash/Core/Session/SessionAuthenticator.swift index b4542ed58..df75283b2 100644 --- a/Flipcash/Core/Session/SessionAuthenticator.swift +++ b/Flipcash/Core/Session/SessionAuthenticator.swift @@ -425,9 +425,9 @@ final class SessionAuthenticator { // Drop cached notification previews — they hold chat text in cleartext in the App Group. NotificationPreviewCache.clear() - // Drop cached avatars for the same reason: other people's photographs, unencrypted, in a + // Drop every cached blob for the same reason: other people's photographs, unencrypted, in a // container the next account on the device reads from. - ProfilePictureCache.shared.clear() + BlobCache.clearAll() // Drop the locally collected onramp email — it must not leak into // another account's Coinbase orders. diff --git a/FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift b/FlipcashCore/Sources/FlipcashCore/Images/BlobCache.swift similarity index 62% rename from FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift rename to FlipcashCore/Sources/FlipcashCore/Images/BlobCache.swift index ebc19500d..b5f0b0fc3 100644 --- a/FlipcashCore/Sources/FlipcashCore/Images/ProfilePictureCache.swift +++ b/FlipcashCore/Sources/FlipcashCore/Images/BlobCache.swift @@ -1,5 +1,5 @@ // -// ProfilePictureCache.swift +// BlobCache.swift // FlipcashCore // // Copyright © 2026 Code Inc. All rights reserved. @@ -7,34 +7,53 @@ import Foundation -/// Profile-picture bytes on disk, keyed by blob id. +/// Blob bytes on disk, keyed by blob id. /// -/// A blob is immutable — a user who changes their picture gets a new id — so a hit never needs -/// revalidating, and the entry never goes stale. That is what the URL-keyed HTTP cache cannot do -/// here: `blobDownloadURL` mints a fresh signed URL on every fetch, so the same picture arrives -/// under a different key each time and every launch re-downloads it. +/// A blob is immutable — a user who changes their profile picture gets a new id — so a hit never +/// needs revalidating and an entry never goes stale. That is what the URL-keyed HTTP cache cannot +/// do here: `blobDownloadURL` mints a fresh signed URL on every fetch, so the same bytes arrive +/// under a different key each time and every launch re-downloads them. /// /// It lives in the App Group container so the notification extension and the app are looking at the /// same bytes: the extension can warm an avatar on push arrival and the app draws it at first paint. /// /// Best-effort throughout. A failed read or write costs a re-download, never a broken screen. -public final class ProfilePictureCache: @unchecked Sendable { +/// +/// ## One instance per kind of blob +/// +/// Each kind gets its own directory and its own ceiling — see ``profilePictures``. The budgets are +/// separate because the sizes are: an avatar thumbnail is tens of kilobytes and a full-size chat +/// image is single-digit megabytes, so one shared pool would let a single scroll through an +/// image-heavy conversation evict every avatar on the device. The next launch would be back to +/// blurhashes, which is the problem this cache exists to fix. +public final class BlobCache: @unchecked Sendable { /// The App Group shared by the app and both notification extensions. public static let appGroup = NotificationPreviewCache.appGroup - /// The process-wide instance, in the App Group container. - public static let shared = ProfilePictureCache() + /// Profile-picture thumbnails, for the signed-in user and for everyone they see. + /// + /// Roughly 300 of them. Avatars accumulate for every counterparty the user ever encounters, so + /// the directory needs a ceiling; the coldest entries go first. + public static let profilePictures = BlobCache(name: "Avatars", limitBytes: 24 * 1024 * 1024) - /// Roughly 300 avatar thumbnails. Avatars accumulate for every counterparty the user ever sees, - /// so the directory needs a ceiling; the coldest entries go first. - public static let defaultLimitBytes = 24 * 1024 * 1024 + /// The parent of every kind's directory. + /// + /// Having one lets ``clearAll()`` delete the lot without a registry of kinds to keep in sync. A + /// kind left out of such a registry would leave other people's photographs on disk through a + /// logout, and nothing would surface the omission. + private static var rootDirectory: URL { + let container = FileManager.default + .containerURL(forSecurityApplicationGroupIdentifier: BlobCache.appGroup) + ?? URL.cachesDirectory + return container.appendingPathComponent("Blobs", isDirectory: true) + } private let directory: URL private let limitBytes: Int private let fileManager = FileManager.default - /// Serializes eviction against concurrent writes. Avatars are fetched from several screens at + /// Serializes eviction against concurrent writes. Blobs are fetched from several screens at /// once, and two overlapping evictions would each size the directory before the other's delete. private let lock = NSLock() @@ -43,16 +62,17 @@ public final class ProfilePictureCache: @unchecked Sendable { /// `containerURL(forSecurityApplicationGroupIdentifier:)` resolves differently on macOS, /// where this package's tests run, than it does on a device. /// - limitBytes: the ceiling for the whole directory. - public init(directory: URL, limitBytes: Int = ProfilePictureCache.defaultLimitBytes) { + public init(directory: URL, limitBytes: Int) { self.directory = directory self.limitBytes = limitBytes } - private convenience init() { - let container = FileManager.default - .containerURL(forSecurityApplicationGroupIdentifier: ProfilePictureCache.appGroup) - ?? URL.cachesDirectory - self.init(directory: container.appendingPathComponent("Avatars", isDirectory: true)) + /// One kind's cache, in its own subdirectory of the App Group container. + private convenience init(name: String, limitBytes: Int) { + self.init( + directory: BlobCache.rootDirectory.appendingPathComponent(name, isDirectory: true), + limitBytes: limitBytes + ) } // MARK: - Reading - @@ -60,7 +80,7 @@ public final class ProfilePictureCache: @unchecked Sendable { /// The cached bytes for a blob, or nil on a miss. /// /// A hit touches the file's modification date, which is what eviction orders by — so the - /// pictures on the screen the user just left survive a round of eviction. + /// entries backing the screen the user just left survive a round of eviction. public func data(for blobID: BlobID) -> Data? { let url = fileURL(for: blobID) guard let data = try? Data(contentsOf: url) else { return nil } @@ -96,14 +116,19 @@ public final class ProfilePictureCache: @unchecked Sendable { evictDownToLimit() } - /// Deletes every cached picture. Call on logout: the bytes are other users' photographs sitting - /// unencrypted in a container the next account on the device would read from. + /// Deletes every blob this cache holds. public func clear() { lock.lock() defer { lock.unlock() } try? fileManager.removeItem(at: directory) } + /// Deletes every cached blob of every kind. Call on logout: these are other people's + /// photographs sitting unencrypted in a container the next account on the device reads from. + public static func clearAll() { + try? FileManager.default.removeItem(at: rootDirectory) + } + // MARK: - Internal - /// `ID.description` is the blob's 32 hex characters, which is already a valid file name. diff --git a/FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift b/FlipcashCore/Tests/FlipcashCoreTests/BlobCacheTests.swift similarity index 75% rename from FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift rename to FlipcashCore/Tests/FlipcashCoreTests/BlobCacheTests.swift index 7ad7c988c..7ff0b08f5 100644 --- a/FlipcashCore/Tests/FlipcashCoreTests/ProfilePictureCacheTests.swift +++ b/FlipcashCore/Tests/FlipcashCoreTests/BlobCacheTests.swift @@ -1,5 +1,5 @@ // -// ProfilePictureCacheTests.swift +// BlobCacheTests.swift // FlipcashCoreTests // // Copyright © 2026 Code Inc. All rights reserved. @@ -9,13 +9,13 @@ import Testing import Foundation @testable import FlipcashCore -@Suite("Profile picture cache") -struct ProfilePictureCacheTests { +@Suite("Blob cache") +struct BlobCacheTests { - private func cache(limitBytes: Int = 1 << 20) -> ProfilePictureCache { - ProfilePictureCache( + private func cache(limitBytes: Int = 1 << 20) -> BlobCache { + BlobCache( directory: FileManager.default.temporaryDirectory - .appendingPathComponent("avatars-\(UUID().uuidString)", isDirectory: true), + .appendingPathComponent("blobs-\(UUID().uuidString)", isDirectory: true), limitBytes: limitBytes ) } @@ -51,9 +51,9 @@ struct ProfilePictureCacheTests { /// bytes have to live on disk under a name derived from the blob id alone — no in-process index. @Test func anotherInstanceOverTheSameDirectoryReadsTheSameBytes() { let directory = FileManager.default.temporaryDirectory - .appendingPathComponent("avatars-\(UUID().uuidString)", isDirectory: true) - let writer = ProfilePictureCache(directory: directory, limitBytes: 1 << 20) - let reader = ProfilePictureCache(directory: directory, limitBytes: 1 << 20) + .appendingPathComponent("blobs-\(UUID().uuidString)", isDirectory: true) + let writer = BlobCache(directory: directory, limitBytes: 1 << 20) + let reader = BlobCache(directory: directory, limitBytes: 1 << 20) writer.write(Data([9, 9, 9]), for: blob(4)) @@ -119,4 +119,23 @@ struct ProfilePictureCacheTests { #expect(cache.data(for: blob(2)) == Data(repeating: 2, count: 50)) } + + // MARK: - Kinds - + + /// Each kind of blob gets its own directory and its own ceiling. Sharing one pool would let a + /// scroll through an image-heavy conversation — megabytes apiece — evict every avatar on the + /// device, which is the bug this cache exists to fix. + @Test func oneKindOverItsLimitDoesNotEvictAnother() { + let avatars = cache(limitBytes: 1 << 20) + let images = cache(limitBytes: 4096) + + avatars.write(Data(repeating: 0xAA, count: 1024), for: blob(1)) + + // Enough writes to push `images` well past its own ceiling. + for byte in UInt8(10)...UInt8(20) { + images.write(Data(repeating: byte, count: 2048), for: blob(byte)) + } + + #expect(avatars.data(for: blob(1))?.count == 1024) + } }