In-place NetworkExtension profiles (#715)

Profiles are being maintained in two places: - Core Data - NetworkExtension Core Data is redundant for local profiles, so make NetworkExtension the only source of truth.
2024-10-10 16:03:02 +02:00 · 2024-10-10 16:03:02 +02:00 · 0aac8cd9f3
parent 6d479a7059
commit 0aac8cd9f3
14 changed files with 168 additions and 71 deletions
--- a/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
+++ b/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@ -9,6 +9,15 @@
        "version" : "1.7.18"
      }
    },
+    {
+      "identity" : "generic-json-swift",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/zoul/generic-json-swift",
+      "state" : {
+        "revision" : "0a06575f4038b504e78ac330913d920f1630f510",
+        "version" : "2.0.2"
+      }
+    },
    {
      "identity" : "kvitto",
      "kind" : "remoteSourceControl",
@ -32,7 +41,7 @@
      "kind" : "remoteSourceControl",
      "location" : "git@github.com:passepartoutvpn/passepartoutkit-source",
      "state" : {
-        "revision" : "7efa18eb75b7a102781be3c62cd31a08607f03c8"
+        "revision" : "90267688fa16be83e7f75f26d5eb5b3094b309ec"
      }
    },
    {
--- a/Passepartout/Library/Package.swift
+++ b/Passepartout/Library/Package.swift
@ -31,7 +31,7 @@ let package = Package(
    ],
    dependencies: [
 //        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.8.0"),
-        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "7efa18eb75b7a102781be3c62cd31a08607f03c8"),
+        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "90267688fa16be83e7f75f26d5eb5b3094b309ec"),
 //        .package(path: "../../../passepartoutkit-source"),
        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", from: "0.8.0"),
 //        .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", revision: "031863a1cd683962a7dfe68e20b91fa820a1ecce"),
@ -47,6 +47,7 @@ let package = Package(
        .target(
            name: "AppData",
            dependencies: [
+                "AppLibrary",
                .product(name: "PassepartoutKit", package: "passepartoutkit-source")
            ]
        ),
@ -64,7 +65,6 @@ let package = Package(
        .target(
            name: "AppLibrary",
            dependencies: [
-                "AppData",
                "CommonLibrary",
                "Kvitto",
                "LegacyV2",
--- a/Passepartout/Library/Sources/AppDataProfiles/AppData+Profiles.swift
+++ b/Passepartout/Library/Sources/AppDataProfiles/AppData+Profiles.swift
@ -28,10 +28,10 @@ import CoreData
 import Foundation

 extension AppData {
-    public static var cdProfilesModel: NSManagedObjectModel {
+    public static let cdProfilesModel: NSManagedObjectModel = {
        guard let model: NSManagedObjectModel = .mergedModel(from: [.module]) else {
            fatalError("Unable to build Core Data model (Profiles v3)")
        }
        return model
-    }
+    }()
 }
--- a/Passepartout/Library/Sources/AppDataProfiles/CDProfileRepositoryV3.swift
+++ b/Passepartout/Library/Sources/AppDataProfiles/CDProfileRepositoryV3.swift
@ -24,6 +24,7 @@
 //

 import AppData
+import AppLibrary
 import CoreData
 import Foundation
 import PassepartoutKit
@ -34,9 +35,13 @@ extension AppData {
        registry: Registry,
        coder: ProfileCoder,
        context: NSManagedObjectContext,
+        observingResults: Bool,
        onResultError: ((Error) -> CoreDataResultAction)?
    ) -> any ProfileRepository {
-        let repository = CoreDataRepository<CDProfileV3, Profile>(context: context) {
+        let repository = CoreDataRepository<CDProfileV3, Profile>(
+            context: context,
+            observingResults: observingResults
+        ) {
            $0.sortDescriptors = [
                .init(key: "name", ascending: true, selector: #selector(NSString.caseInsensitiveCompare)),
                .init(key: "lastUpdate", ascending: true)
--- a/Passepartout/Library/Sources/AppLibrary/Business/InMemoryProfileRepository.swift
+++ b/Passepartout/Library/Sources/AppLibrary/Business/InMemoryProfileRepository.swift
@ -1,5 +1,5 @@
 //
-//  MockProfileRepository.swift
+//  InMemoryProfileRepository.swift
 //  Passepartout
 //
 //  Created by Davide De Rosa on 8/11/24.
@ -28,7 +28,7 @@ import Foundation
 import PassepartoutKit
 import UtilsLibrary

-public final class MockProfileRepository: ProfileRepository {
+public final class InMemoryProfileRepository: ProfileRepository {
    private var profiles: [Profile] {
        didSet {
            profilesSubject.send(EntitiesResult(profiles, isFiltering: false))
--- a/Passepartout/Library/Sources/AppLibrary/Business/NEProfileRepository.swift
+++ b/Passepartout/Library/Sources/AppLibrary/Business/NEProfileRepository.swift
@ -0,0 +1,87 @@
+//
+//  NEProfileRepository.swift
+//  Passepartout
+//
+//  Created by Davide De Rosa on 10/10/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 Combine
+import Foundation
+import PassepartoutKit
+import UtilsLibrary
+
+public final class NEProfileRepository: ProfileRepository {
+    private let repository: NETunnelManagerRepository
+
+    private let profilesSubject: CurrentValueSubject<[Profile], Never>
+
+    private var subscription: AnyCancellable?
+
+    public init(repository: NETunnelManagerRepository) {
+        self.repository = repository
+        profilesSubject = CurrentValueSubject([])
+
+        subscription = repository
+            .managersPublisher
+            .sink { [weak self] allManagers in
+                let profiles = allManagers.values.compactMap {
+                    try? repository.profile(from: $0)
+                }
+                self?.profilesSubject.send(profiles)
+            }
+
+        Task {
+            do {
+                try await repository.load()
+            } catch {
+                pp_log(.app, .fault, "Unable to load NE profiles: \(error)")
+            }
+        }
+    }
+
+    public var entitiesPublisher: AnyPublisher<EntitiesResult<Profile>, Never> {
+        profilesSubject
+            .map {
+                EntitiesResult($0, isFiltering: false)
+            }
+            .eraseToAnyPublisher()
+    }
+
+    public func filter(byFormat format: String, arguments: [Any]?) async throws {
+        assertionFailure("Unused by ProfileManager")
+    }
+
+    public func resetFilter() async throws {
+        assertionFailure("Unused by ProfileManager")
+    }
+
+    public func saveEntities(_ entities: [Profile]) async throws {
+        for profile in entities {
+            try await repository.save(profile, connect: false, title: \.name)
+        }
+    }
+
+    public func removeEntities(withIds ids: [UUID]) async throws {
+        for profileId in ids {
+            try await repository.remove(profileId: profileId)
+        }
+    }
+}
--- a/Passepartout/Library/Sources/AppLibrary/Business/ProfileManager.swift
+++ b/Passepartout/Library/Sources/AppLibrary/Business/ProfileManager.swift
@ -23,7 +23,6 @@
 //  along with Passepartout.  If not, see <http://www.gnu.org/licenses/>.
 //

-import AppData
 import Combine
 import Foundation
 import PassepartoutKit
@ -37,10 +36,6 @@ public final class ProfileManager: ObservableObject {
        case remove([Profile.ID])
    }

-    public var beforeSave: ((Profile) async throws -> Void)?
-
-    public var afterRemove: (([Profile.ID]) async -> Void)?
-
    private let repository: any ProfileRepository

    private let remoteRepository: (any ProfileRepository)?
@ -64,7 +59,7 @@ public final class ProfileManager: ObservableObject {

    // for testing/previews
    public init(profiles: [Profile]) {
-        repository = MockProfileRepository(profiles: profiles)
+        repository = InMemoryProfileRepository(profiles: profiles)
        remoteRepository = nil
        self.profiles = []
        allProfiles = profiles.reduce(into: [:]) {
@ -122,7 +117,6 @@ extension ProfileManager {
        do {
            let existingProfile = allProfiles[profile.id]
            if existingProfile == nil || profile != existingProfile {
-               try await beforeSave?(profile)
               try await repository.saveEntities([profile])

                allProfiles[profile.id] = profile
@ -160,7 +154,6 @@ extension ProfileManager {
            // remove local profiles
            var newAllProfiles = allProfiles
            try await repository.removeEntities(withIds: profileIds)
-            await afterRemove?(profileIds)
            profileIds.forEach {
                newAllProfiles.removeValue(forKey: $0)
            }
@ -245,7 +238,6 @@ extension ProfileManager {
    public func observeObjects(searchDebounce: Int = 200) {
        repository
            .entitiesPublisher
-            .first()
            .receive(on: DispatchQueue.main)
            .sink { [weak self] in
                self?.reloadLocalProfiles($0)
--- a/Passepartout/Library/Sources/AppLibrary/Business/ProfileRepository.swift
+++ b/Passepartout/Library/Sources/AppLibrary/Business/ProfileRepository.swift
--- a/Passepartout/Library/Sources/AppUI/Business/AppContext.swift
+++ b/Passepartout/Library/Sources/AppUI/Business/AppContext.swift
@ -69,13 +69,6 @@ public final class AppContext: ObservableObject {
        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()
--- a/Passepartout/Library/Sources/AppUI/Mock/Mock.swift
+++ b/Passepartout/Library/Sources/AppUI/Mock/Mock.swift
@ -23,7 +23,6 @@
 //  along with Passepartout.  If not, see <http://www.gnu.org/licenses/>.
 //

-import AppData
 import AppLibrary
 import Combine
 import Foundation
--- a/Passepartout/Library/Sources/UtilsLibrary/Business/CoreDataRepository.swift
+++ b/Passepartout/Library/Sources/UtilsLibrary/Business/CoreDataRepository.swift
@ -48,6 +48,8 @@ public actor CoreDataRepository<CD, T>: NSObject,

    private let context: NSManagedObjectContext

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

    private let toMapper: (T, NSManagedObjectContext) throws -> CD
@ -61,6 +63,7 @@ public actor CoreDataRepository<CD, T>: NSObject,

    public init(
        context: NSManagedObjectContext,
+        observingResults: Bool,
        beforeFetch: ((NSFetchRequest<CD>) -> Void)? = nil,
        fromMapper: @escaping (CD) throws -> T?,
        toMapper: @escaping (T, NSManagedObjectContext) throws -> CD,
@ -72,6 +75,7 @@ public actor CoreDataRepository<CD, T>: NSObject,

        self.entityName = entityName
        self.context = context
+        self.observingResults = observingResults
        self.fromMapper = fromMapper
        self.toMapper = toMapper
        self.onResultError = onResultError
@ -161,6 +165,9 @@ public actor CoreDataRepository<CD, T>: NSObject,

    // XXX: triggers on entity insert/update/delete and reloads/remaps ALL into entitiesSubject
    public nonisolated func controllerDidChangeContent(_ controller: NSFetchedResultsController<any NSFetchRequestResult>) {
+        guard observingResults else {
+            return
+        }
        guard let cdController = controller as? NSFetchedResultsController<CD> else {
            fatalError("Unable to upcast results to \(CD.self)")
        }
--- a/Passepartout/Shared/Shared+AppLibrary.swift
+++ b/Passepartout/Shared/Shared+AppLibrary.swift
@ -27,41 +27,27 @@ import AppData
 import AppDataProfiles
 import AppLibrary
 import CommonLibrary
+import CoreData
 import Foundation
 import PassepartoutKit
 import UtilsLibrary

 extension ProfileManager {
    static let shared: ProfileManager = {
-        let model = AppData.cdProfilesModel
-
-        let store = CoreDataPersistentStore(
-            logger: .default,
-            containerName: BundleConfiguration.mainString(for: .profilesContainerName),
-            model: model,
-            cloudKitIdentifier: nil,
-            author: nil
-        )
-        let repository = AppData.cdProfileRepositoryV3(
-            registry: .shared,
-            coder: CodableProfileCoder(),
-            context: store.context
-        ) { error in
-            pp_log(.app, .error, "Unable to decode local result: \(error)")
-            return .ignore
-        }
+        let repository = localProfileRepository

        let remoteStore = CoreDataPersistentStore(
            logger: .default,
            containerName: BundleConfiguration.mainString(for: .remoteProfilesContainerName),
-            model: model,
+            model: coreDataModel,
            cloudKitIdentifier: BundleConfiguration.mainString(for: .cloudKitId),
            author: nil
        )
        let remoteRepository = AppData.cdProfileRepositoryV3(
            registry: .shared,
            coder: CodableProfileCoder(),
-            context: remoteStore.context
+            context: remoteStore.context,
+            observingResults: true
        ) { error in
            pp_log(.app, .error, "Unable to decode remote result: \(error)")
            return .ignore
@ -71,6 +57,10 @@ extension ProfileManager {
    }()
 }

+private var coreDataModel: NSManagedObjectModel {
+    AppData.cdProfilesModel
+}
+
 #if targetEnvironment(simulator)

 extension Tunnel {
@ -79,15 +69,42 @@ extension Tunnel {
    )
 }

+private var localProfileRepository: any ProfileRepository {
+    let store = CoreDataPersistentStore(
+        logger: .default,
+        containerName: BundleConfiguration.mainString(for: .profilesContainerName),
+        model: coreDataModel,
+        cloudKitIdentifier: nil,
+        author: nil
+    )
+    return AppData.cdProfileRepositoryV3(
+        registry: .shared,
+        coder: CodableProfileCoder(),
+        context: store.context,
+        observingResults: false
+    ) { error in
+        pp_log(.app, .error, "Unable to decode local result: \(error)")
+        return .ignore
+    }
+}
+
 #else

 extension Tunnel {
    static let shared = Tunnel(
-        strategy: NETunnelStrategy(
-            bundleIdentifier: BundleConfiguration.mainString(for: .tunnelId),
-            encoder: .shared,
-            environment: .shared
-        )
+        strategy: NETunnelStrategy(repository: neRepository)
+    )
+}
+
+private var localProfileRepository: any ProfileRepository {
+    NEProfileRepository(repository: neRepository)
+}
+
+private var neRepository: NETunnelManagerRepository {
+    NETunnelManagerRepository(
+        bundleIdentifier: BundleConfiguration.mainString(for: .tunnelId),
+        coder: Registry.sharedProtocolCoder,
+        environment: .shared
    )
 }

--- a/Passepartout/Shared/Shared.swift
+++ b/Passepartout/Shared/Shared.swift
@ -59,6 +59,15 @@ extension Registry {
            )
        ]
    )
+
+    static var sharedProtocolCoder: KeychainNEProtocolCoder {
+        KeychainNEProtocolCoder(
+            tunnelBundleIdentifier: BundleConfiguration.mainString(for: .tunnelId),
+            registry: .shared,
+            coder: CodableProfileCoder(),
+            keychain: AppleKeychain(group: BundleConfiguration.mainString(for: .keychainGroupId))
+        )
+    }
 }

 extension TunnelEnvironment where Self == AppGroupEnvironment {
@ -69,24 +78,3 @@ extension TunnelEnvironment where Self == AppGroupEnvironment {
        )
    }
 }
-
-extension NEProtocolEncoder where Self == KeychainNEProtocolCoder {
-    static var shared: Self {
-        sharedProtocolCoder
-    }
-}
-
-extension NEProtocolDecoder where Self == KeychainNEProtocolCoder {
-    static var shared: Self {
-        sharedProtocolCoder
-    }
-}
-
-private var sharedProtocolCoder: KeychainNEProtocolCoder {
-    KeychainNEProtocolCoder(
-        tunnelBundleIdentifier: BundleConfiguration.mainString(for: .tunnelId),
-        registry: .shared,
-        coder: CodableProfileCoder(),
-        keychain: AppleKeychain(group: BundleConfiguration.mainString(for: .keychainGroupId))
-    )
-}
--- a/Passepartout/Tunnel/PacketTunnelProvider.swift
+++ b/Passepartout/Tunnel/PacketTunnelProvider.swift
@ -38,7 +38,7 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
        )
        fwd = try await NEPTPForwarder(
            provider: self,
-            decoder: .shared,
+            decoder: Registry.sharedProtocolCoder,
            registry: .shared,
            environment: .shared
        )