Fine-tune profile management with additional attributes (#807)

Additions to the domain: - Update rather than replace existing Core Data profile - Attach ProfileAttributes to Profile.userInfo - Store one-off `fingerprint` UUID on each save With the above in place, fix and improve ProfileManager to: - Use `fingerprint` to compare local/remote profiles in history and thus avoid local re-import of shared profiles - Use `deletingRemotely` to delete local profiles when removed from the remote repository (default false) - Use `isIncluded` filter to exclude certain profiles from the local repository (default nil)
2024-11-03 23:35:45 +01:00 · 2024-11-03 23:35:45 +01:00 · a22584c630
parent d59f408db8
commit a22584c630
15 changed files with 249 additions and 57 deletions
--- a/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
+++ b/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@ -41,7 +41,7 @@
      "kind" : "remoteSourceControl",
      "location" : "git@github.com:passepartoutvpn/passepartoutkit-source",
      "state" : {
-        "revision" : "b32b63ab8e09883f965737bb6214dfb81e38283a"
+        "revision" : "1b6bf03bb94e650852faabaa6b2161fe8b478151"
      }
    },
    {
--- a/Passepartout/Library/Package.swift
+++ b/Passepartout/Library/Package.swift
@ -40,7 +40,7 @@ let package = Package(
    ],
    dependencies: [
 //        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.9.0"),
-        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "b32b63ab8e09883f965737bb6214dfb81e38283a"),
+        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "1b6bf03bb94e650852faabaa6b2161fe8b478151"),
 //        .package(path: "../../../passepartoutkit-source"),
        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", from: "0.9.1"),
 //        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", revision: "031863a1cd683962a7dfe68e20b91fa820a1ecce"),
--- a/Passepartout/Library/Sources/AppDataProfiles/CDProfileRepositoryV3.swift
+++ b/Passepartout/Library/Sources/AppDataProfiles/CDProfileRepositoryV3.swift
@ -48,27 +48,56 @@ extension AppData {
                .init(key: "lastUpdate", ascending: true)
            ]
        } fromMapper: {
-            guard let encoded = $0.encoded else {
-                return nil
-            }
-            return try registry.decodedProfile(from: encoded, with: coder)
+            try fromMapper($0, registry: registry, coder: coder)
        } toMapper: {
-            let encoded = try registry.encodedProfile($0, with: coder)
-
-            let cdProfile = CDProfileV3(context: $1)
-            cdProfile.uuid = $0.id
-            cdProfile.name = $0.name
-            cdProfile.encoded = encoded
-            cdProfile.lastUpdate = Date()
-            return cdProfile
+            try toMapper($0, $1, $2, registry: registry, coder: coder)
        } onResultError: {
            onResultError?($0) ?? .ignore
        }
-
        return repository
    }
 }

+private extension AppData {
+    static func fromMapper(
+        _ cdEntity: CDProfileV3,
+        registry: Registry,
+        coder: ProfileCoder
+    ) throws -> Profile? {
+        guard let encoded = cdEntity.encoded else {
+            return nil
+        }
+        let profile = try registry.decodedProfile(from: encoded, with: coder)
+        var builder = profile.builder()
+        builder.attributes = ProfileAttributes(
+            lastUpdate: cdEntity.lastUpdate,
+            fingerprint: cdEntity.fingerprint
+        )
+        return try builder.tryBuild()
+    }
+
+    static func toMapper(
+        _ profile: Profile,
+        _ oldCdEntity: CDProfileV3?,
+        _ context: NSManagedObjectContext,
+        registry: Registry,
+        coder: ProfileCoder
+    ) throws -> CDProfileV3 {
+        let encoded = try registry.encodedProfile(profile, with: coder)
+
+        let cdProfile = oldCdEntity ?? CDProfileV3(context: context)
+        cdProfile.uuid = profile.id
+        cdProfile.name = profile.name
+        cdProfile.encoded = encoded
+
+        let attributes = profile.attributes
+        cdProfile.lastUpdate = attributes.lastUpdate
+        cdProfile.fingerprint = attributes.fingerprint
+
+        return cdProfile
+    }
+}
+
 // MARK: - Specialization

 extension CDProfileV3: CoreDataUniqueEntity {
--- a/Passepartout/Library/Sources/AppDataProfiles/Domain/CDProfileV3.swift
+++ b/Passepartout/Library/Sources/AppDataProfiles/Domain/CDProfileV3.swift
@ -36,4 +36,5 @@ final class CDProfileV3: NSManagedObject {
    @NSManaged var name: String?
    @NSManaged var encoded: String?
    @NSManaged var lastUpdate: Date?
+    @NSManaged var fingerprint: UUID?
 }
--- a/Passepartout/Library/Sources/AppDataProfiles/Profiles.xcdatamodeld/ProfilesV3.xcdatamodel/contents
+++ b/Passepartout/Library/Sources/AppDataProfiles/Profiles.xcdatamodeld/ProfilesV3.xcdatamodel/contents
@ -1,7 +1,8 @@
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="22758" systemVersion="23G93" minimumToolsVersion="Automatic" sourceLanguage="Swift" usedWithSwiftData="YES" userDefinedModelVersionIdentifier="">
+<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="22758" systemVersion="23H124" minimumToolsVersion="Automatic" sourceLanguage="Swift" usedWithSwiftData="YES" userDefinedModelVersionIdentifier="">
    <entity name="CDProfileV3" representedClassName="CDProfileV3" elementID="CDProfile" versionHashModifier="1" syncable="YES">
        <attribute name="encoded" optional="YES" attributeType="String" allowsCloudEncryption="YES"/>
+        <attribute name="fingerprint" optional="YES" attributeType="UUID" usesScalarValueType="NO"/>
        <attribute name="lastUpdate" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
        <attribute name="name" optional="YES" attributeType="String"/>
        <attribute name="uuid" optional="YES" attributeType="UUID" usesScalarValueType="NO"/>
--- a/Passepartout/Library/Sources/AppUIMain/Views/App/AppCoordinator.swift
+++ b/Passepartout/Library/Sources/AppUIMain/Views/App/AppCoordinator.swift
@ -191,10 +191,8 @@ extension AppCoordinator {

    func enterDetail(of profile: Profile) {
        profilePath = NavigationPath()
-        profileEditor.editProfile(
-            profile,
-            isShared: profileManager.isRemotelyShared(profileWithId: profile.id)
-        )
+        let isShared = profileManager.isRemotelyShared(profileWithId: profile.id)
+        profileEditor.editProfile(profile, isShared: isShared)
        present(.editProfile)
    }

--- a/Passepartout/Library/Sources/CommonLibrary/Business/InMemoryProfileRepository.swift
+++ b/Passepartout/Library/Sources/CommonLibrary/Business/InMemoryProfileRepository.swift
@ -45,7 +45,7 @@ public final class InMemoryProfileRepository: ProfileRepository {
        profilesSubject.eraseToAnyPublisher()
    }

-    public func saveProfile(_ profile: Profile) async throws {
+    public func saveProfile(_ profile: Profile) {
        pp_log(.app, .info, "Save profile: \(profile.id))")
        if let index = profiles.firstIndex(where: { $0.id == profile.id }) {
            profiles[index] = profile
@ -54,7 +54,7 @@ public final class InMemoryProfileRepository: ProfileRepository {
        }
    }

-    public func removeProfiles(withIds ids: [Profile.ID]) async throws {
+    public func removeProfiles(withIds ids: [Profile.ID]) {
        pp_log(.app, .info, "Remove profiles: \(ids)")
        profiles = profiles.filter {
            !ids.contains($0.id)
--- a/Passepartout/Library/Sources/CommonLibrary/Business/NEProfileRepository.swift
+++ b/Passepartout/Library/Sources/CommonLibrary/Business/NEProfileRepository.swift
@ -91,7 +91,9 @@ private extension NEProfileRepository {
    func onLoadedManagers(_ managers: [Profile.ID: NETunnelProviderManager]) {
        let profiles = managers.values.compactMap {
            do {
-                return try repository.profile(from: $0)
+                let profile = try repository.profile(from: $0)
+                pp_log(.app, .debug, "Attributes for profile \(profile.id): \(profile.attributes)")
+                return profile
            } catch {
                pp_log(.app, .error, "Unable to decode profile from NE manager '\($0.localizedDescription ?? "")': \(error)")
                return nil
@ -107,7 +109,7 @@ private extension NEProfileRepository {
                managers.keys.contains($0.id)
            }

-        let removedProfilesDesc = profilesSubject
+        let removedProfilesDescription = profilesSubject
            .value
            .filter {
                !managers.keys.contains($0.id)
@ -116,7 +118,7 @@ private extension NEProfileRepository {
                "\($0.name)(\($0.id)"
            }

-        pp_log(.app, .info, "Sync profiles removed externally: \(removedProfilesDesc)")
+        pp_log(.app, .info, "Sync profiles removed externally: \(removedProfilesDescription)")

        profilesSubject.send(profiles)
    }
--- a/Passepartout/Library/Sources/CommonLibrary/Business/ProfileManager.swift
+++ b/Passepartout/Library/Sources/CommonLibrary/Business/ProfileManager.swift
@ -41,6 +41,10 @@ public final class ProfileManager: ObservableObject {

    private let remoteRepository: (any ProfileRepository)?

+    private let deletingRemotely: Bool
+
+    private let isIncluded: ((Profile) -> Bool)?
+
    @Published
    private var profiles: [Profile]

@ -63,6 +67,8 @@ public final class ProfileManager: ObservableObject {
        repository = InMemoryProfileRepository(profiles: profiles)
        backupRepository = nil
        remoteRepository = nil
+        deletingRemotely = false
+        isIncluded = nil
        self.profiles = []
        allProfiles = profiles.reduce(into: [:]) {
            $0[$1.id] = $1
@ -77,11 +83,16 @@ public final class ProfileManager: ObservableObject {
    public init(
        repository: any ProfileRepository,
        backupRepository: (any ProfileRepository)? = nil,
-        remoteRepository: (any ProfileRepository)?
+        remoteRepository: (any ProfileRepository)?,
+        deletingRemotely: Bool = false,
+        isIncluded: ((Profile) -> Bool)? = nil
    ) {
+        precondition(!deletingRemotely || remoteRepository != nil, "deletingRemotely requires a non-nil remoteRepository")
        self.repository = repository
        self.backupRepository = backupRepository
        self.remoteRepository = remoteRepository
+        self.deletingRemotely = deletingRemotely
+        self.isIncluded = isIncluded
        profiles = []
        allProfiles = [:]
        allRemoteProfiles = [:]
@ -120,39 +131,39 @@ extension ProfileManager {
    }

    public func save(_ profile: Profile, isShared: Bool? = nil) async throws {
-        pp_log(.app, .notice, "Save profile \(profile.id)...")
+
+        // inject attributes
+        var builder = profile.builder()
+        builder.attributes.lastUpdate = Date()
+        builder.attributes.fingerprint = UUID()
+        let historifiedProfile = try builder.tryBuild()
+
+        pp_log(.app, .notice, "Save profile \(historifiedProfile.id)...")
        do {
-            let existingProfile = allProfiles[profile.id]
-            if existingProfile == nil || profile != existingProfile {
-                try await repository.saveProfile(profile)
-
-                if let backupRepository {
-                    Task.detached {
-                        try? await backupRepository.saveProfile(profile)
-                    }
+            try await repository.saveProfile(historifiedProfile)
+            if let backupRepository {
+                Task.detached {
+                    try await backupRepository.saveProfile(historifiedProfile)
                }
-
-                allProfiles[profile.id] = profile
-                didChange.send(.save(profile))
-            } else {
-                pp_log(.app, .notice, "Profile \(profile.id) not modified, not saving")
            }
+            allProfiles[historifiedProfile.id] = historifiedProfile
+            didChange.send(.save(historifiedProfile))
        } catch {
-            pp_log(.app, .fault, "Unable to save profile \(profile.id): \(error)")
+            pp_log(.app, .fault, "Unable to save profile \(historifiedProfile.id): \(error)")
            throw error
        }
        do {
            if let isShared, let remoteRepository {
                if isShared {
-                    pp_log(.app, .notice, "Enable remote sharing of profile \(profile.id)...")
-                    try await remoteRepository.saveProfile(profile)
+                    pp_log(.app, .notice, "Enable remote sharing of profile \(historifiedProfile.id)...")
+                    try await remoteRepository.saveProfile(historifiedProfile)
                } else {
-                    pp_log(.app, .notice, "Disable remote sharing of profile \(profile.id)...")
-                    try await remoteRepository.removeProfiles(withIds: [profile.id])
+                    pp_log(.app, .notice, "Disable remote sharing of profile \(historifiedProfile.id)...")
+                    try await remoteRepository.removeProfiles(withIds: [historifiedProfile.id])
                }
            }
        } catch {
-            pp_log(.app, .fault, "Unable to save/remove remote profile \(profile.id): \(error)")
+            pp_log(.app, .fault, "Unable to save/remove remote profile \(historifiedProfile.id): \(error)")
            throw error
        }
    }
@ -190,7 +201,7 @@ extension ProfileManager {
    }
 }

-// MARK: - Remote
+// MARK: - Remote/Attributes

 extension ProfileManager {
    public func isRemotelyShared(profileWithId profileId: Profile.ID) -> Bool {
@ -288,6 +299,21 @@ private extension ProfileManager {
        allProfiles = result.reduce(into: [:]) {
            $0[$1.id] = $1
        }
+
+        if let isIncluded {
+            let idsToRemove: [Profile.ID] = allProfiles
+                .filter {
+                    !isIncluded($0.value)
+                }
+                .map(\.key)
+
+            if !idsToRemove.isEmpty {
+                pp_log(.app, .info, "Delete non-included local profile: \(idsToRemove)")
+                Task.detached {
+                    try await self.repository.removeProfiles(withIds: idsToRemove)
+                }
+            }
+        }
    }

    func reloadRemoteProfiles(_ result: [Profile]) {
@ -295,6 +321,17 @@ private extension ProfileManager {
        allRemoteProfiles = result.reduce(into: [:]) {
            $0[$1.id] = $1
        }
+
+        if deletingRemotely {
+            let idsToRemove = Set(allProfiles.keys).subtracting(Set(allRemoteProfiles.keys))
+            if !idsToRemove.isEmpty {
+                pp_log(.app, .info, "Delete local profiles removed remotely: \(idsToRemove)")
+                Task.detached {
+                    try await self.repository.removeProfiles(withIds: Array(idsToRemove))
+                }
+            }
+        }
+
        objectWillChange.send()
    }

@ -303,9 +340,24 @@ private extension ProfileManager {
        let profilesToImport = result
        pp_log(.app, .info, "Try to import remote profiles: \(result.map(\.id))")

+        let allFingerprints = allProfiles.values.reduce(into: [:]) {
+            $0[$1.id] = $1.attributes.fingerprint
+        }
+
        Task.detached { [weak self] in
            for remoteProfile in profilesToImport {
                do {
+                    guard self?.isIncluded?(remoteProfile) ?? true else {
+                        pp_log(.app, .info, "Delete non-included remote profile \(remoteProfile.id)")
+                        try? await self?.repository.removeProfiles(withIds: [remoteProfile.id])
+                        continue
+                    }
+                    if let localFingerprint = allFingerprints[remoteProfile.id] {
+                        guard remoteProfile.attributes.fingerprint != localFingerprint else {
+                            pp_log(.app, .info, "Skip re-importing local profile \(remoteProfile.id)")
+                            continue
+                        }
+                    }
                    pp_log(.app, .notice, "Import remote profile \(remoteProfile.id)...")
                    try await self?.save(remoteProfile)
                } catch {
--- a/Passepartout/Library/Sources/CommonLibrary/Business/ProviderFavoritesManager.swift
+++ b/Passepartout/Library/Sources/CommonLibrary/Business/ProviderFavoritesManager.swift
@ -43,11 +43,11 @@ public final class ProviderFavoritesManager: ObservableObject {

    public var serverIds: Set<String> {
        get {
-            allFavorites.servers(forModuleWithID: moduleId)
+            allFavorites.servers(forModuleWithId: moduleId)
        }
        set {
            objectWillChange.send()
-            allFavorites.setServers(newValue, forModuleWithID: moduleId)
+            allFavorites.setServers(newValue, forModuleWithId: moduleId)
        }
    }

--- a/Passepartout/Library/Sources/CommonLibrary/Domain/ProfileAttributes.swift
+++ b/Passepartout/Library/Sources/CommonLibrary/Domain/ProfileAttributes.swift
@ -0,0 +1,84 @@
+//
+//  ProfileAttributes.swift
+//  Passepartout
+//
+//  Created by Davide De Rosa on 11/3/24.
+//  Copyright (c) 2024 Davide De Rosa. All rights reserved.
+//
+//  https://github.com/passepartoutvpn
+//
+//  This file is part of Passepartout.
+//
+//  Passepartout is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  Passepartout is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with Passepartout.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+import CommonUtils
+import Foundation
+import PassepartoutKit
+
+public struct ProfileAttributes: Hashable, Codable {
+    public var lastUpdate: Date?
+
+    public var fingerprint: UUID?
+
+    public init() {
+    }
+
+    public init(
+        lastUpdate: Date?,
+        fingerprint: UUID?
+    ) {
+        self.lastUpdate = lastUpdate
+        self.fingerprint = fingerprint
+    }
+}
+
+extension ProfileAttributes: ProfileUserInfoTransformable {
+    public var userInfo: [String: AnyHashable]? {
+        do {
+            let data = try JSONEncoder().encode(self)
+            return try JSONSerialization.jsonObject(with: data) as? [String: AnyHashable] ?? [:]
+        } catch {
+            pp_log(.app, .error, "Unable to encode ProfileAttributes to dictionary: \(error)")
+            return [:]
+        }
+    }
+
+    public init?(userInfo: [String: AnyHashable]?) {
+        do {
+            let data = try JSONSerialization.data(withJSONObject: userInfo ?? [:])
+            self = try JSONDecoder().decode(ProfileAttributes.self, from: data)
+        } catch {
+            pp_log(.app, .error, "Unable to decode ProfileAttributes from dictionary: \(error)")
+            return nil
+        }
+    }
+}
+
+extension Profile {
+    public var attributes: ProfileAttributes {
+        userInfo() ?? ProfileAttributes()
+    }
+}
+
+extension Profile.Builder {
+    public var attributes: ProfileAttributes {
+        get {
+            userInfo() ?? ProfileAttributes()
+        }
+        set {
+            setUserInfo(newValue)
+        }
+    }
+}
--- a/Passepartout/Library/Sources/CommonLibrary/Domain/ProviderFavoriteServers.swift
+++ b/Passepartout/Library/Sources/CommonLibrary/Domain/ProviderFavoriteServers.swift
@ -32,11 +32,11 @@ public struct ProviderFavoriteServers {
        map = [:]
    }

-    public func servers(forModuleWithID moduleId: UUID) -> Set<String> {
+    public func servers(forModuleWithId moduleId: UUID) -> Set<String> {
        map[moduleId] ?? []
    }

-    public mutating func setServers(_ servers: Set<String>, forModuleWithID moduleId: UUID) {
+    public mutating func setServers(_ servers: Set<String>, forModuleWithId moduleId: UUID) {
        map[moduleId] = servers
    }
 }
--- a/Passepartout/Library/Sources/CommonUtils/Business/CoreDataRepository.swift
+++ b/Passepartout/Library/Sources/CommonUtils/Business/CoreDataRepository.swift
@ -52,7 +52,7 @@ public actor CoreDataRepository<CD, T>: NSObject,

    private let fromMapper: (CD) throws -> T?

-    private let toMapper: (T, NSManagedObjectContext) throws -> CD
+    private let toMapper: (T, CD?, NSManagedObjectContext) throws -> CD

    private let onResultError: ((Error) -> CoreDataResultAction)?

@ -66,7 +66,7 @@ public actor CoreDataRepository<CD, T>: NSObject,
        observingResults: Bool,
        beforeFetch: ((NSFetchRequest<CD>) -> Void)? = nil,
        fromMapper: @escaping (CD) throws -> T?,
-        toMapper: @escaping (T, NSManagedObjectContext) throws -> CD,
+        toMapper: @escaping (T, CD?, NSManagedObjectContext) throws -> CD,
        onResultError: ((Error) -> CoreDataResultAction)? = nil
    ) {
        guard let entityName = CD.entity().name else {
@ -127,9 +127,11 @@ public actor CoreDataRepository<CD, T>: NSObject,
                    existingIds
                )
                let existing = try context.fetch(request)
-                existing.forEach(context.delete)
                for entity in entities {
-                    _ = try self.toMapper(entity, context)
+                    let oldCdEntity = existing.first {
+                        $0.uuid == entity.uuid
+                    }
+                    _ = try self.toMapper(entity, oldCdEntity, context)
                }
                try context.save()
            } catch {
--- a/Passepartout/Library/Sources/UILibrary/Domain/EditableProfile.swift
+++ b/Passepartout/Library/Sources/UILibrary/Domain/EditableProfile.swift
@ -23,6 +23,7 @@
 //  along with Passepartout.  If not, see <http://www.gnu.org/licenses/>.
 //

+import CommonLibrary
 import Foundation
 import PassepartoutKit

@ -37,6 +38,8 @@ public struct EditableProfile: MutableProfileType {

    public var modulesMetadata: [UUID: ModuleMetadata]?

+    public var userInfo: [String: AnyHashable]?
+
    public func builder() throws -> Profile.Builder {
        var builder = Profile.Builder(id: id)
        builder.modules = try modules.compactMap {
@ -63,10 +66,23 @@ public struct EditableProfile: MutableProfileType {
            $0[$1.key] = metadata
        }

+        builder.userInfo = userInfo
+
        return builder
    }
 }

+extension EditableProfile {
+    var attributes: ProfileAttributes {
+        get {
+            userInfo() ?? ProfileAttributes()
+        }
+        set {
+            setUserInfo(newValue)
+        }
+    }
+}
+
 extension Profile {
    public func editable() -> EditableProfile {
        EditableProfile(
@ -74,7 +90,8 @@ extension Profile {
            name: name,
            modules: modulesBuilders,
            activeModulesIds: activeModulesIds,
-            modulesMetadata: modulesMetadata
+            modulesMetadata: modulesMetadata,
+            userInfo: userInfo
        )
    }

--- a/Passepartout/Library/Tests/UILibraryTests/ProfileEditorTests.swift
+++ b/Passepartout/Library/Tests/UILibraryTests/ProfileEditorTests.swift
@ -236,7 +236,13 @@ extension ProfileEditorTests {
            .sink {
                switch $0 {
                case .save(let savedProfile):
-                    XCTAssertEqual(savedProfile, profile)
+                    do {
+                        let lhs = try savedProfile.withoutUserInfo()
+                        let rhs = try profile.withoutUserInfo()
+                        XCTAssertEqual(lhs, rhs)
+                    } catch {
+                        XCTFail(error.localizedDescription)
+                    }
                    exp.fulfill()

                default: