Improvements to NSE (#4992)

1 files changed, 61 insertions, 12 deletions

diff --git a/modules/BlueskyNSE/NotificationService.swift b/modules/BlueskyNSE/NotificationService.swift
index f863eaf22..481402890 100644
--- a/modules/BlueskyNSE/NotificationService.swift
+++ b/modules/BlueskyNSE/NotificationService.swift
@@ -2,46 +2,80 @@ import UserNotifications
 import UIKit
 
 let APP_GROUP = "group.app.bsky"
+typealias ContentHandler = (UNNotificationContent) -> Void
+
+// This extension allows us to do some processing of the received notification
+// data before displaying the notification to the user. In our use case, there
+// are a few particular things that we want to do:
+//
+// - Determine whether we should play a sound for the notification
+// - Download and display any images for the notification
+// - Update the badge count accordingly
+//
+// The extension may or may not create a new process to handle a notification.
+// It is also possible that multiple notifications will be processed by the
+// same instance of `NotificationService`, though these will happen in
+// parallel.
+//
+// Because multiple instances of `NotificationService` may exist, we should
+// be careful in accessing preferences that will be mutated _by the
+// extension itself_. For example, we should not worry about `playChatSound`
+// changing, since we never mutate that value within the extension itself.
+// However, since we mutate `badgeCount` frequently, we should ensure that
+// these updates always run sync with each other and that the have access
+// to the most recent values.
 
 class NotificationService: UNNotificationServiceExtension {
-  var prefs = UserDefaults(suiteName: APP_GROUP)
+  private var contentHandler: ContentHandler?
+  private var bestAttempt: UNMutableNotificationContent?
 
   override func didReceive(_ request: UNNotificationRequest, withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void) {
-    guard let bestAttempt = createCopy(request.content),
+    self.contentHandler = contentHandler
+
+    guard let bestAttempt = NSEUtil.createCopy(request.content),
           let reason = request.content.userInfo["reason"] as? String
     else {
       contentHandler(request.content)
       return
     }
 
+    self.bestAttempt = bestAttempt
     if reason == "chat-message" {
       mutateWithChatMessage(bestAttempt)
     } else {
       mutateWithBadge(bestAttempt)
     }
 
+    // Any image downloading (or other network tasks) should be handled at the end
+    // of this block. Otherwise, if there is a timeout and serviceExtensionTimeWillExpire
+    // gets called, we might not have all the needed mutations completed in time.
+
     contentHandler(bestAttempt)
   }
 
   override func serviceExtensionTimeWillExpire() {
-    // If for some reason the alloted time expires, we don't actually want to display a notification
+    guard let contentHandler = self.contentHandler,
+          let bestAttempt = self.bestAttempt else {
+      return
+    }
+    contentHandler(bestAttempt)
   }
 
-  func createCopy(_ content: UNNotificationContent) -> UNMutableNotificationContent? {
-    return content.mutableCopy() as? UNMutableNotificationContent
-  }
+  // MARK: Mutations
 
   func mutateWithBadge(_ content: UNMutableNotificationContent) {
-    var count = prefs?.integer(forKey: "badgeCount") ?? 0
-    count += 1
+    NSEUtil.shared.prefsQueue.sync {
+      var count = NSEUtil.shared.prefs?.integer(forKey: "badgeCount") ?? 0
+      count += 1
 
-    // Set the new badge number for the notification, then store that value for using later
-    content.badge = NSNumber(value: count)
-    prefs?.setValue(count, forKey: "badgeCount")
+      // Set the new badge number for the notification, then store that value for using later
+      content.badge = NSNumber(value: count)
+      NSEUtil.shared.prefs?.setValue(count, forKey: "badgeCount")
+    }
   }
 
   func mutateWithChatMessage(_ content: UNMutableNotificationContent) {
-    if self.prefs?.bool(forKey: "playSoundChat") == true {
+    if NSEUtil.shared.prefs?.bool(forKey: "playSoundChat") == true {
       mutateWithDmSound(content)
     }
   }
@@ -54,3 +88,18 @@ class NotificationService: UNNotificationServiceExtension {
     content.sound = UNNotificationSound(named: UNNotificationSoundName(rawValue: "dm.aiff"))
   }
 }
+
+// NSEUtil's purpose is to create a shared instance of `UserDefaults` across
+// `NotificationService` instances. It also includes a queue so that we can process
+// updates to `UserDefaults` in parallel.
+
+private class NSEUtil {
+  static let shared = NSEUtil()
+
+  var prefs = UserDefaults(suiteName: APP_GROUP)
+  var prefsQueue = DispatchQueue(label: "NSEPrefsQueue")
+
+  static func createCopy(_ content: UNNotificationContent) -> UNMutableNotificationContent? {
+    return content.mutableCopy() as? UNMutableNotificationContent
+  }
+}