From ff88b3562d1b5e7bc804ab546b2cf3f3a70d7f23 Mon Sep 17 00:00:00 2001 From: Davide Date: Tue, 10 Dec 2024 20:33:35 +0100 Subject: [PATCH] Clean up dangling module preferences (#999) Erase on module removal, reference is otherwise lost forever. --- .../CDModulePreferencesRepositoryV3.swift | 7 +++++++ .../Views/Profile/ProfileCoordinator.swift | 7 +++++-- .../Business/ModulePreferences.swift | 4 ++++ .../Business/PreferencesManager.swift | 3 +++ .../Strategy/ModulePreferencesRepository.swift | 2 ++ .../UILibrary/Business/ProfileEditor.swift | 16 ++++++++++++++-- .../UILibraryTests/ProfileEditorTests.swift | 2 +- 7 files changed, 36 insertions(+), 5 deletions(-) diff --git a/Library/Sources/AppDataPreferences/Strategy/CDModulePreferencesRepositoryV3.swift b/Library/Sources/AppDataPreferences/Strategy/CDModulePreferencesRepositoryV3.swift index b07bbfa7..69e4266f 100644 --- a/Library/Sources/AppDataPreferences/Strategy/CDModulePreferencesRepositoryV3.swift +++ b/Library/Sources/AppDataPreferences/Strategy/CDModulePreferencesRepositoryV3.swift @@ -106,6 +106,13 @@ private final class CDModulePreferencesRepositoryV3: ModulePreferencesRepository } } + func erase() { + context.performAndWait { + entity.excludedEndpoints?.forEach(context.delete) + context.delete(entity) + } + } + func save() throws { try context.performAndWait { guard context.hasChanges else { diff --git a/Library/Sources/AppUIMain/Views/Profile/ProfileCoordinator.swift b/Library/Sources/AppUIMain/Views/Profile/ProfileCoordinator.swift index d79aee7c..d36f1c54 100644 --- a/Library/Sources/AppUIMain/Views/Profile/ProfileCoordinator.swift +++ b/Library/Sources/AppUIMain/Views/Profile/ProfileCoordinator.swift @@ -43,6 +43,9 @@ struct ProfileCoordinator: View { @EnvironmentObject private var iapManager: IAPManager + @EnvironmentObject + private var preferencesManager: PreferencesManager + let profileManager: ProfileManager let profileEditor: ProfileEditor @@ -133,7 +136,7 @@ private extension ProfileCoordinator { // standard: always save, warn if purchase required func onCommitEditingStandard() async throws { - let savedProfile = try await profileEditor.save(to: profileManager) + let savedProfile = try await profileEditor.save(to: profileManager, preferencesManager: preferencesManager) do { try iapManager.verify(savedProfile) } catch AppError.ineligibleProfile(let requiredFeatures) { @@ -151,7 +154,7 @@ private extension ProfileCoordinator { paywallReason = .init(requiredFeatures) return } - try await profileEditor.save(to: profileManager) + try await profileEditor.save(to: profileManager, preferencesManager: preferencesManager) onDismiss() } diff --git a/Library/Sources/CommonLibrary/Business/ModulePreferences.swift b/Library/Sources/CommonLibrary/Business/ModulePreferences.swift index 553897ec..8d7ee9be 100644 --- a/Library/Sources/CommonLibrary/Business/ModulePreferences.swift +++ b/Library/Sources/CommonLibrary/Business/ModulePreferences.swift @@ -51,6 +51,10 @@ public final class ModulePreferences: ObservableObject, ModulePreferencesReposit repository?.removeExcludedEndpoint(endpoint) } + public func erase() { + repository?.erase() + } + public func save() throws { try repository?.save() } diff --git a/Library/Sources/CommonLibrary/Business/PreferencesManager.swift b/Library/Sources/CommonLibrary/Business/PreferencesManager.swift index a31a0e56..80318702 100644 --- a/Library/Sources/CommonLibrary/Business/PreferencesManager.swift +++ b/Library/Sources/CommonLibrary/Business/PreferencesManager.swift @@ -70,6 +70,9 @@ private final class DummyModulePreferencesRepository: ModulePreferencesRepositor func removeExcludedEndpoint(_ endpoint: ExtendedEndpoint) { } + func erase() { + } + func save() throws { } } diff --git a/Library/Sources/CommonLibrary/Strategy/ModulePreferencesRepository.swift b/Library/Sources/CommonLibrary/Strategy/ModulePreferencesRepository.swift index bfa12049..a74b65e2 100644 --- a/Library/Sources/CommonLibrary/Strategy/ModulePreferencesRepository.swift +++ b/Library/Sources/CommonLibrary/Strategy/ModulePreferencesRepository.swift @@ -34,5 +34,7 @@ public protocol ModulePreferencesRepository { func removeExcludedEndpoint(_ endpoint: ExtendedEndpoint) + func erase() + func save() throws } diff --git a/Library/Sources/UILibrary/Business/ProfileEditor.swift b/Library/Sources/UILibrary/Business/ProfileEditor.swift index bf6e6383..7989af2b 100644 --- a/Library/Sources/UILibrary/Business/ProfileEditor.swift +++ b/Library/Sources/UILibrary/Business/ProfileEditor.swift @@ -188,7 +188,6 @@ extension ProfileEditor { // update local view editableProfile.modules = profile.modulesBuilders() - removedModules.removeAll() return profile } @@ -204,10 +203,23 @@ extension ProfileEditor { } @discardableResult - public func save(to profileManager: ProfileManager) async throws -> Profile { + public func save(to profileManager: ProfileManager, preferencesManager: PreferencesManager) async throws -> Profile { do { let newProfile = try build() try await profileManager.save(newProfile, isLocal: true, remotelyShared: isShared) + + removedModules.keys.forEach { + do { + pp_log(.App.profiles, .info, "Erase preferences for removed module \($0)") + let repository = try preferencesManager.preferencesRepository(forModuleWithId: $0) + repository.erase() + try repository.save() + } catch { + pp_log(.App.profiles, .error, "Unable to erase preferences for removed module \($0): \(error)") + } + } + removedModules.removeAll() + return newProfile } catch { pp_log(.App.profiles, .fault, "Unable to save edited profile: \(error)") diff --git a/Library/Tests/UILibraryTests/ProfileEditorTests.swift b/Library/Tests/UILibraryTests/ProfileEditorTests.swift index cd7a6cc9..15ce4182 100644 --- a/Library/Tests/UILibraryTests/ProfileEditorTests.swift +++ b/Library/Tests/UILibraryTests/ProfileEditorTests.swift @@ -251,7 +251,7 @@ extension ProfileEditorTests { } .store(in: &subscriptions) - try await sut.save(to: manager) + try await sut.save(to: manager, preferencesManager: PreferencesManager()) await fulfillment(of: [exp]) } }