Make SiteList.loadAll a static method

ios/NebulaNetworkExtension/SiteList.swift

@@ -1,108 +1,103 @@
 import NetworkExtension
 
 class SiteList {
-    private var sites = [String: Site]()
-    
     /// Gets the root directory that can be used to share files between the UI and VPN process. Does ensure the directory exists
     static func getRootDir() throws -> URL {
         let fileManager = FileManager.default
         let rootDir = fileManager.containerURL(forSecurityApplicationGroupIdentifier: "group.net.defined.mobileNebula")!
-        
+
-        if (!fileManager.fileExists(atPath: rootDir.absoluteString)) {
+        if !fileManager.fileExists(atPath: rootDir.absoluteString) {
             try fileManager.createDirectory(at: rootDir, withIntermediateDirectories: true)
         }
-        
+
         return rootDir
     }
-    
+
     /// Gets the directory where all sites live, $rootDir/sites. Does ensure the directory exists
     static func getSitesDir() throws -> URL {
         let fileManager = FileManager.default
         let sitesDir = try getRootDir().appendingPathComponent("sites", isDirectory: true)
-        if (!fileManager.fileExists(atPath: sitesDir.absoluteString)) {
+        if !fileManager.fileExists(atPath: sitesDir.absoluteString) {
             try fileManager.createDirectory(at: sitesDir, withIntermediateDirectories: true)
         }
         return sitesDir
     }
-    
+
     /// Gets the directory where a single site would live, $rootDir/sites/$siteID
     static func getSiteDir(id: String, create: Bool = false) throws -> URL {
         let fileManager = FileManager.default
         let siteDir = try getSitesDir().appendingPathComponent(id, isDirectory: true)
-        if (create && !fileManager.fileExists(atPath: siteDir.absoluteString)) {
+        if create && !fileManager.fileExists(atPath: siteDir.absoluteString) {
             try fileManager.createDirectory(at: siteDir, withIntermediateDirectories: true)
         }
         return siteDir
     }
-    
+
     /// Gets the file that represents the site configuration, $rootDir/sites/$siteID/config.json
     static func getSiteConfigFile(id: String, createDir: Bool) throws -> URL {
-        return try getSiteDir(id: id, create: createDir).appendingPathComponent("config", isDirectory: false).appendingPathExtension("json")
+        return try getSiteDir(id: id, create: createDir).appendingPathComponent("config", isDirectory: false)
+            .appendingPathExtension("json")
     }
-    
+
     /// Gets the file that represents the site log output, $rootDir/sites/$siteID/log
     static func getSiteLogFile(id: String, createDir: Bool) throws -> URL {
         return try getSiteDir(id: id, create: createDir).appendingPathComponent("logs", isDirectory: false)
     }
-    
+
-    init(completion: @escaping ([String: Site]?, (any Error)?) -> ()) {
+    static func loadAll(completion: @escaping ([String: Site]?, (any Error)?) -> Void) {
-#if targetEnvironment(simulator)
+        #if targetEnvironment(simulator)
-        SiteList.loadAllFromFS { sites, err in
+            SiteList.loadAllFromFS { sites, err in
-            if sites != nil {
+
-                self.sites = sites!
+                completion(sites, err)
             }
-            completion(sites, err)
+        #else
-        }
+            SiteList.loadAllFromNETPM { sites, err in
-#else
+
-        SiteList.loadAllFromNETPM { sites, err in
+                completion(sites, err)
-            if sites != nil {
-                self.sites = sites!
             }
-            completion(sites, err)
+        #endif
-        }
-#endif
     }
-    
+
-    private static func loadAllFromFS(completion: @escaping ([String: Site]?, (any Error)?) -> ()) {
+    private static func loadAllFromFS(completion: @escaping ([String: Site]?, (any Error)?) -> Void) {
         let fileManager = FileManager.default
         var siteDirs: [URL]
         var sites = [String: Site]()
-        
+
         do {
             siteDirs = try fileManager.contentsOfDirectory(at: getSitesDir(), includingPropertiesForKeys: nil)
-            
+
         } catch {
             completion(nil, error)
             return
         }
-        
+
         siteDirs.forEach { path in
             do {
                 let site = try Site(path: path.appendingPathComponent("config").appendingPathExtension("json"))
                 sites[site.id] = site
-                
+
             } catch {
                 print(error)
                 try? fileManager.removeItem(at: path)
                 print("Deleted non conforming site \(path)")
             }
         }
-        
+
         completion(sites, nil)
     }
-    
+
-    private static func loadAllFromNETPM(completion: @escaping ([String: Site]?, (any Error)?) -> ()) {
+    private static func loadAllFromNETPM(completion: @escaping ([String: Site]?, (any Error)?) -> Void) {
         var sites = [String: Site]()
-        
+
         // dispatchGroup is used to ensure we have migrated all sites before returning them
         // If there are no sites to migrate, there are never any entrants
         let dispatchGroup = DispatchGroup()
-        
+
-        NETunnelProviderManager.loadAllFromPreferences() { newManagers, err in
+        NETunnelProviderManager.loadAllFromPreferences { newManagers, err in
-            if (err != nil) {
+            if err != nil {
                 return completion(nil, err)
             }
-            
+
             newManagers?.forEach { manager in
                 do {
                     let site = try Site(manager: manager)
@@ -112,14 +107,14 @@ class SiteList {
                             if error != nil {
                                 print("Error while migrating site to fs: \(error!.localizedDescription)")
                             }
-                            
+
                             print("Migrated site to fs: \(site.name)")
                             site.needsToMigrateToFS = false
                             dispatchGroup.leave()
                         }
                     }
                     sites[site.id] = site
-                    
+
                 } catch {
                     //TODO: notify the user about this
                     print("Deleted non conforming site \(manager) \(error)")
@@ -127,14 +122,10 @@ class SiteList {
                     //TODO: delete from disk, we need to try and discover the site id though
                 }
             }
-            
+
             dispatchGroup.notify(queue: .main) {
                 completion(sites, nil)
             }
         }
     }
-    
-    func getSites() -> [String: Site] {
-        return sites
-    }
 }

ios/Runner/DNUpdate.swift

@@ -6,8 +6,8 @@ actor DNUpdater {
     private let timer = RepeatingTimer(timeInterval: 15 * 60)  // 15 * 60 is 15 minutes
     private let log = Logger(subsystem: "net.defined.mobileNebula", category: "DNUpdater")
 
-    func updateAll(onUpdate: @escaping (Site) -> Void) {
+    private func updateAll(onUpdate: @escaping (Site) -> Void) {
-        _ = SiteList { (sites, _) -> Void in
+        SiteList.loadAll(completion: { (sites, _) -> Void in
             if let unwrappedSites = sites?.values {
                 // NEVPN seems to force us onto the main thread and we are about to make network calls that
                 // could block for a while. Push ourselves onto another thread to avoid blocking the UI.
@@ -25,7 +25,7 @@ actor DNUpdater {
 
             }
 
-        }
+        })
     }
 
     func updateAllLoop(onUpdate: @escaping (Site) -> Void) {

ios/Runner/Sites.swift

@@ -1,10 +1,10 @@
-import NetworkExtension
 import MobileNebula
+import NetworkExtension
 
 class SiteContainer {
     var site: Site
     var updater: SiteUpdater
-    
+
     init(site: Site, updater: SiteUpdater) {
         self.site = site
         self.updater = updater
@@ -14,39 +14,39 @@ class SiteContainer {
 class Sites {
     private var containers = [String: SiteContainer]()
     private var messenger: (any FlutterBinaryMessenger)?
-    
+
     init(messenger: (any FlutterBinaryMessenger)?) {
         self.messenger = messenger
     }
 
-    func loadSites(completion: @escaping ([String: Site]?, (any Error)?) -> ()) {
+    func loadSites(completion: @escaping ([String: Site]?, (any Error)?) -> Void) {
-        _ = SiteList { (sites, err) in
+        SiteList.loadAll(completion: { (sites, err) in
-            if (err != nil) {
+            if err != nil {
                 return completion(nil, err)
             }
-            
+
-            sites?.values.forEach{ site in
+            sites?.values.forEach { site in
                 var updater = self.containers[site.id]?.updater
-                if (updater != nil) {
+                if updater != nil {
                     updater!.setSite(site: site)
                 } else {
                     updater = SiteUpdater(messenger: self.messenger!, site: site)
                 }
                 self.containers[site.id] = SiteContainer(site: site, updater: updater!)
             }
-            
+
             let justSites = self.containers.mapValues {
                 return $0.site
             }
             completion(justSites, nil)
-        }
+        })
     }
-    
+
-    func deleteSite(id: String, callback: @escaping ((any Error)?) -> ()) {
+    func deleteSite(id: String, callback: @escaping ((any Error)?) -> Void) {
         if let site = self.containers.removeValue(forKey: id) {
             _ = KeyChain.delete(key: "\(site.site.id).dnCredentials")
             _ = KeyChain.delete(key: "\(site.site.id).key")
-            
+
             do {
                 let fileManager = FileManager.default
                 let siteDir = try SiteList.getSiteDir(id: site.site.id)
@@ -54,39 +54,39 @@ class Sites {
             } catch {
                 print("Failed to delete site from fs: \(error.localizedDescription)")
             }
-            
+
-#if !targetEnvironment(simulator)
+            #if !targetEnvironment(simulator)
-            site.site.manager!.removeFromPreferences(completionHandler: callback)
+                site.site.manager!.removeFromPreferences(completionHandler: callback)
-            return
+                return
-#endif
+            #endif
         }
-        
+
         // Nothing to remove
         callback(nil)
     }
-    
+
     func getSite(id: String) -> Site? {
         return self.containers[id]?.site
     }
-    
+
     func getUpdater(id: String) -> SiteUpdater? {
         return self.containers[id]?.updater
     }
-    
+
     func getContainer(id: String) -> SiteContainer? {
         return self.containers[id]
     }
 }
 
 class SiteUpdater: NSObject, FlutterStreamHandler {
-    private var eventSink: FlutterEventSink?;
+    private var eventSink: FlutterEventSink?
-    private var eventChannel: FlutterEventChannel;
+    private var eventChannel: FlutterEventChannel
     private var site: Site
     private var notification: Any?
     public var startFunc: (() -> Void)?
     private var configFd: Int32? = nil
     private var configObserver: (any DispatchSourceFileSystemObject)? = nil
-    
+
     init(messenger: any FlutterBinaryMessenger, site: Site) {
         do {
             let configPath = try SiteList.getSiteConfigFile(id: site.id, createDir: false)
@@ -95,18 +95,18 @@ class SiteUpdater: NSObject, FlutterStreamHandler {
                 fileDescriptor: self.configFd!,
                 eventMask: .write
             )
-            
+
         } catch {
             // SiteList.getSiteConfigFile should never throw because we are not creating it here
             self.configObserver = nil
         }
-        
+
         eventChannel = FlutterEventChannel(name: "net.defined.nebula/\(site.id)", binaryMessenger: messenger)
         self.site = site
         super.init()
-        
+
         eventChannel.setStreamHandler(self)
-        
+
         self.configObserver?.setEventHandler(handler: self.configUpdated)
         self.configObserver?.setCancelHandler {
             if self.configFd != nil {
@@ -114,72 +114,74 @@ class SiteUpdater: NSObject, FlutterStreamHandler {
             }
             self.configObserver = nil
         }
-        
+
         self.configObserver?.resume()
     }
-    
+
     func setSite(site: Site) {
         self.site = site
     }
 
     /// onListen is called when flutter code attaches an event listener
     func onListen(withArguments arguments: Any?, eventSink events: @escaping FlutterEventSink) -> FlutterError? {
-        eventSink = events;
+        eventSink = events
-        
+
-#if !targetEnvironment(simulator)
+        #if !targetEnvironment(simulator)
-        if site.manager == nil {
+            if site.manager == nil {
-            //TODO: The dn updater path seems to race to build a site that lacks a manager. The UI does not display this error
+                //TODO: The dn updater path seems to race to build a site that lacks a manager. The UI does not display this error
-            // and a another listen should occur and succeed.
+                // and a another listen should occur and succeed.
-            return FlutterError(code: "Internal Error", message: "Flutter manager was not present", details: nil)
+                return FlutterError(code: "Internal Error", message: "Flutter manager was not present", details: nil)
-        }
-        
-        self.notification = NotificationCenter.default.addObserver(forName: NSNotification.Name.NEVPNStatusDidChange, object: site.manager!.connection , queue: nil) { n in
-            let oldConnected = self.site.connected
-            self.site.status = statusString[self.site.manager!.connection.status]
-            self.site.connected = statusMap[self.site.manager!.connection.status]
-            
-            // Check to see if we just moved to connected and if we have a start function to call when that happens
-            if self.site.connected! && oldConnected != self.site.connected && self.startFunc != nil {
-                self.startFunc!()
-                self.startFunc = nil
             }
-            
+
-            self.update(connected: self.site.connected!)
+            self.notification = NotificationCenter.default.addObserver(
-        }
+                forName: NSNotification.Name.NEVPNStatusDidChange, object: site.manager!.connection, queue: nil
-#endif
+            ) { n in
+                let oldConnected = self.site.connected
+                self.site.status = statusString[self.site.manager!.connection.status]
+                self.site.connected = statusMap[self.site.manager!.connection.status]
+
+                // Check to see if we just moved to connected and if we have a start function to call when that happens
+                if self.site.connected! && oldConnected != self.site.connected && self.startFunc != nil {
+                    self.startFunc!()
+                    self.startFunc = nil
+                }
+
+                self.update(connected: self.site.connected!)
+            }
+        #endif
         return nil
     }
-    
+
     /// onCancel is called when the flutter listener stops listening
     func onCancel(withArguments arguments: Any?) -> FlutterError? {
-        if (self.notification != nil) {
+        if self.notification != nil {
             NotificationCenter.default.removeObserver(self.notification!)
         }
         return nil
     }
-    
+
     /// update is a way to send information to the flutter listener and generally should not be used directly
     func update(connected: Bool, replaceSite: Site? = nil) {
-        if (replaceSite != nil) {
+        if replaceSite != nil {
             site = replaceSite!
         }
         site.connected = connected
         site.status = connected ? "Connected" : "Disconnected"
-        
+
         let encoder = JSONEncoder()
         let data = try! encoder.encode(site)
         self.eventSink?(String(data: data, encoding: .utf8))
     }
-    
+
     private func configUpdated() {
         if self.site.connected != true {
             return
         }
-        
+
         guard let newSite = try? Site(manager: self.site.manager!) else {
             return
         }
-            
+
         self.update(connected: newSite.connected ?? false, replaceSite: newSite)
     }
 }