Maintain one configuration per profile (#636)

Helps with automation. Install the VPN configuration before persisting a
profile, so that the 1:1 reference with OS settings is maintained.
Likewise, uninstall the VPN configuration after removing a profile.

This before-save hook also resolves a problem with multiple imports,
where multiple VPN permission alerts coalesce if no VPN configuration is
installed. Now the first import waits for the permission synchronously.

Fixes #618

Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

@@ -32,7 +32,7 @@
       "kind" : "remoteSourceControl",
       "location" : "git@github.com:passepartoutvpn/passepartoutkit",
       "state" : {
-        "revision" : "ed3f54281b672af0f1127f00033579a36a9afed5"
+        "revision" : "263bedc756d07eb107d7bfe3b50dbc5db28675d4"
       }
     },
     {

Passepartout/Library/Package.swift

@@ -31,7 +31,7 @@ let package = Package(
     ],
     dependencies: [
 //        .package(url: "git@github.com:passepartoutvpn/passepartoutkit", from: "0.7.0"),
-        .package(url: "git@github.com:passepartoutvpn/passepartoutkit", revision: "ed3f54281b672af0f1127f00033579a36a9afed5"),
+        .package(url: "git@github.com:passepartoutvpn/passepartoutkit", revision: "263bedc756d07eb107d7bfe3b50dbc5db28675d4"),
 //        .package(path: "../../../passepartoutkit"),
         .package(url: "git@github.com:passepartoutvpn/passepartoutkit-openvpn-openssl", from: "0.6.0"),
 //        .package(path: "../../../passepartoutkit-openvpn-openssl"),

Passepartout/Library/Sources/AppLibrary/Business/ProfileManager.swift

@@ -27,17 +27,26 @@ import AppData
 import Combine
 import Foundation
 import PassepartoutKit
+import UtilsLibrary
 
 @MainActor
 public final class ProfileManager: ObservableObject {
-    public let didSave: PassthroughSubject<Profile, Never>
+    public enum Event {
+        case save(Profile)
 
-    public var didUpdate: AnyPublisher<[Profile], Never> {
+        case remove([Profile.ID])
-        $profiles.eraseToAnyPublisher()
+
+        case update([Profile])
     }
 
+    public var beforeSave: ((Profile) async throws -> Void)?
+
+    public var afterRemove: (([Profile.ID]) async -> Void)?
+
+    public let didChange: PassthroughSubject<Event, Never>
+
     @Published
-    var profiles: [Profile]
+    private var profiles: [Profile]
 
     private var allProfileIds: Set<Profile.ID>
 
@@ -49,7 +58,7 @@ public final class ProfileManager: ObservableObject {
 
     // for testing/previews
     public init(profiles: [Profile]) {
-        didSave = PassthroughSubject()
+        didChange = PassthroughSubject()
         self.profiles = profiles.sorted {
             $0.name.lowercased() < $1.name.lowercased()
         }
@@ -57,21 +66,21 @@ public final class ProfileManager: ObservableObject {
         repository = MockProfileRepository(profiles: profiles)
         searchSubject = CurrentValueSubject("")
         subscriptions = []
-
-        observeObjects(searchDebounce: 0)
     }
 
-    public init(repository: any ProfileRepository, searchDebounce: Int = 200) {
+    public init(repository: any ProfileRepository) {
-        didSave = PassthroughSubject()
+        didChange = PassthroughSubject()
         profiles = []
         allProfileIds = []
         self.repository = repository
         searchSubject = CurrentValueSubject("")
         subscriptions = []
-
+    }
-        observeObjects(searchDebounce: searchDebounce)
 }
 
+// MARK: - CRUD
+
+extension ProfileManager {
     public var hasProfiles: Bool {
         !profiles.isEmpty
     }
@@ -98,8 +107,9 @@ public final class ProfileManager: ObservableObject {
 
     public func save(_ profile: Profile) async throws {
         do {
+            try await beforeSave?(profile)
             try await repository.saveEntities([profile])
-            didSave.send(profile)
+            didChange.send(.save(profile))
         } catch {
             pp_log(.app, .fault, "Unable to save profile \(profile.id): \(error)")
             throw error
@@ -114,6 +124,8 @@ public final class ProfileManager: ObservableObject {
         do {
             allProfileIds.subtract(profileIds)
             try await repository.removeEntities(withIds: profileIds)
+            await afterRemove?(profileIds)
+            didChange.send(.remove(profileIds))
         } catch {
             pp_log(.app, .fault, "Unable to remove profiles \(profileIds): \(error)")
         }
@@ -124,6 +136,8 @@ public final class ProfileManager: ObservableObject {
     }
 }
 
+// MARK: - Shortcuts
+
 extension ProfileManager {
     public func new(withName name: String) -> Profile {
         var builder = Profile.Builder()
@@ -149,35 +163,6 @@ extension ProfileManager {
 }
 
 private extension ProfileManager {
-    func observeObjects(searchDebounce: Int) {
-        repository
-            .entitiesPublisher
-            .receive(on: DispatchQueue.main)
-            .sink { [weak self] in
-                guard let self else {
-                    return
-                }
-                self.profiles = $0.entities
-                if !$0.isFiltering {
-                    allProfileIds = Set($0.entities.map(\.id))
-                }
-            }
-            .store(in: &subscriptions)
-
-        searchSubject
-            .debounce(for: .milliseconds(searchDebounce), scheduler: DispatchQueue.main)
-            .sink { [weak self] search in
-                Task {
-                    guard !search.isEmpty else {
-                        try await self?.repository.resetFilter()
-                        return
-                    }
-                    try await self?.repository.filter(byName: search)
-                }
-            }
-            .store(in: &subscriptions)
-    }
-
     func firstUniqueName(from name: String) -> String {
         let allNames = profiles.map(\.name)
         var newName = name
@@ -191,3 +176,52 @@ private extension ProfileManager {
         }
     }
 }
+
+// MARK: - Observation
+
+extension ProfileManager {
+    public func observeObjects(searchDebounce: Int = 200) {
+        repository
+            .entitiesPublisher
+            .receive(on: DispatchQueue.main)
+            .sink { [weak self] in
+                self?.notifyUpdatedEntities($0)
+            }
+            .store(in: &subscriptions)
+
+        searchSubject
+            .debounce(for: .milliseconds(searchDebounce), scheduler: DispatchQueue.main)
+            .sink { [weak self] in
+                self?.performSearch($0)
+            }
+            .store(in: &subscriptions)
+    }
+}
+
+private extension ProfileManager {
+    func notifyUpdatedEntities(_ result: EntitiesResult<Profile>) {
+        let oldProfiles = profiles.reduce(into: [:]) {
+            $0[$1.id] = $1
+        }
+        let newProfiles = result.entities
+        let updatedProfiles = newProfiles.filter {
+            $0 != oldProfiles[$0.id] // includes new profiles
+        }
+
+        if !result.isFiltering {
+            allProfileIds = Set(newProfiles.map(\.id))
+        }
+        profiles = newProfiles
+        didChange.send(.update(updatedProfiles))
+    }
+
+    func performSearch(_ search: String) {
+        Task {
+            guard !search.isEmpty else {
+                try await repository.resetFilter()
+                return
+            }
+            try await repository.filter(byName: search)
+        }
+    }
+}

Passepartout/Library/Sources/AppUI/AppUI.swift

@@ -32,10 +32,6 @@ public struct AppUI {
 
     public static func configure(with context: AppContext) {
         assertMissingModuleImplementations()
-        Task {
-            await context.iapManager.reloadReceipt()
-            try await context.tunnel.prepare()
-        }
     }
 }
 

Passepartout/Library/Sources/AppUI/Business/AppContext.swift

@@ -60,61 +60,80 @@ public final class AppContext: ObservableObject {
         self.profileManager = profileManager
         self.tunnel = tunnel
         self.tunnelEnvironment = tunnelEnvironment
-        self.registry = registry
-        self.constants = constants
-        subscriptions = []
-
         connectionObserver = ConnectionObserver(
             tunnel: tunnel,
             environment: tunnelEnvironment,
             interval: constants.connection.refreshInterval
         )
+        self.registry = registry
+        self.constants = constants
+        subscriptions = []
 
+        profileManager.beforeSave = { [weak self] in
+            try await self?.installSavedProfile($0)
+        }
+        profileManager.afterRemove = { [weak self] in
+            self?.uninstallRemovedProfiles(withIds: $0)
+        }
+
+        Task {
+            try await tunnel.prepare()
+            await iapManager.reloadReceipt()
+            connectionObserver.observeObjects()
+            profileManager.observeObjects()
             observeObjects()
         }
     }
+}
+
+// MARK: - Observation
 
 private extension AppContext {
     func observeObjects() {
         profileManager
-            .didSave
+            .didChange
-            .sink { [weak self] profile in
+            .sink { [weak self] event in
-                guard let self else {
+                switch event {
-                    return
+                case .save(let profile):
+                    self?.syncTunnelIfCurrentProfile(profile)
+
+                default:
+                    break
                 }
-                guard profile.id == tunnel.installedProfile?.id else {
+            }
+            .store(in: &subscriptions)
+    }
+}
+
+private extension AppContext {
+    func installSavedProfile(_ profile: Profile) async throws {
+        try await tunnel.install(profile, processor: iapManager)
+    }
+
+    func uninstallRemovedProfiles(withIds profileIds: [Profile.ID]) {
+        Task {
+            for id in profileIds {
+                do {
+                    try await tunnel.uninstall(profileId: id)
+                } catch {
+                    pp_log(.app, .error, "Unable to uninstall profile \(id): \(error)")
+                }
+            }
+        }
+    }
+
+    func syncTunnelIfCurrentProfile(_ profile: Profile) {
+        guard profile.id == tunnel.currentProfile?.id else {
             return
         }
         Task {
             if profile.isInteractive {
-                        try await self.tunnel.disconnect()
+                try await tunnel.disconnect()
                 return
             }
-                    if self.tunnel.status == .active {
+            if tunnel.status == .active {
-                        try await self.tunnel.reconnect(with: profile, processor: self.iapManager)
+                try await tunnel.connect(with: profile, processor: iapManager)
-                    } else {
-                        try await self.tunnel.reinstate(profile, processor: self.iapManager)
             }
         }
     }
-            .store(in: &subscriptions)
-
-        profileManager
-            .didUpdate
-            .sink { [weak self] _ in
-                guard let self else {
-                    return
-                }
-                guard let installedProfile = tunnel.installedProfile else {
-                    return
-                }
-                guard profileManager.exists(withId: installedProfile.id) else {
-                    Task {
-                        try await self.tunnel.disconnect()
-                    }
-                    return
-                }
-            }
-            .store(in: &subscriptions)
-    }
 }

Passepartout/Library/Sources/AppUI/Business/ConnectionObserver.swift

@@ -65,13 +65,9 @@ public final class ConnectionObserver: ObservableObject {
         self.environment = environment
         self.interval = interval
         subscriptions = []
-
-        observeObjects()
-    }
     }
 
-private extension ConnectionObserver {
+    public func observeObjects() {
-    func observeObjects() {
         tunnel
             .$status
             .receive(on: DispatchQueue.main)

Passepartout/Library/Sources/AppUI/Business/Tunnel+Extensions.swift

@@ -32,28 +32,14 @@ protocol ProfileProcessor {
 }
 
 extension Tunnel {
-    func reinstate(_ profile: Profile, processor: ProfileProcessor) async throws {
+    func install(_ profile: Profile, processor: ProfileProcessor) async throws {
-        try await install(profile, processor: processor)
+        let newProfile = try processor.processedProfile(profile)
+        try await install(newProfile, connect: false, title: \.name)
     }
 
     func connect(with profile: Profile, processor: ProfileProcessor) async throws {
-        try await install(profile, processor: processor)
+        let newProfile = try processor.processedProfile(profile)
-        guard !Task.isCancelled else {
+        try await install(newProfile, connect: true, title: \.name)
-            return
-        }
-        try await connect()
-    }
-
-    func reconnect(with profile: Profile, processor: ProfileProcessor) async throws {
-        try await disconnect()
-        guard !Task.isCancelled else {
-            return
-        }
-        try await install(profile, processor: processor)
-        guard !Task.isCancelled else {
-            return
-        }
-        try await connect()
     }
 
     func currentLog(parameters: Constants.Log) async -> [String] {
@@ -70,10 +56,3 @@ extension Tunnel {
         }
     }
 }
-
-private extension Tunnel {
-    func install(_ profile: Profile, processor: ProfileProcessor) async throws {
-        let newProfile = try processor.processedProfile(profile)
-        try await install(profile: newProfile, title: \.name)
-    }
-}

Passepartout/Library/Sources/AppUI/Business/TunnelInstallationProviding.swift

@@ -36,25 +36,25 @@ protocol TunnelInstallationProviding {
 struct TunnelInstallation {
     let header: ProfileHeader
 
-    let isEnabled: Bool
+    let onDemand: Bool
 }
 
 @MainActor
 extension TunnelInstallationProviding {
     var installation: TunnelInstallation? {
-        guard let installedProfile = tunnel.installedProfile else {
+        guard let currentProfile = tunnel.currentProfile else {
             return nil
         }
         guard let header = profileManager.headers.first(where: {
-            $0.id == installedProfile.id
+            $0.id == currentProfile.id
         }) else {
             return nil
         }
-        return TunnelInstallation(header: header, isEnabled: installedProfile.isEnabled)
+        return TunnelInstallation(header: header, onDemand: currentProfile.onDemand)
     }
 
-    var installedProfile: Profile? {
+    var currentProfile: Profile? {
-        guard let id = tunnel.installedProfile?.id else {
+        guard let id = tunnel.currentProfile?.id else {
             return nil
         }
         return profileManager.profile(withId: id)

Passepartout/Library/Sources/AppUI/L10n/SwiftGen+Strings.swift

@@ -265,8 +265,6 @@ public enum Strings {
     public static let storage = Strings.tr("Localizable", "global.storage", fallback: "Storage")
     /// Subnet
     public static let subnet = Strings.tr("Localizable", "global.subnet", fallback: "Subnet")
-    /// Uninstall
-    public static let uninstall = Strings.tr("Localizable", "global.uninstall", fallback: "Uninstall")
     /// Unknown
     public static let unknown = Strings.tr("Localizable", "global.unknown", fallback: "Unknown")
     /// Username

Passepartout/Library/Sources/AppUI/Resources/en.lproj/Localizable.strings

@@ -58,7 +58,6 @@
 "global.status" = "Status";
 "global.storage" = "Storage";
 "global.subnet" = "Subnet";
-"global.uninstall" = "Uninstall";
 "global.unknown" = "Unknown";
 "global.username" = "Username";
 "global.version" = "Version";

Passepartout/Library/Sources/AppUI/Views/App/ProfileGridView.swift

@@ -85,7 +85,7 @@ private extension ProfileGridView {
         InstalledProfileView(
             layout: .grid,
             profileManager: profileManager,
-            profile: installedProfile,
+            profile: currentProfile,
             tunnel: tunnel,
             interactiveManager: interactiveManager,
             errorHandler: errorHandler,
@@ -95,7 +95,7 @@ private extension ProfileGridView {
             )
         )
         .contextMenu {
-            installedProfile.map {
+            currentProfile.map {
                 ProfileContextMenu(
                     profileManager: profileManager,
                     tunnel: tunnel,
@@ -121,7 +121,7 @@ private extension ProfileGridView {
             withMarker: true,
             onEdit: onEdit
         )
-        .themeGridCell(isSelected: header.id == nextProfileId ?? tunnel.installedProfile?.id)
+        .themeGridCell(isSelected: header.id == nextProfileId ?? tunnel.currentProfile?.id)
         .contextMenu {
             ProfileContextMenu(
                 profileManager: profileManager,

Passepartout/Library/Sources/AppUI/Views/App/ProfileListView.swift

@@ -76,7 +76,7 @@ private extension ProfileListView {
         InstalledProfileView(
             layout: .list,
             profileManager: profileManager,
-            profile: installedProfile,
+            profile: currentProfile,
             tunnel: tunnel,
             interactiveManager: interactiveManager,
             errorHandler: errorHandler,
@@ -86,7 +86,7 @@ private extension ProfileListView {
             )
         )
         .contextMenu {
-            installedProfile.map {
+            currentProfile.map {
                 ProfileContextMenu(
                     profileManager: profileManager,
                     tunnel: tunnel,

Passepartout/Library/Sources/AppUI/Views/UI/ProfileContextMenu.swift

@@ -52,9 +52,6 @@ struct ProfileContextMenu: View {
         profileEditButton
         profileDuplicateButton
         Divider()
-        if isInstalledProfile {
-            tunnelUninstallButton
-        }
         profileRemoveButton
     }
 }
@@ -86,12 +83,6 @@ private extension ProfileContextMenu {
         }
     }
 
-    var tunnelUninstallButton: some View {
-        TunnelUninstallButton(tunnel: tunnel) {
-            ThemeImageLabel(Strings.Global.uninstall, .tunnelUninstall)
-        }
-    }
-
     var profileEditButton: some View {
         Button {
             onEdit(header)

Passepartout/Library/Sources/AppUI/Views/UI/ProfileRowView.swift

@@ -71,7 +71,7 @@ struct ProfileRowView: View, TunnelContextProviding {
 private extension ProfileRowView {
     var markerView: some View {
         ThemeImage(header.id == nextProfileId ? .pending : statusImage)
-            .opacity(header.id == nextProfileId || header.id == tunnel.installedProfile?.id ? 1.0 : 0.0)
+            .opacity(header.id == nextProfileId || header.id == tunnel.currentProfile?.id ? 1.0 : 0.0)
             .frame(width: 24.0)
     }
 

Passepartout/Library/Sources/AppUI/Views/UI/TunnelRestartButton.swift

@@ -55,7 +55,7 @@ struct TunnelRestartButton<Label>: View where Label: View {
             pendingTask?.cancel()
             pendingTask = Task {
                 do {
-                    try await tunnel.reconnect(with: profile, processor: iapManager)
+                    try await tunnel.connect(with: profile, processor: iapManager)
                 } catch {
                     errorHandler.handle(
                         error,

Passepartout/Library/Sources/AppUI/Views/UI/TunnelToggleButton.swift

@@ -77,7 +77,7 @@ struct TunnelToggleButton<Label>: View, TunnelContextProviding, ThemeProviding w
 
 private extension TunnelToggleButton {
     var isInstalled: Bool {
-        profile?.id == tunnel.installedProfile?.id
+        profile?.id == tunnel.currentProfile?.id
     }
 
     var canConnect: Bool {
@@ -127,7 +127,7 @@ private extension TunnelToggleButton {
                     try await tunnel.disconnect()
                 }
             } else {
-                try await tunnel.reconnect(with: profile, processor: iapManager)
+                try await tunnel.connect(with: profile, processor: iapManager)
             }
         } catch {
             errorHandler.handle(

Passepartout/Library/Sources/AppUI/Views/UI/TunnelUninstallButton.swift

@@ -1,50 +0,0 @@
-//
-//  TunnelUninstallButton.swift
-//  Passepartout
-//
-//  Created by Davide De Rosa on 9/7/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 PassepartoutKit
-import SwiftUI
-import UtilsLibrary
-
-struct TunnelUninstallButton<Label>: View where Label: View {
-
-    @ObservedObject
-    var tunnel: Tunnel
-
-    let label: () -> Label
-
-    @State
-    private var pendingTask: Task<Void, Error>?
-
-    var body: some View {
-        Button {
-            pendingTask?.cancel()
-            pendingTask = Task {
-                try await tunnel.uninstall()
-            }
-        } label: {
-            label()
-        }
-    }
-}

Passepartout/Library/Tests/AppUITests/ConnectionObserverTests.swift

@@ -37,10 +37,10 @@ extension ConnectionObserverTests {
         let env = InMemoryEnvironment()
         let tunnel = Tunnel(strategy: FakeTunnelStrategy(environment: env))
         let sut = ConnectionObserver(tunnel: tunnel, environment: env, interval: 0.1)
+        sut.observeObjects()
 
         let profile = try Profile.Builder().tryBuild()
-        try await tunnel.install(profile: profile, title: \.name)
+        try await tunnel.install(profile, connect: true, title: \.name)
-        try await tunnel.connect()
         env.setEnvironmentValue(.crypto, forKey: TunnelEnvironmentKeys.lastErrorCode)
 
         try await tunnel.disconnect()
@@ -52,15 +52,16 @@ extension ConnectionObserverTests {
         let env = InMemoryEnvironment()
         let tunnel = Tunnel(strategy: FakeTunnelStrategy(environment: env))
         let sut = ConnectionObserver(tunnel: tunnel, environment: env, interval: 0.1)
+        sut.observeObjects()
 
         let profile = try Profile.Builder().tryBuild()
-        try await tunnel.install(profile: profile, title: \.name)
+        try await tunnel.install(profile, connect: false, title: \.name)
 
         let dataCount = DataCount(500, 700)
         env.setEnvironmentValue(dataCount, forKey: TunnelEnvironmentKeys.dataCount)
         XCTAssertEqual(sut.dataCount, nil)
 
-        try await tunnel.connect()
+        try await tunnel.install(profile, connect: true, title: \.name)
         try await Task.sleep(for: .milliseconds(200))
         XCTAssertEqual(sut.dataCount, dataCount)
     }

Passepartout/Library/Tests/AppUITests/ProfileEditorTests.swift

@@ -230,10 +230,16 @@ extension ProfileEditorTests {
 
         let exp = expectation(description: "Save")
         manager
-            .didSave
+            .didChange
             .sink {
-                XCTAssertEqual($0, profile)
+                switch $0 {
+                case .save(let savedProfile):
+                    XCTAssertEqual(savedProfile, profile)
                     exp.fulfill()
+
+                default:
+                    break
+                }
             }
             .store(in: &subscriptions)
 

Passepartout/Library/Tests/AppUITests/ProfileImporterTests.swift

@@ -57,11 +57,17 @@ extension ProfileImporterTests {
 
         let exp = expectation(description: "Save")
         profileManager
-            .didSave
+            .didChange
-            .sink { profile in
+            .sink {
+                switch $0 {
+                case .save(let profile):
                     XCTAssertEqual(profile.modules.count, 1)
                     XCTAssertTrue(profile.modules.first is SomeModule)
                     exp.fulfill()
+
+                default:
+                    break
+                }
             }
             .store(in: &subscriptions)
 
@@ -82,11 +88,17 @@ extension ProfileImporterTests {
 
         let exp = expectation(description: "Save")
         profileManager
-            .didSave
+            .didChange
-            .sink { profile in
+            .sink {
+                switch $0 {
+                case .save(let profile):
                     XCTAssertEqual(profile.modules.count, 1)
                     XCTAssertTrue(profile.modules.first is SomeModule)
                     exp.fulfill()
+
+                default:
+                    break
+                }
             }
             .store(in: &subscriptions)