refactor push notification registration and fix re-registration issue #1501

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed a bug where toggles in the settings screen were white instead of green when toggled on. [#1251](https://github.com/planetary-social/nos/issues/1251)
 - Added routing to profile when tapping on follow notification. [#1447](https://github.com/planetary-social/nos/issues/1447)
 - Localized follows notifications. [#1446](https://github.com/planetary-social/nos/issues/1446)
+- Fixed issue where push notifications were not re-registered after account change. [#1501](https://github.com/planetary-social/nos/issues/1501)
 
 ### Internal Changes
 - Use NIP-92 media metadata to display media in the proper orientation. Currently behind the “Enable new media display” feature flag. [#1172](https://github.com/planetary-social/nos/issues/1172)

Nos.xcodeproj/project.pbxproj

@@ -125,6 +125,8 @@
 		3FFB1D9729A6BBEC002A755D /* Collection+SafeSubscript.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3FFB1D9529A6BBEC002A755D /* Collection+SafeSubscript.swift */; };
 		3FFB1D9C29A7DF9D002A755D /* StackedAvatarsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3FFB1D9B29A7DF9D002A755D /* StackedAvatarsView.swift */; };
 		3FFF3BD029A9645F00DD0B72 /* AuthorReference+CoreDataClass.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F43C47529A9625700E896A0 /* AuthorReference+CoreDataClass.swift */; };
+		500899F32C95C1F900834588 /* PushNotificationRegistrar.swift in Sources */ = {isa = PBXBuildFile; fileRef = 502B6C3C2C9462A400446316 /* PushNotificationRegistrar.swift */; };
+		502B6C3D2C9462A400446316 /* PushNotificationRegistrar.swift in Sources */ = {isa = PBXBuildFile; fileRef = 502B6C3C2C9462A400446316 /* PushNotificationRegistrar.swift */; };
 		5044546E2C90726A00251A7E /* Event+Fetching.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5044546D2C90726A00251A7E /* Event+Fetching.swift */; };
 		504454702C90728500251A7E /* Event+Hydration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5044546F2C90728500251A7E /* Event+Hydration.swift */; };
 		504454712C90728E00251A7E /* Event+Fetching.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5044546D2C90726A00251A7E /* Event+Fetching.swift */; };
@@ -641,6 +643,7 @@
 		3FFB1D9229A6BBCE002A755D /* EventReference+CoreDataClass.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "EventReference+CoreDataClass.swift"; sourceTree = "<group>"; };
 		3FFB1D9529A6BBEC002A755D /* Collection+SafeSubscript.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "Collection+SafeSubscript.swift"; sourceTree = "<group>"; };
 		3FFB1D9B29A7DF9D002A755D /* StackedAvatarsView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StackedAvatarsView.swift; sourceTree = "<group>"; };
+		502B6C3C2C9462A400446316 /* PushNotificationRegistrar.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PushNotificationRegistrar.swift; sourceTree = "<group>"; };
 		5044546D2C90726A00251A7E /* Event+Fetching.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Event+Fetching.swift"; sourceTree = "<group>"; };
 		5044546F2C90728500251A7E /* Event+Hydration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Event+Hydration.swift"; sourceTree = "<group>"; };
 		5045540C2C81E10C0044ECAE /* EditableAvatarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditableAvatarView.swift; sourceTree = "<group>"; };
@@ -1530,6 +1533,7 @@
 				C9EF84CE2C24D63000182B6F /* MockRelayService.swift */,
 				0320C1142BFE63DC00C4C080 /* MockRelaySubscriptionManager.swift */,
 				5BC0D9CB2B867B9D005D6980 /* NamesAPI.swift */,
+				502B6C3C2C9462A400446316 /* PushNotificationRegistrar.swift */,
 				C936B4612A4CB01C00DF1EB9 /* PushNotificationService.swift */,
 				C9A8015D2BD0177D006E29B2 /* ReportPublisher.swift */,
 				5B8805192A21027C00E21F06 /* SHA256Key.swift */,
@@ -2190,6 +2194,7 @@
 				C9A0DAE429C69F0C00466635 /* HighlightedText.swift in Sources */,
 				C94D855C2991479900749478 /* NoteComposer.swift in Sources */,
 				5B79F6532BA11B08002DA9BE /* WizardSheetDescriptionText.swift in Sources */,
+				502B6C3D2C9462A400446316 /* PushNotificationRegistrar.swift in Sources */,
 				5B6EB48E29EDBE0E006E750C /* NoteParser.swift in Sources */,
 				C9F84C23298DC7B900C6714D /* SettingsView.swift in Sources */,
 				03C8B4962C6D065900A07CCD /* ImageViewer.swift in Sources */,
@@ -2522,6 +2527,7 @@
 				C9F0BB6C29A503D6000547FC /* PublicKey.swift in Sources */,
 				C9DEC06F2989668E0078B43A /* Relay+CoreDataClass.swift in Sources */,
 				C9ADB13F29929F1F0075E7F8 /* String+Hex.swift in Sources */,
+				500899F32C95C1F900834588 /* PushNotificationRegistrar.swift in Sources */,
 				C973AB622A323167002AED16 /* Author+CoreDataProperties.swift in Sources */,
 				C93EC2F529C34C860012EE2A /* NSPredicate+Bool.swift in Sources */,
 				C9DEC05B298950A90078B43A /* String+Lorem.swift in Sources */,

Nos/AppDelegate.swift

@@ -24,7 +24,7 @@ class AppDelegate: NSObject, UIApplicationDelegate {
     ) {
         Task {
             do {
-                try await pushNotificationService.registerForNotifications(with: deviceToken, user: currentUser)
+                try await pushNotificationService.registerForNotifications(currentUser, with: deviceToken)
             } catch {
                 Log.optional(error, "failed to register for push notifications")
             }

Nos/Service/CurrentUser.swift

@@ -98,13 +98,13 @@ import Dependencies
 
     // TODO: this is fragile
     // Reset CurrentUser state
-    @MainActor func reset() {
+    @MainActor private func reset() {
         onboardingRelays = []
         subscriptions = []
         setUp()
     }
 
-    @MainActor func setUp() {
+    @MainActor private func setUp() {
         if let keyPair {
             do {
                 author = try Author.findOrCreate(by: keyPair.publicKeyHex, context: viewContext)
@@ -122,6 +122,8 @@ import Dependencies
 
                 Task {
                     await subscribe()
+                    // Listen for notifications
+                    await pushNotificationService.listen(for: self)
                     refreshFriendMetadata()
                 }
             } catch {
@@ -201,8 +203,6 @@ import Dependencies
         subscriptions.append(
             await relayService.fetchEvents(matching: importantEventsFilter)
         )
-        // Listen for notifications
-        await pushNotificationService.listen(for: self)
     }
 
     private var friendMetadataTask: Task<Void, any Error>?

Nos/Service/PushNotificationRegistrar.swift

@@ -0,0 +1,108 @@
+import CoreData
+import Dependencies
+import Foundation
+
+/// Registers for push notifications.
+final class PushNotificationRegistrar {
+
+    enum PushNotificationError: LocalizedError {
+        case missingPubkey
+        case missingDeviceToken
+        case unexpected
+
+        var errorDescription: String? {
+            switch self {
+            case .missingPubkey:        "The pubkey is required"
+            case .missingDeviceToken:   "Device token is required"
+            case .unexpected:           "Something unexpected happened"
+            }
+        }
+    }
+
+#if DEBUG
+    private static let notificationServiceRelayURL = URL(string: "wss://dev-notifications.nos.social")!
+#else
+    private static let notificationServiceRelayURL = URL(string: "wss://notifications.nos.social")!
+#endif
+
+    @Dependency(\.relayService) private var relayService
+    @Dependency(\.analytics) private var analytics
+    @Dependency(\.currentUser) private var currentUser
+
+    private var deviceToken: Data?
+    private var registeredPubkey: String?
+    private var registrationTask: Task<Void, Error>?
+
+    /// Registers a user for push notifications by publishing a registration event. Calls to
+    /// this function are handled serially.
+    /// - Parameters:
+    ///   - user: The user to register for notifications.
+    ///   - deviceToken: The device token to register with. A cached token will be used if none is passed in.
+    ///   - context: The Core Data context to use.
+    func register(_ user: CurrentUser, with deviceToken: Data? = nil, context: NSManagedObjectContext) async throws {
+        guard let userKey = user.publicKeyHex, let keyPair = user.keyPair else {
+            throw PushNotificationError.missingPubkey
+        }
+
+        try await registrationTask?.value   // wait for the previous task to finish
+
+        guard let token = deviceToken ?? self.deviceToken else {
+            throw PushNotificationError.missingDeviceToken
+        }
+
+        self.deviceToken = token
+
+        guard userKey != registeredPubkey else {
+            return  // already registered this user
+        }
+
+        registrationTask = Task {
+            do {
+                let jsonEvent = JSONEvent(
+                    pubKey: userKey,
+                    kind: .notificationServiceRegistration,
+                    tags: [],
+                    content: try await registrationContent(deviceToken: token, user: user)
+                )
+                try await relayService.publish(
+                    event: jsonEvent,
+                    to: Self.notificationServiceRelayURL,
+                    signingKey: keyPair,
+                    context: context
+                )
+                registeredPubkey = userKey
+            } catch {
+                analytics.pushNotificationRegistrationFailed(reason: error.localizedDescription)
+                throw error
+            }
+        }
+
+        try await registrationTask?.value
+    }
+
+    /// Builds the string needed for the `content` field.
+    private func registrationContent(deviceToken: Data, user: CurrentUser) async throws -> String {
+        guard let publicKeyHex = currentUser.publicKeyHex else {
+            throw PushNotificationError.unexpected
+        }
+        let relays: [RegistrationRelayAddress] = await relayService.relayAddresses(for: user).map {
+            RegistrationRelayAddress(address: $0.absoluteString)
+        }
+        let content = Registration(
+            apnsToken: deviceToken.hexString,
+            publicKey: publicKeyHex,
+            relays: relays
+        )
+        return String(decoding: try JSONEncoder().encode(content), as: UTF8.self)
+    }
+}
+
+fileprivate struct Registration: Encodable {
+    let apnsToken: String
+    let publicKey: String
+    let relays: [RegistrationRelayAddress]
+}
+
+fileprivate struct RegistrationRelayAddress: Encodable {
+    let address: String
+}

Nos/Service/PushNotificationService.swift

@@ -10,13 +10,6 @@ import Combine
 /// all new events and creates `NosNotification`s and displays them when appropriate.
 @MainActor class PushNotificationService: 
     NSObject, ObservableObject, NSFetchedResultsControllerDelegate, UNUserNotificationCenterDelegate {
-
-    enum PushNotificationError: LocalizedError {
-        case unexpected
-        var errorDescription: String? {
-            "Something unexpected happened"
-        }
-    }
 
     // MARK: - Public Properties
 
@@ -55,19 +48,15 @@ import Combine
     @Dependency(\.userDefaults) private var userDefaults
     @Dependency(\.currentUser) private var currentUser
 
-    #if DEBUG
-    private let notificationServiceAddress = "wss://dev-notifications.nos.social"
-    #else
-    private let notificationServiceAddress = "wss://notifications.nos.social"
-    #endif
-
     private var notificationWatcher: NSFetchedResultsController<Event>?
     private var relaySubscription: SubscriptionCancellable?
     private var currentAuthor: Author? 
     private lazy var modelContext: NSManagedObjectContext = {
         persistenceController.newBackgroundContext()
     }()
 
+    private lazy var registrar = PushNotificationRegistrar()
+
     // MARK: - Setup
 
     func listen(for user: CurrentUser) async {
@@ -112,33 +101,18 @@ import Combine
         )
 
         await updateBadgeCount()
-    }
-
-    func registerForNotifications(with token: Data, user: CurrentUser) async throws {
-        guard let userKey = user.publicKeyHex, let keyPair = user.keyPair else {
-            // TODO: throw
-            return
-        }
 
         do {
-            let jsonEvent = JSONEvent(
-                pubKey: userKey,
-                kind: EventKind.notificationServiceRegistration,
-                tags: [],
-                content: try await createRegistrationContent(deviceToken: token, user: user)
-            )
-            try await self.relayService.publish(
-                event: jsonEvent, 
-                to: URL(string: self.notificationServiceAddress)!, 
-                signingKey: keyPair, 
-                context: self.modelContext
-            )
+            try await registrar.register(user, context: modelContext)
         } catch {
-            analytics.pushNotificationRegistrationFailed(reason: error.localizedDescription)
-            throw error
+            Log.optional(error, "failed to register for push notifications")
         }
     }
 
+    func registerForNotifications(_ user: CurrentUser, with deviceToken: Data) async throws {
+        try await registrar.register(user, with: deviceToken, context: modelContext)
+    }
+
     // MARK: - Helpers
 
     func requestNotificationPermissionsFromUser() {
@@ -153,8 +127,8 @@ import Combine
         }
     }
 
-    /// Recomputes the number of unread notifications for the `currentAuthor` and published the new new value to 
-    /// `badgeCount` and updates the application badge icon. 
+    /// Recomputes the number of unread notifications for the `currentAuthor`, publishes the new value to
+    /// `badgeCount`, and updates the application badge icon.
     func updateBadgeCount() async {
         var badgeCount = 0
         if currentAuthor != nil {
@@ -169,22 +143,6 @@ import Combine
 
     // MARK: - Internal
 
-    /// Builds the string needed for the `content` field in the special 
-    private func createRegistrationContent(deviceToken: Data, user: CurrentUser) async throws -> String {
-        guard let publicKeyHex = currentUser.publicKeyHex else {
-            throw PushNotificationError.unexpected
-        }
-        let relays: [RegistrationRelayAddress] = await relayService.relayAddresses(for: user).map {
-            RegistrationRelayAddress(address: $0.absoluteString)
-        }
-        let content = Registration(
-            apnsToken: deviceToken.hexString,
-            publicKey: publicKeyHex,
-            relays: relays
-        )
-        return String(decoding: try JSONEncoder().encode(content), as: UTF8.self) 
-    }
-
     /// Tells the system to display a notification for the given event if it's appropriate. This will create a 
     /// NosNotification record in the database.
     @MainActor private func showNotificationIfNecessary(for eventID: RawEventID) async {
@@ -295,16 +253,6 @@ import Combine
 
 class MockPushNotificationService: PushNotificationService {
     override func listen(for user: CurrentUser) async { }
-    override func registerForNotifications(with token: Data, user: CurrentUser) async throws { }
+    override func registerForNotifications(_ user: CurrentUser, with deviceToken: Data) async throws { }
     override func requestNotificationPermissionsFromUser() { }
 }
-
-fileprivate struct Registration: Codable {
-    var apnsToken: String
-    var publicKey: String
-    var relays: [RegistrationRelayAddress]
-}
-
-fileprivate struct RegistrationRelayAddress: Codable {
-    var address: String
-}