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

Fix Runtime Crash Due to Unlocked Access to Shared Properties #21

Merged
merged 2 commits into from
Jul 1, 2024
Merged
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
2 changes: 2 additions & 0 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ jobs:
uses: StanfordSpezi/.github/.github/workflows/create-and-upload-coverage-report.yml@v2
with:
coveragereports: SpeziFHIR-Package.xcresult TestApp.xcresult
secrets:
token: ${{ secrets.CODECOV_TOKEN }}
57 changes: 51 additions & 6 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,16 @@
// SPDX-License-Identifier: MIT
//

import class Foundation.ProcessInfo
import PackageDescription


#if swift(<6)
let strictConcurrency: SwiftSetting = .enableExperimentalFeature("StrictConcurrency")
#else
let strictConcurrency: SwiftSetting = .enableUpcomingFeature("StrictConcurrency")
#endif

let package = Package(
name: "SpeziFHIR",
defaultLocalization: "en",
Expand All @@ -32,7 +39,7 @@ let package = Package(
.package(url: "https://github.com/StanfordSpezi/SpeziStorage.git", from: "1.0.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziChat.git", .upToNextMinor(from: "0.2.0")),
.package(url: "https://github.com/StanfordSpezi/SpeziSpeech.git", from: "1.0.0")
],
] + swiftLintPackage(),
targets: [
.target(
name: "SpeziFHIR",
Expand All @@ -41,15 +48,23 @@ let package = Package(
.product(name: "ModelsR4", package: "FHIRModels"),
.product(name: "ModelsDSTU2", package: "FHIRModels"),
.product(name: "HealthKitOnFHIR", package: "HealthKitOnFHIR")
]
],
swiftSettings: [
strictConcurrency
],
plugins: [] + swiftLintPlugin()
),
.target(
name: "SpeziFHIRHealthKit",
dependencies: [
.target(name: "SpeziFHIR"),
.product(name: "HealthKitOnFHIR", package: "HealthKitOnFHIR"),
.product(name: "SpeziHealthKit", package: "SpeziHealthKit")
]
],
swiftSettings: [
strictConcurrency
],
plugins: [] + swiftLintPlugin()
),
.target(
name: "SpeziFHIRLLM",
Expand All @@ -65,7 +80,11 @@ let package = Package(
],
resources: [
.process("Resources")
]
],
swiftSettings: [
strictConcurrency
],
plugins: [] + swiftLintPlugin()
),
.target(
name: "SpeziFHIRMockPatients",
Expand All @@ -75,13 +94,39 @@ let package = Package(
],
resources: [
.process("Resources")
]
],
swiftSettings: [
strictConcurrency
],
plugins: [] + swiftLintPlugin()
),
.testTarget(
name: "SpeziFHIRTests",
dependencies: [
.target(name: "SpeziFHIR")
]
],
swiftSettings: [
strictConcurrency
],
plugins: [] + swiftLintPlugin()
)
]
)


func swiftLintPlugin() -> [Target.PluginUsage] {
// Fully quit Xcode and open again with `open --env SPEZI_DEVELOPMENT_SWIFTLINT /Applications/Xcode.app`
if ProcessInfo.processInfo.environment["SPEZI_DEVELOPMENT_SWIFTLINT"] != nil {
[.plugin(name: "SwiftLintBuildToolPlugin", package: "SwiftLint")]
} else {
[]
}
}

func swiftLintPackage() -> [PackageDescription.Package.Dependency] {
if ProcessInfo.processInfo.environment["SPEZI_DEVELOPMENT_SWIFTLINT"] != nil {
[.package(url: "https://github.com/realm/SwiftLint.git", .upToNextMinor(from: "0.55.1"))]
} else {
[]
}
}
94 changes: 59 additions & 35 deletions Sources/SpeziFHIR/FHIRStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import Combine
import Foundation
import Observation
import class ModelsR4.Bundle
import enum ModelsDSTU2.ResourceProxy
Expand All @@ -18,25 +19,32 @@ 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 class FHIRStore: Module, EnvironmentAccessible, DefaultInitializable {
private let lock = NSLock()
@ObservationIgnored private var _resources: [FHIRResource]


/// Allergy intolerances.
public var allergyIntolerances: [FHIRResource] {
access(keyPath: \.allergyIntolerances)
return _resources.filter { $0.category == .allergyIntolerance }
return lock.withLock {
_resources.filter { $0.category == .allergyIntolerance }
}
}

/// Conditions.
public var conditions: [FHIRResource] {
access(keyPath: \.conditions)
return _resources.filter { $0.category == .condition }
return lock.withLock {
_resources.filter { $0.category == .condition }
}
}

/// Diagnostics.
public var diagnostics: [FHIRResource] {
access(keyPath: \.diagnostics)
return _resources.filter { $0.category == .diagnostic }
return lock.withLock {
_resources.filter { $0.category == .diagnostic }
}
}

/// Encounters.
Expand All @@ -48,31 +56,41 @@ public class FHIRStore: Module, EnvironmentAccessible, DefaultInitializable {
/// Immunizations.
public var immunizations: [FHIRResource] {
access(keyPath: \.immunizations)
return _resources.filter { $0.category == .immunization }
return lock.withLock {
_resources.filter { $0.category == .immunization }
}
}

/// Medications.
public var medications: [FHIRResource] {
access(keyPath: \.medications)
return _resources.filter { $0.category == .medication }
return lock.withLock {
_resources.filter { $0.category == .medication }
}
}

/// Observations.
public var observations: [FHIRResource] {
access(keyPath: \.observations)
return _resources.filter { $0.category == .observation }
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 _resources.filter { $0.category == .other }
return lock.withLock {
_resources.filter { $0.category == .other }
}
}

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


Expand All @@ -86,20 +104,24 @@ public class FHIRStore: Module, EnvironmentAccessible, DefaultInitializable {
/// - Parameter resource: The `FHIRResource` to be inserted.
public func insert(resource: FHIRResource) {
withMutation(keyPath: resource.storeKeyPath) {
_resources.append(resource)
lock.withLock {
_resources.append(resource)
}
}
}

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

withMutation(keyPath: resource.storeKeyPath) {
_resources.removeAll(where: { $0.id == resourceId })
lock.withLock {
guard let resource = _resources.first(where: { $0.id == resourceId }) else {
return
}

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

Expand All @@ -116,25 +138,27 @@ public class FHIRStore: Module, EnvironmentAccessible, DefaultInitializable {

/// Removes all resources from the store.
public func removeAllResources() {
// 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)
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)
}
}
}
2 changes: 1 addition & 1 deletion Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// SPDX-License-Identifier: MIT
//

import HealthKit
@preconcurrency import HealthKit
import HealthKitOnFHIR
import ModelsDSTU2
import ModelsR4
Expand Down
2 changes: 1 addition & 1 deletion Sources/SpeziFHIRLLM/Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@
}
},
"Interpretation Prompt" : {
"comment" : "Title of the multiple resources interpretation prompt.\nTitle of the interpretation prompt.",
"comment" : "Title of the interpretation prompt.\nTitle of the multiple resources interpretation prompt.",
"localizations" : {
"de" : {
"stringUnit" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//

import Foundation
import class ModelsR4.Bundle
@preconcurrency import class ModelsR4.Bundle


extension Foundation.Bundle {
Expand Down
Loading
Loading