Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes data race in LocationModule #52

Merged
merged 4 commits into from
Dec 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading