Added null check for notificationId to fix issue #590 (#592)

* Created check for notificationId to fix #590
* Some notificationIds are coming through as null
* Most likely non-OneSignal notifications slipping through and causing a crash when adding the null notificationId to the received notificationId array
* Added a null check and unit test to make sure no null or blank string notificationIds are tracked for confirmed deliveries and outcomes

* We will now clean any notifications with invalid notification ids
* Each time we go to get the indirect notifications we must iterate over the cached received notifications
* While we iterate we clean the array as well and save once done

Author: mikechoch

iOS_SDK/OneSignalSDK/Source/OSOutcomesUtils.h

@@ -49,6 +49,7 @@
 
 // Methods for received notification ids within the notification limit and attribution window
 + (NSArray *)getCachedReceivedNotifications;
-+ (void)saveReceivedNotificationFromBackground:(NSString *)notificationId;
++ (void)saveReceivedNotifications:(NSArray *)notifications;
++ (void)saveReceivedNotificationFromBackground:(NSString * _Nonnull)notificationId;
 
 @end

iOS_SDK/OneSignalSDK/Source/OSOutcomesUtils.m

@@ -149,7 +149,7 @@ + (void)saveReceivedNotifications:(NSArray *)notifications {
 /*
  Saves a received indirect notification into the NSUSerDefaults at a limit equivalent to the indirect notification limit param
  */
-+ (void)saveReceivedNotificationFromBackground:(NSString * _Nullable)notificationId {
++ (void)saveReceivedNotificationFromBackground:(NSString * _Nonnull)notificationId {
     [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:[NSString stringWithFormat:@"saveReceivedNotificationFromBackground notificationId: %@",
                                                        notificationId]];
 

iOS_SDK/OneSignalSDK/Source/OneSignalNotificationServiceExtensionHandler.m

@@ -50,11 +50,13 @@ + (UNMutableNotificationContent*)didReceiveNotificationExtensionRequest:(UNNotif
     // Track receieved
     [OneSignalTrackFirebaseAnalytics trackReceivedEvent:payload];
 
-    // Track Receive Receipt
-    [OneSignal.receiveReceiptsController sendReceiveReceiptWithNotificationId:payload.notificationID];
-
-    // Save receieved notification id
-    [OSOutcomesUtils saveReceivedNotificationFromBackground:payload.notificationID];
+    let receivedNotificationId = payload.notificationID;
+    if (receivedNotificationId && ![receivedNotificationId isEqualToString:@""]) {
+        // Track confirmed delivery
+        [OneSignal.receiveReceiptsController sendReceiveReceiptWithNotificationId:receivedNotificationId];
+        // Save received notification id
+        [OSOutcomesUtils saveReceivedNotificationFromBackground:receivedNotificationId];
+    }
 
     // Action Buttons
     [self addActionButtonsToExtentionRequest:request

iOS_SDK/OneSignalSDK/Source/OneSignalSessionManager.m

@@ -204,22 +204,27 @@ - (OSSessionResult *)getSessionResult {
  Get the current notifications ids that influenced the session
  */
 - (NSArray *)getIndirectNotificationIds {
-    NSArray *receivedNotifications = [OSOutcomesUtils getCachedReceivedNotifications];
-    if (!receivedNotifications || [receivedNotifications count] == 0)
-        // Unattributed session
-        return nil;
+    NSMutableArray *receivedNotifications = [[NSMutableArray alloc] initWithArray:[OSOutcomesUtils getCachedReceivedNotifications]];
+    if (!receivedNotifications || receivedNotifications.count == 0)
+        return nil; // Unattributed session
 
     NSMutableArray *notificationsIds = [NSMutableArray new];
     NSInteger attributionWindowInSeconds = [OSOutcomesUtils getIndirectAttributionWindow] * 60;
     NSTimeInterval currentTime = [[NSDate date] timeIntervalSince1970];
 
-    for (OSIndirectNotification *notification in receivedNotifications) {
-        long difference = currentTime - notification.timestamp;
-        if (difference <= attributionWindowInSeconds) {
-            [notificationsIds addObject:notification.notificationId];
+    // Add only valid notifications within the attribution window
+    for (var i = 0; i < receivedNotifications.count; i++) {
+        OSIndirectNotification* notification = [receivedNotifications objectAtIndex:i];
+        if (notification.notificationId && ![notification.notificationId isEqualToString:@""]) {
+            long difference = currentTime - notification.timestamp;
+            if (difference <= attributionWindowInSeconds)
+                [notificationsIds addObject:notification.notificationId];
         }
+        else
+            [receivedNotifications removeObjectAtIndex:i--];
     }
-    
+    // Cache updated notifications as received, this removes notifications with invalid notification ids
+    [OSOutcomesUtils saveReceivedNotifications:receivedNotifications.copy];
     return notificationsIds;
 }
 

iOS_SDK/OneSignalSDK/UnitTests/OutcomeIntegrationTests.m

@@ -198,6 +198,42 @@ - (void)testDirectSession_overridesIndirectSession_andSendsOnFocus {
     XCTAssertEqualObjects(OneSignal.sessionManager.getNotificationIds, @[@"test_notification_2"]);
 }
 
+- (void)testSavingNullReceivedNotificationId {
+    // 1. Open app
+    [UnitTestCommonMethods initOneSignalAndThreadWait];
+    
+    // 2. Close the app for 31 seconds
+    [UnitTestCommonMethods backgroundApp];
+    [NSDateOverrider advanceSystemTimeBy:31];
+    
+    // 3. Receive 2 notifications, one blank id and one null id
+    [UnitTestCommonMethods receiveNotification:@"" wasOpened:NO];
+    [UnitTestCommonMethods receiveNotification:nil wasOpened:NO];
+    
+    // 4. Open app
+    [UnitTestCommonMethods foregroundApp];
+    [UnitTestCommonMethods initOneSignalAndThreadWait];
+    
+    // 5. Make sure the session is UNATTRIBUTED and has 0 notifications
+    XCTAssertEqual(OneSignal.sessionManager.getSession, UNATTRIBUTED);
+    XCTAssertEqual(OneSignal.sessionManager.getNotificationIds.count, 0);
+    
+    // 6. Close the app for 31 seconds
+    [UnitTestCommonMethods backgroundApp];
+    [NSDateOverrider advanceSystemTimeBy:31];
+    
+    // 7. Receive 1 notification
+    [UnitTestCommonMethods receiveNotification:@"test_notification_1" wasOpened:NO];
+    
+    // 8. Open app
+    [UnitTestCommonMethods foregroundApp];
+    [UnitTestCommonMethods initOneSignalAndThreadWait];
+    
+    // 9. Make sure the session is INDIRECT and has 1 notifications
+    XCTAssertEqual(OneSignal.sessionManager.getSession, INDIRECT);
+    XCTAssertEqual(OneSignal.sessionManager.getNotificationIds.count, 1);
+}
+
 - (void)testIndirectSession_afterReceiveingNotifications {
     // 1. Open app
     [UnitTestCommonMethods initOneSignalAndThreadWait];

iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.m

@@ -263,6 +263,10 @@ + (void)answerNotificationPrompt:(BOOL)accept {
 + (void)receiveNotification:(NSString *)notificationId wasOpened:(BOOL)opened {
     // Create notification content
     UNMutableNotificationContent *content = [[UNMutableNotificationContent alloc] init];
+    
+    if (!notificationId)
+        notificationId = @"";
+    
     content.userInfo = [self createNotificationUserInfo:notificationId];
 
     // Create notification request
@@ -271,10 +275,10 @@ + (void)receiveNotification:(NSString *)notificationId wasOpened:(BOOL)opened {
     // Entry point for the NSE
     [OneSignalNotificationServiceExtensionHandler didReceiveNotificationExtensionRequest:request withMutableNotificationContent:content];
 
-    [self handleNotificationReceived:notificationId messageDict:content.userInfo wasOpened:opened];
+    [self handleNotificationReceived:content.userInfo wasOpened:opened];
 }
 
-+ (void)handleNotificationReceived:(NSString*)notificationId messageDict:(NSDictionary*)messageDict wasOpened:(BOOL)opened {
++ (void)handleNotificationReceived:(NSDictionary*)messageDict wasOpened:(BOOL)opened {
     BOOL foreground = UIApplication.sharedApplication.applicationState != UIApplicationStateBackground;
     BOOL isActive = UIApplication.sharedApplication.applicationState == UIApplicationStateActive;