Fixes data race in LocationModule

StanfordBDHG · Dec 28, 2024 · a93a7ee · a93a7ee
1 parent d934aa8
commit a93a7ee
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 19 deletions.
diff --git a/LifeSpace.xcodeproj/project.pbxproj b/LifeSpace.xcodeproj/project.pbxproj
@@ -80,6 +80,7 @@
 		63F4C39D2BBCCD200033D985 /* LocationUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63F4C39C2BBCCD200033D985 /* LocationUtils.swift */; };
 		63F4C39F2BBCCDB70033D985 /* Date+Helpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63F4C39E2BBCCDB70033D985 /* Date+Helpers.swift */; };
 		63F4C3A32BBCE79B0033D985 /* LocationPermissions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63F4C3A22BBCE79B0033D985 /* LocationPermissions.swift */; };
+		63FA22C32D2045B600A15017 /* LocationStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63FA22C22D2045B300A15017 /* LocationStorage.swift */; };
 		653A2551283387FE005D4D48 /* LifeSpace.swift in Sources */ = {isa = PBXBuildFile; fileRef = 653A2550283387FE005D4D48 /* LifeSpace.swift */; };
 		653A255528338800005D4D48 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 653A255428338800005D4D48 /* Assets.xcassets */; };
 		653A256228338800005D4D48 /* LifeSpaceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 653A256128338800005D4D48 /* LifeSpaceTests.swift */; };
@@ -169,6 +170,7 @@
 		63F4C39C2BBCCD200033D985 /* LocationUtils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationUtils.swift; sourceTree = "<group>"; };
 		63F4C39E2BBCCDB70033D985 /* Date+Helpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Date+Helpers.swift"; sourceTree = "<group>"; };
 		63F4C3A22BBCE79B0033D985 /* LocationPermissions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationPermissions.swift; sourceTree = "<group>"; };
+		63FA22C22D2045B300A15017 /* LocationStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationStorage.swift; sourceTree = "<group>"; };
 		653A254D283387FE005D4D48 /* LifeSpace.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = LifeSpace.app; sourceTree = BUILT_PRODUCTS_DIR; };
 		653A2550283387FE005D4D48 /* LifeSpace.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LifeSpace.swift; sourceTree = "<group>"; };
 		653A255428338800005D4D48 /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
@@ -338,6 +340,7 @@
 		637AA5CF2BBDA686007BD7A3 /* Location */ = {
 			isa = PBXGroup;
 			children = (
+				63FA22C22D2045B300A15017 /* LocationStorage.swift */,
 				63F4C39A2BBCCCF80033D985 /* LocationModule.swift */,
 				63F4C39C2BBCCD200033D985 /* LocationUtils.swift */,
 				63497B6F2BBF6ECE001F8419 /* LocationDataPoint.swift */,
@@ -658,6 +661,7 @@
 			files = (
 				2FE5DC4129EDD7EE004B9AB4 /* StorageKeys.swift in Sources */,
 				2FE5DCB129EE6107004B9AB4 /* AccountOnboarding.swift in Sources */,
+				63FA22C32D2045B600A15017 /* LocationStorage.swift in Sources */,
 				2FE5DC3A29EDD7CA004B9AB4 /* Welcome.swift in Sources */,
 				634FFF672C169F40005E8217 /* LifeSpaceConsent.swift in Sources */,
 				634E38482CDE7A7400B16E20 /* LogLevel.swift in Sources */,

diff --git a/LifeSpace/Location/LocationModule.swift b/LifeSpace/Location/LocationModule.swift
@@ -19,9 +19,14 @@ public class LocationModule: NSObject, CLLocationManagerDelegate, Module, Defaul
     @Published var authorizationStatus = CLLocationManager().authorizationStatus
     @Published var canShowRequestMessage = true
 
-    public var allLocations = [CLLocationCoordinate2D]()
+    private let storage = LocationStorage()
     public var onLocationsUpdated: (([CLLocationCoordinate2D]) -> Void)?
-    private var lastSaved: (location: CLLocationCoordinate2D, date: Date)?
+
+    public var allLocations: [CLLocationCoordinate2D] {
+        get async {
+            await storage.allLocations
+        }
+    }
 
     override public required init() {
         super.init()
@@ -64,8 +69,13 @@ public class LocationModule: NSObject, CLLocationManagerDelegate, Module, Defaul
     public func fetchLocations() async {
         do {
             if let locations = try await standard?.fetchLocations() {
-                self.allLocations = locations
-                self.onLocationsUpdated?(self.allLocations)
+                await storage.updateLocations(locations)
+                if let callback = onLocationsUpdated {
+                    let currentLocations = await storage.getAllLocations()
+                    await MainActor.run {
+                        callback(currentLocations)
+                    }
+                }
             }
         } catch {
             logger.error("Error fetching locations: \(error.localizedDescription)")
@@ -79,7 +89,7 @@ public class LocationModule: NSObject, CLLocationManagerDelegate, Module, Defaul
         let shouldAddLocation = await determineIfShouldAddLocation(coordinate)
 
         if shouldAddLocation {
-            updateLocalLocations(with: coordinate)
+            await updateLocalLocations(with: coordinate)
             await saveLocation(coordinate)
         }
     }
@@ -94,7 +104,7 @@ public class LocationModule: NSObject, CLLocationManagerDelegate, Module, Defaul
 
         /// Check if there is a previously saved point, so we can calculate the distance between that and the current point.
         /// If there's no previously saved point, we can save the current point
-        guard let lastSaved else {
+        guard let lastSaved = await storage.lastSaved else {
             return true
         }
 
@@ -113,10 +123,17 @@ public class LocationModule: NSObject, CLLocationManagerDelegate, Module, Defaul
 
     /// Updates the local set of locations and the map with the latest location
     /// - Parameter coordinate: The `CLLocationCoordinate2D` of the location to be saved.
-    private func updateLocalLocations(with coordinate: CLLocationCoordinate2D) {
-        allLocations.append(coordinate)
-        onLocationsUpdated?(allLocations)
-        lastSaved = (location: coordinate, date: Date())
+    private func updateLocalLocations(with coordinate: CLLocationCoordinate2D) async {
+        await storage.appendLocation(coordinate)
+
+        if let callback = onLocationsUpdated {
+            let locations = await storage.getAllLocations()
+            await MainActor.run {
+                callback(locations)
+            }
+        }
+
+        await storage.updateLastSaved(location: coordinate, date: Date())
     }
 
     /// Saves a location to Firestore via the Standard.

diff --git a/LifeSpace/Location/LocationStorage.swift b/LifeSpace/Location/LocationStorage.swift
@@ -0,0 +1,33 @@
+//
+//  LocationStorage.swift
+//  LifeSpace
+//
+//  Created by Vishnu Ravi on 12/28/24.
+//
+
+import CoreLocation
+
+internal actor LocationStorage {
+    var allLocations: [CLLocationCoordinate2D]
+    var lastSaved: (location: CLLocationCoordinate2D, date: Date)?
+
+    init() {
+        self.allLocations = []
+    }
+
+    func updateLocations(_ locations: [CLLocationCoordinate2D]) {
+        self.allLocations = locations
+    }
+
+    func appendLocation(_ coordinate: CLLocationCoordinate2D) {
+        self.allLocations.append(coordinate)
+    }
+
+    func updateLastSaved(location: CLLocationCoordinate2D, date: Date) {
+        self.lastSaved = (location: location, date: date)
+    }
+
+    func getAllLocations() -> [CLLocationCoordinate2D] {
+        allLocations
+    }
+}
diff --git a/LifeSpace/Map/MapboxView.swift b/LifeSpace/Map/MapboxView.swift
@@ -65,15 +65,23 @@ public class MapManagerView: UIViewController {
             return
         }
 
-        self.mapView.mapboxMap.onNext(event: .mapLoaded) { _ in
-            let locations = locationModule.allLocations
-            do {
-                try self.addGeoJSONSource(with: locations)
-                try self.addCircleLayer(sourceId: Constants.geoSourceId)
-                self.centerCamera(at: locations.last, zoomLevel: Constants.zoomLevel)
-                self.setupDynamicLocationUpdates()
-            } catch {
-                print("[MapboxMap] Error: \(error.localizedDescription)")
+        self.mapView.mapboxMap.onNext(event: .mapLoaded) { [weak self] _ in
+            guard let self = self else {
+                return
+            }
+
+            Task {
+                let locations = await locationModule.allLocations
+                do {
+                    try await MainActor.run {
+                        try self.addGeoJSONSource(with: locations)
+                        try self.addCircleLayer(sourceId: Constants.geoSourceId)
+                        self.centerCamera(at: locations.last, zoomLevel: Constants.zoomLevel)
+                        self.setupDynamicLocationUpdates()
+                    }
+                } catch {
+                    print("[MapboxMap] Error: \(error.localizedDescription)")
+                }
             }
         }
     }