♻️ Reworked ValetServicesManager w/ DataManager actor

2025-12-21 03:10:06 +01:00 · 2025-11-15 11:59:37 +01:00
parent 5f9c2f8bf5
commit ac0b57c63f
3 changed files with 182 additions and 132 deletions
--- a/Monitor.xcodeproj/project.pbxproj
+++ b/Monitor.xcodeproj/project.pbxproj
@@ -104,6 +104,10 @@
 		03C099452EA15C8E00B76D43 /* Container+Real.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C099432EA15C8B00B76D43 /* Container+Real.swift */; };
 		03C099462EA15C8E00B76D43 /* Container+Real.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C099432EA15C8B00B76D43 /* Container+Real.swift */; };
 		03C099472EA15C8E00B76D43 /* Container+Real.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C099432EA15C8B00B76D43 /* Container+Real.swift */; };
+		03C29A772EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C29A762EC88E2C00FBA25E /* ValetServicesDataManager.swift */; };
+		03C29A782EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C29A762EC88E2C00FBA25E /* ValetServicesDataManager.swift */; };
+		03C29A792EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C29A762EC88E2C00FBA25E /* ValetServicesDataManager.swift */; };
+		03C29A7A2EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03C29A762EC88E2C00FBA25E /* ValetServicesDataManager.swift */; };
 		03CC1FE52E3D22120050FC18 /* InstallHomebrew.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03CC1FE42E3D220F0050FC18 /* InstallHomebrew.swift */; };
 		03CC1FE62E3D22120050FC18 /* InstallHomebrew.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03CC1FE42E3D220F0050FC18 /* InstallHomebrew.swift */; };
 		03CC1FE72E3D22120050FC18 /* InstallHomebrew.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03CC1FE42E3D220F0050FC18 /* InstallHomebrew.swift */; };
@@ -1043,6 +1047,7 @@
 		03BFF5262E312C39007F96FA /* Startup+Timers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Startup+Timers.swift"; sourceTree = "<group>"; };
 		03BFF52B2E313240007F96FA /* StatusMenu+Driver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "StatusMenu+Driver.swift"; sourceTree = "<group>"; };
 		03C099432EA15C8B00B76D43 /* Container+Real.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Container+Real.swift"; sourceTree = "<group>"; };
+		03C29A762EC88E2C00FBA25E /* ValetServicesDataManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ValetServicesDataManager.swift; sourceTree = "<group>"; };
 		03CC1FE42E3D220F0050FC18 /* InstallHomebrew.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InstallHomebrew.swift; sourceTree = "<group>"; };
 		03CC1FF32E3D230B0050FC18 /* ZshRunCommand.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ZshRunCommand.swift; sourceTree = "<group>"; };
 		03D846242EB6344A006EFE3C /* DomainListVC+Window.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DomainListVC+Window.swift"; sourceTree = "<group>"; };
@@ -1998,6 +2003,7 @@
 				C45B9148295607F400F4EC78 /* Service.swift */,
 				C45E76132854A65300B4FE0C /* ServicesManager.swift */,
 				C45B914D295608E300F4EC78 /* ValetServicesManager.swift */,
+				03C29A762EC88E2C00FBA25E /* ValetServicesDataManager.swift */,
 				C45B91522956123A00F4EC78 /* FakeServicesManager.swift */,
 			);
 			path = Services;
@@ -2897,6 +2903,7 @@
 				C4EC1E73279DFCF40010F296 /* Events.swift in Sources */,
 				C44067FB27E25FD70045BD4E /* DomainListTLSCell.swift in Sources */,
 				C4C8900528F0E3D100CE5E97 /* RealFileSystem.swift in Sources */,
+				03C29A782EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */,
 				C4A81CA428C67101008DD9D1 /* PMTableView.swift in Sources */,
 				C4927F0B27B2DFC200C55AFD /* Errors.swift in Sources */,
 				C4CE7F9629683B43000102CF /* PhpVersionNumberCollection.swift in Sources */,
@@ -3053,6 +3060,7 @@
 				C471E86228F9BB650021E251 /* AddProxyVC.swift in Sources */,
 				C471E86328F9BB650021E251 /* PMTableView.swift in Sources */,
 				C471E86428F9BB650021E251 /* Warning.swift in Sources */,
+				03C29A772EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */,
 				C40175BA2903108900763A68 /* ValetInteractor.swift in Sources */,
 				C43931C729C4BD610069165B /* PhpVersionManagerView.swift in Sources */,
 				036C390A2E5C8CC5008DAEDF /* PackagistP2Response.swift in Sources */,
@@ -3302,6 +3310,7 @@
 				039C29152E8AA163007F5FAB /* ActiveApi.swift in Sources */,
 				C471E8D528F9BB8F0021E251 /* CheckboxPreferenceView.swift in Sources */,
 				C471E8D728F9BB8F0021E251 /* SelectPreferenceView.swift in Sources */,
+				03C29A7A2EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */,
 				C471E8D928F9BB8F0021E251 /* HotkeyPreferenceView.swift in Sources */,
 				C4611E5D2AEAD2FA0010BE24 /* ConfigManagerView.swift in Sources */,
 				C471E8DA28F9BB8F0021E251 /* Keys.swift in Sources */,
@@ -3596,6 +3605,7 @@
 				031E2B6A2B1525A7007C29E1 /* BrewPhpExtension.swift in Sources */,
 				C4998F0B2617633900B2526E /* PreferencesWindowController.swift in Sources */,
 				C485707228BF453800539B36 /* SwiftUIHelper.swift in Sources */,
+				03C29A792EC88E3100FBA25E /* ValetServicesDataManager.swift in Sources */,
 				C4EA3C482BA4F947007B0BA7 /* CustomButtonStyles.swift in Sources */,
 				C4F2E43B27530F750020E974 /* PhpInstallation.swift in Sources */,
 				C4F780BD25D80B65000DBC97 /* Constants.swift in Sources */,
--- a/phpmon/Domain/App/Services/ValetServicesDataManager.swift
+++ b/phpmon/Domain/App/Services/ValetServicesDataManager.swift
@@ -0,0 +1,125 @@
+//
+//  ValetServicesDataManager.swift
+//  PHP Monitor
+//
+//  Created by Nico Verbruggen on 15/11/2025.
+//  Copyright © 2025 Nico Verbruggen. All rights reserved.
+//
+
+import Foundation
+
+actor ValetServicesDataManager {
+    private let container: Container
+
+    init(_ container: Container) {
+        self.container = container
+    }
+
+    /**
+     The last known state of all Homebrew services (via the `formulae` property).
+     */
+    private(set) var homebrewServices: [HomebrewService] = []
+
+    /**
+     All Homebrew formulae that we need to check the status for.
+
+     This will include the Valet-required services (php, nginx, dnsmasq) but depending
+     on how the user has configured their setup, this may also include other services
+     like databases or such, which may be very helpful.
+     */
+    var formulae: [HomebrewFormula] {
+        let f = HomebrewFormulae(container)
+
+        // We will always include these (required for Valet)
+        var formulae = [f.php, f.nginx, f.dnsmasq]
+
+        // We may also load additional formulae based on Preferences
+        if let customServices = Preferences.custom.services, !customServices.isEmpty {
+            formulae.append(contentsOf: customServices.map { item in
+                return HomebrewFormula(item, elevated: false)
+            })
+        }
+
+        return formulae
+    }
+
+    /**
+     This method allows us to reload the Homebrew services, but we run this command
+     twice (once for user services, and once for root services). Please note that
+     these two commands are executed concurrently.
+
+     If this fails, question marks will be displayed in the menu bar and we will
+     try one more time to reload the services.
+     */
+    func reloadServicesStatus(isRetry: Bool) async -> [HomebrewService] {
+        if !Valet.installed {
+            Log.info("Not reloading services because running in Standalone Mode.")
+            return []
+        }
+
+        return await withTaskGroup(of: [HomebrewService].self) { group in
+            group.addTask {
+                await self.fetchHomebrewServices(elevated: true)
+            }
+
+            group.addTask {
+                await self.fetchHomebrewServices(elevated: false)
+            }
+
+            // Collect all services into a local variable (avoids intermediate state)
+            var collectedServices: [HomebrewService] = []
+
+            for await services in group {
+                collectedServices.append(contentsOf: services)
+            }
+
+            // Single atomic update to actor state after all data is collected
+            self.homebrewServices = collectedServices
+
+            // Do we need to retry?
+            if homebrewServices.isEmpty && !isRetry {
+                Log.warn("Failed to retrieve any Homebrew services data. Retrying once in 2 seconds...")
+                await delay(seconds: 2)
+                return await self.reloadServicesStatus(isRetry: true)
+            }
+
+            return homebrewServices
+        }
+    }
+
+    /**
+     Fetches Homebrew services information for either elevated (root) or user services.
+
+     - Parameter elevated: Whether to fetch services running as root (true) or user (false)
+     - Returns: Array of HomebrewService objects, or empty array if fetching fails
+     */
+    private func fetchHomebrewServices(elevated: Bool) async -> [HomebrewService] {
+        let serviceNames = self.formulae
+            .filter { $0.elevated == elevated }
+            .map { $0.name }
+
+        let command = elevated
+            ? "sudo \(self.container.paths.brew) services info --all --json"
+            : "\(self.container.paths.brew) services info --all --json"
+
+        let output = await self.container.shell.pipe(command).out
+
+        guard let jsonData = output.data(using: .utf8) else {
+            Log.err("Failed to convert \(elevated ? "root" : "user") services output to UTF-8 data.")
+            return []
+        }
+
+        do {
+            return try JSONDecoder()
+                .decode([HomebrewService].self, from: jsonData)
+                .filter { serviceNames.contains($0.name) }
+        } catch {
+            Log.err("Failed to decode \(elevated ? "root" : "user") services JSON: \(error)")
+            return []
+        }
+    }
+
+    func getHomebrewService(named: String) -> HomebrewService? {
+        return homebrewServices.first { $0.name == named }
+    }
+}
--- a/phpmon/Domain/App/Services/ValetServicesManager.swift
+++ b/phpmon/Domain/App/Services/ValetServicesManager.swift
@@ -11,124 +11,36 @@ import Cocoa
 import NVAlert

 class ValetServicesManager: ServicesManager {
+    private let data: ValetServicesDataManager

    override init(_ container: Container) {
+        self.data = ValetServicesDataManager(container)
        super.init(container)

        // Load the initial services state
        Task {
            await self.reloadServicesStatus()

-            Task { @MainActor in
+            await MainActor.run {
                firstRunComplete = true
            }
        }
    }

-    /**
-     The last known state of all Homebrew services.
-     */
-    var homebrewServices: [HomebrewService] = []
-
-    /**
-     This method allows us to reload the Homebrew services, but we run this command
-     twice (once for user services, and once for root services). Please note that
-     these two commands are executed concurrently.
-
-     If this fails, question marks will be displayed in the menu bar and we will
-     try one more time to reload the services.
-     */
    override func reloadServicesStatus() async {
-        await reloadServicesStatus(isRetry: false)
-    }
+        // Fetch data on background (actor-isolated, thread-safe)
+        let homebrewServices = await data.reloadServicesStatus(isRetry: false)

-    private func reloadServicesStatus(isRetry: Bool) async {
-        if !Valet.installed {
-            return Log.info("Not reloading services because running in Standalone Mode.")
-        }
-
-        await withTaskGroup(of: [HomebrewService].self, body: { group in
-            // Retrieve the status of the formulae that run as root
-            group.addTask {
-                await self.fetchHomebrewServices(elevated: true)
+        // Update UI on main thread
+        await MainActor.run {
+            self.services = self.formulae.map { formula in
+                Service(
+                    formula: formula,
+                    service: homebrewServices.first { $0.name == formula.name }
+                )
            }

-            // At the same time, retrieve the status of the formulae that run as user
-            group.addTask {
-                await self.fetchHomebrewServices(elevated: false)
-            }
-
-            // Collect all services into a local variable first (more thread-safe)
-            // Ideally, we'd want to turn this class into an actor in the future
-            var collectedServices: [HomebrewService] = []
-
-            for await services in group {
-                collectedServices.append(contentsOf: services)
-            }
-
-            // Only assign once, after all tasks complete
-            self.homebrewServices = collectedServices
-
-            // If we didn't get any service data and this isn't a retry, try again
-            if collectedServices.isEmpty && !isRetry {
-                Log.warn("Failed to retrieve any Homebrew services data. Retrying once in 2 seconds...")
-                await delay(seconds: 2)
-                await self.reloadServicesStatus(isRetry: true)
-                return
-            }
-
-            // Dispatch the update of the new service wrappers
-            Task { @MainActor in
-                // Ensure both commands complete (but run concurrently)
-                services = formulae.map { formula in
-                    Service(
-                        formula: formula,
-                        service: collectedServices.first(where: { service in
-                            service.name == formula.name
-                        })
-                    )
-                }
-
-                // Broadcast that all services have been updated
-                self.broadcastServicesUpdated()
-            }
-        })
-    }
-
-    /**
-     Fetches Homebrew services information for either elevated (root) or user services.
-
-     - Parameter elevated: Whether to fetch services running as root (true) or user (false)
-     - Returns: Array of HomebrewService objects, or empty array if fetching fails
-     */
-    private func fetchHomebrewServices(elevated: Bool) async -> [HomebrewService] {
-        // Check which formulae we are supposed to be looking for
-        let serviceNames = self.formulae
-            .filter { $0.elevated == elevated }
-            .map { $0.name }
-
-        // Determine which command to run
-        let command = elevated
-            ? "sudo \(self.container.paths.brew) services info --all --json"
-            : "\(self.container.paths.brew) services info --all --json"
-
-        // Run and get the output of the command
-        let output = await self.container.shell.pipe(command).out
-
-        // Attempt to parse the output
-        guard let jsonData = output.data(using: .utf8) else {
-            Log.err("Failed to convert \(elevated ? "root" : "user") services output to UTF-8 data. Output: \(output)")
-            return []
-        }
-
-        // Attempt to decode the JSON output. In certain situations the output may not be valid and this prevents a crash
-        do {
-            return try JSONDecoder()
-                .decode([HomebrewService].self, from: jsonData)
-                .filter { serviceNames.contains($0.name) }
-        } catch {
-            Log.err("Failed to decode \(elevated ? "root" : "user") services JSON: \(error). Output: \(output)")
-            return []
+            self.broadcastServicesUpdated()
        }
    }

@@ -154,48 +66,51 @@ class ValetServicesManager: ServicesManager {

        // Reload the services status to confirm this worked
        await ServicesManager.shared.reloadServicesStatus()
-
-        Task {
-            await presentTroubleshootingForService(named: named)
-        }
+        await presentTroubleshootingForService(named: named)
    }

-    @MainActor func presentTroubleshootingForService(named: String) {
-        let after = self.homebrewServices.first { service in
-            return service.name == named
+    @MainActor func presentTroubleshootingForService(named: String) async {
+        // If we cannot get data from Homebrew, we won't be able to troubleshoot
+        guard let after = await data.getHomebrewService(named: named) else {
+            return
        }

-        guard let after else { return }
+        // If we don't get an error message from Homebrew, we won't be able to troubleshoot
+        guard after.status == "error" else {
+            return
+        }

-        if after.status == "error" {
-            Log.err("The service '\(named)' is now reporting an error.")
+        Log.err("The service '\(named)' is now reporting an error.")

-            guard let errorLogPath = after.error_log_path else {
-                return NVAlert().withInformation(
-                    title: "alert.service_error.title".localized(named),
-                    subtitle: "alert.service_error.subtitle.no_error_log".localized(named),
-                    description: "alert.service_error.extra".localized
-                )
-                .withPrimary(text: "alert.service_error.button.close".localized)
-                .show()
-            }
-
-            NVAlert().withInformation(
+        // If we don't have a path to a log file, show a simplified alert
+        guard let errorLogPath = after.error_log_path else {
+            return NVAlert().withInformation(
                title: "alert.service_error.title".localized(named),
-                subtitle: "alert.service_error.subtitle.error_log".localized(named),
+                subtitle: "alert.service_error.subtitle.no_error_log".localized(named),
                description: "alert.service_error.extra".localized
            )
            .withPrimary(text: "alert.service_error.button.close".localized)
-            .withTertiary(text: "alert.service_error.button.show_log".localized, action: { alert in
-                let url = URL(fileURLWithPath: errorLogPath)
-                if errorLogPath.hasSuffix(".log") {
-                    NSWorkspace.shared.open(url)
-                } else {
-                    NSWorkspace.shared.activateFileViewerSelecting([url])
-                }
-                alert.close(with: .OK)
-            })
            .show()
        }
+
+        // If we do have a path to a log file, show a more complex alert w/ Show Log button
+        NVAlert().withInformation(
+            title: "alert.service_error.title".localized(named),
+            subtitle: "alert.service_error.subtitle.error_log".localized(named),
+            description: "alert.service_error.extra".localized
+        )
+        .withPrimary(text: "alert.service_error.button.close".localized)
+        .withTertiary(text: "alert.service_error.button.show_log".localized, action: { alert in
+            let url = URL(fileURLWithPath: errorLogPath)
+
+            if errorLogPath.hasSuffix(".log") {
+                NSWorkspace.shared.open(url)
+            } else {
+                NSWorkspace.shared.activateFileViewerSelecting([url])
+            }
+
+            alert.close(with: .OK)
+        })
+        .show()
    }
 }