From 2155fe1892193344a64827a86a602067e521fcd9 Mon Sep 17 00:00:00 2001 From: Davide Date: Sat, 19 Oct 2024 18:22:27 +0200 Subject: [PATCH] Optimize updates in NEProfileRepository (#742) Currently, NEProfileRepository decodes profiles from ALL NE managers on any update. This is undesirable considering that: - Profiles are only _added_ by the app - Externally, profiles can only be _removed_ Therefore: - Observe the initial managers to decode the initial profiles from them - Publish values manually on save/delete (to ProfileManager eventually) - Observe the subsequent updates for when a profile is removed externally, i.e. its ID doesn't appear in managers Fixes #741 --- .../xcshareddata/swiftpm/Package.resolved | 2 +- Passepartout/Library/Package.swift | 2 +- .../Business/NEProfileRepository.swift | 75 ++++++++++++++++--- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 74ac9a6c..41b40b06 100644 --- 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" : "9a43e23e9134c3e93926271b2d630be607433fa0" + "revision" : "ead8d1652fc6875f8865a1c8610b0cc35828dee6" } }, { diff --git a/Passepartout/Library/Package.swift b/Passepartout/Library/Package.swift index f8cbc816..1f323394 100644 --- a/Passepartout/Library/Package.swift +++ b/Passepartout/Library/Package.swift @@ -28,7 +28,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: "9a43e23e9134c3e93926271b2d630be607433fa0"), + .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "ead8d1652fc6875f8865a1c8610b0cc35828dee6"), // .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"), diff --git a/Passepartout/Library/Sources/AppLibrary/Business/NEProfileRepository.swift b/Passepartout/Library/Sources/AppLibrary/Business/NEProfileRepository.swift index dae86ec4..a552c3e8 100644 --- a/Passepartout/Library/Sources/AppLibrary/Business/NEProfileRepository.swift +++ b/Passepartout/Library/Sources/AppLibrary/Business/NEProfileRepository.swift @@ -26,6 +26,7 @@ import Combine import CommonLibrary import Foundation +import NetworkExtension import PassepartoutKit public final class NEProfileRepository: ProfileRepository { @@ -35,26 +36,29 @@ public final class NEProfileRepository: ProfileRepository { private let profilesSubject: CurrentValueSubject<[Profile], Never> - private var subscription: AnyCancellable? + private var subscriptions: Set public init(repository: NETunnelManagerRepository, title: @escaping (Profile) -> String) { self.repository = repository self.title = title profilesSubject = CurrentValueSubject([]) + subscriptions = [] - subscription = repository + repository .managersPublisher - .sink { [weak self] allManagers in - let profiles = allManagers.values.compactMap { - do { - return try repository.profile(from: $0) - } catch { - pp_log(.app, .error, "Unable to decode profile from NE manager '\($0.localizedDescription ?? "")': \(error)") - return nil - } - } - self?.profilesSubject.send(profiles) + .first() + .sink { [weak self] in + self?.onLoadedManagers($0) } + .store(in: &subscriptions) + + repository + .managersPublisher + .dropFirst() + .sink { [weak self] in + self?.onUpdatedManagers($0) + } + .store(in: &subscriptions) } public var profilesPublisher: AnyPublisher<[Profile], Never> { @@ -68,11 +72,58 @@ public final class NEProfileRepository: ProfileRepository { public func saveProfile(_ profile: Profile) async throws { try await repository.save(profile, connect: false, title: title) + if let index = profilesSubject.value.firstIndex(where: { $0.id == profile.id }) { + profilesSubject.value[index] = profile + } else { + profilesSubject.value.append(profile) + } } public func removeProfiles(withIds profileIds: [Profile.ID]) async throws { + var removedIds: Set = [] + defer { + profilesSubject.value.removeAll { + removedIds.contains($0.id) + } + } for id in profileIds { try await repository.remove(profileId: id) + removedIds.insert(id) } } } + +private extension NEProfileRepository { + func onLoadedManagers(_ managers: [Profile.ID: NETunnelProviderManager]) { + let profiles = managers.values.compactMap { + do { + return try repository.profile(from: $0) + } catch { + pp_log(.app, .error, "Unable to decode profile from NE manager '\($0.localizedDescription ?? "")': \(error)") + return nil + } + } + profilesSubject.send(profiles) + } + + func onUpdatedManagers(_ managers: [Profile.ID: NETunnelProviderManager]) { + let profiles = profilesSubject + .value + .filter { + managers.keys.contains($0.id) + } + + let removedProfilesDesc = profilesSubject + .value + .filter { + !managers.keys.contains($0.id) + } + .map { + "\($0.name)(\($0.id)" + } + + pp_log(.app, .info, "Sync profiles removed externally: \(removedProfilesDesc)") + + profilesSubject.send(profiles) + } +}