Randomly delay notification received calls to control traffic volume #934
Conversation
We will call the contentHandler with the replacement content to display the notification, but we will not return from the function until confirmed deliveries are completed. This allows us to delay the confirmed delivery server call since the NSE will not be killed until the function finishes. Using dispatch_semaphore in the success/failure block to signal completion.
Ignoring deprecated warnings for existing unit tests The new unit test validates that the contentHandler is fired, and that the dispatch_semaphore is signaled within a second. We do not randomize the time sent for confirmed deliveries in unit tests. The dispatch_semaphore now has a timeout of 30 seconds instead of "forever"
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 3 of 4 files at r2, 2 of 4 files at r3, 4 of 4 files at r6.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @emawby and @Jeasmine)
iOS_SDK/OneSignalSDK/Source/OneSignal.m, line 2347 at r6 (raw file):
// Called from the app's Notification Service Extension. Calls contentHandler() to display the notification + (UNMutableNotificationContent*)didReceiveNotificationExtensionRequest:(UNNotificationRequest*)request withMutableNotificationContent:(UNMutableNotificationContent*)replacementContent withContentHandler:(void (^)(UNNotificationContent * _Nonnull))contentHandler {
nit, the spacing / new lines here look off and don't line up.
iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m, line 51 at r6 (raw file):
} + (UNMutableNotificationContent*)didReceiveNotificationExtensionRequest:(UNNotificationRequest*)request withMutableNotificationContent:(UNMutableNotificationContent*)replacementContent
it, the spacing / new lines here look off and don't line up.
iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m, line 81 at r6 (raw file):
contentHandler(replacementContent); dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); [self onNotificationReceived:receivedNotificationId withBlockingTask:semaphore];
Thinking we want to kick off this task as soon as possible in parallel with downloading the attachment. As downloading the image could take a while.
iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m, line 82 at r6 (raw file):
dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); [self onNotificationReceived:receivedNotificationId withBlockingTask:semaphore]; dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, 30 * NSEC_PER_SEC));
30 * NSEC_PER_SEC, can we make this a constant. Something like MAX_NSE_LIFETIME_SECOUNDS
Since the download media attachment is synchronous we need to kick it off after the confirmed delivery task but before we wait for the semaphore to complete. If a semaphore is not passed (we are using the old entry point) make sure to set delay to 0
There was a problem hiding this comment.
Reviewable status: 6 of 10 files reviewed, 3 unresolved discussions (waiting on @Jeasmine and @jkasten2)
iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m, line 51 at r6 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
it, the spacing / new lines here look off and don't line up.
Done.
iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m, line 81 at r6 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Thinking we want to kick off this task as soon as possible in parallel with downloading the attachment. As downloading the image could take a while.
Done.
iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m, line 82 at r6 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
30 * NSEC_PER_SEC, can we make this a constant. Something likeMAX_NSE_LIFETIME_SECOUNDS
Done.
There was a problem hiding this comment.
Reviewed 4 of 4 files at r7.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Jeasmine)
Notifications sent to a large number of subscribers can result in a very large number of confirmed deliveries hitting our backend at the same time. This PR randomly delays the confirmed delivery between 0 and 25 seconds. In order to do this we need have a blocking task on the NSE process until the confirmed delivery is sent. We don't want to delay the actual receipt of the notification, so this requires that OneSignal is passed the contentHandler from the NSE and calls it before waiting for the confirmed delivery to finish.
To achieve this I have created a new public method and marked the previous method as deprecated.
+ (UNMutableNotificationContent*)didReceiveNotificationExtensionRequest:(UNNotificationRequest* _Nonnull)request withMutableNotificationContent:(UNMutableNotificationContent* _Nullable)replacementContent withContentHandler:(void (^)(UNNotificationContent * _Nullable))contentHandler;OneSignal will now call
contentHandler(replacementContent)to trigger the notification display so this is no longer needed in the customer's NotificationService file. Any additional changes to the notification content will need to take place BEFORE calling OneSignal'sdidReceiveNotificationExtensionRequestmethod.This change is