fix: 3rd party SDK compatibility when SDK forwards push events back t…

…o us (#736)

Sources/MessagingPush/PushHandling/PushEventHandlerProxy.swift

@@ -50,16 +50,26 @@ class PushEventHandlerProxyImpl: PushEventHandlerProxy {
             // Each iteration of the loop waits for the push event to be processed by the delegate.
             for delegate in nestedDelegates.values {
                 await withCheckedContinuation { continuation in
+                    // Some SDKs, like the rn-firebase SDK (RNFBMessagingUNUserNotificationCenter), will call the completionHandler twice.
+                    // Handle that by only responding to the first call and ignoring the rest.
+                    var hasResumed = false
+
                     let nameOfDelegateClass: String = .init(describing: delegate)
 
                     // Using logs to give feedback to customer if 1 or more delegates do not call the async completion handler.
                     // These logs could help in debuggging to determine what delegate did not call the completion handler.
                     self.logger.info("Sending push notification, \(pushAction.push.title), event to: \(nameOfDelegateClass)). Customer.io SDK will wait for async completion handler to be called...")
 
                     delegate.onPushAction(pushAction) {
-                        self.logger.info("Received async completion handler from \(nameOfDelegateClass).")
+                        Task { @MainActor in // in case the delegate calls the completion handler on a background thread, we need to switch back to the main thread.
+                            self.logger.info("Received async completion handler from \(nameOfDelegateClass).")
+
+                            if !hasResumed {
+                                hasResumed = true
 
-                        continuation.resume()
+                                continuation.resume()
+                            }
+                        }
                     }
                 }
             }
@@ -91,20 +101,30 @@ class PushEventHandlerProxyImpl: PushEventHandlerProxy {
             // Each iteration of the loop waits for the push event to be processed by the delegate.
             for delegate in nestedDelegates.values {
                 await withCheckedContinuation { continuation in
+                    // Some SDKs, like the rn-firebase SDK (RNFBMessagingUNUserNotificationCenter), will call the completionHandler twice.
+                    // Handle that by only responding to the first call and ignoring the rest.
+                    var hasResumed = false
+
                     let nameOfDelegateClass: String = .init(describing: delegate)
 
                     // Using logs to give feedback to customer if 1 or more delegates do not call the async completion handler.
                     // These logs could help in debuggging to determine what delegate did not call the completion handler.
                     self.logger.info("Sending push notification, \(push.title), event to: \(nameOfDelegateClass)). Customer.io SDK will wait for async completion handler to be called...")
 
                     delegate.shouldDisplayPushAppInForeground(push, completionHandler: { delegateShouldDisplayPushResult in
-                        self.logger.info("Received async completion handler from \(nameOfDelegateClass).")
+                        Task { @MainActor in // in case the delegate calls the completion handler on a background thread, we need to switch back to the main thread.
+                            self.logger.info("Received async completion handler from \(nameOfDelegateClass).")
 
-                        if delegateShouldDisplayPushResult {
-                            shouldDisplayPush = true
-                        }
+                            if !hasResumed {
+                                hasResumed = true
+
+                                if delegateShouldDisplayPushResult {
+                                    shouldDisplayPush = true
+                                }
 
-                        continuation.resume()
+                                continuation.resume()
+                            }
+                        }
                     })
                 }
             }

Sources/MessagingPush/PushHandling/iOSPushEventListener.swift

@@ -31,6 +31,10 @@ class IOSPushEventListener: PushEventHandler {
         guard !pushHistory.hasHandledPush(pushEvent: .didReceive, pushId: push.pushId, pushDeliveryDate: dateWhenPushDelivered) else {
             // push has already been handled. exit early
             logger.debug("[onPushAction] early exist as the push was already handled: \(pushAction)")
+
+            // We expect this function to only be called by a 3rd party SDK that forwarded the push event to our SDK.
+            // Call the completionHandler so the 3rd party SDK knows we are done processing it.
+            completionHandler()
             return
         }
 
@@ -78,6 +82,11 @@ class IOSPushEventListener: PushEventHandler {
 
             // See notes in didReceive function to learn more about this logic of exiting early when we already have handled a push.
             logger.debug("[shouldDisplayPushAppInForeground] early exit as the push was previously handled: \(push)")
+            // We expect this function to only be called by a 3rd party SDK that forwarded the push event to our SDK.
+            // Call the completionHandler so the 3rd party SDK knows we are done processing it.
+            //
+            // For push notifications sent from CIO, the completionHandler return value is irrelevant. For those sent by third-party SDKs, it's up to that SDK to use the return value or not.
+            completionHandler(false)
             return
         }
 

Tests/MessagingPush/PushHandling/AutomaticPushClickedIntegrationTest.swift

@@ -175,41 +175,114 @@ class AutomaticPushClickedIntegrationTest: IntegrationTest {
         pushClickHandler.assertHandledPushClick(for: givenPush)
     }
 
-    // MARK: prevent infinite loops with notification center proxy
+    // MARK: prevent infinite loops when forwarding push events to other push handlers
 
     /*
-     Some 3rd party SDKs (such as FCM SDK) have an implementation of swizzling that can create an infinite loop with our SDK. .
+     Some 3rd party SDKs (such as FCM SDK) have an implementation of swizzling that can create an infinite loop with our SDK.
 
      Example scenario of infinite loop:
      - iOS delivers a push click event to our SDK via our `UNUserNotificationCenterDelegate` instance.
-     - The push did not come from CIO, so the SDK forwards the event to other `UNUserNotificationCenterDelegate` instances, such as FCM SDK.
+     - Our SDK forwards the push event to other the other `UNUserNotificationCenterDelegate` instances. For example, we forward it to the FCM SDK.
      - The FCM SDK's swizzling implementation involves making a call back to the host app's current UserNotificationCenter.delegate (which is our SDK). See code: https://github.com/firebase/firebase-ios-sdk/blob/5890db966963fd76cfd020d68c0067a7741bef06/FirebaseMessaging/Sources/FIRMessagingRemoteNotificationsProxy.m#L498-L504
-     - This call means that our SDK's `UNUserNotificationCenterDelegate` instance is called *again* to handle this push event. Starting this series of steps over again, creating an infinite loop.
+     - When the FCM SDK does this, it means our SDK's `UNUserNotificationCenterDelegate` instance is called *again* to handle the same push event we're already handling. If our SDK handles this push event again by forwarding the event to the other `UNUserNotificationCenterDelegate` instances, an infinite loop would occur.
      */
 
-    func test_givenMultiplePushClickHandlers_simulateFcmSdkSwizzlingBehavior_expectNoInfiniteLoop() {
+    func test_onPushAction_givenMultiplePushClickHandlers_simulateFcmSdkSwizzlingBehavior_expectNoInfiniteLoop() {
         let givenPush = PushNotificationStub.getPushNotSentFromCIO()
         let givenOtherPushHandler = PushEventHandlerMock()
         let givenPushClickAction = PushNotificationActionStub(push: givenPush, didClickOnPush: true)
 
         let expectOtherClickHandlerHandlesPush = expectation(description: "Other push handler should handle push.")
         expectOtherClickHandlerHandlesPush.expectedFulfillmentCount = 1 // the other push click handler should only be called once, indicating an infinite loop is not created.
         givenOtherPushHandler.onPushActionClosure = { _, onComplete in
+            expectOtherClickHandlerHandlesPush.fulfill()
+
             // Like the FCM SDK does, make a call back to the app's current `UNUserNotificationCenterDelegate` instance. We simulate this by performing a push click, again.
-            self.performPushAction(givenPushClickAction) {}
+            self.performPushAction(givenPushClickAction, withCompletionHandler: onComplete)
+        }
+        addOtherPushEventHandler(givenOtherPushHandler)
+
+        performPushAction(givenPushClickAction)
+
+        waitForExpectations()
 
+        pushClickHandler.assertDidNotHandlePushClick() // CIO SDK should not handle push.
+    }
+
+    func test_shouldDisplayPushAppInForeground_givenMultiplePushClickHandlers_simulateFcmSdkSwizzlingBehavior_expectNoInfiniteLoop() {
+        let givenPush = PushNotificationStub.getPushNotSentFromCIO()
+        let givenOtherPushHandler = PushEventHandlerMock()
+
+        let expectOtherClickHandlerHandlesPush = expectation(description: "Other push handler should handle push.")
+        expectOtherClickHandlerHandlesPush.expectedFulfillmentCount = 1 // the other push click handler should only be called once, indicating an infinite loop is not created.
+        givenOtherPushHandler.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
             expectOtherClickHandlerHandlesPush.fulfill()
-            onComplete()
+
+            // Like the FCM SDK does, make a call back to the app's current `UNUserNotificationCenterDelegate` instance. We simulate this by sending the push event back to us.
+            self.sendPushEventShouldShowPushAppInForeground(givenPush, withCompletionHandler: onComplete)
         }
         addOtherPushEventHandler(givenOtherPushHandler)
 
-        performPushAction(givenPushClickAction)
+        sendPushEventShouldShowPushAppInForeground(givenPush)
 
         waitForExpectations()
 
         pushClickHandler.assertDidNotHandlePushClick() // CIO SDK should not handle push.
     }
 
+    /*
+        Some 3rd party SDKs (such as rnfirebase) when they handle a push event, they call the async completionHandler more then one time. The CIO SDK expects to only
+     receive the completionHandler once, but it needs to handle the scenario where a 3rd party SDK calls it multiple times otherwise the SDK could crash.
+
+     References:
+     1. rnfirebase push event handler (RNFBMessagingUNUserNotificationCenter) can call the completionHandler twice:
+     https://github.com/invertase/react-native-firebase/blob/d849667a1b3614c4a938c1f2bea892758831b368/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BUNUserNotificationCenter.m#L139
+     https://github.com/invertase/react-native-firebase/blob/d849667a1b3614c4a938c1f2bea892758831b368/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BUNUserNotificationCenter.m#L143-L145 (when the CIO SDK gets called as original delegate)
+     */
+
+    func test_onPushAction_givenMultiplePushClickHandlers_thirdPartySdkCallsCompletionHandlerTwice_expectSdkDoesNotCrash() {
+        let givenPush = PushNotificationStub.getPushNotSentFromCIO()
+        let givenOtherPushHandler = PushEventHandlerMock()
+        let givenPushClickAction = PushNotificationActionStub(push: givenPush, didClickOnPush: true)
+
+        let expectOtherClickHandlerHandlesPush = expectation(description: "Other push handler should handle push.")
+        expectOtherClickHandlerHandlesPush.expectedFulfillmentCount = 1 // the other push click handler should only be called once, indicating an infinite loop is not created.
+        givenOtherPushHandler.onPushActionClosure = { _, onComplete in
+            expectOtherClickHandlerHandlesPush.fulfill()
+
+            onComplete() // First, call the completion handler.
+            self.performPushAction(givenPushClickAction, withCompletionHandler: onComplete) // Second, the CIO SDK will call it when it is called.
+        }
+        addOtherPushEventHandler(givenOtherPushHandler)
+
+        performPushAction(givenPushClickAction)
+
+        waitForExpectations()
+
+        // If the test succeeds without crashing and all expectations fulfilled, it's a successful test.
+    }
+
+    func test_shouldDisplayPushAppInForeground_givenMultiplePushClickHandlers_thirdPartySdkCallsCompletionHandlerTwice_expectSdkDoesNotCrash() {
+        let givenPush = PushNotificationStub.getPushNotSentFromCIO()
+        let givenOtherPushHandler = PushEventHandlerMock()
+
+        let expectOtherClickHandlerHandlesPush = expectation(description: "Other push handler should handle push.")
+        expectOtherClickHandlerHandlesPush.expectedFulfillmentCount = 1 // the other push click handler should only be called once, indicating an infinite loop is not created.
+        givenOtherPushHandler.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            expectOtherClickHandlerHandlesPush.fulfill()
+
+            onComplete(false) // First, call the completion handler.
+            self.sendPushEventShouldShowPushAppInForeground(givenPush, withCompletionHandler: onComplete) // Second, the CIO SDK will call it when it is called.
+        }
+        addOtherPushEventHandler(givenOtherPushHandler)
+
+        sendPushEventShouldShowPushAppInForeground(givenPush)
+
+        waitForExpectations()
+
+        // If the test succeeds without crashing and all expectations fulfilled, it's a successful test.
+    }
+
     // MARK: local push notification support
 
     func test_givenClickOnLocalPush_expectEventHandled() {
@@ -279,9 +352,29 @@ extension AutomaticPushClickedIntegrationTest {
     }
 
     func performPushAction(_ pushAction: PushNotificationAction, withCompletionHandler completionHandler: @escaping () -> Void) {
-        pushEventHandler.onPushAction(pushAction, completionHandler: {
-            completionHandler()
-        })
+        pushEventHandler.onPushAction(pushAction, completionHandler: completionHandler)
+    }
+
+    func sendPushEventShouldShowPushAppInForeground(_ push: PushNotification, expectToCallCompletionHandler: Bool = true) {
+        // Note: It's important that we test that the `withContentHandler` callback function gets called either by our SDK (when we handle it), or the 3rd party handler.
+        //       We add an expectation to verify that 1 push click handler calls it.
+        let expectCompletionHandlerCalled = expectation(description: "Expect completion handler called by a click handler")
+
+        if expectToCallCompletionHandler {
+            expectCompletionHandlerCalled.expectedFulfillmentCount = 1 // Test will fail if called 2+ times which could indicate a bug because only 1 push click handler should be calling it.
+        } else {
+            expectCompletionHandlerCalled.isInverted = true
+        }
+
+        sendPushEventShouldShowPushAppInForeground(push) { _ in
+            expectCompletionHandlerCalled.fulfill()
+        }
+
+        waitForExpectations(for: [expectCompletionHandlerCalled])
+    }
+
+    func sendPushEventShouldShowPushAppInForeground(_ push: PushNotification, withCompletionHandler completionHandler: @escaping (Bool) -> Void) {
+        pushEventHandler.shouldDisplayPushAppInForeground(push, completionHandler: completionHandler)
     }
 
     func addOtherPushEventHandler(_ pushEventHandler: PushEventHandler) {