From 0799bbd317322449bb6d406f018069119257fe2d Mon Sep 17 00:00:00 2001 From: Paul Schmiedmayer Date: Mon, 1 Jul 2024 11:50:06 -0700 Subject: [PATCH 1/2] Fix Runtime Crash Due to Unlocked Access to Shared Properties --- Sources/SpeziFHIR/FHIRStore.swift | 94 ++++++++++++------- .../Resources/Localizable.xcstrings | 2 +- .../xcshareddata/swiftpm/Package.resolved | 63 ++++++------- 3 files changed, 87 insertions(+), 72 deletions(-) diff --git a/Sources/SpeziFHIR/FHIRStore.swift b/Sources/SpeziFHIR/FHIRStore.swift index 3e2126c..4f0ad9f 100644 --- a/Sources/SpeziFHIR/FHIRStore.swift +++ b/Sources/SpeziFHIR/FHIRStore.swift @@ -7,6 +7,7 @@ // import Combine +import Foundation import Observation import class ModelsR4.Bundle import enum ModelsDSTU2.ResourceProxy @@ -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. @@ -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 } + } } @@ -86,7 +104,9 @@ 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) + } } } @@ -94,12 +114,14 @@ public class FHIRStore: Module, EnvironmentAccessible, DefaultInitializable { /// /// - 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 }) + } } } @@ -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) + } } } diff --git a/Sources/SpeziFHIRLLM/Resources/Localizable.xcstrings b/Sources/SpeziFHIRLLM/Resources/Localizable.xcstrings index 1d81bbf..8dd7b7d 100644 --- a/Sources/SpeziFHIRLLM/Resources/Localizable.xcstrings +++ b/Sources/SpeziFHIRLLM/Resources/Localizable.xcstrings @@ -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" : { diff --git a/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6f5caf0..811c740 100644 --- a/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -14,8 +14,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordBDHG/HealthKitOnFHIR", "state" : { - "revision" : "825e96007d83ed83f81ee49eb3ebab29d7b7ba2f", - "version" : "0.2.5" + "revision" : "418929f315f37e6d9c8f30f40030bc65b9cc47c9", + "version" : "0.2.8" } }, { @@ -23,26 +23,17 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordBDHG/llama.cpp", "state" : { - "revision" : "b0611c7d3cb049822f9911878514e4706b80e2ac", - "version" : "0.1.8" + "revision" : "6839853a321778906e210a33ee2c6aec52f34c97", + "version" : "0.3.3" } }, { "identity" : "openai", "kind" : "remoteSourceControl", - "location" : "https://github.com/MacPaw/OpenAI", + "location" : "https://github.com/StanfordBDHG/OpenAI", "state" : { - "revision" : "35afc9a6ee127b8f22a85a31aec2036a987478af", - "version" : "0.2.6" - } - }, - { - "identity" : "semaphore", - "kind" : "remoteSourceControl", - "location" : "https://github.com/groue/Semaphore.git", - "state" : { - "revision" : "f1c4a0acabeb591068dea6cffdd39660b86dec28", - "version" : "0.0.8" + "revision" : "1ad95dd531d7c854a3f98f588b0eb68fa83e8a8c", + "version" : "0.2.9" } }, { @@ -50,8 +41,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/Spezi", "state" : { - "revision" : "0ced3efbc2af9513c07ac913ad762c773a00a6c8", - "version" : "1.2.1" + "revision" : "5ada3f3a18c86a658d140c6f4c45ca2e9d5e61ce", + "version" : "1.4.0" } }, { @@ -59,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziChat.git", "state" : { - "revision" : "eae5c15b211f18e09aa98de63ce119629320afeb", - "version" : "0.1.8" + "revision" : "aaa10d71431b78ece8bf29f95c0050632714984d", + "version" : "0.2.0" } }, { @@ -68,8 +59,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziFoundation", "state" : { - "revision" : "0346857e2f1d6fd4b1d950d271be6c82df97107f", - "version" : "1.0.2" + "revision" : "3326feab3dac120c16af63243615592990d33516", + "version" : "1.1.2" } }, { @@ -77,8 +68,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziHealthKit.git", "state" : { - "revision" : "b40695ffa4d1c9d58c5a0ee277640c2343fb5516", - "version" : "0.5.1" + "revision" : "1e9cb5a6036ac7f4ff37ea1c3ed4898103339ad1", + "version" : "0.5.3" } }, { @@ -86,8 +77,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziLLM.git", "state" : { - "revision" : "6892c5dfe258371b6f3287f02b8fec57a611ba70", - "version" : "0.7.0" + "revision" : "3663884f23e55c67c875a97c5da08ed172ea02ac", + "version" : "0.8.3" } }, { @@ -95,8 +86,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziOnboarding", "state" : { - "revision" : "8fb6d9f1a080661c0cc564a93b82ead3c8d44d4f", - "version" : "1.0.2" + "revision" : "8d6dda3501720a1952573439b21a503cbecd9e0f", + "version" : "1.2.0" } }, { @@ -104,8 +95,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziSpeech", "state" : { - "revision" : "a1e1d021d8f605b5e6b23aee773115d7125a57e3", - "version" : "1.0.0" + "revision" : "60b8cdbf6f3d58b0d75eadf30db50f88848069aa", + "version" : "1.0.1" } }, { @@ -113,8 +104,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziStorage", "state" : { - "revision" : "eaed2220375c35400aa69d1f96a8d32b7e66b1c7", - "version" : "1.0.0" + "revision" : "b958df9b31f24800388a7bfc28f457ce7b82556c", + "version" : "1.0.2" } }, { @@ -122,8 +113,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziViews", "state" : { - "revision" : "7210f72d6821d2eeb93438b29cb854a8ce334164", - "version" : "1.2.0" + "revision" : "ff61e6594677572df051b96905cc2a7a12cffd10", + "version" : "1.5.0" } }, { @@ -131,8 +122,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-collections.git", "state" : { - "revision" : "94cf62b3ba8d4bed62680a282d4c25f9c63c2efb", - "version" : "1.1.0" + "revision" : "ee97538f5b81ae89698fd95938896dec5217b148", + "version" : "1.1.1" } }, { From b5cc630b99539772615276c7505100c22d8c0bd3 Mon Sep 17 00:00:00 2001 From: Paul Schmiedmayer Date: Mon, 1 Jul 2024 12:01:20 -0700 Subject: [PATCH 2/2] Update Setup --- .github/workflows/build-and-test.yml | 2 + Package.swift | 57 +++++++++++++++++-- .../FHIRStore+HealthKit.swift | 2 +- .../FoundationBundle+LoadBundle.swift | 2 +- 4 files changed, 55 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 8fac4a6..8750e4b 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -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 }} diff --git a/Package.swift b/Package.swift index b9b0b18..a35f654 100644 --- a/Package.swift +++ b/Package.swift @@ -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", @@ -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", @@ -41,7 +48,11 @@ 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", @@ -49,7 +60,11 @@ let package = Package( .target(name: "SpeziFHIR"), .product(name: "HealthKitOnFHIR", package: "HealthKitOnFHIR"), .product(name: "SpeziHealthKit", package: "SpeziHealthKit") - ] + ], + swiftSettings: [ + strictConcurrency + ], + plugins: [] + swiftLintPlugin() ), .target( name: "SpeziFHIRLLM", @@ -65,7 +80,11 @@ let package = Package( ], resources: [ .process("Resources") - ] + ], + swiftSettings: [ + strictConcurrency + ], + plugins: [] + swiftLintPlugin() ), .target( name: "SpeziFHIRMockPatients", @@ -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 { + [] + } +} diff --git a/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift b/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift index bbd5860..2a18df2 100644 --- a/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift +++ b/Sources/SpeziFHIRHealthKit/FHIRStore+HealthKit.swift @@ -6,7 +6,7 @@ // SPDX-License-Identifier: MIT // -import HealthKit +@preconcurrency import HealthKit import HealthKitOnFHIR import ModelsDSTU2 import ModelsR4 diff --git a/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift b/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift index 8255d8b..0df93ef 100644 --- a/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift +++ b/Sources/SpeziFHIRMockPatients/FoundationBundle+LoadBundle.swift @@ -7,7 +7,7 @@ // import Foundation -import class ModelsR4.Bundle +@preconcurrency import class ModelsR4.Bundle extension Foundation.Bundle {