Bind navigation to ProfileManager directly

- Do the profile loading inside the model

- Allow setting current profile to a transient profile

- Check .placeholder before saving current profile

XXX: avoid loading active profile on iPad portrait.

Passepartout/App/Constants/AppContext.swift

@@ -139,12 +139,8 @@ class AppContext {
         profileManager.observeUpdates()
         vpnManager.observeUpdates()
 
-        if let activeProfileId = appManager.activeProfileId, profileManager.setActiveProfileId(activeProfileId) {
+        if let activeProfileId = appManager.activeProfileId {
-            do {
+            profileManager.setActiveProfileId(activeProfileId)
-                try profileManager.loadCurrentProfile(withId: activeProfileId)
-            } catch {
-                pp_log.warning("Unable to load active profile: \(error)")
-            }
         }
 
         // app

Passepartout/App/Views/OrganizerView+Profiles.swift

@@ -35,32 +35,6 @@ extension OrganizerView {
         // just to observe changes in profiles eligibility
         @ObservedObject private var productManager: ProductManager
         
-        @State private var isFirstLaunch = true
-
-        @State private var presentedProfileId: UUID?
-
-        private var presentedAndLoadedProfileId: Binding<UUID?> {
-            .init {
-                presentedProfileId
-            } set: {
-                guard $0 != presentedProfileId else {
-                    return
-                }
-                guard let id = $0 else {
-                    presentedProfileId = nil
-                    return
-                }
-                presentedProfileId = id
-
-                // load profile contextually with navigation
-                do {
-                    try profileManager.loadCurrentProfile(withId: id)
-                } catch {
-                    pp_log.error("Unable to load profile: \(error)")
-                }
-            }
-        }
-
         init() {
             profileManager = .shared
             providerManager = .shared
@@ -74,15 +48,10 @@ extension OrganizerView {
                 if !profileManager.hasProfiles {
                     emptyView
                 }
-            }
+            }.onAppear {
-
-            // events
-            .onAppear {
                 performMigrationsIfNeeded()
-            }.onChange(of: profileManager.headers) {
-                dismissSelectionIfDeleted(headers: $0)
             }.onReceive(profileManager.didCreateProfile) {
-                presentedAndLoadedProfileId.wrappedValue = $0.id
+                profileManager.currentProfileId = $0.id
             }
         }
         
@@ -116,14 +85,12 @@ extension OrganizerView {
         }
 
         private func profileRow(forHeader header: Profile.Header) -> some View {
-            NavigationLink(tag: header.id, selection: presentedAndLoadedProfileId) {
+            NavigationLink(tag: header.id, selection: $profileManager.currentProfileId) {
                 ProfileView()
             } label: {
                 profileLabel(forHeader: header)
             }.contextMenu {
                 profileMenu(forHeader: header)
-            }.onAppear {
-                presentIfActiveProfile(header.id)
             }
         }
         
@@ -155,46 +122,12 @@ extension OrganizerView {
                 }
         }
 
-        private func presentIfActiveProfile(_ id: UUID) {
-            guard id == profileManager.activeProfileId else {
-                return
-            }
-            presentActiveProfile()
-        }
-        
-        private func presentActiveProfile() {
-            guard isFirstLaunch else {
-                return
-            }
-            isFirstLaunch = false
-
-            guard let activeProfileId = profileManager.activeProfileId else {
-                return
-            }
-
-            // FIXME: iPad portrait/compact, preselecting profile on launch adds ProfileView() twice
-            // can notice becase "Back" needs to be tapped twice to show sidebar
-            if themeIdiom != .pad {
-                presentedProfileId = activeProfileId
-            }
-        }
-        
         private func removeProfiles(at offsets: IndexSet) {
             let currentHeaders = sortedHeaders
             var toDelete: [UUID] = []
             offsets.forEach {
                 toDelete.append(currentHeaders[$0].id)
             }
-            removeProfiles(withIds: toDelete)
-        }
-
-        private func removeProfiles(withIds toDelete: [UUID]) {
-
-            // clear selection before removal to avoid triggering a bogus navigation push
-            if toDelete.contains(profileManager.currentProfile.value.id) {
-                presentedProfileId = nil
-            }
-
             withAnimation {
                 profileManager.removeProfiles(withIds: toDelete)
             }
@@ -205,11 +138,5 @@ extension OrganizerView {
                 await AppManager.shared.doMigrations(profileManager)
             }
         }
-        
-        private func dismissSelectionIfDeleted(headers: [Profile.Header]) {
-            if let _ = presentedProfileId, !profileManager.isCurrentProfileExisting() {
-                presentedProfileId = nil
-            }
-        }
     }
 }

Passepartout/App/Views/OrganizerView+Scene.swift

@@ -40,6 +40,8 @@ extension OrganizerView {
         
         @Binding private var didHandleSubreddit: Bool
         
+        @State private var isFirstLaunch = true
+
         init(alertType: Binding<AlertType?>, didHandleSubreddit: Binding<Bool>) {
             profileManager = .shared
             vpnManager = .shared
@@ -58,8 +60,26 @@ extension OrganizerView {
         }
         
         private func onAppear() {
-            if !didHandleSubreddit {
+            guard didHandleSubreddit else {
                 alertType = .subscribeReddit
+                return
+            }
+
+            //
+            // FIXME: iPad portrait/compact, loading current profile adds ProfileView() twice
+            //
+            // - from MainView
+            // - from NavigationLink destination in OrganizerView
+            //
+            // can notice becase "Back" needs to be tapped twice to show sidebar
+            // workaround: set active profile but do not load as current (prevents NavigationLink activation)
+            //
+            guard isFirstLaunch else {
+                return
+            }
+            isFirstLaunch = false
+            if !isiPadPortrait, let activeProfileId = profileManager.activeProfileId {
+                profileManager.currentProfileId = activeProfileId
             }
         }
 
@@ -83,5 +103,10 @@ extension OrganizerView {
         private func persist() {
             profileManager.persist()
         }
+        
+        private var isiPadPortrait: Bool {
+            let device: UIDevice = .current
+            return device.userInterfaceIdiom == .pad && device.orientation.isPortrait
+        }
     }
 }

Passepartout/App/Views/ProfileView+MainMenu.swift

@@ -212,11 +212,7 @@ extension ProfileView {
                 return
             }
             if switchCurrentProfile {
-                do {
+                profileManager.currentProfileId = copy.id
-                    try profileManager.loadCurrentProfile(withId: copy.id)
-                } catch {
-                    pp_log.warning("Unable to load profile duplicate: \(error)")
-                }
             }
         }
     }

PassepartoutCore/Sources/PassepartoutCore/Managers/VPNManager+Actions.swift

@@ -43,7 +43,7 @@ extension VPNManager {
     }
 
     public func connect(with profileId: UUID) async throws {
-        let result = try profileManager.profileEx(withId: profileId)
+        let result = try profileManager.liveProfileEx(withId: profileId)
         let profile = result.profile
         guard !profileManager.isActiveProfile(profileId) ||
               currentState.vpnStatus != .connected else {
@@ -64,7 +64,7 @@ extension VPNManager {
     }
     
     public func connect(with profileId: UUID, toServer newServerId: String) async throws {
-        let result = try profileManager.profileEx(withId: profileId)
+        let result = try profileManager.liveProfileEx(withId: profileId)
         var profile = result.profile
         guard profile.isProvider else {
             assertionFailure("Profile \(profile.logDescription) is not a provider")

PassepartoutCore/Sources/PassepartoutProfiles/Managers/ProfileManager.swift

@@ -50,8 +50,21 @@ public class ProfileManager: ObservableObject {
     // MARK: Observables
 
     @Published public private(set) var activeProfileId: UUID? {
-        didSet {
+        willSet {
-            pp_log.debug("Active profile updated: \(activeProfileId?.uuidString ?? "nil")")
+            pp_log.debug("Setting active profile: \(activeProfileId?.uuidString ?? "nil")")
+        }
+    }
+
+    @Published public var currentProfileId: UUID? {
+        willSet {
+            pp_log.debug("Setting current profile: \(newValue?.uuidString ?? "nil")")
+            guard let id = newValue else {
+                return
+            }
+            guard let profile = liveProfile(withId: id) else {
+                return
+            }
+            setCurrentProfile(profile)
         }
     }
 
@@ -79,13 +92,12 @@ public class ProfileManager: ObservableObject {
         currentProfile = ObservableProfile()
     }
     
-    public func setActiveProfileId(_ id: UUID) -> Bool {
+    public func setActiveProfileId(_ id: UUID) {
         guard isExistingProfile(withId: id) else {
             pp_log.warning("Active profile \(id) does not exist, ignoring")
-            return false
+            return
         }
         activeProfileId = id
-        return true
     }
 }
 
@@ -139,15 +151,15 @@ extension ProfileManager {
         guard let id = activeProfileId else {
             return nil
         }
-        return profile(withId: id)
+        return liveProfile(withId: id)
     }
 
     public func activateProfile(_ profile: Profile) {
         saveProfile(profile, isActive: true)
     }
 
-    public func profileEx(withId id: UUID) throws -> ProfileEx {
+    public func liveProfileEx(withId id: UUID) throws -> ProfileEx {
-        guard let profile = profile(withId: id) else {
+        guard let profile = liveProfile(withId: id) else {
             pp_log.error("Profile not found: \(id)")
             throw PassepartoutError.missingProfile
         }
@@ -155,7 +167,7 @@ extension ProfileManager {
         return (profile, isProfileReady(profile))
     }
 
-    private func profile(withId id: UUID) -> Profile? {
+    private func liveProfile(withId id: UUID) -> Profile? {
         pp_log.debug("Searching profile \(id)")
 
         // IMPORTANT: fetch live copy first (see intents)
@@ -228,58 +240,75 @@ extension ProfileManager {
     }
     
     public func duplicateProfile(withId id: UUID) -> Profile? {
-        guard let source = profile(withId: id) else {
+        guard let source = liveProfile(withId: id) else {
             return nil
         }
         let copy = source
             .withNewId()
             .renamedUniquely(withLastUpdate: false)
-
+        
         saveProfile(copy, isActive: nil)
         return copy
     }
 
     public func persist() {
-        pp_log.info("Persisting profiles")
+        pp_log.info("Persisting pending profiles")
-        saveCurrentProfile()
+        if !currentProfile.value.isPlaceholder {
+            saveProfile(currentProfile.value, isActive: nil, updateIfCurrent: false)
+        }
     }
 }
 
 // MARK: Observation
 
 extension ProfileManager {
-    public func loadCurrentProfile(withId id: UUID) throws {
+    private func setCurrentProfile(_ profile: Profile) {
         guard !currentProfile.isLoading else {
             pp_log.warning("Already loading another profile")
             return
         }
-        guard id != currentProfile.value.id else {
+        guard profile.id != currentProfile.value.id else {
-            pp_log.debug("Profile \(id) is already current profile")
+            pp_log.debug("Profile \(profile.logDescription) is already current profile")
             return
         }
+
+        pp_log.info("Set current profile: \(profile.logDescription)")
+
+        //
+        // IMPORTANT: this method is called on app launch if there is an active profile, which
+        // means that carelessly calling .saveProfiles() may trigger an unnecessary
+        // willUpdateProfiles() and a potential animation in subscribers (e.g. OrganizerView)
+        //
+        // current profile, when set on launch, is never fetched from pendingProfiles, so we take
+        // care of checking that to avoid an undesired save
+        //
+        var profilesToSave: [Profile] = []
         if isExistingProfile(withId: currentProfile.value.id) {
-            pp_log.info("Committing changes of former current profile \(currentProfile.value.logDescription)")
+            pp_log.info("Defer saving of former current profile \(currentProfile.value.logDescription)")
-            saveCurrentProfile()
+            profilesToSave.append(currentProfile.value)
+        }
+        if let _ = pendingProfiles[profile.id] {
+            pp_log.info("Defer saving of transient current profile \(profile.logDescription)")
+            profilesToSave.append(profile)
+        }
+        defer {
+            if !profilesToSave.isEmpty {
+                strategy.saveProfiles(profilesToSave)
+            }
         }
 
-        let result = try profileEx(withId: id)
+        if isProfileReady(profile) {
-        pp_log.info("Current profile: \(result.profile.logDescription)")
+            currentProfile.value = profile
-        if result.isReady {
-            currentProfile.value = result.profile
         } else {
             currentProfile.isLoading = true
             Task {
-                try await makeProfileReady(result.profile)
+                try await makeProfileReady(profile)
-                currentProfile.value = result.profile
+                currentProfile.value = profile
                 currentProfile.isLoading = false
             }
         }
     }
-    
+
-    public func isCurrentProfileExisting() -> Bool {
-        isExistingProfile(withId: currentProfile.value.id)
-    }
-    
     public func isCurrentProfileActive() -> Bool {
         currentProfile.value.id == activeProfileId
     }
@@ -291,10 +320,6 @@ extension ProfileManager {
     public func activateCurrentProfile() {
         saveProfile(currentProfile.value, isActive: true, updateIfCurrent: false)
     }
-    
-    public func saveCurrentProfile() {
-        saveProfile(currentProfile.value, isActive: nil, updateIfCurrent: false)
-    }
 }
 
 extension ProfileManager {
@@ -366,7 +391,7 @@ extension ProfileManager {
             headers.forEach { dupHeader in
                 let uniqueHeader = dupHeader.renamedUniquely(withLastUpdate: true)
                 pp_log.debug("Renaming duplicate profile \(dupHeader.logDescription) to \(uniqueHeader.logDescription)")
-                guard var uniqueProfile = profile(withId: uniqueHeader.id) else {
+                guard var uniqueProfile = liveProfile(withId: uniqueHeader.id) else {
                     pp_log.warning("Skipping profile \(dupHeader.logDescription) renaming, not found")
                     return
                 }