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

FHIRStore actor isolation #26

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
43 changes: 19 additions & 24 deletions Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import enum ModelsDSTU2.ResourceProxy
extension FHIRResource {
/// Enum representing different categories of FHIR resources.
/// This categorization helps in classifying FHIR resources into common healthcare scenarios and types.
enum FHIRResourceCategory {
enum FHIRResourceCategory: CaseIterable {
/// Represents an observation-type resource (e.g., patient measurements, lab results).
case observation
/// Represents an encounter-type resource (e.g., patient visits, admissions).
Expand All @@ -34,32 +34,27 @@ extension FHIRResource {
case medication
/// Represents other types of resources not covered by the above categories.
case other
}

var storeKeyPath: KeyPath<FHIRStore, [FHIRResource]> {
switch self.category {
case .observation:
\.observations
case .encounter:
\.encounters
case .condition:
\.conditions
case .diagnostic:
\.diagnostics
case .procedure:
\.procedures
case .immunization:
\.immunizations
case .allergyIntolerance:
\.allergyIntolerances
case .medication:
\.medications
case .other:
\.otherResources


/// The ``FHIRStore`` property key path of the resource.
///
/// - Note: Needs to be isolated on `MainActor` as the respective ``FHIRStore`` properties referred to by the `KeyPath` are isolated on the `MainActor`.
@MainActor var storeKeyPath: KeyPath<FHIRStore, [FHIRResource]> {
switch self {
case .observation: \.observations
case .encounter: \.encounters
case .condition: \.conditions
case .diagnostic: \.diagnostics
case .procedure: \.procedures
case .immunization: \.immunizations
case .allergyIntolerance: \.allergyIntolerances
case .medication: \.medications
case .other: \.otherResources
}
}
}


/// Category of the FHIR resource.
///
/// Analyzes the type of the underlying resource and assigns it to an appropriate category.
Expand Down
10 changes: 5 additions & 5 deletions Sources/SpeziFHIR/FHIRResource/FHIRResource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
//

import Foundation
@preconcurrency import ModelsDSTU2
@preconcurrency import ModelsR4
import ModelsDSTU2
import ModelsR4


/// Represents a FHIR (Fast Healthcare Interoperability Resources) entity.
///
/// Handles both DSTU2 and R4 versions, providing a unified interface to interact with different FHIR versions.
public struct FHIRResource: Sendable, Identifiable, Hashable {
public struct FHIRResource: Identifiable, Hashable {
/// Version-specific FHIR resources.
public enum VersionedFHIRResource: Sendable, Hashable {
public enum VersionedFHIRResource: Hashable {
/// R4 version of FHIR resources.
case r4(ModelsR4.Resource) // swiftlint:disable:this identifier_name
// DSTU2 version of FHIR resources.
/// DSTU2 version of FHIR resources.
case dstu2(ModelsDSTU2.Resource)
}

Expand Down
227 changes: 116 additions & 111 deletions Sources/SpeziFHIR/FHIRStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
// SPDX-License-Identifier: MIT
//

import Combine
import Foundation
import Observation
import class ModelsR4.Bundle
import enum ModelsDSTU2.ResourceProxy
Expand All @@ -19,149 +17,156 @@
/// The ``FHIRStore`` is automatically injected in the environment if you use the ``FHIR`` standard or can be used as a standalone module.
@Observable
public final class FHIRStore: Module,
EnvironmentAccessible,
DefaultInitializable,
@unchecked Sendable /* `unchecked` `Sendable` conformance fine as access to `_resources` protected by `NSLock` */ {
private let lock = NSLock()
@ObservationIgnored private var _resources: [FHIRResource]


/// Allergy intolerances.
public var allergyIntolerances: [FHIRResource] {
EnvironmentAccessible,
DefaultInitializable,
Sendable {
@MainActor private var _resources: [FHIRResource] = []


/// `FHIRResource`s with category `allergyIntolerance`.
@MainActor public var allergyIntolerances: [FHIRResource] {
access(keyPath: \.allergyIntolerances)
return lock.withLock {
_resources.filter { $0.category == .allergyIntolerance }
}
return _resources.filter { $0.category == .allergyIntolerance }
}
/// Conditions.
public var conditions: [FHIRResource] {

/// `FHIRResource`s with category `condition`.
philippzagar marked this conversation as resolved.
Show resolved Hide resolved
@MainActor public var conditions: [FHIRResource] {
access(keyPath: \.conditions)
return lock.withLock {
_resources.filter { $0.category == .condition }
}
return _resources.filter { $0.category == .condition }
}
/// Diagnostics.
public var diagnostics: [FHIRResource] {

/// `FHIRResource`s with category `diagnostic`.
@MainActor public var diagnostics: [FHIRResource] {
access(keyPath: \.diagnostics)
return lock.withLock {
_resources.filter { $0.category == .diagnostic }
}
return _resources.filter { $0.category == .diagnostic }
}
/// Encounters.
public var encounters: [FHIRResource] {

/// `FHIRResource`s with category `encounter`.
@MainActor public var encounters: [FHIRResource] {
access(keyPath: \.encounters)
return _resources.filter { $0.category == .encounter }
}
/// Immunizations.
public var immunizations: [FHIRResource] {

/// `FHIRResource`s with category `immunization`
@MainActor public var immunizations: [FHIRResource] {
access(keyPath: \.immunizations)
return lock.withLock {
_resources.filter { $0.category == .immunization }
}
return _resources.filter { $0.category == .immunization }
}
/// Medications.
public var medications: [FHIRResource] {

/// `FHIRResource`s with category `medication`.
@MainActor public var medications: [FHIRResource] {
access(keyPath: \.medications)
return lock.withLock {
_resources.filter { $0.category == .medication }
}
return _resources.filter { $0.category == .medication }
}
/// Observations.
public var observations: [FHIRResource] {

/// `FHIRResource`s with category `observation`.
@MainActor public var observations: [FHIRResource] {
access(keyPath: \.observations)
return lock.withLock {
_resources.filter { $0.category == .observation }
}
return _resources.filter { $0.category == .observation }
}

/// Other resources that could not be classified on the other categories.
public var otherResources: [FHIRResource] {
access(keyPath: \.otherResources)
return lock.withLock {
_resources.filter { $0.category == .other }
}
}

/// Procedures.
public var procedures: [FHIRResource] {

/// `FHIRResource`s with category `procedure`.
@MainActor public var procedures: [FHIRResource] {
access(keyPath: \.procedures)
return lock.withLock {
_resources.filter { $0.category == .procedure }
}
return _resources.filter { $0.category == .procedure }
}


public required init() {
self._resources = []

/// `FHIRResource`s with category `other`.
@MainActor public var otherResources: [FHIRResource] {
access(keyPath: \.otherResources)
return _resources.filter { $0.category == .other }
}


/// Inserts a FHIR resource into the store.


/// Create an empty ``FHIRStore``.
public required init() {}


/// Inserts a FHIR resource into the ``FHIRStore``.
///
/// - Parameter resource: The `FHIRResource` to be inserted.
@MainActor
public func insert(resource: FHIRResource) {
withMutation(keyPath: resource.storeKeyPath) {
lock.withLock {
_resources.append(resource)
}
}
_$observationRegistrar.willSet(self, keyPath: resource.category.storeKeyPath)

_resources.append(resource)

_$observationRegistrar.didSet(self, keyPath: resource.category.storeKeyPath)
}
/// Removes a FHIR resource from the store.

/// Inserts a ``Collection`` of FHIR resources into the ``FHIRStore``.
///
/// - Parameter resource: The `FHIRResource` identifier to be inserted.
public func remove(resource resourceId: FHIRResource.ID) {
lock.withLock {
guard let resource = _resources.first(where: { $0.id == resourceId }) else {
return
/// - Parameter resources: The `FHIRResource`s to be inserted.
public func insert<T: Collection>(resources: sending T) async where T.Element == FHIRResource {
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this method just be isolated to MainActor (and sync) instead of requiring that the input must be supplied sending? Seems a bit overly restrictive?

let resourceCategories = Set(resources.map { $0.category })

await MainActor.run {
for category in resourceCategories {
_$observationRegistrar.willSet(self, keyPath: category.storeKeyPath)
}

withMutation(keyPath: resource.storeKeyPath) {
_resources.removeAll(where: { $0.id == resourceId })

self._resources.append(contentsOf: resources)

for category in resourceCategories {
_$observationRegistrar.didSet(self, keyPath: category.storeKeyPath)
}
}
}
/// Loads resources from a given FHIR `Bundle`.

/// Loads resources from a given FHIR `Bundle` into the ``FHIRStore``.
///
/// - Parameter bundle: The FHIR `Bundle` containing resources to be loaded.
public func load(bundle: Bundle) {
public func load(bundle: sending Bundle) async {
let resourceProxies = bundle.entry?.compactMap { $0.resource } ?? []

var resources: [FHIRResource] = []

for resourceProxy in resourceProxies {
insert(resource: FHIRResource(resource: resourceProxy.get(), displayName: resourceProxy.displayName))
if Task.isCancelled {
return

Check warning on line 125 in Sources/SpeziFHIR/FHIRStore.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFHIR/FHIRStore.swift#L125

Added line #L125 was not covered by tests
}

resources.append(
FHIRResource(
resource: resourceProxy.get(),
displayName: resourceProxy.displayName
)
)
}

if Task.isCancelled {
return

Check warning on line 137 in Sources/SpeziFHIR/FHIRStore.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFHIR/FHIRStore.swift#L137

Added line #L137 was not covered by tests
}

await insert(resources: resources)
}

/// Removes a FHIR resource from the ``FHIRStore``.
///
/// - Parameter resource: The `FHIRResource` identifier to be inserted.
@MainActor
public func remove(resource resourceId: FHIRResource.ID) {
guard let resource = _resources.first(where: { $0.id == resourceId }) else {
return
}

_$observationRegistrar.willSet(self, keyPath: resource.category.storeKeyPath)

_resources.removeAll { $0.id == resourceId }

_$observationRegistrar.didSet(self, keyPath: resource.category.storeKeyPath)
}

/// Removes all resources from the store.

/// Removes all resources from the ``FHIRStore``.
@MainActor
public func removeAllResources() {
lock.withLock {
// Not really ideal but seems to be a path to ensure that all observables are called.
_$observationRegistrar.willSet(self, keyPath: \.allergyIntolerances)
_$observationRegistrar.willSet(self, keyPath: \.conditions)
_$observationRegistrar.willSet(self, keyPath: \.diagnostics)
_$observationRegistrar.willSet(self, keyPath: \.encounters)
_$observationRegistrar.willSet(self, keyPath: \.immunizations)
_$observationRegistrar.willSet(self, keyPath: \.medications)
_$observationRegistrar.willSet(self, keyPath: \.observations)
_$observationRegistrar.willSet(self, keyPath: \.otherResources)
_$observationRegistrar.willSet(self, keyPath: \.procedures)
_resources = []
_$observationRegistrar.didSet(self, keyPath: \.allergyIntolerances)
_$observationRegistrar.didSet(self, keyPath: \.conditions)
_$observationRegistrar.didSet(self, keyPath: \.diagnostics)
_$observationRegistrar.didSet(self, keyPath: \.encounters)
_$observationRegistrar.didSet(self, keyPath: \.immunizations)
Copy link
Member

Choose a reason for hiding this comment

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

Something that this PR is changing is that previously the observation pattern is much more granular. If you were just interested in a certain category, the view would just update if an element in this category was changed. Might this impact performance for our use case?

_$observationRegistrar.didSet(self, keyPath: \.medications)
_$observationRegistrar.didSet(self, keyPath: \.observations)
_$observationRegistrar.didSet(self, keyPath: \.otherResources)
_$observationRegistrar.didSet(self, keyPath: \.procedures)
for category in FHIRResource.FHIRResourceCategory.allCases {
_$observationRegistrar.willSet(self, keyPath: category.storeKeyPath)
}

_resources = []

for category in FHIRResource.FHIRResourceCategory.allCases {
_$observationRegistrar.didSet(self, keyPath: category.storeKeyPath)
}
}
}
4 changes: 2 additions & 2 deletions Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public func add(sample: HKSample) async {
do {
let resource = try await transform(sample: sample)
insert(resource: resource)
await insert(resource: resource)

Check warning on line 32 in Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift#L32

Added line #L32 was not covered by tests
} catch {
print("Could not transform HKSample: \(error)")
}
Expand All @@ -38,7 +38,7 @@
/// Remove a HealthKit sample delete object from the FHIR store.
/// - Parameter sample: The sample delete object that should be removed.
public func remove(sample: HKDeletedObject) async {
remove(resource: sample.uuid.uuidString)
await remove(resource: sample.uuid.uuidString)

Check warning on line 41 in Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift#L41

Added line #L41 was not covered by tests
}


Expand Down
Loading
Loading