Re: [PATCH damus] Move Kingfisher data to the Caches directory

[William Casarin]

On Sat, Apr 26, 2025 at 05:04:09PM -0700, Daniel D’Aquino wrote:
>This commit moves Kingfisher data to Apple's designated caches folder
>to avoid it from being backed up to iCloud.
>
>Closes: https://github.com/damus-io/damus/issues/2993
>Changelog-Fixed: Fixed issue where cached images would be backed up to iCloud
>Signed-off-by: Daniel D’Aquino <dan...@daquino.me>
>
>Closes: https://github.com/damus-io/damus/pull/3002
>---

LGTM

Reviewed-by: William Casarin <jb...@jb55.com>

> damus.xcodeproj/project.pbxproj | 8 +++
> damus/Util/ImageCacheMigrations.swift | 79 +++++++++++++++++++++++++++
> damus/damusApp.swift | 46 +---------------
> 3 files changed, 89 insertions(+), 44 deletions(-)
> create mode 100644 damus/Util/ImageCacheMigrations.swift
>
>diff --git a/damus.xcodeproj/project.pbxproj b/damus.xcodeproj/project.pbxproj
>index 52d69d6b4..42b935407 100644
>--- a/damus.xcodeproj/project.pbxproj
>+++ b/damus.xcodeproj/project.pbxproj
>@@ -1703,6 +1703,9 @@
> D7F360262CEBBD8B009D34DA /* PresentFullScreenItemNotify.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7EB00AF2CD59C8300660C07 /* PresentFullScreenItemNotify.swift */; };
> D7F360272CEBBDC0009D34DA /* DamusVideoControlsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7EFBA362CC322F300F45588 /* DamusVideoControlsView.swift */; };
> D7F360292CEBBE34009D34DA /* CodeScanner in Frameworks */ = {isa = PBXBuildFile; productRef = D7F360282CEBBE34009D34DA /* CodeScanner */; };
>+ D7FA46E52DBDAA7E002C9BB0 /* ImageCacheMigrations.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7FA46E42DBDAA75002C9BB0 /* ImageCacheMigrations.swift */; };
>+ D7FA46E62DBDAA7E002C9BB0 /* ImageCacheMigrations.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7FA46E42DBDAA75002C9BB0 /* ImageCacheMigrations.swift */; };
>+ D7FA46E72DBDAA7E002C9BB0 /* ImageCacheMigrations.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7FA46E42DBDAA75002C9BB0 /* ImageCacheMigrations.swift */; };
> D7FB10A72B0C371A00FA8D42 /* Log.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C2B10272A7B0F5C008AA43E /* Log.swift */; };
> D7FB14222BE5970000398331 /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = D7FB14212BE5970000398331 /* PrivacyInfo.xcprivacy */; };
> D7FB14252BE5A9A800398331 /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = D7FB14242BE5A9A800398331 /* PrivacyInfo.xcprivacy */; };
>@@ -2574,6 +2577,7 @@
> D7EDED2D2B128E8A0018B19C /* CollectionExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CollectionExtension.swift; sourceTree = "<group>"; };
> D7EDED322B12ACAE0018B19C /* DamusUserDefaults.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusUserDefaults.swift; sourceTree = "<group>"; };
> D7EFBA362CC322F300F45588 /* DamusVideoControlsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusVideoControlsView.swift; sourceTree = "<group>"; };
>+ D7FA46E42DBDAA75002C9BB0 /* ImageCacheMigrations.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageCacheMigrations.swift; sourceTree = "<group>"; };
> D7FB14212BE5970000398331 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
> D7FB14242BE5A9A800398331 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
> D7FD12252BD345A700CF195B /* FirstAidSettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FirstAidSettingsView.swift; sourceTree = "<group>"; };
>@@ -3312,6 +3316,7 @@
> 4C7FF7D628233637009601DB /* Util */ = {
> isa = PBXGroup;
> children = (
>+ D7FA46E42DBDAA75002C9BB0 /* ImageCacheMigrations.swift */,
> D73B74E02D8365B40067BDBC /* ExtraFonts.swift */,
> D7DB93042D66A43B00DA1EE5 /* Undistractor.swift */,
> D73E5F7E2C6AA066007EB227 /* DamusAliases.swift */,
>@@ -4675,6 +4680,7 @@
> 4CCEB7AE29B53D260078AA28 /* SearchingEventView.swift in Sources */,
> 4CF0ABE929844AF100D66079 /* AnyCodable.swift in Sources */,
> BA3759932ABCCEBA0018D73B /* CameraModel.swift in Sources */,
>+ D7FA46E72DBDAA7E002C9BB0 /* ImageCacheMigrations.swift in Sources */,
> D7100C5A2B76FD5100C59298 /* LogoView.swift in Sources */,
> 4C0A3F8F280F640A000448DE /* ThreadModel.swift in Sources */,
> 4C3AC79F2833115300E1F516 /* FollowButtonView.swift in Sources */,
>@@ -5217,6 +5223,7 @@
> 82D6FB432CD99F7900C925F4 /* KeychainStorage.swift in Sources */,
> 82D6FB442CD99F7900C925F4 /* Bech32.swift in Sources */,
> 82D6FB452CD99F7900C925F4 /* InputDismissKeyboard.swift in Sources */,
>+ D7FA46E62DBDAA7E002C9BB0 /* ImageCacheMigrations.swift in Sources */,
> 82D6FB462CD99F7900C925F4 /* Constants.swift in Sources */,
> 82D6FB472CD99F7900C925F4 /* LinkView.swift in Sources */,
> D7DB1FDF2D5A78CE00CF06DA /* NIP44.swift in Sources */,
>@@ -5681,6 +5688,7 @@
> D73E5E9F2C6A97F4007EB227 /* CreateAccountModel.swift in Sources */,
> D73E5EA12C6A97F4007EB227 /* SignalModel.swift in Sources */,
> 5CB017272D42C5C400A9ED05 /* TransactionsView.swift in Sources */,
>+ D7FA46E52DBDAA7E002C9BB0 /* ImageCacheMigrations.swift in Sources */,
> D73E5EA22C6A97F4007EB227 /* FollowTarget.swift in Sources */,
> D73E5EA32C6A97F4007EB227 /* BookmarksManager.swift in Sources */,
> D73E5EA42C6A97F4007EB227 /* EventsModel.swift in Sources */,
>diff --git a/damus/Util/ImageCacheMigrations.swift b/damus/Util/ImageCacheMigrations.swift
>new file mode 100644
>index 000000000..c50117fd4
>--- /dev/null
>+++ b/damus/Util/ImageCacheMigrations.swift
>@@ -0,0 +1,79 @@
>+//
>+// ImageCacheMigrations.swift
>+// damus
>+//
>+// Created by Daniel D’Aquino on 2025-04-26.
>+//
>+
>+import Foundation
>+import Kingfisher
>+
>+struct ImageCacheMigrations {
>+ static func migrateKingfisherCacheIfNeeded() {
>+ let fileManager = FileManager.default
>+ let defaults = UserDefaults.standard
>+ let migration1Key = "KingfisherCacheMigrated" // Never ever changes
>+ let migration2Key = "KingfisherCacheMigratedV2" // Never ever changes
>+
>+ let migration1Done = defaults.bool(forKey: migration1Key)
>+ let migration2Done = defaults.bool(forKey: migration2Key)
>+
>+ guard !migration1Done || !migration2Done else {
>+ // All migrations are already done. Skip.
>+ return
>+ }
>+
>+ let oldCachePath = migration1Done ? migration1KingfisherCachePath() : migration0KingfisherCachePath()
>+
>+ // New shared cache location
>+ let newCachePath = kingfisherCachePath().path
>+
>+ if fileManager.fileExists(atPath: oldCachePath) {
>+ do {
>+ // Move the old cache to the new location
>+ try fileManager.moveItem(atPath: oldCachePath, toPath: newCachePath)
>+ Log.info("Successfully migrated Kingfisher cache to %s", for: .storage, newCachePath)
>+ } catch {
>+ do {
>+ // Cache data is not essential, fallback to deleting the cache and starting all over
>+ // It's better than leaving significant garbage data stuck indefinitely on the user's phone
>+ try fileManager.removeItem(atPath: newCachePath)
>+ try fileManager.removeItem(atPath: oldCachePath)
>+ }
>+ catch {
>+ Log.error("Failed to migrate cache: %s", for: .storage, error.localizedDescription)
>+ return // Do not mark them as complete, we can try again next time the user reloads the app
>+ }
>+ }
>+ }
>+
>+ // Mark migrations as complete
>+ defaults.set(true, forKey: migration1Key)
>+ defaults.set(true, forKey: migration2Key)
>+ }
>+
>+ static private func migration0KingfisherCachePath() -> String {
>+ // Implementation note: These are old, so they should not be changed
>+ let defaultCache = ImageCache.default
>+ return defaultCache.diskStorage.directoryURL.path
>+ }
>+
>+ static private func migration1KingfisherCachePath() -> String {
>+ // Implementation note: These are old, so they are hard-coded on purpose, because we can't change these values from the past.
>+ let groupURL = FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: "group.com.damus")!
>+ return groupURL.appendingPathComponent("ImageCache").path
>+ }
>+
>+ /// The latest path for kingfisher to store cached images on.
>+ ///
>+ /// Documentation references:
>+ /// - https://developer.apple.com/documentation/foundation/filemanager/containerurl(forsecurityapplicationgroupidentifier:)#:~:text=The%20system%20creates%20only%20the%20Library/Caches%20subdirectory%20automatically
>+ /// - https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html#:~:text=Put%20data%20cache,files%20as%20needed.
>+ static func kingfisherCachePath() -> URL {
>+ let groupURL = FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: Constants.DAMUS_APP_GROUP_IDENTIFIER)!
>+ return groupURL
>+ .appendingPathComponent("Library")
>+ .appendingPathComponent("Caches")
>+ .appendingPathComponent(Constants.IMAGE_CACHE_DIRNAME)
>+ }
>+}
>diff --git a/damus/damusApp.swift b/damus/damusApp.swift
>index 5db118a79..a53f57842 100644
>--- a/damus/damusApp.swift
>+++ b/damus/damusApp.swift
>@@ -80,7 +80,7 @@ class AppDelegate: NSObject, UIApplicationDelegate, UNUserNotificationCenterDele
> UNUserNotificationCenter.current().delegate = self
> SKPaymentQueue.default().add(StoreObserver.standard)
> registerNotificationCategories()
>- migrateKingfisherCacheIfNeeded()
>+ ImageCacheMigrations.migrateKingfisherCacheIfNeeded()
> configureKingfisherCache()
> return true
> }
>@@ -113,50 +113,8 @@ class AppDelegate: NSObject, UIApplicationDelegate, UNUserNotificationCenterDele
> completionHandler()
> }
>
>- private func migrateKingfisherCacheIfNeeded() {
>- let fileManager = FileManager.default
>- let defaults = UserDefaults.standard
>- let migrationKey = "KingfisherCacheMigrated"
>-
>- // Check if migration has already been done
>- guard !defaults.bool(forKey: migrationKey) else { return }
>-
>- // Get the default Kingfisher cache (before we override it)
>- let defaultCache = ImageCache.default
>- let oldCachePath = defaultCache.diskStorage.directoryURL.path
>-
>- // New shared cache location
>- guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: Constants.DAMUS_APP_GROUP_IDENTIFIER) else { return }
>- let newCachePath = groupURL.appendingPathComponent(Constants.IMAGE_CACHE_DIRNAME).path
>-
>- // Check if the old cache exists
>- if fileManager.fileExists(atPath: oldCachePath) {
>- do {
>- // Move the old cache to the new location
>- try fileManager.moveItem(atPath: oldCachePath, toPath: newCachePath)
>- print("Successfully migrated Kingfisher cache to \(newCachePath)")
>- } catch {
>- print("Failed to migrate cache: \(error)")
>- // Optionally, copy instead of move if you want to preserve the old cache as a fallback
>- do {
>- try fileManager.copyItem(atPath: oldCachePath, toPath: newCachePath)
>- print("Copied cache instead due to error")
>- } catch {
>- print("Failed to copy cache: \(error)")
>- }
>- }
>- }
>-
>- // Mark migration as complete
>- defaults.set(true, forKey: migrationKey)
>- }
>-
> private func configureKingfisherCache() {
>- guard let groupURL = FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: Constants.DAMUS_APP_GROUP_IDENTIFIER) else {
>- return
>- }
>-
>- let cachePath = groupURL.appendingPathComponent(Constants.IMAGE_CACHE_DIRNAME)
>+ let cachePath = ImageCacheMigrations.kingfisherCachePath()
> if let cache = try? ImageCache(name: "sharedCache", cacheDirectoryURL: cachePath) {
> KingfisherManager.shared.cache = cache
> }