Rewrite AppContext event handlers (#839)

Loading remote profiles before local profiles may cause duplicated NE
managers. This happened because if local profiles are empty, any remote
profile is imported regardless of their former existence in the local
store. The importer just doesn't know.

Therefore, revisit the sequence of AppContext registrations:

- First off
- Skip Tunnel prepare() because NEProfileRepository.fetch() does it
already
- NE is both Tunnel and ProfileRepository, so calling tunnel.prepare()
loads local NE profiles twice
- onLaunch() - **run this once and before anything else**
  - Read local profiles
  - Reload in-app receipt
  - Observe in-app eligibility → Triggers onEligibleFeatures()
  - Observe profile save → Triggers onSaveProfile()
  - Fetch providers index
- onForeground()
  - Read local profiles
  - Read remote profiles, and toggle CloudKit sync based on eligibility
- onEligibleFeatures()
  - Read remote profiles, and toggle CloudKit sync based on eligibility
- onSaveProfile()
  - Reconnect if necessary
This commit is contained in:
Davide 2024-11-10 17:51:28 +01:00 committed by GitHub
parent fdbed7442c
commit 21340e9f56
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 289 additions and 198 deletions

View File

@ -41,7 +41,7 @@
"kind" : "remoteSourceControl",
"location" : "git@github.com:passepartoutvpn/passepartoutkit-source",
"state" : {
"revision" : "b31816d060e40583a27d22ea5c59cc686c057aaf"
"revision" : "3a4c78af67dfe181acc657a5539ee3d62d1c9361"
}
},
{

View File

@ -36,10 +36,5 @@ final class AppDelegate: NSObject {
func configure(with uiConfiguring: UILibraryConfiguring) {
UILibrary(uiConfiguring)
.configure(with: context)
Task {
pp_log(.app, .notice, "Fetch providers index...")
try await context.providerManager.fetchIndex(from: API.shared)
}
}
}

View File

@ -46,7 +46,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: "b31816d060e40583a27d22ea5c59cc686c057aaf"),
.package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "3a4c78af67dfe181acc657a5539ee3d62d1c9361"),
// .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"),

View File

@ -41,19 +41,23 @@ extension AppData {
) -> ProfileRepository {
let repository = CoreDataRepository<CDProfileV3, Profile>(
context: context,
observingResults: observingResults
) {
observingResults: observingResults,
beforeFetch: {
$0.sortDescriptors = [
.init(key: "name", ascending: true, selector: #selector(NSString.caseInsensitiveCompare)),
.init(key: "lastUpdate", ascending: false)
]
} fromMapper: {
},
fromMapper: {
try fromMapper($0, registry: registry, coder: coder)
} toMapper: {
},
toMapper: {
try toMapper($0, $1, registry: registry, coder: coder)
} onResultError: {
},
onResultError: {
onResultError?($0) ?? .ignore
}
)
return repository
}
}
@ -112,6 +116,10 @@ extension CoreDataRepository: ProfileRepository where T == Profile {
.eraseToAnyPublisher()
}
public func fetchProfiles() async throws -> [Profile] {
try await fetchAllEntities()
}
public func saveProfile(_ profile: Profile) async throws {
try await saveEntities([profile])
}

View File

@ -98,11 +98,6 @@ extension ExtendedTunnel {
tunnel.currentProfile
}
public func prepare(purge: Bool) async throws {
pp_log(.app, .notice, "Prepare tunnel and purge stale data (\(purge))...")
try await tunnel.prepare(purge: purge)
}
public func install(_ profile: Profile) async throws {
pp_log(.app, .notice, "Install profile \(profile.id)...")
let newProfile = try processedProfile(profile)

View File

@ -45,6 +45,10 @@ public final class InMemoryProfileRepository: ProfileRepository {
profilesSubject.eraseToAnyPublisher()
}
public func fetchProfiles() async throws -> [Profile] {
profiles
}
public func saveProfile(_ profile: Profile) {
pp_log(.App.profiles, .info, "Save profile: \(profile.id))")
if let index = profiles.firstIndex(where: { $0.id == profile.id }) {

View File

@ -43,14 +43,6 @@ public final class NEProfileRepository: ProfileRepository {
profilesSubject = CurrentValueSubject([])
subscriptions = []
repository
.managersPublisher
.first()
.sink { [weak self] in
self?.onLoadedManagers($0)
}
.store(in: &subscriptions)
repository
.managersPublisher
.dropFirst()
@ -64,6 +56,20 @@ public final class NEProfileRepository: ProfileRepository {
profilesSubject.eraseToAnyPublisher()
}
public func fetchProfiles() async throws -> [Profile] {
let managers = try await repository.fetch()
let profiles = managers.compactMap {
do {
return try repository.profile(from: $0)
} catch {
pp_log(.App.profiles, .error, "Unable to decode profile from NE manager '\($0.localizedDescription ?? "")': \(error)")
return nil
}
}
profilesSubject.send(profiles)
return profiles
}
public func saveProfile(_ profile: Profile) async throws {
try await repository.save(profile, forConnecting: false, title: title)
if let index = profilesSubject.value.firstIndex(where: { $0.id == profile.id }) {
@ -74,6 +80,9 @@ public final class NEProfileRepository: ProfileRepository {
}
public func removeProfiles(withIds profileIds: [Profile.ID]) async throws {
guard !profileIds.isEmpty else {
return
}
var removedIds: Set<Profile.ID> = []
defer {
profilesSubject.value.removeAll {
@ -92,18 +101,6 @@ public final class NEProfileRepository: ProfileRepository {
}
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.profiles, .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

View File

@ -286,9 +286,15 @@ private extension ProfileManager {
// MARK: - Observation
extension ProfileManager {
public func observeObjects(searchDebounce: Int = 200) {
public func observeLocal(searchDebounce: Int = 200) async throws {
subscriptions.removeAll()
let initialProfiles = try await repository.fetchProfiles()
reloadLocalProfiles(initialProfiles)
repository
.profilesPublisher
.dropFirst()
.receive(on: DispatchQueue.main)
.sink { [weak self] in
self?.reloadLocalProfiles($0)
@ -303,35 +309,29 @@ extension ProfileManager {
.store(in: &subscriptions)
}
public func enableRemoteImporting(_ isRemoteImportingEnabled: Bool) {
public func observeRemote(_ isRemoteImportingEnabled: Bool) async throws {
guard let remoteRepositoryBlock else {
// preconditionFailure("Missing remoteRepositoryBlock")
return
}
guard remoteRepository == nil || isRemoteImportingEnabled != self.isRemoteImportingEnabled else {
return
}
self.isRemoteImportingEnabled = isRemoteImportingEnabled
remoteSubscriptions.removeAll()
remoteRepository = remoteRepositoryBlock(isRemoteImportingEnabled)
remoteRepository?
.profilesPublisher
.first()
.receive(on: DispatchQueue.main)
.sink { [weak self] in
self?.loadInitialRemoteProfiles($0)
remoteRepository = remoteRepositoryBlock(isRemoteImportingEnabled)
if let initialProfiles = try await remoteRepository?.fetchProfiles() {
reloadRemoteProfiles(initialProfiles, importing: false)
}
.store(in: &remoteSubscriptions)
remoteRepository?
.profilesPublisher
.dropFirst()
.receive(on: DispatchQueue.main)
.sink { [weak self] in
self?.reloadRemoteProfiles($0)
self?.reloadRemoteProfiles($0, importing: true)
}
.store(in: &remoteSubscriptions)
}
@ -343,6 +343,7 @@ private extension ProfileManager {
allProfiles = result.reduce(into: [:]) {
$0[$1.id] = $1
}
// objectWillChange implicit from updating profiles in didSet
// should not be imported at all, but you never know
if let processor {
@ -361,21 +362,17 @@ private extension ProfileManager {
}
}
func loadInitialRemoteProfiles(_ result: [Profile]) {
pp_log(.App.profiles, .info, "Load initial remote profiles: \(result.map(\.id))")
allRemoteProfiles = result.reduce(into: [:]) {
$0[$1.id] = $1
}
objectWillChange.send()
}
func reloadRemoteProfiles(_ result: [Profile]) {
func reloadRemoteProfiles(_ result: [Profile], importing: Bool) {
pp_log(.App.profiles, .info, "Reload remote profiles: \(result.map(\.id))")
allRemoteProfiles = result.reduce(into: [:]) {
$0[$1.id] = $1
}
objectWillChange.send()
guard importing else {
return
}
Task.detached { [weak self] in
guard let self else {
return

View File

@ -30,6 +30,8 @@ import PassepartoutKit
public protocol ProfileRepository {
var profilesPublisher: AnyPublisher<[Profile], Never> { get }
func fetchProfiles() async throws -> [Profile]
func saveProfile(_ profile: Profile) async throws
func removeProfiles(withIds profileIds: [Profile.ID]) async throws

View File

@ -27,6 +27,8 @@ import Foundation
import PassepartoutKit
public enum AppError: Error {
case couldNotLaunch(reason: Error)
case emptyProducts
case emptyProfileName

View File

@ -279,6 +279,9 @@ private extension IAPManager {
pp_log(.App.iap, .info, "App level (custom): \(userLevel)")
} else {
let isBeta = await SandboxChecker().isBeta
guard userLevel == .undefined else {
return
}
userLevel = isBeta ? .beta : .freemium
pp_log(.App.iap, .info, "App level: \(userLevel)")
}

View File

@ -89,16 +89,6 @@ public actor CoreDataRepository<CD, T>: NSObject,
sectionNameKeyPath: nil,
cacheName: nil
)
super.init()
resultsController.delegate = self
do {
try resultsController.performFetch()
sendResults(from: resultsController)
} catch {
//
}
}
public nonisolated var entitiesPublisher: AnyPublisher<EntitiesResult<T>, Never> {
@ -114,6 +104,10 @@ public actor CoreDataRepository<CD, T>: NSObject,
try await filter(byPredicate: nil)
}
public func fetchAllEntities() async throws -> [T] {
try await filter(byPredicate: nil)
}
public func saveEntities(_ entities: [T]) async throws {
try await context.perform { [weak self] in
guard let self else {
@ -154,7 +148,6 @@ public actor CoreDataRepository<CD, T>: NSObject,
do {
let existing = try context.fetch(request)
existing.forEach(context.delete)
try context.save()
} catch {
context.rollback()
@ -173,7 +166,9 @@ public actor CoreDataRepository<CD, T>: NSObject,
guard let cdController = controller as? NSFetchedResultsController<CD> else {
fatalError("Unable to upcast results to \(CD.self)")
}
sendResults(from: cdController)
Task.detached { [weak self] in
await self?.sendResults(from: cdController)
}
}
}
@ -182,7 +177,12 @@ private extension CoreDataRepository {
case mapping(Error)
}
func filter(byPredicate predicate: NSPredicate?) async throws {
nonisolated func newFetchRequest() -> NSFetchRequest<CD> {
NSFetchRequest(entityName: entityName)
}
@discardableResult
func filter(byPredicate predicate: NSPredicate?) async throws -> [T] {
let request = resultsController.fetchRequest
request.predicate = predicate
resultsController = NSFetchedResultsController(
@ -193,20 +193,24 @@ private extension CoreDataRepository {
)
resultsController.delegate = self
try resultsController.performFetch()
sendResults(from: resultsController)
return await sendResults(from: resultsController)
}
nonisolated func newFetchRequest() -> NSFetchRequest<CD> {
NSFetchRequest(entityName: entityName)
@discardableResult
func sendResults(from controller: NSFetchedResultsController<CD>) async -> [T] {
await context.perform {
self.unsafeSendResults(from: controller)
}
}
nonisolated func sendResults(from controller: NSFetchedResultsController<CD>) {
Task.detached { [weak self] in
await self?.context.perform { [weak self] in
@discardableResult
func unsafeSendResults(from controller: NSFetchedResultsController<CD>) -> [T] {
guard let cdEntities = controller.fetchedObjects else {
return
return []
}
var entitiesToDelete: [CD] = []
// strip duplicates by sort order (first entry wins)
var knownUUIDs = Set<UUID>()
cdEntities.forEach {
@ -215,7 +219,7 @@ private extension CoreDataRepository {
}
guard !knownUUIDs.contains(uuid) else {
NSLog("Strip duplicate \(String(describing: CD.self)) with UUID \(uuid)")
self?.context.delete($0)
entitiesToDelete.append($0)
return
}
knownUUIDs.insert(uuid)
@ -224,11 +228,11 @@ private extension CoreDataRepository {
do {
let entities = try cdEntities.compactMap {
do {
return try self?.fromMapper($0)
return try fromMapper($0)
} catch {
switch self?.onResultError?(error) {
switch onResultError?(error) {
case .discard:
self?.context.delete($0)
entitiesToDelete.append($0)
case .halt:
throw ResultError.mapping(error)
@ -240,14 +244,22 @@ private extension CoreDataRepository {
}
}
try self?.context.save()
if !entitiesToDelete.isEmpty {
do {
entitiesToDelete.forEach(context.delete)
try context.save()
} catch {
NSLog("Unable to delete Core Data entities: \(error)")
context.rollback()
}
}
let result = EntitiesResult(entities, isFiltering: controller.fetchRequest.predicate != nil)
self?.entitiesSubject.send(result)
entitiesSubject.send(result)
return result.entities
} catch {
NSLog("Unable to send Core Data entities: \(error)")
}
}
return []
}
}
}

View File

@ -37,11 +37,7 @@ public final class StoreKitHelper<ProductType>: InAppHelper where ProductType: R
private var nativeProducts: [ProductType: InAppProduct]
private var activeTransactions: Set<Transaction> {
didSet {
didUpdateSubject.send()
}
}
private var activeTransactions: Set<Transaction>
private let didUpdateSubject: PassthroughSubject<Void, Never>
@ -96,11 +92,13 @@ extension StoreKitHelper {
}
switch try await skProduct.purchase() {
case .success(let verificationResult):
if let transaction = try? verificationResult.payloadValue {
guard let transaction = try? verificationResult.payloadValue else {
break
}
activeTransactions.insert(transaction)
didUpdateSubject.send()
await transaction.finish()
return .done
}
case .pending:
return .pending
@ -143,5 +141,6 @@ private extension StoreKitHelper {
}
}
self.activeTransactions = activeTransactions
didUpdateSubject.send()
}
}

View File

@ -41,7 +41,9 @@ public final class AppContext: ObservableObject {
public let providerManager: ProviderManager
private var isActivating = false
private var launchTask: Task<Void, Error>?
private var pendingTask: Task<Void, Never>?
private var subscriptions: Set<AnyCancellable>
@ -58,70 +60,167 @@ public final class AppContext: ObservableObject {
self.tunnel = tunnel
self.providerManager = providerManager
subscriptions = []
observeObjects()
}
public func onApplicationActive() {
guard !isActivating else {
return
}
isActivating = true
pp_log(.app, .notice, "Application became active")
Task {
await withTaskGroup(of: Void.self) { group in
group.addTask {
do {
try await self.tunnel.prepare(purge: true)
} catch {
pp_log(.app, .fault, "Unable to prepare tunnel: \(error)")
}
}
group.addTask { [weak self] in
guard let self else {
return
}
await iapManager.reloadReceipt()
}
}
isActivating = false
}
}
}
// MARK: - Observation
// invoked by AppDelegate
extension AppContext {
public func onApplicationActive() {
Task {
// TODO: ###, should handle AppError.couldNotLaunch (although extremely rare)
try await onForeground()
}
}
}
// invoked on internal events
private extension AppContext {
func observeObjects() {
iapManager
.observeObjects()
func onLaunch() async throws {
pp_log(.app, .notice, "Application did launch")
pp_log(.App.profiles, .info, "Read and observe local profiles...")
try await profileManager.observeLocal()
iapManager.observeObjects()
await iapManager.reloadReceipt()
iapManager
.$eligibleFeatures
.removeDuplicates()
.sink { [weak self] in
self?.syncEligibleFeatures($0)
.sink { [weak self] eligible in
Task {
try await self?.onEligibleFeatures(eligible)
}
}
.store(in: &subscriptions)
profileManager
.observeObjects()
profileManager
.didChange
.sink { [weak self] event in
switch event {
case .save(let profile):
self?.syncTunnelIfCurrentProfile(profile)
Task {
try await self?.onSaveProfile(profile)
}
default:
break
}
}
.store(in: &subscriptions)
do {
pp_log(.app, .notice, "Fetch providers index...")
try await providerManager.fetchIndex(from: API.shared)
} catch {
pp_log(.app, .error, "Unable to fetch providers index: \(error)")
}
}
func onForeground() async throws {
let didLaunch = try await waitForTasks()
guard !didLaunch else {
return // foreground is redundant after launch
}
pp_log(.app, .notice, "Application did enter foreground")
pendingTask = Task {
do {
pp_log(.App.profiles, .info, "Refresh local profiles observers...")
try await profileManager.observeLocal()
} catch {
pp_log(.App.profiles, .error, "Unable to re-observe local profiles: \(error)")
}
await iapManager.reloadReceipt()
}
await pendingTask?.value
pendingTask = nil
}
func onEligibleFeatures(_ features: Set<AppFeature>) async throws {
try await waitForTasks()
pp_log(.app, .notice, "Application did update eligible features")
pendingTask = Task {
let isEligible = features.contains(.sharing)
do {
pp_log(.App.profiles, .info, "Refresh remote profiles observers (eligible=\(isEligible), CloudKit=\(isCloudKitEnabled))...")
try await profileManager.observeRemote(isEligible && isCloudKitEnabled)
} catch {
pp_log(.App.profiles, .error, "Unable to re-observe remote profiles: \(error)")
}
}
await pendingTask?.value
pendingTask = nil
}
func onSaveProfile(_ profile: Profile) async throws {
try await waitForTasks()
pp_log(.app, .notice, "Application did save profile (\(profile.id))")
guard profile.id == tunnel.currentProfile?.id else {
pp_log(.app, .debug, "Profile \(profile.id) is not current, do nothing")
return
}
guard [.active, .activating].contains(tunnel.status) else {
pp_log(.app, .debug, "Connection is not active (\(tunnel.status)), do nothing")
return
}
pendingTask = Task {
do {
if profile.isInteractive {
pp_log(.app, .info, "Profile \(profile.id) is interactive, disconnect")
try await tunnel.disconnect()
return
}
do {
pp_log(.app, .info, "Reconnect profile \(profile.id)")
try await tunnel.connect(with: profile)
} catch {
pp_log(.app, .error, "Unable to reconnect profile \(profile.id), disconnect: \(error)")
try await tunnel.disconnect()
}
} catch {
pp_log(.app, .error, "Unable to reinstate connection on save profile \(profile.id): \(error)")
}
}
await pendingTask?.value
pendingTask = nil
}
@discardableResult
func waitForTasks() async throws -> Bool {
var didLaunch = false
// must launch once before anything else
if launchTask == nil {
launchTask = Task {
do {
try await onLaunch()
} catch {
launchTask = nil // redo launch
throw AppError.couldNotLaunch(reason: error)
}
}
didLaunch = true
}
// will throw on .couldNotLaunch
// next wait will re-attempt launch (launchTask == nil)
try await launchTask?.value
// wait for pending task if any
await pendingTask?.value
pendingTask = nil
return didLaunch
}
}
// MARK: - Helpers
private extension AppContext {
var isCloudKitEnabled: Bool {
#if os(tvOS)
@ -130,29 +229,4 @@ private extension AppContext {
FileManager.default.ubiquityIdentityToken != nil
#endif
}
func syncEligibleFeatures(_ eligible: Set<AppFeature>) {
let canImport = eligible.contains(.sharing)
profileManager.enableRemoteImporting(canImport && isCloudKitEnabled)
}
func syncTunnelIfCurrentProfile(_ profile: Profile) {
guard profile.id == tunnel.currentProfile?.id else {
return
}
Task {
guard [.active, .activating].contains(tunnel.status) else {
return
}
if profile.isInteractive {
try await tunnel.disconnect()
return
}
do {
try await tunnel.connect(with: profile)
} catch {
try await tunnel.disconnect()
}
}
}
}

View File

@ -32,6 +32,9 @@ extension AppError: LocalizedError {
public var errorDescription: String? {
let V = Strings.Errors.App.self
switch self {
case .couldNotLaunch(let reason):
return reason.localizedDescription
case .emptyProducts:
return V.emptyProducts