Skip to content

Commit

Permalink
Fixes data race in LocationModule (#52)
Browse files Browse the repository at this point in the history
  • Loading branch information
vishnuravi authored Dec 28, 2024
1 parent d934aa8 commit 7f524a9
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 49 deletions.
10 changes: 7 additions & 3 deletions LifeSpace.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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>"; };
Expand Down Expand Up @@ -338,6 +340,7 @@
637AA5CF2BBDA686007BD7A3 /* Location */ = {
isa = PBXGroup;
children = (
63FA22C22D2045B300A15017 /* LocationStorage.swift */,
63F4C39A2BBCCCF80033D985 /* LocationModule.swift */,
63F4C39C2BBCCD200033D985 /* LocationUtils.swift */,
63497B6F2BBF6ECE001F8419 /* LocationDataPoint.swift */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -820,7 +824,7 @@
CODE_SIGN_ENTITLEMENTS = "LifeSpace/Supporting Files/LifeSpace.entitlements";
CODE_SIGN_IDENTITY = "Apple Development";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 2;
CURRENT_PROJECT_VERSION = 3;
DEVELOPMENT_ASSET_PATHS = "";
DEVELOPMENT_TEAM = "";
ENABLE_PREVIEWS = YES;
Expand Down Expand Up @@ -1022,7 +1026,7 @@
CODE_SIGN_ENTITLEMENTS = "LifeSpace/Supporting Files/LifeSpace.entitlements";
CODE_SIGN_IDENTITY = "Apple Development";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 2;
CURRENT_PROJECT_VERSION = 3;
DEVELOPMENT_ASSET_PATHS = "";
DEVELOPMENT_TEAM = "";
ENABLE_PREVIEWS = YES;
Expand Down Expand Up @@ -1070,7 +1074,7 @@
CODE_SIGN_IDENTITY = "Apple Development";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Distribution";
CODE_SIGN_STYLE = Manual;
CURRENT_PROJECT_VERSION = 2;
CURRENT_PROJECT_VERSION = 3;
DEVELOPMENT_ASSET_PATHS = "";
DEVELOPMENT_TEAM = "";
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = "";
Expand Down
9 changes: 7 additions & 2 deletions LifeSpace/Account/AccountSheet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ struct AccountSheet: View {

@AppStorage(StorageKeys.studyID) var studyID = "unknownStudyID"
@AppStorage(StorageKeys.trackingPreference) private var trackingOn = true
@AppStorage(StorageKeys.lastSurveyTransmissionDate) private var lastSurveyTransmissionDate = "Not set"
@AppStorage(StorageKeys.lastLocationTransmissionDate) private var lastLocationTransmissionDate = "Not set"

@AppStorage(StorageKeys.lastSurveyTransmissionDate) private var lastSurveyTransmissionDate = "None"
@AppStorage(StorageKeys.lastLocationTransmissionDate) private var lastLocationTransmissionDate = "None"
@AppStorage(StorageKeys.lastHealthKitTransmissionDate) private var lastHealthKitTransmissionDate = "None"

var body: some View {
NavigationStack {
Expand Down Expand Up @@ -116,6 +118,9 @@ struct AccountSheet: View {
Section(header: Text("LAST_LOCATION_TRANSMISSION_SECTION")) {
Text(lastLocationTransmissionDate)
}
Section(header: Text("LAST_HEALTHKIT_TRANSMISSION_SECTION")) {
Text(lastHealthKitTransmissionDate)
}
}
}
}
Expand Down
37 changes: 27 additions & 10 deletions LifeSpace/Location/LocationModule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.getAllLocations()
}
}

override public required init() {
super.init()
Expand Down Expand Up @@ -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.updateAllLocations(locations)
if let callback = onLocationsUpdated {
let currentLocations = await storage.getAllLocations()
await MainActor.run {
callback(currentLocations)
}
}
}
} catch {
logger.error("Error fetching locations: \(error.localizedDescription)")
Expand All @@ -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)
}
}
Expand All @@ -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.getLastSaved() else {
return true
}

Expand All @@ -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.
Expand Down
61 changes: 61 additions & 0 deletions LifeSpace/Location/LocationStorage.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//
// LocationStorage.swift
// LifeSpace
//
// Created by Vishnu Ravi on 12/28/24.
//

import CoreLocation

/// An actor that manages storage and retrieval of location coordinates.
/// This class provides thread-safe access to location data and maintains both a collection
/// of locations and information about the last saved location.
actor LocationStorage {
/// Array storing all recorded location coordinates
private var allLocations: [CLLocationCoordinate2D]

/// Tuple containing the most recently saved location and its timestamp.
/// We use this in `LocationModule` to handle daily location resets.
private var lastSaved: (location: CLLocationCoordinate2D, date: Date)?

init() {
self.allLocations = []
}

/// Adds a new location coordinate to the collection
/// - Parameter coordinate: The location coordinate to append
func appendLocation(_ coordinate: CLLocationCoordinate2D) {
self.allLocations.append(coordinate)
}

/// Updates the entire collection of locations with a new array
/// - Parameter locations: Array of coordinates to replace the existing locations
func updateAllLocations(_ locations: [CLLocationCoordinate2D]) {
self.allLocations = locations
}

/// Retrieves all stored location coordinates
/// - Returns: Array of all stored location coordinates
func getAllLocations() -> [CLLocationCoordinate2D] {
allLocations
}

/// Removes all stored locations from the collection
func clearAllLocations() {
allLocations.removeAll()
}

/// Retrieves the most recently saved location and its timestamp
/// - Returns: Tuple containing the location coordinate and save date, or nil if no location has been saved
func getLastSaved() -> (location: CLLocationCoordinate2D, date: Date)? {
lastSaved
}

/// Updates the most recently saved location with a new coordinate and timestamp
/// - Parameters:
/// - location: The location coordinate to save
/// - date: The timestamp of when the location was saved
func updateLastSaved(location: CLLocationCoordinate2D, date: Date) {
self.lastSaved = (location: location, date: date)
}
}
26 changes: 17 additions & 9 deletions LifeSpace/Map/MapboxView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
}
}
}
}
Expand Down
25 changes: 12 additions & 13 deletions LifeSpace/Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,22 @@
}
}
},
"LAST_HEALTHKIT_TRANSMISSION_SECTION" : {
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "Last HealthKit Transmission"
}
}
}
},
"LAST_LOCATION_TRANSMISSION_SECTION" : {
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "Last Successful Location Data Transmission"
"value" : "Last Location Transmission"
}
}
}
Expand All @@ -340,7 +350,7 @@
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "Last Successful Survey Transmission"
"value" : "Last Survey Transmission"
}
}
}
Expand Down Expand Up @@ -465,17 +475,6 @@
}
}
},
"LOGS" : {
"extractionState" : "stale",
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "Logs"
}
}
}
},
"LOGS_SHARE_PREVIEW_TITLE" : {

},
Expand Down
2 changes: 2 additions & 0 deletions LifeSpace/SharedContext/StorageKeys.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@ enum StorageKeys {
static let lastSurveyTransmissionDate = "lastSurveyTransmissionDate"
/// `Date`s containing the timestamp of the last successful transmission for location data.
static let lastLocationTransmissionDate = "lastLocationTransmissionDate"
/// `Date`s containing the timestamp of the last successful transmission for HealthKit data.
static let lastHealthKitTransmissionDate = "lastHealthKitTransmissionDate"
}
Loading

0 comments on commit 7f524a9

Please sign in to comment.