Fix registerRemoteNotifications action would never return (#115)

# Fix registerRemoteNotifications action would never return

## ♻️ Current situation & Problem
#98 introduced the `registerRemoteNotifications` action. However, the
knowledge source that stores the continuation was never persisted in the
store. Therefore, the continuation was never resumed and allocated
resources forever.
This was fixed with this PR. We originally discussed supporting
cancellation of the registration processed, however I decided against as
it wasn't clear to properly handle cancellation (e.g., automatically
call the unregister action? How to handle the delegate call that might
arrive later. Especially when another call to the action might incur in
the mean time).
This PR changes the behavior when concurrently calling the
registerRemoteNotifications action. Now, the second caller will wait for
the first call to complete instead of throwing a concurrence access
error. This helps to share this resources with, e.g., multiple modules
trying to retrieve an device token.

**To emphasize: `registerRemoteNotifications` works on simulator
devices!**

## ⚙️ Release Notes 
* Fixed an issue where the `registerRemoteNotifications` actions would
never return.


## 📚 Documentation
Slightly updated docs to provide more guidance around requesting
authorization for notifications.


## ✅ Testing
Added a unit test that verifies that the action returns.


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).

---------

Co-authored-by: Paul Schmiedmayer <[email protected]>

Sources/Spezi/Capabilities/Notifications/RegisterRemoteNotificationsAction.swift

@@ -10,15 +10,25 @@ import SpeziFoundation
 import SwiftUI
 
 
-private final class RemoteNotificationContinuation: DefaultProvidingKnowledgeSource, Sendable {
+@MainActor
+private final class RemoteNotificationContinuation: KnowledgeSource, Sendable {
     typealias Anchor = SpeziAnchor
 
-    static let defaultValue = RemoteNotificationContinuation()
+    fileprivate(set) var continuation: CheckedContinuation<Data, Error>?
+    fileprivate(set) var access = AsyncSemaphore()
 
-    @MainActor
-    var continuation: CheckedContinuation<Data, Error>?
 
     init() {}
+
+
+    @MainActor
+    func resume(with result: Result<Data, Error>) {
+        if let continuation {
+            self.continuation = nil
+            access.signal()
+            continuation.resume(with: result)
+        }
+    }
 }
 
 
@@ -33,24 +43,40 @@ private final class RemoteNotificationContinuation: DefaultProvidingKnowledgeSou
 ///
 /// Below is a short code example on how to use this action within your ``Module``.
 ///
+/// - Warning: Registering for Remote Notifications on Simulator devices might not be possible if your are not signed into an Apple ID on the host machine.
+///     The method might throw a [`TimeoutError`](https://swiftpackageindex.com/stanfordspezi/spezifoundation/documentation/spezifoundation/timeouterror)
+///     in such a case.
+///
 /// ```swift
+/// import SpeziFoundation
+///
 /// class ExampleModule: Module {
 ///     @Application(\.registerRemoteNotifications)
 ///     var registerRemoteNotifications
 ///
-///     func handleNotificationsAllowed() async throws {
-///         let deviceToken = try await registerRemoteNotifications()
+///     func handleNotificationsPermissions() async throws {
+///         // Make sure to request notifications permissions before registering for remote notifications
+///         try await UNUserNotificationCenter.current().requestAuthorization(options: [.alert, .badge, .sound])
+///
+///
+///         do {
+///             let deviceToken = try await registerRemoteNotifications()
+///         } catch let error as TimeoutError {
+/// #if targetEnvironment(simulator)
+///             return // override logic when running within a simulator
+/// #else
+///             throw error
+/// #endif
+///         }
+///
 ///         // .. send the device token to your remote server that generates push notifications
 ///     }
 /// }
 /// ```
-public struct RegisterRemoteNotificationsAction {
-    /// Errors occurring when registering for remote notifications.
-    public enum ActionError: Error {
-        /// The action was called while we were still waiting to receive a response from the previous one.
-        case concurrentAccess
-    }
-
+///
+/// > Tip: Make sure to request authorization by calling [`requestAuthorization(options:completionHandler:)`](https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/requestauthorization(options:completionhandler:))
+///     to have your remote notifications be able to display alerts, badges or use sound. Otherwise, all remote notifications will be delivered silently.
+public struct RegisterRemoteNotificationsAction: Sendable {
     private weak var spezi: Spezi?
 
     init(_ spezi: Spezi) {
@@ -65,28 +91,41 @@ public struct RegisterRemoteNotificationsAction {
     ///     For more information refer to the documentation of
     ///     [`application(_:didRegisterForRemoteNotificationsWithDeviceToken:)`](https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622958-application).
     /// - Throws: Registration might fail if the user's device isn't connected to the network or
-    ///     if your app is not properly configured for remote notifications. It might also throw in the
-    ///     rare circumstance where you make a call to this method while another one is still ongoing.
-    ///     Try again to register at a later point in time.
-    @MainActor
+    ///     if your app is not properly configured for remote notifications. It might also throw a `TimeoutError` when running on a simulator device running on a host
+    ///     that is not connected to an Apple ID.
     @discardableResult
+    @MainActor
     public func callAsFunction() async throws -> Data {
         guard let spezi else {
             preconditionFailure("RegisterRemoteNotificationsAction was used in a scope where Spezi was not available anymore!")
         }
 
+
 #if os(watchOS)
         let application = _Application.shared()
 #else
         let application = _Application.shared
-#endif
+#endif // os(watchOS)
+
+        let registration: RemoteNotificationContinuation
+        if let existing = spezi.storage[RemoteNotificationContinuation.self] {
+            registration = existing
+        } else {
+            registration = RemoteNotificationContinuation()
+            spezi.storage[RemoteNotificationContinuation.self] = registration
+        }
+
+        try await registration.access.waitCheckingCancellation()
 
-        let registration = spezi.storage[RemoteNotificationContinuation.self]
-        if registration.continuation != nil {
-            throw ActionError.concurrentAccess
+#if targetEnvironment(simulator)
+        async let _ = withTimeout(of: .seconds(5)) { @MainActor in
+            spezi.logger.warning("Registering for remote notifications seems to be not possible on this simulator device. Timing out ...")
+            spezi.storage[RemoteNotificationContinuation.self]?.resume(with: .failure(TimeoutError()))
         }
+#endif
 
         return try await withCheckedThrowingContinuation { continuation in
+            assert(registration.continuation == nil, "continuation wasn't nil")
             registration.continuation = continuation
             application.registerForRemoteNotifications()
         }
@@ -100,20 +139,46 @@ extension Spezi {
     /// For more information refer to the [`registerForRemoteNotifications()`](https://developer.apple.com/documentation/uikit/uiapplication/1623078-registerforremotenotifications)
     /// documentation for `UIApplication` or for the respective equivalent for your current platform.
     ///
+    /// - Note: For more information on the general topic on how to register your app with APNs,
+    ///     refer to the [Registering your app with APNs](https://developer.apple.com/documentation/usernotifications/registering-your-app-with-apns)
+    ///     article.
+    ///
     /// Below is a short code example on how to use this action within your ``Module``.
     ///
+    /// - Warning: Registering for Remote Notifications on Simulator devices might not be possible if your are not signed into an Apple ID on the host machine.
+    ///     The method might throw a [`TimeoutError`](https://swiftpackageindex.com/stanfordspezi/spezifoundation/documentation/spezifoundation/timeouterror)
+    ///     in such a case.
+    ///
     /// ```swift
+    /// import SpeziFoundation
+    ///
     /// class ExampleModule: Module {
     ///     @Application(\.registerRemoteNotifications)
     ///     var registerRemoteNotifications
     ///
-    ///     func handleNotificationsAllowed() async throws {
-    ///         let deviceToken = try await registerRemoteNotifications()
+    ///     func handleNotificationsPermissions() async throws {
+    ///         // Make sure to request notifications permissions before registering for remote notifications
+    ///         try await UNUserNotificationCenter.current().requestAuthorization(options: [.alert, .badge, .sound])
+    ///
+    ///
+    ///         do {
+    ///             let deviceToken = try await registerRemoteNotifications()
+    ///         } catch let error as TimeoutError {
+    /// #if targetEnvironment(simulator)
+    ///             return // override logic when running within a simulator
+    /// #else
+    ///             throw error
+    /// #endif
+    ///         }
+    ///
     ///         // .. send the device token to your remote server that generates push notifications
     ///     }
     /// }
     /// ```
     ///
+    /// > Tip: Make sure to request authorization by calling [`requestAuthorization(options:completionHandler:)`](https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/requestauthorization(options:completionhandler:))
+    ///     to have your remote notifications be able to display alerts, badges or use sound. Otherwise, all remote notifications will be delivered silently.
+    ///
     /// ## Topics
     /// ### Action
     /// - ``RegisterRemoteNotificationsAction``
@@ -126,24 +191,26 @@ extension Spezi {
 extension RegisterRemoteNotificationsAction {
     @MainActor
     static func handleDeviceTokenUpdate(_ spezi: Spezi, _ deviceToken: Data) {
-        let registration = spezi.storage[RemoteNotificationContinuation.self]
-        guard let continuation = registration.continuation else {
-            // might also be called if, e.g., app is restored from backup and is automatically registered for remote notifications.
-            // This can be handled through the `NotificationHandler` protocol.
+        guard let registration = spezi.storage[RemoteNotificationContinuation.self] else {
             return
         }
-        registration.continuation = nil
-        continuation.resume(returning: deviceToken)
+
+        // might also be called if, e.g., app is restored from backup and is automatically registered for remote notifications.
+        // This can be handled through the `NotificationHandler` protocol.
+
+        registration.resume(with: .success(deviceToken))
     }
 
     @MainActor
     static func handleFailedRegistration(_ spezi: Spezi, _ error: Error) {
-        let registration = spezi.storage[RemoteNotificationContinuation.self]
-        guard let continuation = registration.continuation else {
-            spezi.logger.warning("Received a call to \(#function) while we were not waiting for a notifications registration request.")
+        guard let registration = spezi.storage[RemoteNotificationContinuation.self] else {
             return
         }
-        registration.continuation = nil
-        continuation.resume(throwing: error)
+
+        if registration.continuation == nil {
+            spezi.logger.warning("Received a call to \(#function) while we were not waiting for a notifications registration request.")
+        }
+
+        registration.resume(with: .failure(error))
     }
 }

Sources/Spezi/Capabilities/Notifications/UnregisterRemoteNotificationsAction.swift

@@ -27,7 +27,7 @@ import SwiftUI
 ///     }
 /// }
 /// ```
-public struct UnregisterRemoteNotificationsAction {
+public struct UnregisterRemoteNotificationsAction: Sendable {
     init() {}
 
 

Sources/Spezi/Spezi/Spezi.swift

@@ -79,9 +79,13 @@ import XCTRuntimeAssertions
 /// - ``launchOptions``
 /// - ``spezi``
 ///
-/// ### Actions
+/// ### Remote Notifications
 /// - ``registerRemoteNotifications``
 /// - ``unregisterRemoteNotifications``
+///
+/// ### Dynamically Loading Modules
+/// - ``loadModule(_:ownership:)``
+/// - ``unloadModule(_:)``
 @Observable
 public final class Spezi: Sendable {
     static let logger = Logger(subsystem: "edu.stanford.spezi", category: "Spezi")
@@ -93,7 +97,7 @@ public final class Spezi: Sendable {
     /// A shared repository to store any `KnowledgeSource`s restricted to the ``SpeziAnchor``.
     ///
     /// Every `Module` automatically conforms to `KnowledgeSource` and is stored within this storage object.
-    fileprivate(set) nonisolated(unsafe) var storage: SpeziStorage // nonisolated, writes are all isolated to @MainActor, just reads are non-isolated
+    nonisolated(unsafe) var storage: SpeziStorage // nonisolated, writes are all isolated to @MainActor, just reads are non-isolated
 
     /// Key is either a UUID for `@Modifier` or `@Model` property wrappers, or a `ModuleReference` for `EnvironmentAccessible` modifiers.
     @MainActor private var _viewModifiers: OrderedDictionary<AnyHashable, any ViewModifier> = [:]

Tests/UITests/TestApp/LifecycleHandler/LifecycleHandlerTestsView.swift

@@ -14,7 +14,7 @@ import XCTSpezi
 struct LifecycleHandlerTestsView: View {
     @Environment(LifecycleHandlerModel.self)
     var model
-    
+
 
     var body: some View {
         VStack {

Tests/UITests/TestApp/RemoteNotifications/NotificationModule.swift

@@ -0,0 +1,19 @@
+//
+// This source file is part of the Stanford Spezi open-source project
+//
+// SPDX-FileCopyrightText: 2024 Stanford University and the project authors (see CONTRIBUTORS.md)
+//
+// SPDX-License-Identifier: MIT
+//
+
+import Spezi
+
+
+@MainActor
+class NotificationModule: Module, EnvironmentAccessible {
+    @Application(\.registerRemoteNotifications)
+    var registerRemoteNotifications
+
+    @Application(\.unregisterRemoteNotifications)
+    var unregisterRemoteNotifications
+}

Tests/UITests/TestApp/RemoteNotifications/RemoteNotificationsTestView.swift

@@ -0,0 +1,75 @@
+//
+// This source file is part of the Stanford Spezi open-source project
+//
+// SPDX-FileCopyrightText: 2024 Stanford University and the project authors (see CONTRIBUTORS.md)
+//
+// SPDX-License-Identifier: MIT
+//
+
+import SwiftUI
+
+
+struct RemoteNotificationsTestView: View {
+    @Environment(NotificationModule.self)
+    private var notificationModule
+
+    @State private var token: Data?
+    @State private var error: Error?
+
+    @State private var task: Task<Void, Never>?
+
+    var body: some View {
+        List { // swiftlint:disable:this closure_body_length
+            Section("Token") {
+                HStack {
+                    Text(verbatim: "Token")
+                    Spacer()
+                    if let token {
+                        Text(token.description)
+                            .foregroundStyle(.green)
+                    } else if let error = error as? LocalizedError,
+                              let description = error.errorDescription ?? error.failureReason {
+                        Text(verbatim: description)
+                            .foregroundStyle(.red)
+                    } else if error != nil {
+                        Text(verbatim: "failed")
+                            .foregroundStyle(.red)
+                    } else {
+                        Text(verbatim: "none")
+                            .foregroundStyle(.secondary)
+                    }
+                }
+                    .accessibilityElement(children: .combine)
+                    .accessibilityIdentifier("token-field")
+            }
+
+            Section("Actions") {
+                Button("Register") {
+                    task = Task { @MainActor in
+                        do {
+                            token = try await notificationModule.registerRemoteNotifications()
+                        } catch {
+                            self.error = error
+                        }
+                    }
+                }
+                Button("Unregister") {
+                    notificationModule.unregisterRemoteNotifications()
+                    token = nil
+                    error = nil
+                }
+            }
+        }
+            .onDisappear {
+                task?.cancel()
+            }
+    }
+}
+
+
+#if DEBUG
+#Preview {
+    RemoteNotificationsTestView()
+        .environment(NotificationModule())
+}
+#endif

Tests/UITests/TestApp/Shared/TestAppDelegate.swift

@@ -19,6 +19,7 @@ class TestAppDelegate: SpeziAppDelegate {
             }
             ModuleWithModifier()
             ModuleWithModel()
+            NotificationModule()
         }
     }
 }

Tests/UITests/TestApp/SpeziTests.swift

@@ -14,7 +14,8 @@ enum SpeziTests: String, TestAppTests {
     case viewModifier = "ViewModifier"
     case lifecycleHandler = "LifecycleHandler"
     case model = "Model"
-
+    case notifications = "Remote Notifications"
+
 
     func view(withNavigationPath path: Binding<NavigationPath>) -> some View {
         switch self {
@@ -24,6 +25,8 @@ enum SpeziTests: String, TestAppTests {
             LifecycleHandlerTestsView()
         case .model:
             ModelTestView()
+        case .notifications:
+            RemoteNotificationsTestView()
         }
     }
 }

Tests/UITests/TestApp/TestApp.entitlements

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>aps-environment</key>
+	<string>development</string>
+	<key>com.apple.developer.aps-environment</key>
+	<string>development</string>
+</dict>
+</plist>

Tests/UITests/TestApp/TestApp.entitlements.license

@@ -0,0 +1,5 @@
+This source file is part of the Stanford Spezi open-source project
+
+SPDX-FileCopyrightText: 2024 Stanford University and the project authors (see CONTRIBUTORS.md)
+
+SPDX-License-Identifier: MIT