From 557fbbef97e4e37290644469f520b094b9c46a96 Mon Sep 17 00:00:00 2001 From: Bryan Montz Date: Sun, 15 Sep 2024 07:03:02 -0500 Subject: [PATCH 1/2] refactor push notification registration and fix re-registration issue #1501 --- CHANGELOG.md | 1 + Nos.xcodeproj/project.pbxproj | 6 ++ Nos/AppDelegate.swift | 2 +- Nos/Service/CurrentUser.swift | 8 +- Nos/Service/PushNotificationRegistrar.swift | 108 ++++++++++++++++++++ Nos/Service/PushNotificationService.swift | 74 ++------------ 6 files changed, 131 insertions(+), 68 deletions(-) create mode 100644 Nos/Service/PushNotificationRegistrar.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 631f878af..3bc907538 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/Nos.xcodeproj/project.pbxproj b/Nos.xcodeproj/project.pbxproj index 73caa9fbc..3926d4d3a 100644 --- a/Nos.xcodeproj/project.pbxproj +++ b/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 = ""; }; 3FFB1D9529A6BBEC002A755D /* Collection+SafeSubscript.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "Collection+SafeSubscript.swift"; sourceTree = ""; }; 3FFB1D9B29A7DF9D002A755D /* StackedAvatarsView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StackedAvatarsView.swift; sourceTree = ""; }; + 502B6C3C2C9462A400446316 /* PushNotificationRegistrar.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PushNotificationRegistrar.swift; sourceTree = ""; }; 5044546D2C90726A00251A7E /* Event+Fetching.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Event+Fetching.swift"; sourceTree = ""; }; 5044546F2C90728500251A7E /* Event+Hydration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Event+Hydration.swift"; sourceTree = ""; }; 5045540C2C81E10C0044ECAE /* EditableAvatarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditableAvatarView.swift; sourceTree = ""; }; @@ -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 */, diff --git a/Nos/AppDelegate.swift b/Nos/AppDelegate.swift index fa9d1334f..f460855ca 100644 --- a/Nos/AppDelegate.swift +++ b/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") } diff --git a/Nos/Service/CurrentUser.swift b/Nos/Service/CurrentUser.swift index ecdfb008a..17b885c95 100644 --- a/Nos/Service/CurrentUser.swift +++ b/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? diff --git a/Nos/Service/PushNotificationRegistrar.swift b/Nos/Service/PushNotificationRegistrar.swift new file mode 100644 index 000000000..dc28ec2ff --- /dev/null +++ b/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? + + /// 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 createRegistrationContent(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 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) + } +} + +fileprivate struct Registration: Encodable { + let apnsToken: String + let publicKey: String + let relays: [RegistrationRelayAddress] +} + +fileprivate struct RegistrationRelayAddress: Encodable { + let address: String +} diff --git a/Nos/Service/PushNotificationService.swift b/Nos/Service/PushNotificationService.swift index 075bb978d..6b173ab3c 100644 --- a/Nos/Service/PushNotificationService.swift +++ b/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,12 +48,6 @@ 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? private var relaySubscription: SubscriptionCancellable? private var currentAuthor: Author? @@ -68,6 +55,8 @@ import Combine 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 -} From 73f580fe6970e1e35469d41bbd5bf638135284e6 Mon Sep 17 00:00:00 2001 From: Bryan Montz Date: Tue, 17 Sep 2024 06:32:15 -0500 Subject: [PATCH 2/2] updated doc comment and made minor tweak to function name --- Nos/Service/PushNotificationRegistrar.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Nos/Service/PushNotificationRegistrar.swift b/Nos/Service/PushNotificationRegistrar.swift index dc28ec2ff..a730dda0a 100644 --- a/Nos/Service/PushNotificationRegistrar.swift +++ b/Nos/Service/PushNotificationRegistrar.swift @@ -62,7 +62,7 @@ final class PushNotificationRegistrar { pubKey: userKey, kind: .notificationServiceRegistration, tags: [], - content: try await createRegistrationContent(deviceToken: token, user: user) + content: try await registrationContent(deviceToken: token, user: user) ) try await relayService.publish( event: jsonEvent, @@ -80,8 +80,8 @@ final class PushNotificationRegistrar { try await registrationTask?.value } - /// Builds the string needed for the `content` field in the special - private func createRegistrationContent(deviceToken: Data, user: CurrentUser) async throws -> String { + /// 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 }