Merge pull request #728 from OneSignal/fix_register_user_race_condition

Fix register user race condition
OneSignal · Aug 25, 2020 · e1b8940 · e1b8940
2 parents 965fa40 + e721cdf
commit e1b8940
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 8 deletions.
diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m
@@ -1557,12 +1557,15 @@ + (BOOL)shouldRegisterNow {
         return false;
 
     // Don't make a 2nd on_session if have in inflight one
+    onesignal_Log(ONE_S_LL_VERBOSE, [NSString stringWithFormat:@"shouldRegisterNow:waitingForOneSReg: %d", waitingForOneSReg]);
     if (waitingForOneSReg)
         return false;
 
+    onesignal_Log(ONE_S_LL_VERBOSE, [NSString stringWithFormat:@"shouldRegisterNow:isImmediatePlayerCreateOrOnSession: %d", [self isImmediatePlayerCreateOrOnSession]]);
     if ([self isImmediatePlayerCreateOrOnSession])
         return true;
 
+    onesignal_Log(ONE_S_LL_VERBOSE, [NSString stringWithFormat:@"shouldRegisterNow:isOnSessionSuccessfulForCurrentState: %d", isOnSessionSuccessfulForCurrentState]);
     if (isOnSessionSuccessfulForCurrentState)
         return false;
 
@@ -1583,6 +1586,7 @@ + (BOOL)shouldRegisterNow {
 }
 
 + (void)registerUserAfterDelay {
+    [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"registerUserAfterDelay"];
     [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(registerUser) object:nil];
     [OneSignalHelper performSelector:@selector(registerUser) onMainThreadOnObject:self withObject:nil afterDelay:reattemptRegistrationInterval];
 }
@@ -1598,23 +1602,44 @@ + (void)registerUser {
     if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:nil])
         return;
 
-    // We should delay registration if we are waiting on APNS
-    // But if APNS hasn't responded within 30 seconds,
-    // we should continue and register the user.
-    if (waitingForApnsResponse && initializationTime && [[NSDate date] timeIntervalSinceDate:initializationTime] < maxApnsWait) {
+    if ([self shouldRegisterUserAfterDelay]) {
         [self registerUserAfterDelay];
         return;
     }
 
+    [self registerUserNow];
+}
+
++(void)registerUserNow {
+    [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"registerUserNow"];
+
     if (!serialQueue)
         serialQueue = dispatch_queue_create("com.onesignal.regiseruser", DISPATCH_QUEUE_SERIAL);
-
-   dispatch_async(serialQueue, ^{
+    
+    dispatch_async(serialQueue, ^{
         [self registerUserInternal];
-    });
+     });
+}
+
+// We should delay registration if we are waiting on APNS
+// But if APNS hasn't responded within 30 seconds (maxApnsWait),
+// we should continue and register the user.
++ (BOOL)shouldRegisterUserAfterDelay {
+    [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:[NSString stringWithFormat:@"registerUser:waitingForApnsResponse: %d", waitingForApnsResponse]];
+    [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:[NSString stringWithFormat:@"registerUser:initializationTime: %@", initializationTime]];
+
+    // If there isn't an initializationTime yet then the SDK hasn't finished initializing so we should delay
+    if (!initializationTime)
+        return true;
+
+    if (!waitingForApnsResponse)
+        return false;
+
+    return [[NSDate date] timeIntervalSinceDate:initializationTime] < maxApnsWait;
 }
 
 + (void)registerUserInternal {
+    [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"registerUserInternal"];
 
     // return if the user has not granted privacy permissions
     if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:nil])

diff --git a/iOS_SDK/OneSignalSDK/UnitTests/Shadows/UIApplicationOverrider.m b/iOS_SDK/OneSignalSDK/UnitTests/Shadows/UIApplicationOverrider.m
@@ -98,7 +98,7 @@ +(void) setDidFailRegistarationErrorCode:(NSInteger)value {
 }
 
 +(void)setBlockApnsResponse:(BOOL)block {
-    blockApnsResponse = true;
+    blockApnsResponse = block;
 }
 
 + (void)setAPNSTokenLength:(int)tokenLength {

diff --git a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m
@@ -859,6 +859,35 @@ - (void)testPromptedButNeveranswerNotificationPrompt {
     XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"notification_types"], @-19);
 }
 
+// This test covers migrating to OneSignal with the following senario;
+// 1. App was released publicly to the AppStore with push on with another provider.
+// 2. User imports all existing push tokens into OneSignal.
+// 3. OneSignal is added to their app.
+// 4. Ensure that identifier is always send with the player create, to prevent duplicated players
+- (void)testNotificationPermissionsAcceptedBeforeAddingOneSiganl_waitsForAPNSTokenBeforePlayerCreate {
+    // 1. Set that notification permissions are already enabled.
+    UNUserNotificationCenterOverrider.notifTypesOverride = 7;
+    UNUserNotificationCenterOverrider.authorizationStatus = [NSNumber numberWithInteger:UNAuthorizationStatusAuthorized];
+
+    // 2. Setup delay of APNs reaponse
+    [UIApplicationOverrider setBlockApnsResponse:true];
+
+    // 3. Init OneSignal
+    [UnitTestCommonMethods initOneSignalAndThreadWait];
+    [NSObjectOverrider runPendingSelectors];
+
+    // 4. Don't make a network call right away
+    XCTAssertNil(OneSignalClientOverrider.lastHTTPRequest);
+
+    // 5. Simulate APNs now giving us a push token
+    [UIApplicationOverrider setBlockApnsResponse:false];
+    [UnitTestCommonMethods runBackgroundThreads];
+
+    // 6. Ensure we registered with push token and it has the correct notification_types
+    XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"notification_types"], @15);
+    XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"identifier"], @"0000000000000000000000000000000000000000000000000000000000000000");
+}
+
 - (void)testNotificationTypesWhenAlreadyAcceptedWithAutoPromptOffOnFristStartPreIos10 {
     OneSignalHelperOverrider.mockIOSVersion = 8;
     [UnitTestCommonMethods setCurrentNotificationPermission:true];