VIT-7712: Start and stop HKQuery inside handle lock

Sources/VitalHealthKit/HealthKit/Abstractions.swift

@@ -429,8 +429,7 @@ struct StatisticsQueryDependencies {
       )
 
       @Sendable func handle(
-        _ query: HKStatisticsCollectionQuery,
-        collection: HKStatisticsCollection?,
+        _ collection: HKStatisticsCollection?,
         error: Error?,
         continuation: CancellableQueryHandle<[VitalStatistics]>.Continuation
       ) {
@@ -500,7 +499,7 @@ struct StatisticsQueryDependencies {
 
       let handle = CancellableQueryHandle { continuation in
         query.initialResultsHandler = { query, collection, error in
-          handle(query, collection: collection, error: error, continuation: continuation)
+          handle(collection, error: error, continuation: continuation)
         }
         return query
       }
@@ -609,7 +608,7 @@ final class CancellableQueryHandle<Result>: @unchecked Sendable {
 
   @discardableResult
   private func transition(to newState: State) -> UnsafeContinuation<Result, any Error>? {
-    let (doWork, continuation): ((() -> Void)?, UnsafeContinuation<Result, any Error>?) = lock.withLock {
+    let continuation: UnsafeContinuation<Result, any Error>? = lock.withLock {
       switch (state, newState) {
       case let (.idle, .running(store, query, _)):
 
@@ -618,44 +617,42 @@ final class CancellableQueryHandle<Result>: @unchecked Sendable {
           try await Task.sleep(nanoseconds: NSEC_PER_SEC * timeoutSeconds)
           self?.cancel()
         }
-        let doWork = { store.execute(query) }
-        return (doWork, nil)
+        store.execute(query)
+        return nil
 
       case 
         let (.running(store, query, continuation), .cancelled),
         let (.running(store, query, continuation), .completed):
 
         state = newState
         watchdog?.cancel()
-        let doWork = { store.stop(query) }
-        return (doWork, continuation)
+        store.stop(query)
+        return continuation
 
       case
         let (.cancelled, .running(_, _, continuation)),
         let (.completed, .running(_, _, continuation)):
 
         // The handle is cancelled before it started running.
         // Don't start the query, but make sure the continuation is called.
-        let doWork = { continuation.resume(throwing: CancellationError()) }
-        return (doWork, nil)
+        continuation.resume(throwing: CancellationError())
+        return nil
 
       case (.completed, .cancelled), (.cancelled, .cancelled), (.idle, .cancelled):
         // Not illegal, can gracefully ignore
-        return (nil, nil)
+        return nil
 
       case (.cancelled, .completed):
         // Not illgeal, can gracefully ignore
         // Any registered continuation would have been called backed already
         // during running -> cancelled|completed.
-        return (nil, nil)
+        return nil
 
       case (.completed, .completed), (.running, .running), (.idle, .completed), (_, .idle):
         fatalError("Illegal CancellableQueryHandle state transition")
       }
     }
 
-    doWork?()
-
     return continuation
   }
 

Tests/VitalHealthKitTests/SampleTypeTests.swift

@@ -16,7 +16,7 @@ private class MockHealthStore: HKHealthStore {
         to: CompletionHandler.self
       )
       DispatchQueue.global().async {
-        handler(query, [], [], nil, nil)
+        handler(query, nil, nil, nil, ExpectedError())
       }
 
     case let query as HKStatisticsCollectionQuery:
@@ -33,7 +33,7 @@ private class MockHealthStore: HKHealthStore {
         to: ResultHandler.self
       )
       DispatchQueue.global().async {
-        handler(query, [], nil)
+        handler(query, nil, ExpectedError())
       }
 
     default:
@@ -53,43 +53,29 @@ private class MockHealthStore: HKHealthStore {
 class SampleTypeTests: XCTestCase {
 
   @available(iOS 16.0, *)
-  func test_observed_types_are_mappable_to_resources() {
-    let success = self.expectation(description: "test_body")
-    let thrownError = OSAllocatedUnfairLock<Error?>(initialState: nil)
+  func test_observed_types_are_mappable_to_resources() async throws {
+    let resources = observedSampleTypes()
+      .flatMap({ $0 })
+      .flatMap(VitalHealthKitStore.sampleTypeToVitalResource)
 
-    Task {
+    for resource in resources {
       do {
-        let resources = observedSampleTypes()
-          .flatMap({ $0 })
-          .flatMap(VitalHealthKitStore.sampleTypeToVitalResource)
-
-        for resource in resources {
-          do {
-            print(resource)
-            _ = try await read(
-              resource: VitalHealthKitStore.remapResource(resource),
-              healthKitStore: MockHealthStore(),
-              vitalStorage: VitalHealthKitStorage(storage: .debug),
-              instruction: SyncInstruction(stage: .daily, query: Date() ..< Date()),
-              options: ReadOptions()
-            )
-          } catch let error {
-            switch error {
-            case is ExpectedError:
-              continue
-            default:
-              throw error
-            }
-          }
-        }
-        success.fulfill()
+        _ = try await read(
+          resource: VitalHealthKitStore.remapResource(resource),
+          healthKitStore: MockHealthStore(),
+          vitalStorage: VitalHealthKitStorage(storage: .debug),
+          instruction: SyncInstruction(stage: .daily, query: Date() ..< Date()),
+          options: ReadOptions()
+        )
       } catch let error {
-        thrownError.withLock { $0 = error }
+        switch error {
+        case is ExpectedError:
+          continue
+        default:
+          throw error
+        }
       }
     }
-
-    wait(for: [success], timeout: 3.0)
-    XCTAssertNil(thrownError.withLock { $0 })
   }
 
   func test_all_quantity_sample_types_have_mapped_unit() async {