Skip to content

Commit

Permalink
Introduce error hierarchy
Browse files Browse the repository at this point in the history
  • Loading branch information
Supereg committed Oct 28, 2024
1 parent 3752b6d commit b536a87
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 94 deletions.
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ let package = Package(
.library(name: "XCTSpezi", targets: ["XCTSpezi"])
],
dependencies: [
.package(url: "https://github.com/StanfordSpezi/SpeziFoundation.git", from: "2.0.0-beta.1"),
.package(url: "https://github.com/StanfordSpezi/SpeziFoundation.git", from: "2.0.0"),
.package(url: "https://github.com/StanfordBDHG/XCTRuntimeAssertions.git", from: "1.1.1"),
.package(url: "https://github.com/apple/swift-collections.git", from: "1.1.1")
] + swiftLintPackage(),
Expand Down
122 changes: 87 additions & 35 deletions Sources/Spezi/Spezi/Spezi.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ import XCTRuntimeAssertions
public final class Spezi: Sendable {
static let logger = Logger(subsystem: "edu.stanford.spezi", category: "Spezi")

@TaskLocal static var moduleInitContext: ModuleDescription?

let standard: any Standard

/// A shared repository to store any `KnowledgeSource`s restricted to the ``SpeziAnchor``.
Expand Down Expand Up @@ -188,9 +186,13 @@ public final class Spezi: Sendable {
self.standard = standard
self.storage = consume storage

self.loadModules(modules, ownership: .spezi)
// load standard separately, such that all module loading takes precedence
self.loadModule(standard, ownership: .spezi)
do {
try self.loadModules(modules, ownership: .spezi)
// load standard separately, such that all module loading takes precedence
try self.loadModules([standard], ownership: .spezi)
} catch {
preconditionFailure(error.description)
}
}

/// Load a new Module.
Expand All @@ -211,11 +213,15 @@ public final class Spezi: Sendable {
/// - ``ModuleOwnership``
@MainActor
public func loadModule(_ module: any Module, ownership: ModuleOwnership = .spezi) {
loadModules([module], ownership: ownership)
do {
try loadModules([module], ownership: ownership)
} catch {
preconditionFailure(error.description)
}
}

@MainActor
private func loadModules(_ modules: [any Module], ownership: ModuleOwnership) {
func loadModules(_ modules: [any Module], ownership: ModuleOwnership) throws(SpeziModuleError) {
precondition(Self.moduleInitContext == nil, "Modules cannot be loaded within the `configure()` method.")

purgeWeaklyReferenced()
Expand All @@ -230,24 +236,24 @@ public final class Spezi: Sendable {
do {
try dependencyManager.resolve()
} catch {
preconditionFailure(error.description)
throw .dependency(error)
}

implicitlyCreatedModules.formUnion(dependencyManager.implicitlyCreatedModules)

// we pass through the whole list of modules once to collect all @Provide values
for module in dependencyManager.initializedModules {
Self.$moduleInitContext.withValue(module.moduleDescription) {
withModuleInitContext(module.moduleDescription) {
module.collectModuleValues(into: &storage)
}
}

for module in dependencyManager.initializedModules {
if requestedModules.contains(ModuleReference(module)) {
// the policy only applies to the requested modules, all other are always managed and owned by Spezi
self.initModule(module, ownership: ownership)
try self.initModule(module, ownership: ownership)
} else {
self.initModule(module, ownership: .spezi)
try self.initModule(module, ownership: .spezi)
}
}

Expand All @@ -269,6 +275,15 @@ public final class Spezi: Sendable {
/// - Parameter module: The Module to unload.
@MainActor
public func unloadModule(_ module: any Module) {
do {
try _unloadModule(module)
} catch {
preconditionFailure(error.description)
}
}

@MainActor
func _unloadModule(_ module: any Module) throws(SpeziModuleError) { // swiftlint:disable:this identifier_name
precondition(Self.moduleInitContext == nil, "Modules cannot be unloaded within the `configure()` method.")

purgeWeaklyReferenced()
Expand All @@ -280,7 +295,9 @@ public final class Spezi: Sendable {
logger.debug("Unloading module \(type(of: module)) ...")

let dependents = retrieveDependingModules(module.dependencyReference, considerOptionals: false)
precondition(dependents.isEmpty, "Tried to unload Module \(type(of: module)) that is still required by peer Modules: \(dependents)")
if !dependents.isEmpty {
throw SpeziModuleError.moduleStillRequired(module: "\(type(of: module))", dependents: dependents.map { "\(type(of: $0))" })
}

module.clearModule(from: self)

Expand Down Expand Up @@ -317,38 +334,42 @@ public final class Spezi: Sendable {
/// - module: The module to initialize.
/// - ownership: Define the type of ownership when loading the module.
@MainActor
private func initModule(_ module: any Module, ownership: ModuleOwnership) {
private func initModule(_ module: any Module, ownership: ModuleOwnership) throws(SpeziModuleError) {
precondition(!module.isLoaded(in: self), "Tried to initialize Module \(type(of: module)) that was already loaded!")

Self.$moduleInitContext.withValue(module.moduleDescription) {
module.inject(spezi: self)
do {
try withModuleInitContext(module.moduleDescription) { () throws(SpeziPropertyError) in
try module.inject(spezi: self)

// supply modules values to all @Collect
module.injectModuleValues(from: storage)
// supply modules values to all @Collect
module.injectModuleValues(from: storage)

module.configure()
module.configure()

switch ownership {
case .spezi:
module.storeModule(into: self)
case .external:
module.storeWeakly(into: self)
}
switch ownership {
case .spezi:
module.storeModule(into: self)
case .external:
module.storeWeakly(into: self)
}

// If a module is @Observable, we automatically inject it view the `ModelModifier` into the environment.
if let observable = module as? EnvironmentAccessible {
// we can't guarantee weak references for EnvironmentAccessible modules
precondition(ownership != .external, "Modules loaded with self-managed policy cannot conform to `EnvironmentAccessible`.")
_viewModifiers[ModuleReference(module)] = observable.viewModifier
}
// If a module is @Observable, we automatically inject it view the `ModelModifier` into the environment.
if let observable = module as? EnvironmentAccessible {
// we can't guarantee weak references for EnvironmentAccessible modules
precondition(ownership != .external, "Modules loaded with self-managed policy cannot conform to `EnvironmentAccessible`.")
_viewModifiers[ModuleReference(module)] = observable.viewModifier
}

let modifierEntires: [(id: UUID, modifier: any ViewModifier)] = module.viewModifierEntires
// this check is important. Change to viewModifiers re-renders the whole SwiftUI view hierarchy. So avoid to do it unnecessarily
if !modifierEntires.isEmpty {
for entry in modifierEntires.reversed() { // reversed, as we re-reverse things in the `viewModifier` getter
_viewModifiers.updateValue(entry.modifier, forKey: entry.id)
let modifierEntires: [(id: UUID, modifier: any ViewModifier)] = module.viewModifierEntires
// this check is important. Change to viewModifiers re-renders the whole SwiftUI view hierarchy. So avoid to do it unnecessarily
if !modifierEntires.isEmpty {
for entry in modifierEntires.reversed() { // reversed, as we re-reverse things in the `viewModifier` getter
_viewModifiers.updateValue(entry.modifier, forKey: entry.id)
}
}
}
} catch {
throw .property(error)
}
}

Expand Down Expand Up @@ -483,3 +504,34 @@ extension Module {
}
}
}


extension Spezi {
private static let initContextLock = NSLock()
private static nonisolated(unsafe) var _moduleInitContext: ModuleDescription?

private(set) static var moduleInitContext: ModuleDescription? {
get {
initContextLock.withLock {
_moduleInitContext
}
}
set {
initContextLock.withLock {
_moduleInitContext = newValue
}
}
}

@MainActor
func withModuleInitContext<E: Error>(_ context: ModuleDescription, perform action: () throws(E) -> Void) throws(E) {
Self.moduleInitContext = context
defer {
Self.moduleInitContext = nil
}

try action()
}
}

// swiftlint:disable:this file_length
26 changes: 26 additions & 0 deletions Sources/Spezi/Spezi/SpeziModuleError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//
// This source file is part of the Stanford Spezi open-source project
//
// SPDX-FileCopyrightText: 2022 Stanford University and the project authors (see CONTRIBUTORS.md)
//
// SPDX-License-Identifier: MIT
//


enum SpeziModuleError: Error, CustomStringConvertible {
case dependency(DependencyManagerError)
case property(SpeziPropertyError)

case moduleStillRequired(module: String, dependents: [String])

var description: String {
switch self {
case let .dependency(error):
error.description
case let .property(error):
error.description
case let .moduleStillRequired(module, dependents):
"Tried to unload Module \(type(of: module)) that is still required by peer Module(s): \(dependents.joined(separator: ", "))"
}
}
}
40 changes: 37 additions & 3 deletions Sources/Spezi/Spezi/SpeziPropertyWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,19 @@
//


enum SpeziPropertyError: Error {
case unsatisfiedStandardConstraint(constraint: String, standard: String)
}


protocol SpeziPropertyWrapper {
/// Inject the global Spezi instance.
///
/// This call happens right before ``Module/configure()-5pa83`` is called.
/// An empty default implementation is provided.
/// - Parameter spezi: The global ``Spezi/Spezi`` instance.
@MainActor
func inject(spezi: Spezi)
func inject(spezi: Spezi) throws(SpeziPropertyError)

/// Clear the property wrapper state before the Module is unloaded.
@MainActor
Expand All @@ -29,9 +34,9 @@ extension SpeziPropertyWrapper {

extension Module {
@MainActor
func inject(spezi: Spezi) {
func inject(spezi: Spezi) throws(SpeziPropertyError) {
for wrapper in retrieveProperties(ofType: SpeziPropertyWrapper.self) {
wrapper.inject(spezi: spezi)
try wrapper.inject(spezi: spezi)
}
}

Expand All @@ -42,3 +47,32 @@ extension Module {
}
}
}


extension SpeziPropertyError: CustomStringConvertible {
var description: String {
switch self {
case let .unsatisfiedStandardConstraint(constraint, standard):
"""
The `Standard` defined in the `Configuration` does not conform to \(constraint).
Ensure that you define an appropriate standard in your configuration in your `SpeziAppDelegate` subclass ...
```
var configuration: Configuration {
Configuration(standard: \(standard)()) {
// ...
}
}
```
... and that your standard conforms to \(constraint):
```swift
actor \(standard): Standard, \(constraint) {
// ...
}
```
"""
}
}
}
27 changes: 4 additions & 23 deletions Sources/Spezi/Standard/StandardPropertyWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,12 @@ public class _StandardPropertyWrapper<Constraint> {


extension _StandardPropertyWrapper: SpeziPropertyWrapper {
func inject(spezi: Spezi) {
func inject(spezi: Spezi) throws(SpeziPropertyError) {
guard let standard = spezi.standard as? Constraint else {
let standardType = type(of: spezi.standard)
// TODO: allow this to get throwing!
preconditionFailure(
"""
The `Standard` defined in the `Configuration` does not conform to \(String(describing: Constraint.self)).
Ensure that you define an appropriate standard in your configuration in your `SpeziAppDelegate` subclass ...
```
var configuration: Configuration {
Configuration(standard: \(String(describing: standardType))()) {
// ...
}
}
```
... and that your standard conforms to \(String(describing: Constraint.self)):
```swift
actor \(String(describing: standardType)): Standard, \(String(describing: Constraint.self)) {
// ...
}
```
"""
throw SpeziPropertyError.unsatisfiedStandardConstraint(
constraint: String(describing: Constraint.self),
standard: String(describing: standardType)
)
}

Expand Down
16 changes: 0 additions & 16 deletions Tests/SpeziTests/CapabilityTests/ModuleCommunicationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,4 @@ final class ModuleCommunicationTests: XCTestCase {
XCTAssertTrue(Self.collectModule.nothingProvided.isEmpty)
XCTAssertEqual(Self.collectModule.strings, ["Hello World"])
}

@MainActor
func testIllegalAccess() throws {
let delegate = TestApplicationDelegate()

throw XCTSkip("Skipped for now!") // TODO: what the fuck?
try XCTRuntimePrecondition {
_ = Self.collectModule.strings
}

_ = delegate.spezi // ensure init

try XCTRuntimePrecondition {
Self.provideModule.numMaybe2 = 12
}
}
}
14 changes: 10 additions & 4 deletions Tests/SpeziTests/DependenciesTests/DependencyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,16 @@ final class DependencyTests: XCTestCase { // swiftlint:disable:this type_body_le
let module3 = TestModule3()
let spezi = Spezi(standard: DefaultStandard(), modules: [TestModule1(), module3])

throw XCTSkip("Skipped for now!") // TODO: what the fuck?
try XCTRuntimePrecondition {
// cannot unload module that other modules still depend on
spezi.unloadModule(module3)
// cannot unload module that other modules still depend on
XCTAssertThrowsError(try spezi._unloadModule(module3)) { error in
guard let moduleError = error as? SpeziModuleError,
case let .moduleStillRequired(module, dependents) = moduleError else {
XCTFail("Received unexpected error: \(error)")
return
}

XCTAssertEqual(module, "TestModule3")
XCTAssertEqual(Set(dependents), ["TestModule2", "TestModule1"])
}
}

Expand Down
Loading

0 comments on commit b536a87

Please sign in to comment.