fix: Query HealthKit samples over specified date interval#723
fix: Query HealthKit samples over specified date interval#723cbaker6 wants to merge 10 commits intocarekit-apple:mainfrom
Conversation
| // Search over the interval provided by OCKOutcomeQuery if present | ||
| // or else constrain sample query over the current day. | ||
| let dateInterval = outcomeQuery.dateInterval ?? | ||
| Calendar.current.dateInterval(of: .day, for: Date())! |
There was a problem hiding this comment.
Similar to
* feat: All OCKOutcomeQuery sorting * effectiveDate ascending should be false for all single queries * add unit tests
| switch order { | ||
| case .effectiveDate(let ascending): return NSSortDescriptor(keyPath: \OCKCDCarePlan.effectiveDate, ascending: ascending) | ||
| case .title(let ascending): return NSSortDescriptor(keyPath: \OCKCDCarePlan.title, ascending: ascending) | ||
| case .groupIdentifier(let ascending): return NSSortDescriptor(keyPath: \OCKCDCarePlan.groupIdentifier, ascending: ascending) |
There was a problem hiding this comment.
Added groupIdentifier sort to all that was missing
| taskQuery.ids = outcomeQuery.taskIDs | ||
| taskQuery.remoteIDs = outcomeQuery.taskRemoteIDs | ||
| taskQuery.uuids = outcomeQuery.taskUUIDs | ||
| taskQuery.sortDescriptors = outcomeQuery.sortDescriptors.map { descriptor in |
There was a problem hiding this comment.
Pass relevant sort criteria to the task query.
| var query = OCKCarePlanQuery(for: Date()) | ||
| query.limit = 1 | ||
| query.sortDescriptors = [.effectiveDate(ascending: true)] | ||
| query.sortDescriptors = [.effectiveDate(ascending: false)] |
There was a problem hiding this comment.
All of these should be false to always get the newest version related to today, similar to how the fetching task was set up.
| } | ||
|
|
||
| let sortDescriptor = NSSortDescriptor( | ||
| keyPath: \OCKCDOutcome.id, |
There was a problem hiding this comment.
I'm assuming this is leftover from when the OCKCDOutcome.id had the the task occurrence appended to it:
| public struct OCKOutcomeQuery: Equatable, OCKQueryProtocol { | ||
|
|
||
| /// Specifies the order in which query results will be sorted. | ||
| public enum SortDescriptor: Equatable { |
There was a problem hiding this comment.
Add a sort descriptor for OCKOutcomeQuery
| public extension OCKAnyReadOnlyTaskStore { | ||
| func fetchAnyTask(withID id: String, callbackQueue: DispatchQueue = .main, completion: @escaping OCKResultClosure<OCKAnyTask>) { | ||
| var query = OCKTaskQuery(id: id) | ||
| var query = OCKTaskQuery(for: Date()) |
There was a problem hiding this comment.
Switched this so it's aligned with the other Entity fetches. It was setting the id twice instead of querying the current version for today. Without the change, it's possible for this fetch to return a future version of the task since it would have a future effectiveDate.
|
|
||
| var taskV2 = taskV1 | ||
| taskV2.title = "V2" | ||
| taskV2.effectiveDate = Calendar.current.date(byAdding: .year, value: 1, to: Date())! |
There was a problem hiding this comment.
Adjusted this test based on #723 (comment). This effectiveDate essentially could have been next year and it would have returned next years task today, which would be unexpected
98bc61c to
e9be043
Compare
|
@gavirawson-apple this PR has the latest updates and is ready for review |
This fixes a problem where all HealthKit queries using CareKit don't take advantage of the
DateIntervalspecified inOCKOutcomeQueryand instead default to theDateIntervalof the current day, resulting in all HealthKit samples being queried for the whole day. This can become expensive when a developer only needs samples for a small interval. In addition, it doesn't allow querying over days different from the current day. This is because the taskdateIntervalis propagated to generateHKQueryDescriptor's here:CareKit/CareKitStore/CareKitStore/HealthKit/OCKHealthKitPassthroughStore+EventUtilities.swift
Lines 577 to 590 in b16d15c
OCKOutcomeQueryif provided or else default to current dayfetchAnyTask(withID...)to always return the current version based on thetaskIDeffectiveDateandgroupIdentifierto OCKOutcomeQueryeffectiveDateassending asfalsefor all single entity fetches