fix iOS notification polling completion and persistence plus better logs

ntfy/App/AppDelegate.swift

@@ -52,14 +52,37 @@ class AppDelegate: UIResponder, UIApplicationDelegate, ObservableObject {
         // Poll and show new messages as notifications
         let store = Store.shared
         let subscriptionManager = SubscriptionManager(store: store)
-        store.getSubscriptions()?.forEach { subscription in
+        let subscriptions = store.getSubscriptions() ?? []
+        guard !subscriptions.isEmpty else {
+            completionHandler(.noData)
+            return
+        }
+
+        let group = DispatchGroup()
+        let resultQueue = DispatchQueue(label: "io.heckel.ntfy.background-poll-result")
+        var didReceiveNewData = false
+        subscriptions.forEach { subscription in
+            group.enter()
+            guard let baseUrl = subscription.baseUrl else {
+                Log.w(tag, "Skipping background poll notification for subscription with missing baseUrl")
+                group.leave()
+                return
+            }
             subscriptionManager.poll(subscription) { messages in
-                messages.forEach { message in
-                    self.showNotification(subscription, message)
+                if !messages.isEmpty {
+                    resultQueue.sync {
+                        didReceiveNewData = true
+                    }
                 }
+                messages.forEach { message in
+                    self.showNotification(baseUrl: baseUrl, message)
+                }
+                group.leave()
             }
         }
-        completionHandler(.newData)
+        group.notify(queue: .main) {
+            completionHandler(didReceiveNewData ? .newData : .noData)
+        }
     }
     
     func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
@@ -76,8 +99,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate, ObservableObject {
     /// local notification look exactly like the remote one (same userInfo), so that when we tap it, the userNotificationCenter(didReceive) function
     /// has the same information available.
     private func showNotification(_ subscription: Subscription, _ message: Message) {
+        guard let baseUrl = subscription.baseUrl else {
+            Log.w(tag, "Skipping notification for subscription with missing baseUrl")
+            return
+        }
+        showNotification(baseUrl: baseUrl, message)
+    }
+
+    private func showNotification(baseUrl: String, _ message: Message) {
         let content = UNMutableNotificationContent()
-        content.modify(message: message, baseUrl: subscription.baseUrl ?? "?")
+        content.modify(message: message, baseUrl: baseUrl)
     
         let request = UNNotificationRequest(identifier: message.id, content: content, trigger: nil /* now */)
         UNUserNotificationCenter.current().add(request) { (error) in

ntfy/Persistence/Store.swift

@@ -115,24 +115,26 @@ class Store: ObservableObject {
     // MARK: Notifications
     
     func save(notificationFromMessage message: Message, withSubscription subscription: Subscription) {
-        do {
-            let notification = Notification(context: context)
-            notification.id = message.id
-            notification.time = message.time
-            notification.message = message.message ?? ""
-            notification.title = message.title ?? ""
-            notification.priority = (message.priority != nil && message.priority != 0) ? message.priority! : 3
-            notification.tags = message.tags?.joined(separator: ",") ?? ""
-            notification.actions = Actions.shared.encode(message.actions)
-            notification.click = message.click ?? ""
-            notification.subscription = subscription
-            subscription.addToNotifications(notification)
-            subscription.lastNotificationId = message.id
-            Log.d(Store.tag, "Storing notification with ID \(notification.id ?? "<unknown>")")
-            try context.save()
-        } catch let error {
-            Log.w(Store.tag, "Cannot store notification (fromMessage)", error)
-            rollbackAndRefresh()
+        context.performAndWait {
+            do {
+                let notification = Notification(context: context)
+                notification.id = message.id
+                notification.time = message.time
+                notification.message = message.message ?? ""
+                notification.title = message.title ?? ""
+                notification.priority = (message.priority != nil && message.priority != 0) ? message.priority! : 3
+                notification.tags = message.tags?.joined(separator: ",") ?? ""
+                notification.actions = Actions.shared.encode(message.actions)
+                notification.click = message.click ?? ""
+                notification.subscription = subscription
+                subscription.addToNotifications(notification)
+                subscription.lastNotificationId = message.id
+                Log.d(Store.tag, "Storing notification with ID \(notification.id ?? "<unknown>")")
+                try context.save()
+            } catch let error {
+                Log.w(Store.tag, "Cannot store notification (fromMessage)", error)
+                rollbackAndRefresh()
+            }
         }
     }
     

ntfy/Persistence/SubscriptionManager.swift

@@ -47,10 +47,8 @@ struct SubscriptionManager {
             }
             Log.d(tag, "Polling success, \(messages.count) new message(s)", messages)
             if !messages.isEmpty {
-                DispatchQueue.main.sync {
-                    for message in messages {
-                        store.save(notificationFromMessage: message, withSubscription: subscription)
-                    }
+                for message in messages {
+                    store.save(notificationFromMessage: message, withSubscription: subscription)
                 }
             }
             completionHandler(messages)

ntfy/Utils/ApiService.swift

@@ -8,7 +8,7 @@ class ApiService {
     
     func poll(subscription: Subscription, user: BasicUser?, completionHandler: @escaping ([Message]?, Error?) -> Void) {
         guard let url = URL(string: subscription.urlString()) else {
-            // FIXME
+            completionHandler(nil, URLError(.badURL))
             return
         }
         let since = subscription.lastNotificationId ?? "all"

ntfyNSE/NotificationService.swift

@@ -80,7 +80,7 @@ class NotificationService: UNNotificationServiceExtension {
         
         // Poll original server
         let user = store?.getUser(baseUrl: baseUrl)?.toBasicUser()
-        let semaphore = DispatchSemaphore(value: 0)
+        // The extension only needs contentHandler to be called from the async callback
         ApiService.shared.poll(subscription: subscription, messageId: pollId, user: user) { message, error in
             guard let message = message else {
                 Log.w(self.tag, "Error fetching message", error)
@@ -88,13 +88,6 @@ class NotificationService: UNNotificationServiceExtension {
                 return
             }
             self.handleMessage(request, content, baseUrl, message, contentHandler)
-            semaphore.signal()
         }
-        
-        // Note: If notifications only show up as "New message", it may be because the "return" statement
-        // happens before the contentHandler() is called. We add this semaphore here to synchronize the threads.
-        // I don't know if this is necessary, but it feels like the right thing to do.
-        
-        _ = semaphore.wait(timeout: DispatchTime.now() + 25) // 30 seconds is the max for the entire extension
     }
 }