Check Registration (#397)

* Check Registration

• In rare cases, if APNS does not respond with a push token or an error, our SDK was not correctly registering the user regardless.
• This commit adds an additional check to make sure that the user get registered and gets a OneSignal user ID.

* Remove Delay Selector

• Removed the 'checkRegistrationStatus' method from the previous commit as it is unnecessary
• Instead, this PR now uses an NSDate instance that gets instantiated when the SDK is initialized, and uses it to ensure that if APNS does not respond within 25 seconds, it will register with OS anyways.
• This reduces network traffic to onesignal servers.

* Add Test Case

• Adds an integration test to make sure that the SDK will register with OneSignal if APNS doesn't respond within a set amount of time
• Fixes a typo (pendingRegiseterBlock in UIApplicationOverrider)

Author: Nightsd01

iOS_SDK/OneSignalSDK/Source/OneSignal.m

@@ -69,20 +69,6 @@
 #import "OneSignalCommonDefines.h"
 #import "DelayedInitializationParameters.h"
 
-#define NOTIFICATION_TYPE_NONE 0
-#define NOTIFICATION_TYPE_BADGE 1
-#define NOTIFICATION_TYPE_SOUND 2
-#define NOTIFICATION_TYPE_ALERT 4
-#define NOTIFICATION_TYPE_ALL 7
-
-#define ERROR_PUSH_CAPABLILITY_DISABLED    -13
-#define ERROR_PUSH_DELEGATE_NEVER_FIRED    -14
-#define ERROR_PUSH_SIMULATOR_NOT_SUPPORTED -15
-#define ERROR_PUSH_UNKNOWN_APNS_ERROR      -16
-#define ERROR_PUSH_OTHER_3000_ERROR        -17
-#define ERROR_PUSH_NEVER_PROMPTED          -18
-#define ERROR_PUSH_PROMPT_NEVER_ANSWERED   -19
-
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wundeclared-selector"
 
@@ -169,6 +155,11 @@ @implementation OneSignal
 static BOOL delayedInitializationForPrivacyConsent = false;
 DelayedInitializationParameters *delayedInitParameters;
 
+//used to ensure registration occurs even if APNS does not respond
+static NSDate *initializationTime;
+static NSTimeInterval maxApnsWait = APNS_TIMEOUT;
+static NSTimeInterval reattemptRegistrationInterval = REGISTRATION_DELAY_SECONDS;
+
 //the iOS Native SDK will use the plist flag to enable privacy consent
 //however wrapper SDK's will use a method call before initialization
 //this boolean flag is switched on to enable this behavior
@@ -351,6 +342,14 @@ + (void) setWaitingForApnsResponse:(BOOL)value {
     waitingForApnsResponse = value;
 }
 
+// Used for testing purposes to decrease the amount of time the
+// SDK will spend waiting for a response from APNS before it
+// gives up and registers with OneSignal anyways
++ (void)setDelayIntervals:(NSTimeInterval)apnsMaxWait withRegistrationDelay:(NSTimeInterval)registrationDelay {
+    reattemptRegistrationInterval = registrationDelay;
+    maxApnsWait = apnsMaxWait;
+}
+
 + (void)clearStatics {
     app_id = nil;
     _osNotificationSettings = nil;
@@ -368,6 +367,9 @@ + (void)clearStatics {
     _permissionStateChangesObserver = nil;
 
     didCallDownloadParameters = false;
+    
+    maxApnsWait = APNS_TIMEOUT;
+    reattemptRegistrationInterval = REGISTRATION_DELAY_SECONDS;
 }
 
 // Set to false as soon as it's read.
@@ -398,6 +400,8 @@ + (id)initWithLaunchOptions:(NSDictionary*)launchOptions appId:(NSString*)appId
 + (id)initWithLaunchOptions:(NSDictionary*)launchOptions appId:(NSString*)appId handleNotificationReceived:(OSHandleNotificationReceivedBlock)receivedCallback handleNotificationAction:(OSHandleNotificationActionBlock)actionCallback settings:(NSDictionary*)settings {
     NSLog(@"Called init with app ID: %@", appId);
 
+    initializationTime = [NSDate date];
+    
     //Some wrapper SDK's call init multiple times and pass nil/NSNull as the appId on the first call
     //the app ID is required to download parameters, so do not download params until the appID is provided
     if (!didCallDownloadParameters && appId != nil && appId != (id)[NSNull null])
@@ -414,7 +418,7 @@ + (id)initWithLaunchOptions:(NSDictionary*)launchOptions appId:(NSString*)appId
 
     let success = [self initAppId:appId
                  withUserDefaults:userDefaults
-                     withSettings:settings];
+                        withSettings:settings];
 
     if (!success)
         return self;
@@ -476,14 +480,17 @@ + (id)initWithLaunchOptions:(NSDictionary*)launchOptions appId:(NSString*)appId
                 self.inFocusDisplayType = (OSNotificationDisplayType)IFDSetting.integerValue;
         }
 
+        
         if (self.currentSubscriptionState.userId)
             [self registerUser];
         else {
             [self.osNotificationSettings getNotificationPermissionState:^(OSPermissionState *state) {
-                if (state.answeredPrompt)
+                
+                if (state.answeredPrompt) {
                     [self registerUser];
-                else
-                    [self performSelector:@selector(registerUser) withObject:nil afterDelay:30.0f];
+                } else {
+                    [self registerUserAfterDelay];
+                }
             }];
         }
     }
@@ -1225,9 +1232,10 @@ + (void)updateDeviceToken:(NSString*)deviceToken onSuccess:(OSResultSuccessBlock
         // The goal is to only have 1 server call.
         [self.osNotificationSettings getNotificationPermissionState:^(OSPermissionState *status) {
             if (status.answeredPrompt)
-                [OneSignal registerUser];
-            else
+                [self registerUser];
+            else {
                 [self registerUserAfterDelay];
+            }
         }];
         return;
     }
@@ -1297,7 +1305,7 @@ +(BOOL)shouldRegisterNow {
 
 + (void)registerUserAfterDelay {
     [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(registerUser) object:nil];
-    [OneSignalHelper performSelector:@selector(registerUser) onMainThreadOnObject:self withObject:nil afterDelay:30.0f];
+    [OneSignalHelper performSelector:@selector(registerUser) onMainThreadOnObject:self withObject:nil afterDelay:reattemptRegistrationInterval];
 }
 
 static dispatch_queue_t serialQueue;
@@ -1307,12 +1315,14 @@ + (dispatch_queue_t) getRegisterQueue {
 }
 
 + (void)registerUser {
-    
     // return if the user has not granted privacy permissions
     if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:nil])
         return;
 
-    if (waitingForApnsResponse) {
+    // 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) {
         [self registerUserAfterDelay];
         return;
     }

iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h

@@ -64,4 +64,26 @@
 
 #define ONESIGNAL_SUPPORTED_ATTACHMENT_TYPES @[@"aiff", @"wav", @"mp3", @"mp4", @"jpg", @"jpeg", @"png", @"gif", @"mpeg", @"mpg", @"avi", @"m4a", @"m4v"]
 
+// Notification types
+#define NOTIFICATION_TYPE_NONE 0
+#define NOTIFICATION_TYPE_BADGE 1
+#define NOTIFICATION_TYPE_SOUND 2
+#define NOTIFICATION_TYPE_ALERT 4
+#define NOTIFICATION_TYPE_ALL 7
+
+#define ERROR_PUSH_CAPABLILITY_DISABLED    -13
+#define ERROR_PUSH_DELEGATE_NEVER_FIRED    -14
+#define ERROR_PUSH_SIMULATOR_NOT_SUPPORTED -15
+#define ERROR_PUSH_UNKNOWN_APNS_ERROR      -16
+#define ERROR_PUSH_OTHER_3000_ERROR        -17
+#define ERROR_PUSH_NEVER_PROMPTED          -18
+#define ERROR_PUSH_PROMPT_NEVER_ANSWERED   -19
+
+// Registration delay
+#define REGISTRATION_DELAY_SECONDS 30.0
+
+// How long the SDK will wait for APNS to respond
+// before registering the user anyways
+#define APNS_TIMEOUT 25.0
+
 #endif /* OneSignalCommonDefines_h */

iOS_SDK/OneSignalSDK/UnitTests/Shadows/UIApplicationOverrider.h

@@ -31,6 +31,8 @@
 @interface UIApplicationOverrider : NSObject
 +(void)reset;
 
++(void)setBlockApnsResponse:(BOOL)block;
+
 +(void)setCurrentUIApplicationState:(UIApplicationState)value;
 
 +(UILocalNotification*)lastUILocalNotification;

iOS_SDK/OneSignalSDK/UnitTests/Shadows/UIApplicationOverrider.m

@@ -45,7 +45,10 @@ @implementation UIApplicationOverrider
 
 static UIUserNotificationSettings* lastUIUserNotificationSettings;
 
-static BOOL pendingRegiseterBlock;
+static BOOL pendingRegisterBlock;
+
+//mimics no response from APNS
+static BOOL blockApnsResponse;
 
 + (void)load {
     injectToProperClass(@selector(overrideRegisterForRemoteNotifications), @selector(registerForRemoteNotifications), @[], [UIApplicationOverrider class], [UIApplication class]);
@@ -58,8 +61,9 @@ + (void)load {
 }
 
 +(void)reset {
+    blockApnsResponse = false;
     lastUILocalNotification = nil;
-    pendingRegiseterBlock = false;
+    pendingRegisterBlock = false;
     shouldFireDeviceToken = true;
     calledRegisterForRemoteNotifications = false;
     calledCurrentUserNotificationSettings = false;
@@ -87,6 +91,10 @@ +(void) setDidFailRegistarationErrorCode:(NSInteger)value {
     didFailRegistarationErrorCode = value;
 }
 
++(void)setBlockApnsResponse:(BOOL)block {
+    blockApnsResponse = true;
+}
+
 // Keeps UIApplicationMain(...) from looping to continue to the next line.
 - (void) override_run {
     NSLog(@"override_run!!!!!!");
@@ -106,15 +114,15 @@ + (void)helperCallDidRegisterForRemoteNotificationsWithDeviceToken {
         if (!shouldFireDeviceToken)
             return;
 
-        pendingRegiseterBlock = true;
+        pendingRegisterBlock = true;
     });
 }
 
 // callPendingApplicationDidRegisterForRemoteNotificaitonsWithDeviceToken
 + (void)runBackgroundThreads {
-    if (!pendingRegiseterBlock || currentUIApplicationState != UIApplicationStateActive)
+    if (!pendingRegisterBlock || currentUIApplicationState != UIApplicationStateActive || blockApnsResponse)
         return;
-    pendingRegiseterBlock = false;
+    pendingRegisterBlock = false;
 
     id app = [UIApplication sharedApplication];
     id appDelegate = [[UIApplication sharedApplication] delegate];

iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.h

@@ -43,6 +43,12 @@ NSString * serverUrlWithPath(NSString *path);
 
 @end
 
+// Expose OneSignal test methods
+@interface OneSignal (UN_extra)
++ (dispatch_queue_t) getRegisterQueue;
++ (void)setDelayIntervals:(NSTimeInterval)apnsMaxWait withRegistrationDelay:(NSTimeInterval)registrationDelay;
+@end
+
 // START - Start Observers
 
 @interface OSPermissionStateTestObserver : NSObject<OSPermissionObserver> {

iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.m

@@ -48,10 +48,6 @@
     return [NSString stringWithFormat:@"%@%@%@", SERVER_URL, API_VERSION, path];
 }
 
-@interface OneSignal (UN_extra)
-+ (dispatch_queue_t) getRegisterQueue;
-@end
-
 @implementation UnitTestCommonMethods
 
 // Runs any blocks passed to dispatch_async()

iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m

@@ -2007,4 +2007,48 @@ - (void)testFileExtensionPrioritizesURLFileExtension {
     XCTAssertTrue([downloadedGifFilename.supportedFileExtension isEqualToString:@"gif"]);
 }
 
+//integration test that makes sure that if APNS doesn't respond within a certain
+//window of time, the SDK will register the user with onesignal anyways.
+- (void)testRegistersAfterNoApnsResponse {
+    
+    // simulates no response from APNS
+    [UIApplicationOverrider setBlockApnsResponse:true];
+    
+    // Normally the SDK would wait at least 25 seconds to get a response
+    // and 30 seconds between registration attempts.
+    // This would be too long for a test, so we artificially set the
+    // delay times to be very very short.
+    [OneSignal setDelayIntervals:0.001f withRegistrationDelay:0.002f];
+    
+    // add the subscription observer
+    OSSubscriptionStateTestObserver* observer = [OSSubscriptionStateTestObserver new];
+    [OneSignal addSubscriptionObserver:observer];
+    
+    // create an expectation
+    XCTestExpectation *expectation = [self expectationWithDescription:@"onesignal_registration_wait"];
+    expectation.expectedFulfillmentCount = 1;
+    
+    // do not answer the prompt (apns will not respond)
+    [UnitTestCommonMethods setCurrentNotificationPermissionAsUnanswered];
+    
+    [OneSignal initWithLaunchOptions:nil
+                               appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba"
+            handleNotificationAction:nil
+                            settings:@{kOSSettingsKeyAutoPrompt: @false}];
+    
+    [UnitTestCommonMethods runBackgroundThreads];
+    
+    // wait for the registration to be re-attempted.
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.05 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+        [expectation fulfill];
+    });
+    
+    [self waitForExpectations:@[expectation] timeout:0.1];
+    
+    // If APNS didn't respond within X seconds, the SDK
+    // should have registered the user with OneSignal
+    // and should have a user ID
+    XCTAssertTrue(observer->last.to.userId != nil);
+}
+
 @end