From 1bf79ec04866f560cd50448465b56e58074eb1e2 Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Mon, 23 Dec 2024 19:26:31 +0100 Subject: [PATCH 1/9] FHIRStore proper actor isolation implementation --- .../FHIRResource/FHIRResource+Category.swift | 32 ++- Sources/SpeziFHIR/FHIRStore.swift | 193 ++++++++---------- .../FHIRStore+HealthKit.swift | 4 +- .../FHIRBundleSelector.swift | 79 +++---- .../FHIRStore+TestingSupport.swift | 6 +- Tests/UITests/TestApp/ContentView.swift | 6 +- .../FHIRMockDataStorageProviderTests.swift | 10 +- .../UITests/UITests.xcodeproj/project.pbxproj | 8 +- .../xcshareddata/xcschemes/TestApp.xcscheme | 2 +- 9 files changed, 157 insertions(+), 183 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift b/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift index 2a7b0b8..ab949a4 100644 --- a/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift +++ b/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift @@ -35,31 +35,23 @@ extension FHIRResource { /// Represents other types of resources not covered by the above categories. case other } - + + + /// The ``FHIRStore`` property key path of the resource. var storeKeyPath: KeyPath { 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 + 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. diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index 08abdf9..a506510 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -6,8 +6,6 @@ // SPDX-License-Identifier: MIT // -import Combine -import Foundation import Observation import class ModelsR4.Bundle import enum ModelsDSTU2.ResourceProxy @@ -19,149 +17,132 @@ import Spezi /// 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. + EnvironmentAccessible, + DefaultInitializable, + Sendable { + /// Actor-isolated storage for `FHIRResource`s backing the ``FHIRStore``. + private actor Storage { + // Non-isolation required so that resource access via the `FHIRStore` stays sync (required for seamless SwiftUI access). + // Isolation is still guaranteed as the only modifying functions `insert()` and `remove()` are isolated on the `FHIRStore.Storage` actor. + nonisolated(unsafe) fileprivate var _resources: [FHIRResource] = [] + + + func insert(resource: FHIRResource) { + _resources.append(resource) + } + + func remove(resource resourceId: FHIRResource.ID) { + _resources.removeAll { $0.id == resourceId } + } + } + + + private let storage: Storage + + + /// `FHIRResource`s with category `allergyIntolerance`. public var allergyIntolerances: [FHIRResource] { access(keyPath: \.allergyIntolerances) - return lock.withLock { - _resources.filter { $0.category == .allergyIntolerance } - } + return storage._resources.filter { $0.category == .allergyIntolerance } } - - /// Conditions. + + /// `FHIRResource`s with category `condition`. public var conditions: [FHIRResource] { access(keyPath: \.conditions) - return lock.withLock { - _resources.filter { $0.category == .condition } - } + return storage._resources.filter { $0.category == .condition } } - - /// Diagnostics. + + /// `FHIRResource`s with category `diagnostic`. public var diagnostics: [FHIRResource] { access(keyPath: \.diagnostics) - return lock.withLock { - _resources.filter { $0.category == .diagnostic } - } + return storage._resources.filter { $0.category == .diagnostic } } - - /// Encounters. + + /// `FHIRResource`s with category `encounter`. public var encounters: [FHIRResource] { access(keyPath: \.encounters) - return _resources.filter { $0.category == .encounter } + return storage._resources.filter { $0.category == .encounter } } - - /// Immunizations. + + /// `FHIRResource`s with category `immunization`. public var immunizations: [FHIRResource] { access(keyPath: \.immunizations) - return lock.withLock { - _resources.filter { $0.category == .immunization } - } + return storage._resources.filter { $0.category == .immunization } } - - /// Medications. + + /// `FHIRResource`s with category `medication`. public var medications: [FHIRResource] { access(keyPath: \.medications) - return lock.withLock { - _resources.filter { $0.category == .medication } - } + return storage._resources.filter { $0.category == .medication } } - - /// Observations. + + /// `FHIRResource`s with category `observation`. public var observations: [FHIRResource] { access(keyPath: \.observations) - return lock.withLock { - _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 } - } + return storage._resources.filter { $0.category == .observation } } - - /// Procedures. + + /// `FHIRResource`s with category `procedure`. public var procedures: [FHIRResource] { access(keyPath: \.procedures) - return lock.withLock { - _resources.filter { $0.category == .procedure } - } + return storage._resources.filter { $0.category == .procedure } + } + + /// `FHIRResource`s with category `other`. + public var otherResources: [FHIRResource] { + access(keyPath: \.otherResources) + return storage._resources.filter { $0.category == .other } } - - + + + /// Create an empty ``FHIRStore``. public required init() { - self._resources = [] + storage = Storage() } - - - /// Inserts a FHIR resource into the store. + + + /// Inserts a FHIR resource into the ``FHIRStore``. /// /// - Parameter resource: The `FHIRResource` to be inserted. - public func insert(resource: FHIRResource) { - withMutation(keyPath: resource.storeKeyPath) { - lock.withLock { - _resources.append(resource) - } - } + public func insert(resource: FHIRResource) async { + _$observationRegistrar.willSet(self, keyPath: resource.storeKeyPath) + await storage.insert(resource: resource) + _$observationRegistrar.didSet(self, keyPath: resource.storeKeyPath) } - - /// Removes a FHIR resource from the store. + + /// Removes a FHIR resource from 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 - } - - withMutation(keyPath: resource.storeKeyPath) { - _resources.removeAll(where: { $0.id == resourceId }) - } + public func remove(resource resourceId: FHIRResource.ID) async { + guard let resource = storage._resources.first(where: { $0.id == resourceId }) else { + return } + + _$observationRegistrar.willSet(self, keyPath: resource.storeKeyPath) + await storage.remove(resource: resourceId) + _$observationRegistrar.didSet(self, keyPath: resource.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: Bundle) async { let resourceProxies = bundle.entry?.compactMap { $0.resource } ?? [] - + for resourceProxy in resourceProxies { - insert(resource: FHIRResource(resource: resourceProxy.get(), displayName: resourceProxy.displayName)) + await self.insert( + resource: FHIRResource( + resource: resourceProxy.get(), + displayName: resourceProxy.displayName + ) + ) } } - - /// Removes all resources from the store. - 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) - _$observationRegistrar.didSet(self, keyPath: \.medications) - _$observationRegistrar.didSet(self, keyPath: \.observations) - _$observationRegistrar.didSet(self, keyPath: \.otherResources) - _$observationRegistrar.didSet(self, keyPath: \.procedures) + + /// Removes all resources from the ``FHIRStore``. + public func removeAllResources() async { + for resourceId in storage._resources.map(\.id) { + await self.remove(resource: resourceId) } } } diff --git a/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift b/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift index d599058..b44ca28 100644 --- a/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift +++ b/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift @@ -29,7 +29,7 @@ extension FHIRStore { public func add(sample: HKSample) async { do { let resource = try await transform(sample: sample) - insert(resource: resource) + await insert(resource: resource) } catch { print("Could not transform HKSample: \(error)") } @@ -38,7 +38,7 @@ extension FHIRStore { /// 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) } diff --git a/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift b/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift index 8181238..1d34b00 100644 --- a/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift +++ b/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift @@ -6,7 +6,7 @@ // SPDX-License-Identifier: MIT // -import class ModelsR4.Bundle +@preconcurrency import class ModelsR4.Bundle import class ModelsR4.Patient import SpeziFHIR import SwiftUI @@ -16,57 +16,60 @@ import SwiftUI /// /// The View assumes that the bundle contains a `ModelsR4.Patient` resource to identify the bundle and provide a human-readable name. public struct FHIRBundleSelector: View { - private struct PatientIdentifiedBundle: Identifiable { + private struct PatientIdentifiedBundle: Identifiable, Sendable { let id: String let bundle: ModelsR4.Bundle } - - - private let bundles: [PatientIdentifiedBundle] - + + @Environment(FHIRStore.self) private var store + @State private var selectedBundleId: PatientIdentifiedBundle.ID? + + private let bundles: [PatientIdentifiedBundle] + - private var selectedBundle: Binding { - Binding( - get: { - guard let patient = store.otherResources.compactMap({ resource -> ModelsR4.Patient? in - guard case let .r4(resource) = resource.versionedResource, let loadedPatient = resource as? Patient else { - return nil - } - return loadedPatient - }).first else { - return nil - } - - - guard let bundle = bundles.first(where: { patient.identifier == $0.bundle.patient?.identifier }) else { - return nil - } - - return bundle.id - }, - set: { newValue in - guard let newValue, let bundle = bundles.first(where: { $0.id == newValue })?.bundle else { - return - } - - store.removeAllResources() - store.load(bundle: bundle) - } - ) - } - - public var body: some View { Picker( String(localized: "Select Mock Patient", bundle: .module), - selection: selectedBundle + selection: $selectedBundleId ) { ForEach(bundles) { bundle in Text(bundle.bundle.patientName) .tag(bundle.id as String?) } } + // Load the currently stored patient data to update `selectedBundleID` + .task { + let existingResources = store.otherResources + guard + let patient = existingResources.compactMap({ resource -> Patient? in + guard case let .r4(r4Resource) = resource.versionedResource, + let loadedPatient = r4Resource as? Patient else { + return nil + } + + return loadedPatient + }).first, + let matchedBundle = bundles.first(where: { + patient.identifier == $0.bundle.patient?.identifier + }) + else { + return + } + selectedBundleId = matchedBundle.id + } + // Remove existing resources and load the newly selected bundle + .onChange(of: selectedBundleId) { _, newValue in + Task { + guard let newValue, + let selected = bundles.first(where: { $0.id == newValue }) else { + return + } + + await store.removeAllResources() + await store.load(bundle: selected.bundle) + } + } } diff --git a/Sources/SpeziFHIRMockPatients/FHIRStore+TestingSupport.swift b/Sources/SpeziFHIRMockPatients/FHIRStore+TestingSupport.swift index 1458133..b7916de 100644 --- a/Sources/SpeziFHIRMockPatients/FHIRStore+TestingSupport.swift +++ b/Sources/SpeziFHIRMockPatients/FHIRStore+TestingSupport.swift @@ -12,7 +12,7 @@ import SpeziFHIR extension FHIRStore { /// Loads a mock resource into the `FHIRStore` for testing purposes. - public func loadTestingResources() { + public func loadTestingResources() async { let mockObservation = Observation( code: CodeableConcept(coding: [Coding(code: "1234".asFHIRStringPrimitive())]), id: FHIRPrimitive("1234"), @@ -25,7 +25,7 @@ extension FHIRStore { displayName: "Mock Resource" ) - removeAllResources() - insert(resource: mockFHIRResource) + await removeAllResources() + await insert(resource: mockFHIRResource) } } diff --git a/Tests/UITests/TestApp/ContentView.swift b/Tests/UITests/TestApp/ContentView.swift index 09895cb..876eaa7 100644 --- a/Tests/UITests/TestApp/ContentView.swift +++ b/Tests/UITests/TestApp/ContentView.swift @@ -11,8 +11,8 @@ import SwiftUI struct ContentView: View { - @Environment(FHIRStore.self) var fhirStore - @State var presentPatientSelection = false + @Environment(FHIRStore.self) private var fhirStore + @State private var presentPatientSelection = false var body: some View { @@ -26,8 +26,8 @@ struct ContentView: View { Text("Immunizations: \(fhirStore.immunizations.count)") Text("Medications: \(fhirStore.medications.count)") Text("Observations: \(fhirStore.observations.count)") - Text("Other Resources: \(fhirStore.otherResources.count)") Text("Procedures: \(fhirStore.procedures.count)") + Text("Other Resources: \(fhirStore.otherResources.count)") } Section { presentPatientSelectionButton diff --git a/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift b/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift index 6a0eb23..39370cb 100644 --- a/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift +++ b/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift @@ -21,9 +21,9 @@ final class SpeziFHIRTests: XCTestCase { XCTAssert(app.staticTexts["Immunizations: 0"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Medications: 0"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Observations: 0"].waitForExistence(timeout: 2)) - XCTAssert(app.staticTexts["Other Resources: 0"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Procedures: 0"].waitForExistence(timeout: 2)) - + XCTAssert(app.staticTexts["Other Resources: 0"].waitForExistence(timeout: 2)) + app.buttons["Select Mock Patient"].tap() XCTAssert(app.buttons["Jamison785 Denesik803"].waitForExistence(timeout: 20)) @@ -38,9 +38,9 @@ final class SpeziFHIRTests: XCTestCase { XCTAssert(app.staticTexts["Immunizations: 12"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Medications: 31"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Observations: 769"].waitForExistence(timeout: 2)) - XCTAssert(app.staticTexts["Other Resources: 302"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Procedures: 106"].waitForExistence(timeout: 2)) - + XCTAssert(app.staticTexts["Other Resources: 302"].waitForExistence(timeout: 2)) + app.buttons["Select Mock Patient"].tap() XCTAssert(app.buttons["Maye976 Dickinson688"].waitForExistence(timeout: 20)) @@ -55,7 +55,7 @@ final class SpeziFHIRTests: XCTestCase { XCTAssert(app.staticTexts["Immunizations: 11"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Medications: 55"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Observations: 169"].waitForExistence(timeout: 2)) - XCTAssert(app.staticTexts["Other Resources: 322"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Procedures: 225"].waitForExistence(timeout: 2)) + XCTAssert(app.staticTexts["Other Resources: 322"].waitForExistence(timeout: 2)) } } diff --git a/Tests/UITests/UITests.xcodeproj/project.pbxproj b/Tests/UITests/UITests.xcodeproj/project.pbxproj index 0a5207b..031a5d6 100644 --- a/Tests/UITests/UITests.xcodeproj/project.pbxproj +++ b/Tests/UITests/UITests.xcodeproj/project.pbxproj @@ -164,7 +164,7 @@ attributes = { BuildIndependentTargetsInParallel = 1; LastSwiftUpdateCheck = 1410; - LastUpgradeCheck = 1500; + LastUpgradeCheck = 1620; TargetAttributes = { 2F6D139128F5F384007C25D6 = { CreatedOnToolsVersion = 14.1; @@ -412,7 +412,7 @@ SUPPORTS_MAC_DESIGNED_FOR_IPHONE_IPAD = NO; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_STRICT_CONCURRENCY = complete; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = 1; }; name = Debug; @@ -445,7 +445,7 @@ SUPPORTS_MAC_DESIGNED_FOR_IPHONE_IPAD = NO; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_STRICT_CONCURRENCY = complete; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = 1; }; name = Release; @@ -453,7 +453,6 @@ 2F6D13BD28F5F386007C25D6 /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { - ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES; CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 1; DEVELOPMENT_TEAM = 637867499T; @@ -473,7 +472,6 @@ 2F6D13BE28F5F386007C25D6 /* Release */ = { isa = XCBuildConfiguration; buildSettings = { - ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES; CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 1; DEVELOPMENT_TEAM = 637867499T; diff --git a/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme b/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme index ba86621..322d325 100644 --- a/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme +++ b/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme @@ -1,6 +1,6 @@ Date: Mon, 23 Dec 2024 19:50:24 +0100 Subject: [PATCH 2/9] Introduce abstractions in Storage --- Sources/SpeziFHIR/FHIRStore.swift | 38 +++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index a506510..69b87d5 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -24,7 +24,12 @@ public final class FHIRStore: Module, private actor Storage { // Non-isolation required so that resource access via the `FHIRStore` stays sync (required for seamless SwiftUI access). // Isolation is still guaranteed as the only modifying functions `insert()` and `remove()` are isolated on the `FHIRStore.Storage` actor. - nonisolated(unsafe) fileprivate var _resources: [FHIRResource] = [] + nonisolated(unsafe) private var _resources: [FHIRResource] = [] + + + var resourceIds: [FHIRResource.ID] { + _resources.map(\.id) + } func insert(resource: FHIRResource) { @@ -34,6 +39,15 @@ public final class FHIRStore: Module, func remove(resource resourceId: FHIRResource.ID) { _resources.removeAll { $0.id == resourceId } } + + nonisolated func fetch(for category: FHIRResource.FHIRResourceCategory) -> [FHIRResource] { + _resources.filter { $0.category == category } + } + + + subscript(id: FHIRResource.ID) -> FHIRResource? { + _resources.first { $0.id == id } + } } @@ -43,55 +57,55 @@ public final class FHIRStore: Module, /// `FHIRResource`s with category `allergyIntolerance`. public var allergyIntolerances: [FHIRResource] { access(keyPath: \.allergyIntolerances) - return storage._resources.filter { $0.category == .allergyIntolerance } + return storage.fetch(for: .allergyIntolerance) } /// `FHIRResource`s with category `condition`. public var conditions: [FHIRResource] { access(keyPath: \.conditions) - return storage._resources.filter { $0.category == .condition } + return storage.fetch(for: .condition) } /// `FHIRResource`s with category `diagnostic`. public var diagnostics: [FHIRResource] { access(keyPath: \.diagnostics) - return storage._resources.filter { $0.category == .diagnostic } + return storage.fetch(for: .diagnostic) } /// `FHIRResource`s with category `encounter`. public var encounters: [FHIRResource] { access(keyPath: \.encounters) - return storage._resources.filter { $0.category == .encounter } + return storage.fetch(for: .encounter) } /// `FHIRResource`s with category `immunization`. public var immunizations: [FHIRResource] { access(keyPath: \.immunizations) - return storage._resources.filter { $0.category == .immunization } + return storage.fetch(for: .immunization) } /// `FHIRResource`s with category `medication`. public var medications: [FHIRResource] { access(keyPath: \.medications) - return storage._resources.filter { $0.category == .medication } + return storage.fetch(for: .medication) } /// `FHIRResource`s with category `observation`. public var observations: [FHIRResource] { access(keyPath: \.observations) - return storage._resources.filter { $0.category == .observation } + return storage.fetch(for: .observation) } /// `FHIRResource`s with category `procedure`. public var procedures: [FHIRResource] { access(keyPath: \.procedures) - return storage._resources.filter { $0.category == .procedure } + return storage.fetch(for: .procedure) } /// `FHIRResource`s with category `other`. public var otherResources: [FHIRResource] { access(keyPath: \.otherResources) - return storage._resources.filter { $0.category == .other } + return storage.fetch(for: .other) } @@ -114,7 +128,7 @@ public final class FHIRStore: Module, /// /// - Parameter resource: The `FHIRResource` identifier to be inserted. public func remove(resource resourceId: FHIRResource.ID) async { - guard let resource = storage._resources.first(where: { $0.id == resourceId }) else { + guard let resource = await storage[resourceId] else { return } @@ -141,7 +155,7 @@ public final class FHIRStore: Module, /// Removes all resources from the ``FHIRStore``. public func removeAllResources() async { - for resourceId in storage._resources.map(\.id) { + for resourceId in await storage.resourceIds { await self.remove(resource: resourceId) } } From d154f385fe1a131609a7de43f2609e4acb6f8f9f Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Mon, 23 Dec 2024 20:07:54 +0100 Subject: [PATCH 3/9] Isolation fixes --- .../FHIRResource/FHIRResource+Category.swift | 29 +++++++++++-------- Sources/SpeziFHIR/FHIRStore.swift | 19 +++++++----- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift b/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift index ab949a4..3d4cdc4 100644 --- a/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift +++ b/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift @@ -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). @@ -34,22 +34,27 @@ extension FHIRResource { case medication /// Represents other types of resources not covered by the above categories. case other + + + var storeKeyPath: KeyPath { + 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 + } + } } /// The ``FHIRStore`` property key path of the resource. var storeKeyPath: KeyPath { - 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 - } + self.category.storeKeyPath } /// Category of the FHIR resource. diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index 69b87d5..46fcfaf 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -27,11 +27,6 @@ public final class FHIRStore: Module, nonisolated(unsafe) private var _resources: [FHIRResource] = [] - var resourceIds: [FHIRResource.ID] { - _resources.map(\.id) - } - - func insert(resource: FHIRResource) { _resources.append(resource) } @@ -40,6 +35,10 @@ public final class FHIRStore: Module, _resources.removeAll { $0.id == resourceId } } + func removeAll() { + _resources = [] + } + nonisolated func fetch(for category: FHIRResource.FHIRResourceCategory) -> [FHIRResource] { _resources.filter { $0.category == category } } @@ -155,8 +154,14 @@ public final class FHIRStore: Module, /// Removes all resources from the ``FHIRStore``. public func removeAllResources() async { - for resourceId in await storage.resourceIds { - await self.remove(resource: resourceId) + for category in FHIRResource.FHIRResourceCategory.allCases { + _$observationRegistrar.willSet(self, keyPath: category.storeKeyPath) + } + + await storage.removeAll() + + for category in FHIRResource.FHIRResourceCategory.allCases { + _$observationRegistrar.didSet(self, keyPath: category.storeKeyPath) } } } From 74dc7c10ecfa1f6092f6f34a17aa6256739c383c Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Thu, 2 Jan 2025 23:53:23 +0100 Subject: [PATCH 4/9] Implement feedback --- .../FHIRResource/FHIRResource+Category.swift | 12 +- Sources/SpeziFHIR/FHIRStore.swift | 145 +++++++++--------- .../FHIRBundleSelector.swift | 19 ++- Tests/UITests/TestApp/ContentView.swift | 16 ++ 4 files changed, 109 insertions(+), 83 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift b/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift index 3d4cdc4..62ea806 100644 --- a/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift +++ b/Sources/SpeziFHIR/FHIRResource/FHIRResource+Category.swift @@ -36,7 +36,10 @@ extension FHIRResource { case other - var storeKeyPath: KeyPath { + /// 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 { switch self { case .observation: \.observations case .encounter: \.encounters @@ -50,13 +53,8 @@ extension FHIRResource { } } } - - - /// The ``FHIRStore`` property key path of the resource. - var storeKeyPath: KeyPath { - self.category.storeKeyPath - } + /// Category of the FHIR resource. /// /// Analyzes the type of the underlying resource and assigns it to an appropriate category. diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index 46fcfaf..4ad9915 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -20,120 +20,97 @@ public final class FHIRStore: Module, EnvironmentAccessible, DefaultInitializable, Sendable { - /// Actor-isolated storage for `FHIRResource`s backing the ``FHIRStore``. - private actor Storage { - // Non-isolation required so that resource access via the `FHIRStore` stays sync (required for seamless SwiftUI access). - // Isolation is still guaranteed as the only modifying functions `insert()` and `remove()` are isolated on the `FHIRStore.Storage` actor. - nonisolated(unsafe) private var _resources: [FHIRResource] = [] - - - func insert(resource: FHIRResource) { - _resources.append(resource) - } - - func remove(resource resourceId: FHIRResource.ID) { - _resources.removeAll { $0.id == resourceId } - } - - func removeAll() { - _resources = [] - } - - nonisolated func fetch(for category: FHIRResource.FHIRResourceCategory) -> [FHIRResource] { - _resources.filter { $0.category == category } - } - - - subscript(id: FHIRResource.ID) -> FHIRResource? { - _resources.first { $0.id == id } - } - } - - - private let storage: Storage + @MainActor private var _resources: [FHIRResource] = [] /// `FHIRResource`s with category `allergyIntolerance`. - public var allergyIntolerances: [FHIRResource] { + @MainActor public var allergyIntolerances: [FHIRResource] { access(keyPath: \.allergyIntolerances) - return storage.fetch(for: .allergyIntolerance) + return _resources.filter { $0.category == .allergyIntolerance } } /// `FHIRResource`s with category `condition`. - public var conditions: [FHIRResource] { + @MainActor public var conditions: [FHIRResource] { access(keyPath: \.conditions) - return storage.fetch(for: .condition) + return _resources.filter { $0.category == .condition } } /// `FHIRResource`s with category `diagnostic`. - public var diagnostics: [FHIRResource] { + @MainActor public var diagnostics: [FHIRResource] { access(keyPath: \.diagnostics) - return storage.fetch(for: .diagnostic) + return _resources.filter { $0.category == .diagnostic } } /// `FHIRResource`s with category `encounter`. - public var encounters: [FHIRResource] { + @MainActor public var encounters: [FHIRResource] { access(keyPath: \.encounters) - return storage.fetch(for: .encounter) + return _resources.filter { $0.category == .encounter } } - /// `FHIRResource`s with category `immunization`. - public var immunizations: [FHIRResource] { + /// `FHIRResource`s with category `immunization` + @MainActor public var immunizations: [FHIRResource] { access(keyPath: \.immunizations) - return storage.fetch(for: .immunization) + return _resources.filter { $0.category == .immunization } } /// `FHIRResource`s with category `medication`. - public var medications: [FHIRResource] { + @MainActor public var medications: [FHIRResource] { access(keyPath: \.medications) - return storage.fetch(for: .medication) + return _resources.filter { $0.category == .medication } } /// `FHIRResource`s with category `observation`. - public var observations: [FHIRResource] { + @MainActor public var observations: [FHIRResource] { access(keyPath: \.observations) - return storage.fetch(for: .observation) + return _resources.filter { $0.category == .observation } } /// `FHIRResource`s with category `procedure`. - public var procedures: [FHIRResource] { + @MainActor public var procedures: [FHIRResource] { access(keyPath: \.procedures) - return storage.fetch(for: .procedure) + return _resources.filter { $0.category == .procedure } } /// `FHIRResource`s with category `other`. - public var otherResources: [FHIRResource] { + @MainActor public var otherResources: [FHIRResource] { access(keyPath: \.otherResources) - return storage.fetch(for: .other) + return _resources.filter { $0.category == .other } } /// Create an empty ``FHIRStore``. - public required init() { - storage = Storage() - } + public required init() {} /// Inserts a FHIR resource into the ``FHIRStore``. /// /// - Parameter resource: The `FHIRResource` to be inserted. - public func insert(resource: FHIRResource) async { - _$observationRegistrar.willSet(self, keyPath: resource.storeKeyPath) - await storage.insert(resource: resource) - _$observationRegistrar.didSet(self, keyPath: resource.storeKeyPath) + @MainActor + public func insert(resource: FHIRResource) { + _$observationRegistrar.willSet(self, keyPath: resource.category.storeKeyPath) + + _resources.append(resource) + + _$observationRegistrar.didSet(self, keyPath: resource.category.storeKeyPath) } - /// Removes a FHIR resource from the ``FHIRStore``. + /// Inserts a ``Collection`` of FHIR resources into the ``FHIRStore``. /// - /// - Parameter resource: The `FHIRResource` identifier to be inserted. - public func remove(resource resourceId: FHIRResource.ID) async { - guard let resource = await storage[resourceId] else { - return - } + /// - Parameter resources: The `FHIRResource`s to be inserted. + public func insert(resources: sending T) async where T.Element == FHIRResource { + let resourceCategories = Set(resources.map { $0.category }) + + await MainActor.run { + for category in resourceCategories { + _$observationRegistrar.willSet(self, keyPath: category.storeKeyPath) + } - _$observationRegistrar.willSet(self, keyPath: resource.storeKeyPath) - await storage.remove(resource: resourceId) - _$observationRegistrar.didSet(self, keyPath: resource.storeKeyPath) + self._resources.append(contentsOf: resources) + + for category in resourceCategories { + _$observationRegistrar.didSet(self, keyPath: category.storeKeyPath) + } + } } /// Loads resources from a given FHIR `Bundle` into the ``FHIRStore``. @@ -141,24 +118,52 @@ public final class FHIRStore: Module, /// - Parameter bundle: The FHIR `Bundle` containing resources to be loaded. public func load(bundle: Bundle) async { let resourceProxies = bundle.entry?.compactMap { $0.resource } ?? [] + var resources: [FHIRResource] = [] for resourceProxy in resourceProxies { - await self.insert( - resource: FHIRResource( + if Task.isCancelled { + return + } + + resources.append( + FHIRResource( resource: resourceProxy.get(), displayName: resourceProxy.displayName ) ) } + + if Task.isCancelled { + return + } + + 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) async { + 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 ``FHIRStore``. - public func removeAllResources() async { + @MainActor + public func removeAllResources() { for category in FHIRResource.FHIRResourceCategory.allCases { _$observationRegistrar.willSet(self, keyPath: category.storeKeyPath) } - await storage.removeAll() + _resources = [] for category in FHIRResource.FHIRResourceCategory.allCases { _$observationRegistrar.didSet(self, keyPath: category.storeKeyPath) diff --git a/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift b/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift index 1d34b00..6adf87b 100644 --- a/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift +++ b/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift @@ -24,6 +24,7 @@ public struct FHIRBundleSelector: View { @Environment(FHIRStore.self) private var store @State private var selectedBundleId: PatientIdentifiedBundle.ID? + @State private var fhirResourceLoadingTask: Task? private let bundles: [PatientIdentifiedBundle] @@ -41,6 +42,7 @@ public struct FHIRBundleSelector: View { // Load the currently stored patient data to update `selectedBundleID` .task { let existingResources = store.otherResources + guard let patient = existingResources.compactMap({ resource -> Patient? in guard case let .r4(r4Resource) = resource.versionedResource, @@ -56,17 +58,22 @@ public struct FHIRBundleSelector: View { else { return } + selectedBundleId = matchedBundle.id } // Remove existing resources and load the newly selected bundle .onChange(of: selectedBundleId) { _, newValue in - Task { - guard let newValue, - let selected = bundles.first(where: { $0.id == newValue }) else { - return - } + fhirResourceLoadingTask?.cancel() + fhirResourceLoadingTask = nil + + guard let newValue, + let selected = bundles.first(where: { $0.id == newValue }) else { + return + } + + store.removeAllResources() - await store.removeAllResources() + fhirResourceLoadingTask = Task.detached { await store.load(bundle: selected.bundle) } } diff --git a/Tests/UITests/TestApp/ContentView.swift b/Tests/UITests/TestApp/ContentView.swift index 876eaa7..fa66889 100644 --- a/Tests/UITests/TestApp/ContentView.swift +++ b/Tests/UITests/TestApp/ContentView.swift @@ -6,6 +6,7 @@ // SPDX-License-Identifier: MIT // +import ModelsR4 import SpeziFHIR import SwiftUI @@ -36,6 +37,21 @@ struct ContentView: View { .sheet(isPresented: $presentPatientSelection) { MockPatientSelection(presentPatientSelection: $presentPatientSelection) } + .toolbar { + ToolbarItem(placement: .navigationBarTrailing) { + Button { + fhirStore.insert( + resource: .init( + resource: ModelsR4.Account(id: "SuperUniqueFHIRResourceIdentifier", status: .init()), + displayName: "Random Account FHIR Resource" + ) + ) + } label: { + Label("Add", systemImage: "doc.badge.plus") + .accessibilityLabel("Add FHIR Resource") + } + } + } } } From 00b33292e28ebf7ebeff93698dfe8ea01688b615 Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Fri, 3 Jan 2025 00:11:29 +0100 Subject: [PATCH 5/9] Add test cases --- Sources/SpeziFHIR/FHIRStore.swift | 2 +- Tests/UITests/TestApp/ContentView.swift | 17 ++++- .../FHIRMockDataStorageProviderTests.swift | 62 ++++++++++++++++++- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index 4ad9915..d303326 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -144,7 +144,7 @@ public final class FHIRStore: Module, /// /// - Parameter resource: The `FHIRResource` identifier to be inserted. @MainActor - public func remove(resource resourceId: FHIRResource.ID) async { + public func remove(resource resourceId: FHIRResource.ID) { guard let resource = _resources.first(where: { $0.id == resourceId }) else { return } diff --git a/Tests/UITests/TestApp/ContentView.swift b/Tests/UITests/TestApp/ContentView.swift index fa66889..2b45634 100644 --- a/Tests/UITests/TestApp/ContentView.swift +++ b/Tests/UITests/TestApp/ContentView.swift @@ -14,10 +14,12 @@ import SwiftUI struct ContentView: View { @Environment(FHIRStore.self) private var fhirStore @State private var presentPatientSelection = false - + + private let additionalFHIRResourceId = "SuperUniqueFHIRResourceIdentifier" + var body: some View { - NavigationStack { + NavigationStack { // swiftlint:disable:this closure_body_length List { Section { Text("Allergy Intolerances: \(fhirStore.allergyIntolerances.count)") @@ -42,7 +44,7 @@ struct ContentView: View { Button { fhirStore.insert( resource: .init( - resource: ModelsR4.Account(id: "SuperUniqueFHIRResourceIdentifier", status: .init()), + resource: ModelsR4.Account(id: .init(stringLiteral: additionalFHIRResourceId), status: .init()), displayName: "Random Account FHIR Resource" ) ) @@ -51,6 +53,15 @@ struct ContentView: View { .accessibilityLabel("Add FHIR Resource") } } + + ToolbarItem(placement: .automatic) { + Button { + fhirStore.remove(resource: additionalFHIRResourceId) + } label: { + Label("Remove", systemImage: "minus.circle.fill") + .accessibilityLabel("Remove FHIR Resource") + } + } } } } diff --git a/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift b/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift index 39370cb..2dea837 100644 --- a/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift +++ b/Tests/UITests/TestAppUITests/FHIRMockDataStorageProviderTests.swift @@ -10,7 +10,7 @@ import XCTest final class SpeziFHIRTests: XCTestCase { - func testSpeziFHIRMockPatients() throws { + func testMockPatientResources() throws { let app = XCUIApplication() app.launch() @@ -24,8 +24,9 @@ final class SpeziFHIRTests: XCTestCase { XCTAssert(app.staticTexts["Procedures: 0"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Other Resources: 0"].waitForExistence(timeout: 2)) + XCTAssert(app.buttons["Select Mock Patient"].waitForExistence(timeout: 2)) app.buttons["Select Mock Patient"].tap() - + XCTAssert(app.buttons["Jamison785 Denesik803"].waitForExistence(timeout: 20)) app.buttons["Jamison785 Denesik803"].tap() @@ -41,8 +42,9 @@ final class SpeziFHIRTests: XCTestCase { XCTAssert(app.staticTexts["Procedures: 106"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Other Resources: 302"].waitForExistence(timeout: 2)) + XCTAssert(app.buttons["Select Mock Patient"].waitForExistence(timeout: 2)) app.buttons["Select Mock Patient"].tap() - + XCTAssert(app.buttons["Maye976 Dickinson688"].waitForExistence(timeout: 20)) app.buttons["Maye976 Dickinson688"].tap() @@ -58,4 +60,58 @@ final class SpeziFHIRTests: XCTestCase { XCTAssert(app.staticTexts["Procedures: 225"].waitForExistence(timeout: 2)) XCTAssert(app.staticTexts["Other Resources: 322"].waitForExistence(timeout: 2)) } + + func testAddingAndRemovingResources() throws { + let app = XCUIApplication() + app.launch() + + // Add 5 resources + for resourceCount in 0..<5 { + XCTAssert(app.staticTexts["Other Resources: \(resourceCount)"].waitForExistence(timeout: 2)) + + XCTAssert(app.buttons["Add FHIR Resource"].waitForExistence(timeout: 2)) + app.buttons["Add FHIR Resource"].tap() + } + + // Remove added resources + XCTAssert(app.buttons["Remove FHIR Resource"].waitForExistence(timeout: 2)) + app.buttons["Remove FHIR Resource"].tap() + + XCTAssert(app.staticTexts["Other Resources: 0"].waitForExistence(timeout: 2)) + + // Try removing a second time + XCTAssert(app.buttons["Remove FHIR Resource"].waitForExistence(timeout: 2)) + app.buttons["Remove FHIR Resource"].tap() + + XCTAssert(app.staticTexts["Other Resources: 0"].waitForExistence(timeout: 2)) + + // Select mock patient + XCTAssert(app.buttons["Select Mock Patient"].waitForExistence(timeout: 2)) + app.buttons["Select Mock Patient"].tap() + + XCTAssert(app.buttons["Jamison785 Denesik803"].waitForExistence(timeout: 20)) + app.buttons["Jamison785 Denesik803"].tap() + + app.navigationBars["Select Mock Patient"].buttons["Dismiss"].tap() + + XCTAssert(app.staticTexts["Other Resources: 302"].waitForExistence(timeout: 2)) + + // Add resource to mock patient + XCTAssert(app.buttons["Add FHIR Resource"].waitForExistence(timeout: 2)) + app.buttons["Add FHIR Resource"].tap() + + XCTAssert(app.staticTexts["Other Resources: 303"].waitForExistence(timeout: 2)) + + // Remove resource from mock patient + XCTAssert(app.buttons["Remove FHIR Resource"].waitForExistence(timeout: 2)) + app.buttons["Remove FHIR Resource"].tap() + + XCTAssert(app.staticTexts["Other Resources: 302"].waitForExistence(timeout: 2)) + + // Try removing a second time + XCTAssert(app.buttons["Remove FHIR Resource"].waitForExistence(timeout: 2)) + app.buttons["Remove FHIR Resource"].tap() + + XCTAssert(app.staticTexts["Other Resources: 302"].waitForExistence(timeout: 2)) + } } From 5de3f427540c7e5fe1921b8bd253d3176b42596d Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Fri, 3 Jan 2025 00:13:50 +0100 Subject: [PATCH 6/9] Icon change --- Tests/UITests/TestApp/ContentView.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/UITests/TestApp/ContentView.swift b/Tests/UITests/TestApp/ContentView.swift index 2b45634..24e2ba8 100644 --- a/Tests/UITests/TestApp/ContentView.swift +++ b/Tests/UITests/TestApp/ContentView.swift @@ -54,11 +54,11 @@ struct ContentView: View { } } - ToolbarItem(placement: .automatic) { + ToolbarItem { Button { fhirStore.remove(resource: additionalFHIRResourceId) } label: { - Label("Remove", systemImage: "minus.circle.fill") + Label("Remove", systemImage: "folder.badge.minus") .accessibilityLabel("Remove FHIR Resource") } } From d9ef49271f1bf811b460008b1765bbd1713ad686 Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Tue, 7 Jan 2025 21:45:16 +0100 Subject: [PATCH 7/9] FHIRResource not sendable --- Sources/SpeziFHIR/FHIRResource/FHIRResource.swift | 10 +++++----- Sources/SpeziFHIR/FHIRStore.swift | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRResource/FHIRResource.swift b/Sources/SpeziFHIR/FHIRResource/FHIRResource.swift index e191fa9..ba5aa08 100644 --- a/Sources/SpeziFHIR/FHIRResource/FHIRResource.swift +++ b/Sources/SpeziFHIR/FHIRResource/FHIRResource.swift @@ -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) } diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index d303326..1e7d44b 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -116,7 +116,7 @@ public final class FHIRStore: Module, /// 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) async { + public func load(bundle: sending Bundle) async { let resourceProxies = bundle.entry?.compactMap { $0.resource } ?? [] var resources: [FHIRResource] = [] From 1759e7f4b9821ce31577eea111eddfc5184de60d Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Sun, 19 Jan 2025 19:23:04 +0100 Subject: [PATCH 8/9] Change Bulk insert isolation to main actor --- Sources/SpeziFHIR/FHIRStore.swift | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index 1e7d44b..f09c205 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -97,19 +97,18 @@ public final class FHIRStore: Module, /// Inserts a ``Collection`` of FHIR resources into the ``FHIRStore``. /// /// - Parameter resources: The `FHIRResource`s to be inserted. - public func insert(resources: sending T) async where T.Element == FHIRResource { - let resourceCategories = Set(resources.map { $0.category }) + @MainActor + public func insert(resources: T) where T.Element == FHIRResource { + let resourceCategories = Set(resources.map(\.category)) - await MainActor.run { - for category in resourceCategories { - _$observationRegistrar.willSet(self, keyPath: category.storeKeyPath) - } + for category in resourceCategories { + _$observationRegistrar.willSet(self, keyPath: category.storeKeyPath) + } - self._resources.append(contentsOf: resources) + self._resources.append(contentsOf: resources) - for category in resourceCategories { - _$observationRegistrar.didSet(self, keyPath: category.storeKeyPath) - } + for category in resourceCategories { + _$observationRegistrar.didSet(self, keyPath: category.storeKeyPath) } } From 19f5ccd61cbc3b353a86f84abc0fefa77ace859f Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Sun, 19 Jan 2025 20:34:16 +0100 Subject: [PATCH 9/9] Remove ModelsR4 preconcurrency imports --- .../SpeziFHIRMockPatients/FHIRBundleSelector.swift | 6 +++--- .../FHIRMockBundleSelector.swift | 9 +++++++-- .../FoundationBundle+LoadBundle.swift | 14 ++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift b/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift index 6adf87b..35ceb4e 100644 --- a/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift +++ b/Sources/SpeziFHIRMockPatients/FHIRBundleSelector.swift @@ -6,7 +6,7 @@ // SPDX-License-Identifier: MIT // -@preconcurrency import class ModelsR4.Bundle +import class ModelsR4.Bundle import class ModelsR4.Patient import SpeziFHIR import SwiftUI @@ -16,7 +16,7 @@ import SwiftUI /// /// The View assumes that the bundle contains a `ModelsR4.Patient` resource to identify the bundle and provide a human-readable name. public struct FHIRBundleSelector: View { - private struct PatientIdentifiedBundle: Identifiable, Sendable { + private struct PatientIdentifiedBundle: Identifiable { let id: String let bundle: ModelsR4.Bundle } @@ -73,7 +73,7 @@ public struct FHIRBundleSelector: View { store.removeAllResources() - fhirResourceLoadingTask = Task.detached { + fhirResourceLoadingTask = Task { await store.load(bundle: selected.bundle) } } diff --git a/Sources/SpeziFHIRMockPatients/FHIRMockBundleSelector.swift b/Sources/SpeziFHIRMockPatients/FHIRMockBundleSelector.swift index 5c2f7f4..bb2f57d 100644 --- a/Sources/SpeziFHIRMockPatients/FHIRMockBundleSelector.swift +++ b/Sources/SpeziFHIRMockPatients/FHIRMockBundleSelector.swift @@ -6,7 +6,7 @@ // SPDX-License-Identifier: MIT // -@preconcurrency import ModelsR4 +import ModelsR4 import SwiftUI @@ -31,7 +31,12 @@ public struct FHIRMockPatientSelection: View { } } .task { - self.bundles = await ModelsR4.Bundle.mockPatients + _Concurrency.Task.detached { // Workaround but enables us to not mark `import ModelsR4` as `@preconcurrency` + let bundles = await ModelsR4.Bundle.mockPatients + await MainActor.run { + self.bundles = bundles + } + } } } diff --git a/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift b/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift index 0df93ef..0516c81 100644 --- a/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift +++ b/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift @@ -7,7 +7,7 @@ // import Foundation -@preconcurrency import class ModelsR4.Bundle +import class ModelsR4.Bundle extension Foundation.Bundle { @@ -18,14 +18,12 @@ extension Foundation.Bundle { guard let resourceURL = Self.module.url(forResource: name, withExtension: "json") else { fatalError("Could not find the resource \"\(name)\".json in the SpeziFHIRMockPatients Resources folder.") } - - let loadingTask = Task { - let resourceData = try Data(contentsOf: resourceURL) - return try JSONDecoder().decode(Bundle.self, from: resourceData) - } - + do { - return try await loadingTask.value + return try JSONDecoder().decode( + Bundle.self, + from: Data(contentsOf: resourceURL) + ) } catch { fatalError("Could not decode the FHIR bundle named \"\(name).json\": \(error)") }