fix: forward push received app in foreground events to 3rd party call…

…backs

Part of: https://linear.app/customerio/issue/MBL-138/[bug]-ios-customers-receive-callbacks-from-3rd-party-sdks-for-pushes

When a push is received while app in foreground, send the push event to other push event handlers in host app. Giving customers the opportunity to process the push event. Previously, only non-CIO pushes would be sent to other push event handlers.

Notes:

* CIO SDK waits for all push handlers to call completion handler for push processing. Not calling completion handler may result in push not being shown while app is in foreground.

Push event handlers must call async completion handler for CIO SDK to handle push events. Test function `test_givenMultiplePushHandlers_givenCioPushDelivered_givenOtherPushHandlerDoesNotCallCompletionHandler_expectCompletionHandlerDoesNotGetCalled` reproduces scenario.

Waiting for completion handler prevents premature termination of push processing logic by iOS. Apple documentation states completion handler must be called after processing push event. https://developer.apple.com/documentation/usernotifications/unusernotificationcenterdelegate/usernotificationcenter(_:willpresent:withcompletionhandler:)

* The SDK now determines if a push should be shown for non-CIO pushes and fixes bug in `PushEventHandlerProxy`.

This change had to be made as part of waiting for all push handlers to complete async operation.

Behavior before: The push handler that called the async completion handler first would be the one that decides if the push is displayed on the device.

After behavior: The SDK uses Apple's default of not displaying push. The SDK will display the push if at least 1 of the push handlers decides to display push.

This aligns with Apple's default behavior of not displaying a push unless at least one push handler decides to display it.https://developer.apple.com/documentation/usernotifications/unusernotificationcenterdelegate/usernotificationcenter(_:willpresent:withcompletionhandler:)

commit-id:91fe252e

Sources/MessagingPush/PushHandling/PushEventHandlerProxy.swift

@@ -65,8 +65,28 @@ class PushEventHandlerProxyImpl: PushEventHandlerProxy {
             return
         }
 
-        nestedDelegates.forEach { _, delegate in
-            delegate.shouldDisplayPushAppInForeground(push, completionHandler: completionHandler)
+        Task {
+            // 2+ other push event handlers may exist in app. We need to decide if a push should be displayed or not, by combining all the results from all other push handlers.
+            // To do that, we start with Apple's default value of: do not display.
+            // If any of the handlers return result indicating push should be displayed, we return true.
+            // Apple docs: https://developer.apple.com/documentation/usernotifications/unusernotificationcenterdelegate/usernotificationcenter(_:willpresent:withcompletionhandler:)
+            var shouldDisplayPush = false
+
+            // Wait for all other push event handlers to finish before calling the completion handler.
+            // 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
+                    delegate.shouldDisplayPushAppInForeground(push, completionHandler: { delegateShouldDisplayPushResult in
+                        if delegateShouldDisplayPushResult {
+                            shouldDisplayPush = true
+                        }
+
+                        continuation.resume()
+                    })
+                }
+            }
+            // After the loop finishes, call the completion handler to indicate the event has been fully processed by all delegates.
+            completionHandler(shouldDisplayPush)
         }
     }
 }

Sources/MessagingPush/PushHandling/iOSPushEventListener.swift

@@ -83,9 +83,17 @@ class IOSPushEventListener: PushEventHandler {
 
         logger.debug("Push came from CIO. Handle the willPresent event on behalf of the customer.")
 
-        let shouldShowPush = moduleConfig.showPushAppInForeground
+        // Forward event to other push handlers so they can receive a callback about this push event.
+        pushEventHandlerProxy.shouldDisplayPushAppInForeground(push, completionHandler: { _ in
+            // When this block of code executes, the customer is done processing the push event.
+
+            // Because push came from CIO, ignore the return result of other push handlers.
+            // Determine if CIO push should be shown from SDK config
+            let shouldShowPush = self.moduleConfig.showPushAppInForeground
 
-        // Call the completionHandler so customer does not need to. The push came from CIO, so it gets handled by the CIO SDK.
-        completionHandler(shouldShowPush)
+            // The push came from CIO, so it gets handled by the CIO SDK.
+            // Calling the completion handler indicates to the OS that we are done processing the push.
+            completionHandler(shouldShowPush)
+        })
     }
 }

Tests/MessagingPush/PushHandling/AutomaticPushDeliveredAppInForegroundTest.swift

@@ -4,15 +4,14 @@ import Foundation
 import SharedTests
 import XCTest
 
+// When a push is delivered to an iOS device and the app is in the foreground, iOS gives the app the option to either display that push or to not display that push.
 class AutomaticPushDeliveredAppInForegrondTest: IntegrationTest {
     private var pushEventHandler: PushEventHandler!
 
     private let pushClickHandler = PushClickHandlerMock()
     private let pushEventHandlerProxy = PushEventHandlerProxyImpl()
 
-    // MARK: test handling when push delivered, app in foreground
-
-    // When a push is delivered to an iOS device and the app is in the foreground, iOS gives the app the option to either display that push or to not display that push.
+    // MARK: SDK configuration behavior
 
     func test_givenCioPushDelivered_givenSdkConfigDisplayPush_expectPushDisplayed() {
         configureSdk(shouldDisplayPushAppInForeground: true)
@@ -21,7 +20,7 @@ class AutomaticPushDeliveredAppInForegrondTest: IntegrationTest {
 
         let didPushGetDisplayed = deliverPush(givenPush)
 
-        XCTAssertTrue(didPushGetDisplayed)
+        XCTAssertEqual(didPushGetDisplayed, true)
     }
 
     func test_givenCioPushDelivered_givenSdkConfigDoNotDisplayPush_expectPushNotDisplayed() {
@@ -31,10 +30,24 @@ class AutomaticPushDeliveredAppInForegrondTest: IntegrationTest {
 
         let didPushGetDisplayed = deliverPush(givenPush)
 
-        XCTAssertFalse(didPushGetDisplayed)
+        XCTAssertEqual(didPushGetDisplayed, false)
     }
 
-    func test_givenMultiplePushHandlers_givenNonCioPushDelivered_expectHandleEvent() {
+    // MARK: cio SDK only push event handler in app
+
+    func test_givenCioOnlyPushHandler_givenNonCioPushDelivered_expectHandleEvent() {
+        configureSdk(shouldDisplayPushAppInForeground: true)
+
+        let givenPush = PushNotificationStub.getPushNotSentFromCIO()
+
+        let didPushGetDisplayed = deliverPush(givenPush)
+
+        XCTAssertEqual(didPushGetDisplayed, true)
+    }
+
+    // MARK: multiple push handlers in app
+
+    func test_givenOtherPushHandlers_givenNonCioPushDelivered_expectHandleEvent() {
         configureSdk(shouldDisplayPushAppInForeground: false)
         let givenPush = PushNotificationStub.getPushNotSentFromCIO()
         var otherPushHandlerCalled = false
@@ -52,14 +65,81 @@ class AutomaticPushDeliveredAppInForegrondTest: IntegrationTest {
         XCTAssertTrue(otherPushHandlerCalled)
     }
 
-    func test_givenCioOnlyPushHandler_givenNonCioPushDelivered_expectHandleEvent() {
+    func test_givenOtherPushHandlers_givenCioPushDelivered_expectOtherPushHandlerGetsCallback_expectIgnoreResultOfOtherHandler() {
         configureSdk(shouldDisplayPushAppInForeground: true)
+        let givenPush = PushNotificationStub.getPushSentFromCIO()
+        let expectOtherPushHandlerCallbackCalled = expectation(description: "Expect other push handler callback called")
 
-        let givenPush = PushNotificationStub.getPushNotSentFromCIO()
+        let givenOtherPushHandler = PushEventHandlerMock()
+        givenOtherPushHandler.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            // We expect that other push handler gets callback of push event from CIO push
+            expectOtherPushHandlerCallbackCalled.fulfill()
+
+            // We expect that this return result is ignored. The CIO SDK config setting is used instead.
+            onComplete(false)
+        }
+        addOtherPushEventHandler(givenOtherPushHandler)
 
         let didPushGetDisplayed = deliverPush(givenPush)
 
-        XCTAssertTrue(didPushGetDisplayed)
+        waitForExpectations()
+
+        // Check that the decision to display push was determined by SDK config and not from the return result of 3rd party callback.
+        XCTAssertEqual(didPushGetDisplayed, true)
+    }
+
+    // Important to test that 2+ 3rd party push handlers for some use cases.
+    func test_givenMultiplePushHandlers_givenCioPushDelivered_expectOtherPushHandlersGetCallback() {
+        configureSdk(shouldDisplayPushAppInForeground: true)
+        let givenPush = PushNotificationStub.getPushSentFromCIO()
+        let expectOtherPushHandlerCallbackCalled = expectation(description: "Expect other push handler callback called")
+        expectOtherPushHandlerCallbackCalled.expectedFulfillmentCount = 2
+
+        // In order to add 2+ push handlers to SDK, each class needs to have a unique name.
+        // The SDK only accepts unique push event handlers. Creating this class makes each push handler unique.
+        class PushEventHandlerMock2: PushEventHandlerMock {}
+
+        let givenOtherPushHandler1 = PushEventHandlerMock()
+        let givenOtherPushHandler2 = PushEventHandlerMock2()
+        givenOtherPushHandler1.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            expectOtherPushHandlerCallbackCalled.fulfill()
+
+            onComplete(true)
+        }
+        givenOtherPushHandler2.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            expectOtherPushHandlerCallbackCalled.fulfill()
+
+            onComplete(true)
+        }
+        addOtherPushEventHandler(givenOtherPushHandler1)
+        addOtherPushEventHandler(givenOtherPushHandler2)
+
+        _ = deliverPush(givenPush)
+
+        waitForExpectations()
+    }
+
+    // MARK: completion handler
+
+    func test_givenMultiplePushHandlers_givenCioPushDelivered_givenOtherPushHandlerDoesNotCallCompletionHandler_expectCompletionHandlerDoesNotGetCalled() {
+        configureSdk(shouldDisplayPushAppInForeground: true)
+
+        let expectOtherClickHandlerToGetCallback = expectation(description: "Receive a callback")
+        let givenPush = PushNotificationStub.getPushSentFromCIO()
+        let givenOtherPushHandler = PushEventHandlerMock()
+
+        givenOtherPushHandler.shouldDisplayPushAppInForegroundClosure = { _, _ in
+            // Do not call completion handler.
+            expectOtherClickHandlerToGetCallback.fulfill()
+        }
+        addOtherPushEventHandler(givenOtherPushHandler)
+
+        let didPushGetDisplayed = deliverPush(givenPush, expectToCallCompletionHandler: false)
+
+        waitForExpectations(for: [expectOtherClickHandlerToGetCallback])
+
+        // nil result means that completionHandler never got called and returned a result.
+        XCTAssertNil(didPushGetDisplayed)
     }
 }
 
@@ -78,12 +158,16 @@ extension AutomaticPushDeliveredAppInForegrondTest {
         )
     }
 
-    func deliverPush(_ push: PushNotification) -> Bool {
+    func deliverPush(_ push: PushNotification, expectToCallCompletionHandler: Bool = true) -> Bool? {
         // 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.
         let expectCompletionHandlerCalled = expectation(description: "Expect completion handler called by a handler")
-        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.
+        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
+        }
 
-        var returnValueFromPushHandler = false
+        var returnValueFromPushHandler: Bool?
 
         pushEventHandler.shouldDisplayPushAppInForeground(push) { shouldDisplayPush in
             returnValueFromPushHandler = shouldDisplayPush

Tests/MessagingPush/PushHandling/PushEventHandlerProxyTest.swift

@@ -1,19 +1,15 @@
 @testable import CioMessagingPush
-@testable import CioTracking
 import Foundation
 import SharedTests
 import XCTest
 
 class PushEventHandlerProxyTest: UnitTest {
-    var pushEventHandlerProxy: PushEventHandlerProxy!
-
-    private let deepLinkUtilMock = DeepLinkUtilMock()
-    private let customerIOMock = CustomerIOInstanceMock()
+    private var proxy: PushEventHandlerProxyImpl!
 
     override func setUp() {
         super.setUp()
 
-        pushEventHandlerProxy = PushEventHandlerProxyImpl()
+        proxy = PushEventHandlerProxyImpl()
     }
 
     // MARK: thread safety
@@ -43,10 +39,10 @@ class PushEventHandlerProxyTest: UnitTest {
                     completion()
                 }
             }
-            pushEventHandlerProxy.addPushEventHandler(delegate1)
-            pushEventHandlerProxy.addPushEventHandler(delegate2)
+            proxy.addPushEventHandler(delegate1)
+            proxy.addPushEventHandler(delegate2)
 
-            pushEventHandlerProxy.onPushAction(PushNotificationActionStub(push: PushNotificationStub.getPushSentFromCIO(), didClickOnPush: true)) {
+            proxy.onPushAction(PushNotificationActionStub(push: PushNotificationStub.getPushSentFromCIO(), didClickOnPush: true)) {
                 expectCompleteCallingAllDelegates.fulfill()
             }
 
@@ -56,4 +52,138 @@ class PushEventHandlerProxyTest: UnitTest {
             ], enforceOrder: true)
         }
     }
+
+    func test_shouldDisplayPushAppInForeground_ensureThreadSafetyCallingDelegates() {
+        runTest(numberOfTimes: 100) { // Ensure no race conditions by running test many times.
+            let givenPush = PushNotificationStub.getPushSentFromCIO()
+
+            let delegate1 = PushEventHandlerMock()
+            class PushEventHandlerMock2: PushEventHandlerMock {}
+            let delegate2 = PushEventHandlerMock2()
+
+            let expectDelegatesReceiveEvent = expectation(description: "delegate1 received event")
+            expectDelegatesReceiveEvent.expectedFulfillmentCount = 2 // 1 for each delegate. We do not care what order the delegates get called as long as all get called.
+            let expectCompleteCallingAllDelegates = expectation(description: "complete calling all delegates")
+
+            // When each delegate gets called, have them call the completion handler on different threads to test the proxy is thread safe.
+            delegate1.shouldDisplayPushAppInForegroundClosure = { _, completion in
+                expectDelegatesReceiveEvent.fulfill()
+
+                self.runOnMain {
+                    completion(false)
+                }
+            }
+            delegate2.shouldDisplayPushAppInForegroundClosure = { _, completion in
+                expectDelegatesReceiveEvent.fulfill()
+
+                self.runOnBackground {
+                    completion(true)
+                }
+            }
+            proxy.addPushEventHandler(delegate1)
+            proxy.addPushEventHandler(delegate2)
+
+            proxy.shouldDisplayPushAppInForeground(givenPush, completionHandler: { actualShouldDisplayPush in
+                // Assert that the 1 delegate that returns `true` is the return result.
+                XCTAssertTrue(actualShouldDisplayPush)
+
+                expectCompleteCallingAllDelegates.fulfill()
+            })
+
+            wait(for: [
+                expectDelegatesReceiveEvent,
+                expectCompleteCallingAllDelegates
+            ], enforceOrder: true)
+        }
+    }
+
+    // MARK: shouldDisplayPushAppInForeground
+
+    func test_shouldDisplayPushAppInForeground_givenNoHandlers_expectTrue() {
+        let push = PushNotificationStub.getPushSentFromCIO()
+        var actual: Bool!
+
+        proxy.shouldDisplayPushAppInForeground(push) { value in
+            actual = value
+        }
+
+        XCTAssertTrue(actual)
+    }
+
+    func test_shouldDisplayPushAppInForeground_givenOneHandler_expectReturnResultFromHandler() async throws {
+        let push = PushNotificationStub.getPushSentFromCIO()
+        var actual: Bool!
+
+        let handler = PushEventHandlerMock()
+        handler.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            onComplete(true)
+        }
+        proxy.addPushEventHandler(handler)
+
+        await waitForAsyncOperation { asyncComplete in
+            self.proxy.shouldDisplayPushAppInForeground(push) { value in
+                actual = value
+                asyncComplete()
+            }
+        }
+
+        XCTAssertTrue(actual)
+    }
+
+    // The SDK's logic to combine the return results is: return `false`, unless at least 1 push handler returns `true`.
+    // To test that, we will perform multiple checks in the test function and watch the results change after each check.
+    func test_shouldDisplayPushAppInForeground_givenMultipleHandlers_expectCombineReturnResults() async throws {
+        let push = PushNotificationStub.getPushSentFromCIO()
+        var actual: Bool!
+
+        // First, test that `false` is the default return result.
+
+        let handler1 = PushEventHandlerMock()
+        handler1.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            onComplete(false)
+        }
+        proxy.addPushEventHandler(handler1)
+
+        await waitForAsyncOperation { asyncComplete in
+            self.proxy.shouldDisplayPushAppInForeground(push) { value in
+                actual = value
+                asyncComplete()
+            }
+        }
+
+        XCTAssertFalse(actual)
+
+        // Next, add another push handler that's return result is `true`.
+        // We expect return result to now be `true`, since 1 handler returned `true`.
+        class PushEventHandlerMock2: PushEventHandlerMock {}
+        let handler2 = PushEventHandlerMock2()
+        handler2.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            onComplete(true)
+        }
+        proxy.addPushEventHandler(handler2)
+
+        await waitForAsyncOperation { asyncComplete in
+            self.proxy.shouldDisplayPushAppInForeground(push) { value in
+                actual = value
+                asyncComplete()
+            }
+        }
+        XCTAssertTrue(actual)
+
+        // Finally, check that once 1 handler returns `true`, the return result is always `true`.
+        class PushEventHandlerMock3: PushEventHandlerMock {}
+        let handler3 = PushEventHandlerMock3()
+        handler3.shouldDisplayPushAppInForegroundClosure = { _, onComplete in
+            onComplete(false)
+        }
+        proxy.addPushEventHandler(handler3)
+
+        await waitForAsyncOperation { asyncComplete in
+            self.proxy.shouldDisplayPushAppInForeground(push) { value in
+                actual = value
+                asyncComplete()
+            }
+        }
+        XCTAssertTrue(actual)
+    }
 }