Skip to content

Commit 5cddd6a

Browse files
theospearsclaude
authored and
krugerk
committed
Update data points based on requestId not date (#665)
## Summary Previously when syncing HealthKit data points it was done based on the daystamp. However, we've been assigning unique requestIds for a while now, so switch to instead updating via dataPoint. ## Validation Updated unit tests --------- Co-authored-by: Claude <[email protected]>
1 parent 91f5d1e commit 5cddd6a

File tree

2 files changed

+187
-69
lines changed

2 files changed

+187
-69
lines changed

‎BeeKit/Managers/DataPointManager.swift‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,14 @@ public actor DataPointManager {
3030
}
3131
}
3232

33-
private func updateDatapoint(goal : Goal, datapoint : DataPoint, datapointValue : NSNumber) async throws {
33+
private func updateDatapoint(goal : Goal, datapoint : DataPoint, datapointValue : NSNumber, comment: String) async throws {
3434
let val = datapoint.value
35-
if datapointValue == val {
35+
if datapointValue == val && comment == datapoint.comment {
3636
return
3737
}
3838
let params = [
3939
"value": "\(datapointValue)",
40-
"comment": "Auto-updated via Apple Health",
40+
"comment": comment,
4141
]
4242
let _ = try await requestManager.put(url: "api/v1/users/{username}/goals/\(goal.slug)/datapoints/\(datapoint.id).json", parameters: params)
4343
}
@@ -103,37 +103,52 @@ public actor DataPointManager {
103103
let datapoints = try await datapointsSince(goal: goal, daystamp: try! Daystamp(fromString: firstDaystamp.description))
104104
let realDatapoints = datapoints.filter{ !$0.isDummy && !$0.isInitial }
105105

106-
for newDataPoint in healthKitDataPoints {
107-
try await self.updateToMatchDataPoint(goal: goal, newDataPoint: newDataPoint, recentDatapoints: realDatapoints)
106+
let healthKitDataPointsByDay = Dictionary(grouping: healthKitDataPoints) { $0.daystamp }
107+
108+
try await withThrowingTaskGroup(of: Void.self) { group in
109+
for (daystamp, dayDataPoints) in healthKitDataPointsByDay {
110+
group.addTask {
111+
let existingDatapointsForDay = await self.datapointsMatchingDaystamp(datapoints: realDatapoints, daystamp: daystamp)
112+
try await self.updateToMatchDataPointsForDay(goal: goal, newDataPoints: dayDataPoints, existingDatapoints: existingDatapointsForDay)
113+
}
114+
}
108115
}
109116
}
110117

111-
private func updateToMatchDataPoint(goal: Goal, newDataPoint : BeeDataPoint, recentDatapoints: [DataPoint]) async throws {
112-
var matchingDatapoints = datapointsMatchingDaystamp(datapoints: recentDatapoints, daystamp: newDataPoint.daystamp)
113-
if matchingDatapoints.count == 0 {
114-
// If there are not already data points for this day, do not add points
115-
// from before the creation of the goal. This avoids immediate derailment
116-
//on do less goals, and excessive safety buffer on do-more goals.
117-
if newDataPoint.daystamp < goal.initDaystamp {
118-
return
118+
private func updateToMatchDataPointsForDay(goal: Goal, newDataPoints: [BeeDataPoint], existingDatapoints: [DataPoint]) async throws {
119+
try await withThrowingTaskGroup(of: Void.self) { group in
120+
var processedDatapoints: Set<String> = []
121+
122+
for newDataPoint in newDataPoints {
123+
let matchingDatapoint = existingDatapoints.first { $0.requestid == newDataPoint.requestid }
124+
125+
if let existingDatapoint = matchingDatapoint {
126+
if !isApproximatelyEqual(existingDatapoint.value.doubleValue, newDataPoint.value.doubleValue) || existingDatapoint.comment != newDataPoint.comment {
127+
group.addTask {
128+
self.logger.notice("Updating datapoint for \(goal.id) with requestId \(newDataPoint.requestid, privacy: .public) from \(existingDatapoint.value) to \(newDataPoint.value)")
129+
try await self.updateDatapoint(goal: goal, datapoint: existingDatapoint, datapointValue: newDataPoint.value, comment: newDataPoint.comment)
130+
}
131+
}
132+
processedDatapoints.insert(existingDatapoint.requestid)
133+
} else if newDataPoint.daystamp >= goal.initDaystamp {
134+
// If there are not already data points for this requestId, do not add points
135+
// from before the creation of the goal. This avoids immediate derailment
136+
// on do less goals, and excessive safety buffer on do-more goals.
137+
group.addTask {
138+
let urText = "\(newDataPoint.daystamp.day) \(newDataPoint.value) \"\(newDataPoint.comment)\""
139+
self.logger.notice("Creating new datapoint for \(goal.id, privacy: .public) with requestId \(newDataPoint.requestid, privacy: .public): \(newDataPoint.value, privacy: .private)")
140+
try await self.postDatapoint(goal: goal, urText: urText, requestId: newDataPoint.requestid)
141+
}
142+
}
119143
}
120-
121-
let urText = "\(newDataPoint.daystamp.day) \(newDataPoint.value) \"\(newDataPoint.comment)\""
122-
let requestId = newDataPoint.requestid
123-
124-
logger.notice("Creating new datapoint for \(goal.id, privacy: .public) on \(newDataPoint.daystamp, privacy: .public): \(newDataPoint.value, privacy: .private)")
125-
126-
try await postDatapoint(goal: goal, urText: urText, requestId: requestId)
127-
} else if matchingDatapoints.count >= 1 {
128-
let firstDatapoint = matchingDatapoints.remove(at: 0)
129-
for datapoint in matchingDatapoints {
130-
try await deleteDatapoint(goal: goal, datapoint: datapoint)
131-
}
132-
133-
if !isApproximatelyEqual(firstDatapoint.value.doubleValue, newDataPoint.value.doubleValue) {
134-
logger.notice("Updating datapoint for \(goal.id) on \(firstDatapoint.daystamp, privacy: .public) from \(firstDatapoint.value) to \(newDataPoint.value)")
135-
136-
try await updateDatapoint(goal: goal, datapoint: firstDatapoint, datapointValue: newDataPoint.value)
144+
145+
for existingDatapoint in existingDatapoints {
146+
if !processedDatapoints.contains(existingDatapoint.requestid) {
147+
group.addTask {
148+
self.logger.notice("Deleting obsolete datapoint for \(goal.id) with requestId \(existingDatapoint.requestid, privacy: .public)")
149+
try await self.deleteDatapoint(goal: goal, datapoint: existingDatapoint)
150+
}
151+
}
137152
}
138153
}
139154
}

‎BeeKitTests/DataPointManagerTests.swift‎

Lines changed: 142 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,29 @@ class DataPointManagerTests: XCTestCase {
8181
user = nil
8282
}
8383

84-
func testSynchronizesPoints() async throws {
84+
func testSynchronizesPointsByRequestId() async throws {
8585
let apiResponse = [
86-
// Matching data point to be updated
86+
// Existing datapoint with requestId that should be updated
8787
[
88-
"id": "normal3",
88+
"id": "existing1",
8989
"value": 10,
9090
"daystamp": "20221201",
91-
"comment": "Normal datapoint",
91+
"comment": "Old comment",
9292
"updated_at": 1000,
9393
"is_dummy": false,
94-
"is_initial": false
94+
"is_initial": false,
95+
"requestid": "hk_workout_1"
96+
],
97+
// Existing datapoint without matching requestId that should be deleted
98+
[
99+
"id": "obsolete1",
100+
"value": 20,
101+
"daystamp": "20221201",
102+
"comment": "Should be deleted",
103+
"updated_at": 1001,
104+
"is_dummy": false,
105+
"is_initial": false,
106+
"requestid": "hk_workout_old"
95107
],
96108
// Metadata points which should be ignored
97109
[
@@ -102,60 +114,151 @@ class DataPointManagerTests: XCTestCase {
102114
"updated_at": 2000,
103115
"is_dummy": true,
104116
"is_initial": false
105-
],
106-
[
107-
"id": "initial2",
108-
"value": 30,
109-
"daystamp": "20221203",
110-
"comment": "Initial datapoint",
111-
"updated_at": 3000,
112-
"is_dummy": false,
113-
"is_initial": true
114-
],
115-
// Additional datapoint that will be deleted (multiple datapoints for same daystamp)
116-
[
117-
"id": "duplicate1",
118-
"value": 11,
119-
"daystamp": "20221201",
120-
"comment": "Duplicate datapoint to be deleted",
121-
"updated_at": 1001,
122-
"is_dummy": false,
123-
"is_initial": false
124-
],
117+
]
125118
]
126119

127120
mockRequestManager.responses["api/v1/users/{username}/goals/test-goal/datapoints.json"] = apiResponse
128121

129-
let healthKitDatapoint = MockHealthKitDataPoint(
122+
let updatedHealthKitDatapoint = MockHealthKitDataPoint(
130123
daystamp: try Daystamp(fromString: "20221201"),
131124
value: NSNumber(value: 15),
132-
comment: "Updated from HealthKit",
133-
requestid: "hk_test"
125+
comment: "Updated workout comment",
126+
requestid: "hk_workout_1"
134127
)
135128

136129
let newHealthKitDatapoint = MockHealthKitDataPoint(
137-
daystamp: try Daystamp(fromString: "20221204"),
130+
daystamp: try Daystamp(fromString: "20221201"),
138131
value: NSNumber(value: 25),
139-
comment: "New HealthKit datapoint",
140-
requestid: "hk_new"
132+
comment: "New workout",
133+
requestid: "hk_workout_2"
141134
)
142135

143-
try! await dataPointManager.updateToMatchDataPoints(goalID: goal.objectID, healthKitDataPoints: [healthKitDatapoint, newHealthKitDatapoint])
136+
try! await dataPointManager.updateToMatchDataPoints(goalID: goal.objectID, healthKitDataPoints: [updatedHealthKitDatapoint, newHealthKitDatapoint])
144137

145-
// Should update the matching data point
138+
// Should update the existing datapoint by requestId
146139
XCTAssertEqual(mockRequestManager.putCalls.count, 1)
147-
XCTAssertTrue(mockRequestManager.putCalls[0].url.contains("normal3"))
140+
XCTAssertTrue(mockRequestManager.putCalls[0].url.contains("existing1"))
148141
XCTAssertEqual(mockRequestManager.putCalls[0].parameters["value"] as? String, "15")
142+
XCTAssertEqual(mockRequestManager.putCalls[0].parameters["comment"] as? String, "Updated workout comment")
149143

150-
// Should delete the duplicate data point
144+
// Should delete the obsolete datapoint
151145
XCTAssertEqual(mockRequestManager.deleteCalls.count, 1)
152-
XCTAssertTrue(mockRequestManager.deleteCalls[0].contains("duplicate1"))
146+
XCTAssertTrue(mockRequestManager.deleteCalls[0].contains("obsolete1"))
153147

154-
// Should create a new data point
148+
// Should create a new datapoint
155149
XCTAssertEqual(mockRequestManager.addDatapointCalls.count, 1)
156-
XCTAssertEqual(mockRequestManager.addDatapointCalls[0].urtext, "4 25 \"New HealthKit datapoint\"")
150+
XCTAssertEqual(mockRequestManager.addDatapointCalls[0].urtext, "1 25 \"New workout\"")
157151
XCTAssertEqual(mockRequestManager.addDatapointCalls[0].slug, "test-goal")
158-
XCTAssertEqual(mockRequestManager.addDatapointCalls[0].requestId, "hk_new")
152+
XCTAssertEqual(mockRequestManager.addDatapointCalls[0].requestId, "hk_workout_2")
153+
}
154+
155+
func testDeletesRemovedWorkouts() async throws {
156+
let apiResponse = [
157+
[
158+
"id": "workout1",
159+
"value": 30,
160+
"daystamp": "20221201",
161+
"comment": "Morning run",
162+
"updated_at": 1000,
163+
"is_dummy": false,
164+
"is_initial": false,
165+
"requestid": "hk_workout_uuid_1"
166+
],
167+
[
168+
"id": "workout2",
169+
"value": 45,
170+
"daystamp": "20221201",
171+
"comment": "Evening bike",
172+
"updated_at": 1001,
173+
"is_dummy": false,
174+
"is_initial": false,
175+
"requestid": "hk_workout_uuid_2"
176+
]
177+
]
178+
179+
mockRequestManager.responses["api/v1/users/{username}/goals/test-goal/datapoints.json"] = apiResponse
180+
181+
// Only one workout remains in HealthKit
182+
let remainingWorkout = MockHealthKitDataPoint(
183+
daystamp: try Daystamp(fromString: "20221201"),
184+
value: NSNumber(value: 30),
185+
comment: "Morning run",
186+
requestid: "hk_workout_uuid_1"
187+
)
188+
189+
try! await dataPointManager.updateToMatchDataPoints(goalID: goal.objectID, healthKitDataPoints: [remainingWorkout])
190+
191+
// Should not update the matching workout (same value/comment)
192+
XCTAssertEqual(mockRequestManager.putCalls.count, 0)
193+
194+
// Should delete the removed workout
195+
XCTAssertEqual(mockRequestManager.deleteCalls.count, 1)
196+
XCTAssertTrue(mockRequestManager.deleteCalls[0].contains("workout2"))
197+
198+
// Should not create any new datapoints
199+
XCTAssertEqual(mockRequestManager.addDatapointCalls.count, 0)
200+
}
201+
202+
func testMultipleDaysWithMultipleWorkouts() async throws {
203+
let apiResponse = [
204+
[
205+
"id": "day1_workout1",
206+
"value": 30,
207+
"daystamp": "20221201",
208+
"comment": "Run",
209+
"updated_at": 1000,
210+
"is_dummy": false,
211+
"is_initial": false,
212+
"requestid": "uuid_1"
213+
],
214+
[
215+
"id": "day2_workout1",
216+
"value": 45,
217+
"daystamp": "20221202",
218+
"comment": "Bike",
219+
"updated_at": 1001,
220+
"is_dummy": false,
221+
"is_initial": false,
222+
"requestid": "uuid_2"
223+
]
224+
]
225+
226+
mockRequestManager.responses["api/v1/users/{username}/goals/test-goal/datapoints.json"] = apiResponse
227+
228+
let day1Workouts = [
229+
MockHealthKitDataPoint(
230+
daystamp: try Daystamp(fromString: "20221201"),
231+
value: NSNumber(value: 30),
232+
comment: "Run",
233+
requestid: "uuid_1"
234+
),
235+
MockHealthKitDataPoint(
236+
daystamp: try Daystamp(fromString: "20221201"),
237+
value: NSNumber(value: 20),
238+
comment: "Yoga",
239+
requestid: "uuid_3"
240+
)
241+
]
242+
243+
let day2Workouts = [
244+
MockHealthKitDataPoint(
245+
daystamp: try Daystamp(fromString: "20221202"),
246+
value: NSNumber(value: 60),
247+
comment: "Long bike ride",
248+
requestid: "uuid_4"
249+
)
250+
]
251+
252+
try! await dataPointManager.updateToMatchDataPoints(goalID: goal.objectID, healthKitDataPoints: day1Workouts + day2Workouts)
253+
254+
// Should delete day2 old workout, but not update day1 unchanged workout
255+
XCTAssertEqual(mockRequestManager.deleteCalls.count, 1)
256+
XCTAssertTrue(mockRequestManager.deleteCalls[0].contains("day2_workout1"))
257+
258+
// Should create 2 new workouts (day1 yoga, day2 bike)
259+
XCTAssertEqual(mockRequestManager.addDatapointCalls.count, 2)
260+
XCTAssertTrue(mockRequestManager.addDatapointCalls.contains { $0.requestId == "uuid_3" })
261+
XCTAssertTrue(mockRequestManager.addDatapointCalls.contains { $0.requestId == "uuid_4" })
159262
}
160263

161264
private func createTestGoalJSON() -> JSON {

0 commit comments

Comments
 (0)