Skip to content

Conversation

@cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Feb 23, 2025

This fixes a problem where all HealthKit queries using CareKit don't take advantage of the DateInterval specified in OCKOutcomeQuery and instead default to the DateInterval of 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 task dateInterval is propagated to generate HKQueryDescriptor's here:

// Create a lookup table to quickly locate all events for a specific sample type
let eventsGroupedByQuantityType = Dictionary(grouping: events) {
extractQuantityType(from: $0.task)
}
// Create a single query descriptor for each quantity type. The date interval
// for the query descriptor predicate is determined by the events for the quantity type.
let queryDescriptors = eventsGroupedByQuantityType.map { sampleType, events -> HKQueryDescriptor in
let predicates = events.map { makePredicate(for: $0.scheduleEvent) }
let predicate = NSCompoundPredicate(orPredicateWithSubpredicates: predicates)
return HKQueryDescriptor(sampleType: sampleType, predicate: predicate)
}

  • Use the date interval specified by OCKOutcomeQuery if provided or else default to current day
  • Fix fetchAnyTask(withID...) to always return the current version based on the taskID
  • Add sort descriptors for effectiveDate and groupIdentifier to OCKOutcomeQuery
  • Sort by effectiveDate assending as false for all single entity fetches

@cbaker6 cbaker6 changed the title fix: Query HealthKitSamples over specified date interval fix: Query HealthKit samples over specified date interval Feb 23, 2025
// 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())!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to

// We need to explicitly set the date interval in case it's nil
// to ensure that we fetch the most recent version of a task in a
// given date interval, and not all versions of the task
let dateInterval = latestTaskVersionsQuery.dateInterval ??
Calendar.current.dateInterval(of: .day, for: Date())!
latestTaskVersionsQuery.dateInterval = dateInterval

* 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
Copy link
Contributor Author

@cbaker6 cbaker6 Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this is leftover from when the OCKCDOutcome.id had the the task occurrence appended to it:

public var id: String { taskUUID.uuidString + "_\(taskOccurrenceIndex)" }

public struct OCKOutcomeQuery: Equatable, OCKQueryProtocol {

/// Specifies the order in which query results will be sorted.
public enum SortDescriptor: Equatable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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())
Copy link
Contributor Author

@cbaker6 cbaker6 Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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())!
Copy link
Contributor Author

@cbaker6 cbaker6 Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@cbaker6 cbaker6 force-pushed the updateHealthKitOutcomeDateInterval branch from 98bc61c to e9be043 Compare April 24, 2025 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant