From 90c4d2485449be4988dc796a90844e29216969e6 Mon Sep 17 00:00:00 2001 From: Brad Hesse Date: Mon, 3 Dec 2018 17:15:14 -0800 Subject: [PATCH 1/4] Setting External ID's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Adds setExternalUserId() and removeExternalUserId() as public functions to the OneSignal SDK • Adds tests to verify the request is working correctly and the API request is correctly formatted • Adds a test to verify that removeExternalUserId() works the same except it sends an empty string (which is what the API wants) --- iOS_SDK/OneSignalSDK/Source/OneSignal.h | 3 ++ iOS_SDK/OneSignalSDK/Source/OneSignal.m | 44 ++++++++++++++++ iOS_SDK/OneSignalSDK/Source/Requests.h | 4 ++ iOS_SDK/OneSignalSDK/Source/Requests.m | 14 ++++++ iOS_SDK/OneSignalSDK/UnitTests/RequestTests.m | 10 +++- iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m | 50 +++++++++++++++++++ 6 files changed, 124 insertions(+), 1 deletion(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.h b/iOS_SDK/OneSignalSDK/Source/OneSignal.h index 3094348c1..4272344f2 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.h +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.h @@ -457,6 +457,9 @@ typedef void (^OSEmailSuccessBlock)(); + (void)setEmail:(NSString * _Nonnull)email; + (void)setEmail:(NSString * _Nonnull)email withEmailAuthHashToken:(NSString * _Nullable)hashToken; ++ (void)setExternalUserId:(NSString * _Nonnull)externalId; ++ (void)removeExternalUserId; + @end #pragma clang diagnostic pop diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m index a23e756f9..27d659bad 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.m +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.m @@ -192,6 +192,9 @@ @implementation OneSignal static BOOL providesAppNotificationSettings = false; +static BOOL performedOnSessionRequest = false; +static NSString *pendingExternalUserId; + static OSNotificationDisplayType _inFocusDisplayType = OSNotificationDisplayTypeInAppAlert; + (void)setInFocusDisplayType:(OSNotificationDisplayType)value { NSInteger op = value; @@ -386,6 +389,9 @@ + (void)clearStatics { maxApnsWait = APNS_TIMEOUT; reattemptRegistrationInterval = REGISTRATION_DELAY_SECONDS; + + performedOnSessionRequest = false; + pendingExternalUserId = nil; } // Set to false as soon as it's read. @@ -1476,6 +1482,14 @@ + (void)registerUserInternal { ONESIGNAL_VERSION, @"sdk", nil]; + // should be set to true even before the API request is finished + performedOnSessionRequest = true; + + if (pendingExternalUserId) { + dataDic[@"external_user_id"] = pendingExternalUserId; + pendingExternalUserId = nil; + } + if (deviceModel) dataDic[@"device_model"] = deviceModel; @@ -2283,6 +2297,36 @@ + (void)emailChangedWithNewEmailPlayerId:(NSString * _Nullable)emailPlayerId { }]; } ++ (void)setExternalUserId:(NSString * _Nonnull)externalId { + + // return if the user has not granted privacy permissions + if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:@"setExternalUserId:"]) + return; + + if (!performedOnSessionRequest) { + // will be sent as part of the registration/on_session request + pendingExternalUserId = externalId; + return; + } else if (!self.currentSubscriptionState.userId || !self.app_id) { + [self onesignal_Log:ONE_S_LL_WARN message:[NSString stringWithFormat:@"Attempted to set external-userID while %@ is not set", self.app_id == nil ? @"app ID" : @"OneSignal user ID"]]; + return; + } + + [OneSignalClient.sharedClient executeRequest:[OSRequestUpdateExternalUserId withUserId:externalId withOneSignalUserId:self.currentSubscriptionState.userId appId:self.app_id] onSuccess:nil onFailure:^(NSError *error) { + [self onesignal_Log:ONE_S_LL_ERROR message:[NSString stringWithFormat:@"Encountered an error while attempting to set external ID: %@", error.description]]; + }]; +} + ++ (void)removeExternalUserId { + + // return if the user has not granted privacy permissions + if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:@"removeExternalUserId"]) + return; + + [self setExternalUserId:@""]; +} + + @end // Swizzles UIApplication class to swizzling the following: diff --git a/iOS_SDK/OneSignalSDK/Source/Requests.h b/iOS_SDK/OneSignalSDK/Source/Requests.h index 486cb5462..fdf49352b 100644 --- a/iOS_SDK/OneSignalSDK/Source/Requests.h +++ b/iOS_SDK/OneSignalSDK/Source/Requests.h @@ -94,5 +94,9 @@ NS_ASSUME_NONNULL_END + (instancetype _Nonnull)withUserId:(NSString * _Nonnull)userId appId:(NSString * _Nonnull)appId state:(NSString * _Nonnull)state type:(NSNumber * _Nonnull)type activeTime:(NSNumber * _Nonnull)activeTime netType:(NSNumber * _Nonnull)netType emailAuthToken:(NSString * _Nullable)emailAuthHash; @end +@interface OSRequestUpdateExternalUserId : OneSignalRequest ++ (instancetype _Nonnull)withUserId:(NSString * _Nullable)externalId withOneSignalUserId:(NSString * _Nonnull)userId appId:(NSString * _Nonnull)appId; +@end + #endif /* Requests_h */ diff --git a/iOS_SDK/OneSignalSDK/Source/Requests.m b/iOS_SDK/OneSignalSDK/Source/Requests.m index 232a6f4a5..5dbfe6dd0 100644 --- a/iOS_SDK/OneSignalSDK/Source/Requests.m +++ b/iOS_SDK/OneSignalSDK/Source/Requests.m @@ -319,3 +319,17 @@ + (instancetype _Nonnull)withUserId:(NSString * _Nonnull)userId appId:(NSString return request; } @end + +@implementation OSRequestUpdateExternalUserId + ++ (instancetype _Nonnull)withUserId:(NSString * _Nullable)externalId withOneSignalUserId:(NSString *)userId appId:(NSString *)appId { + let request = [OSRequestUpdateExternalUserId new]; + NSLog(@"App ID: %@, external ID: %@", appId, externalId); + request.parameters = @{@"app_id" : appId, @"external_user_id" : externalId ?: @""}; + request.method = PUT; + request.path = [NSString stringWithFormat:@"players/%@", userId]; + + return request; +} + +@end diff --git a/iOS_SDK/OneSignalSDK/UnitTests/RequestTests.m b/iOS_SDK/OneSignalSDK/UnitTests/RequestTests.m index 17bf3c06f..f73bf6aee 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/RequestTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/RequestTests.m @@ -268,6 +268,14 @@ - (void)testOnFocus { XCTAssertTrue(checkHttpBody(secondRequest.urlRequest.HTTPBody, @{@"app_id" : testAppId, @"state" : @"test_state", @"type" : @1, @"active_time" : @2, @"net_type" : @3})); } - +- (void)testSendExternalUserId { + let request = [OSRequestUpdateExternalUserId withUserId:@"test_external" withOneSignalUserId:testUserId appId:testAppId]; + + let correctUrl = correctUrlWithPath([NSString stringWithFormat:@"players/%@", testUserId]); + + XCTAssertTrue([correctUrl isEqualToString:request.urlRequest.URL.absoluteString]); + + XCTAssertTrue(checkHttpBody(request.urlRequest.HTTPBody, @{@"app_id" : testAppId, @"external_user_id" : @"test_external"})); +} @end diff --git a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m index 51931ebab..096e15891 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m @@ -2152,4 +2152,54 @@ - (void)testInvalidButtonFormat { XCTAssertTrue(notification.actionButtons.count == 0); } +- (void)testSetExternalUserIdWithRegistration { + let testExternalId = @"i_am_a_test_external_id"; + + [OneSignal setExternalUserId:testExternalId]; + + [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" + handleNotificationAction:nil + settings:nil]; + + [UnitTestCommonMethods runBackgroundThreads]; + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalId); + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); +} + +- (void)testSetExternalUserIdAfterRegistration { + let testExternalId = @"i_am_a_test_external_id"; + + [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" + handleNotificationAction:nil + settings:nil]; + + [UnitTestCommonMethods runBackgroundThreads]; + + [OneSignal setExternalUserId:testExternalId]; + + [UnitTestCommonMethods runBackgroundThreads]; + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalId); +} + +- (void)testRemoveExternalUserId { + [OneSignal setExternalUserId:@"i_am_a_test_external_id"]; + + [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" + handleNotificationAction:nil + settings:nil]; + + [UnitTestCommonMethods runBackgroundThreads]; + + [OneSignal removeExternalUserId]; + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @""); +} + @end From 19734fae324b2d60cb5b320c9fa1bed6a5ed337a Mon Sep 17 00:00:00 2001 From: Brad Hesse Date: Mon, 3 Dec 2018 17:18:23 -0800 Subject: [PATCH 2/4] Add External ID To Demo Project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • We have a (very) simple dev project for the SDK. This commit adds a text field and button to set an arbitrary external User ID, as well as a button to remove the ID --- .../Base.lproj/Main.storyboard | 75 ++++++++++++++++--- .../OneSignalDevApp/ViewController.h | 2 +- .../OneSignalDevApp/ViewController.m | 17 +++++ 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/iOS_SDK/OneSignalDevApp/OneSignalDevApp/Base.lproj/Main.storyboard b/iOS_SDK/OneSignalDevApp/OneSignalDevApp/Base.lproj/Main.storyboard index f8652c39b..34edbb9ac 100644 --- a/iOS_SDK/OneSignalDevApp/OneSignalDevApp/Base.lproj/Main.storyboard +++ b/iOS_SDK/OneSignalDevApp/OneSignalDevApp/Base.lproj/Main.storyboard @@ -1,13 +1,11 @@ - + - - - + @@ -24,7 +22,7 @@ - + @@ -44,7 +42,7 @@ - + @@ -86,14 +84,62 @@ + + + + + + + + + + + + + + + + + + + + + + + + + @@ -105,20 +151,25 @@ + + - + + + + diff --git a/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.h b/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.h index 2d49dc108..80be98b5a 100644 --- a/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.h +++ b/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.h @@ -30,7 +30,7 @@ #import -@interface ViewController : UIViewController +@interface ViewController : UIViewController @end diff --git a/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.m b/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.m index 7744fc6ef..6e2a295fb 100644 --- a/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.m +++ b/iOS_SDK/OneSignalDevApp/OneSignalDevApp/ViewController.m @@ -36,6 +36,7 @@ @interface ViewController () @property (weak, nonatomic) IBOutlet UITextField *textField; @property (weak, nonatomic) IBOutlet UIActivityIndicatorView *activityIndicatorView; @property (weak, nonatomic) IBOutlet UISegmentedControl *consentSegmentedControl; +@property (weak, nonatomic) IBOutlet UITextField *externalIdTextField; @end @@ -48,6 +49,9 @@ - (void)viewDidLoad { self.activityIndicatorView.hidden = true; self.consentSegmentedControl.selectedSegmentIndex = (NSInteger)![OneSignal requiresUserPrivacyConsent]; + + self.textField.delegate = self; + self.externalIdTextField.delegate = self; } - (void)changeAnimationState:(BOOL)animating { @@ -129,5 +133,18 @@ - (IBAction)consentSegmentedControlValueChanged:(UISegmentedControl *)sender { [OneSignal consentGranted:(bool)sender.selectedSegmentIndex]; } +- (IBAction)setExternalIdButtonPressed:(UIButton *)sender { + [OneSignal setExternalUserId:self.externalIdTextField.text]; +} + +- (IBAction)removeExternalIdButtonPressed:(UIButton *)sender { + [OneSignal removeExternalUserId]; +} + +#pragma mark UITextFieldDelegate Methods +-(BOOL)textFieldShouldReturn:(UITextField *)textField { + [textField resignFirstResponder]; + return false; +} @end From 9c4820f20a6e4af9e0b32dee7c3c642a34016f50 Mon Sep 17 00:00:00 2001 From: Brad Hesse Date: Tue, 4 Dec 2018 12:51:56 -0800 Subject: [PATCH 3/4] De-Duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Persists the most recently set external user ID so that if a developer sets it every time a user opens the app (setting the same ID every time), the SDK does not send unnecessary duplicate API requests • Adds tests to verify that unnecessary API requests don't occur in this scenario --- iOS_SDK/OneSignalSDK/Source/OneSignal.m | 31 ++++++++-- .../Source/OneSignalCommonDefines.h | 1 + iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m | 59 +++++++++++++++---- 3 files changed, 76 insertions(+), 15 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m index 27d659bad..b9905390b 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.m +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.m @@ -1485,10 +1485,10 @@ + (void)registerUserInternal { // should be set to true even before the API request is finished performedOnSessionRequest = true; - if (pendingExternalUserId) { + if (pendingExternalUserId && ![self.existingExternalUserId isEqualToString:pendingExternalUserId]) dataDic[@"external_user_id"] = pendingExternalUserId; - pendingExternalUserId = nil; - } + + pendingExternalUserId = nil; if (deviceModel) dataDic[@"device_model"] = deviceModel; @@ -1645,7 +1645,13 @@ + (void)registerUserInternal { } - [[NSUserDefaults standardUserDefaults] synchronize]; + // If the external user ID was sent as part of this request, we need to save it + // The 'successfullySentExternalUserId' method already calls NSUserDefaults synchronize + // so there is no need to call it again + if (dataDic[@"external_user_id"]) + [self successfullySentExternalUserId:dataDic[@"external_user_id"]]; + else + [[NSUserDefaults standardUserDefaults] synchronize]; } onFailure:^(NSDictionary *errors) { waitingForOneSReg = false; @@ -2303,6 +2309,9 @@ + (void)setExternalUserId:(NSString * _Nonnull)externalId { if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:@"setExternalUserId:"]) return; + if ([self.existingExternalUserId isEqualToString:externalId]) + return; + if (!performedOnSessionRequest) { // will be sent as part of the registration/on_session request pendingExternalUserId = externalId; @@ -2312,11 +2321,23 @@ + (void)setExternalUserId:(NSString * _Nonnull)externalId { return; } - [OneSignalClient.sharedClient executeRequest:[OSRequestUpdateExternalUserId withUserId:externalId withOneSignalUserId:self.currentSubscriptionState.userId appId:self.app_id] onSuccess:nil onFailure:^(NSError *error) { + [OneSignalClient.sharedClient executeRequest:[OSRequestUpdateExternalUserId withUserId:externalId withOneSignalUserId:self.currentSubscriptionState.userId appId:self.app_id] onSuccess:^(NSDictionary *result) { + // the success/fail callbacks always execute on the main thread + [self successfullySentExternalUserId:externalId]; + } onFailure:^(NSError *error) { [self onesignal_Log:ONE_S_LL_ERROR message:[NSString stringWithFormat:@"Encountered an error while attempting to set external ID: %@", error.description]]; }]; } ++ (NSString *)existingExternalUserId { + return [NSUserDefaults.standardUserDefaults stringForKey:EXTERNAL_USER_ID] ?: @""; +} + ++ (void)successfullySentExternalUserId:(NSString *)externalId { + [NSUserDefaults.standardUserDefaults setObject:externalId forKey:EXTERNAL_USER_ID]; + [NSUserDefaults.standardUserDefaults synchronize]; +} + + (void)removeExternalUserId { // return if the user has not granted privacy permissions diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h b/iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h index 82ad6a778..9b7d50941 100644 --- a/iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h +++ b/iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h @@ -52,6 +52,7 @@ #define PERMISSION_ACCEPTED @"ONESIGNAL_ACCEPTED_NOTIFICATION_LAST" #define PERMISSION_PROVISIONAL_STATUS @"ONESIGNAL_PROVISIONAL_AUTHORIZATION_LAST" #define PERMISSION_PROVIDES_NOTIFICATION_SETTINGS @"OS_APP_PROVIDES_NOTIFICATION_SETTINGS" +#define EXTERNAL_USER_ID @"OS_EXTERNAL_USER_ID" // To avoid undefined symbol compiler errors on older versions of Xcode, // instead of using UNAuthorizationOptionProvisional directly, we will use diff --git a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m index 096e15891..ded7db0ca 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m @@ -92,12 +92,16 @@ @interface UnitTests : XCTestCase @end -@implementation UnitTests +@implementation UnitTests { + NSString *testExternalUserId; +} // Called before each test. - (void)setUp { [super setUp]; + testExternalUserId = @"i_am_a_test_external_user_id"; + OneSignalHelperOverrider.mockIOSVersion = 10; [OneSignalUNUserNotificationCenter setUseiOS10_2_workaround:true]; @@ -2153,9 +2157,7 @@ - (void)testInvalidButtonFormat { } - (void)testSetExternalUserIdWithRegistration { - let testExternalId = @"i_am_a_test_external_id"; - - [OneSignal setExternalUserId:testExternalId]; + [OneSignal setExternalUserId:testExternalUserId]; [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil @@ -2163,31 +2165,29 @@ - (void)testSetExternalUserIdWithRegistration { [UnitTestCommonMethods runBackgroundThreads]; - XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalId); + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalUserId); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); } - (void)testSetExternalUserIdAfterRegistration { - let testExternalId = @"i_am_a_test_external_id"; - [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil settings:nil]; [UnitTestCommonMethods runBackgroundThreads]; - [OneSignal setExternalUserId:testExternalId]; + [OneSignal setExternalUserId:testExternalUserId]; [UnitTestCommonMethods runBackgroundThreads]; XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); - XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalId); + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalUserId); } - (void)testRemoveExternalUserId { - [OneSignal setExternalUserId:@"i_am_a_test_external_id"]; + [OneSignal setExternalUserId:testExternalUserId]; [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil @@ -2202,4 +2202,43 @@ - (void)testRemoveExternalUserId { XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @""); } +// Tests to make sure that the SDK will not send an external ID if it already successfully sent the same ID +- (void)testDoesntSendExistingExternalUserIdAfterRegistration { + [OneSignal setExternalUserId:testExternalUserId]; + + [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" + handleNotificationAction:nil + settings:nil]; + + [UnitTestCommonMethods runBackgroundThreads]; + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); + + [OneSignal setExternalUserId:testExternalUserId]; + + // the PUT request to set external ID should not happen since the external ID + // is the same as it was during registration + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); +} + +- (void)testDoesntSendExistingExternalUserIdBeforeRegistration { + //mimics a previous session where the external user ID was set + [NSUserDefaults.standardUserDefaults setObject:testExternalUserId forKey:EXTERNAL_USER_ID]; + [NSUserDefaults.standardUserDefaults synchronize]; + + [OneSignal setExternalUserId:testExternalUserId]; + + [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" + handleNotificationAction:nil + settings:nil]; + + [UnitTestCommonMethods runBackgroundThreads]; + + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); + + // the registration request should not have included external user ID + // since it had been set already to the same value in a previous session + XCTAssertNil(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"]); +} + @end From 285d693dfee57add217b02a52ffe6be797ce2a49 Mon Sep 17 00:00:00 2001 From: Brad Hesse Date: Tue, 4 Dec 2018 16:39:58 -0800 Subject: [PATCH 4/4] Email Player External User ID's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • I had implemented external user ID for Push players but it turns out we want to support it for Email user records as well. • This commit adds external_user_id to email user records, in a similar matter to how we execute simultaneous requests for sendTags and so on. • Added an integration test in EmailTests to make sure external ID methods work correctly, duplicate requests don't generate unnecessary API traffic, etc. • Changes the OneSignalClientOverrider so that it maintains an array of executed OneSignalRequest (API request) objects. This makes testing more flexible --- iOS_SDK/OneSignalSDK/Source/OneSignal.m | 12 +++++++-- iOS_SDK/OneSignalSDK/UnitTests/EmailTests.m | 25 +++++++++++++++++++ .../Shadows/OneSignalClientOverrider.h | 2 ++ .../Shadows/OneSignalClientOverrider.m | 19 ++++++++------ .../UnitTests/UnitTestCommonMethods.h | 2 ++ iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m | 24 ++++++++---------- 6 files changed, 60 insertions(+), 24 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m index b9905390b..581833fee 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.m +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.m @@ -2321,10 +2321,18 @@ + (void)setExternalUserId:(NSString * _Nonnull)externalId { return; } - [OneSignalClient.sharedClient executeRequest:[OSRequestUpdateExternalUserId withUserId:externalId withOneSignalUserId:self.currentSubscriptionState.userId appId:self.app_id] onSuccess:^(NSDictionary *result) { + let requests = [NSMutableDictionary new]; + requests[@"push"] = [OSRequestUpdateExternalUserId withUserId:externalId withOneSignalUserId:self.currentSubscriptionState.userId appId:self.app_id]; + + if (self.currentEmailSubscriptionState.emailUserId && (self.currentEmailSubscriptionState.requiresEmailAuth == false || self.currentEmailSubscriptionState.emailAuthCode)) + requests[@"email"] = [OSRequestUpdateExternalUserId withUserId:externalId withOneSignalUserId:self.currentEmailSubscriptionState.emailUserId appId:self.app_id]; + + [OneSignalClient.sharedClient executeSimultaneousRequests:requests withSuccess:^(NSDictionary *results) { // the success/fail callbacks always execute on the main thread [self successfullySentExternalUserId:externalId]; - } onFailure:^(NSError *error) { + } onFailure:^(NSDictionary *errors) { + // if either request fails, this block is executed + NSError *error = errors[@"push"] ?: errors[@"email"]; [self onesignal_Log:ONE_S_LL_ERROR message:[NSString stringWithFormat:@"Encountered an error while attempting to set external ID: %@", error.description]]; }]; } diff --git a/iOS_SDK/OneSignalSDK/UnitTests/EmailTests.m b/iOS_SDK/OneSignalSDK/UnitTests/EmailTests.m index 43ba4c2cb..85b5864ec 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/EmailTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/EmailTests.m @@ -544,4 +544,29 @@ -(void)testEmailSubscriptionDescription { [OneSignalClientOverrider reset:self]; } +- (void)testSetExternalIdForEmailPlayer { + [UnitTestCommonMethods runBackgroundThreads]; + [self setupEmailTest]; + + [OneSignal setEmail:@"test@test.com"]; + [UnitTestCommonMethods runBackgroundThreads]; + + int currentRequestCount = OneSignalClientOverrider.networkRequestCount; + + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; + [UnitTestCommonMethods runBackgroundThreads]; + + let emailPlayerId = OneSignal.getPermissionSubscriptionState.emailSubscriptionStatus.emailUserId; + XCTAssertEqual(OneSignalClientOverrider.networkRequestCount, currentRequestCount + 2); + + for (OneSignalRequest *request in OneSignalClientOverrider.executedRequests) + if ([request isKindOfClass:[OSRequestUpdateExternalUserId class]] && [request.urlRequest.URL.absoluteString containsString:emailPlayerId]) + XCTAssertEqualObjects(request.parameters[@"external_user_id"], TEST_EXTERNAL_USER_ID); + + // lastly, check to make sure that calling setExternalUserId() again with the same + // ID doesn't create a duplicate API request + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; + XCTAssertEqual(OneSignalClientOverrider.networkRequestCount, currentRequestCount + 2); +} + @end diff --git a/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.h b/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.h index 4d9a433b9..236527224 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.h +++ b/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.h @@ -28,6 +28,7 @@ #import #import #import +#import "OneSignalRequest.h" @interface OneSignalClientOverrider : NSObject +(void)reset:(XCTestCase*)testInstance; @@ -44,5 +45,6 @@ +(BOOL)hasExecutedRequestOfType:(Class)type; +(void)setShouldUseProvisionalAuth:(BOOL)provisional; +(void)disableExecuteRequestOverride:(BOOL)disable; ++(NSArray *)executedRequests; @end diff --git a/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m b/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m index d8a84d49a..2764930f2 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m @@ -31,7 +31,6 @@ #import "OneSignal.h" #import "OneSignalHelper.h" #import "OneSignalClient.h" -#import "OneSignalRequest.h" #import "OneSignalSelectorHelpers.h" #import "Requests.h" #import "OneSignalCommonDefines.h" @@ -49,9 +48,9 @@ @implementation OneSignalClientOverrider static dispatch_queue_t executionQueue; static NSString *lastHTTPRequestType; static BOOL requiresEmailAuth = false; -static NSMutableArray *executedRequestTypes; static BOOL shouldUseProvisionalAuthorization = false; //new in iOS 12 (aka Direct to History) static BOOL disableOverride = false; +static NSMutableArray *executedRequests; + (void)load { serialMockMainLooper = dispatch_queue_create("com.onesignal.unittest", DISPATCH_QUEUE_SERIAL); @@ -63,7 +62,7 @@ + (void)load { executionQueue = dispatch_queue_create("com.onesignal.execution", NULL); - executedRequestTypes = [[NSMutableArray alloc] init]; + executedRequests = [NSMutableArray new]; } // Calling this function twice results in reversing the swizzle @@ -81,7 +80,7 @@ - (void)overrideExecuteSimultaneousRequests:(NSDictionary *results = [NSMutableDictionary new]; for (NSString *key in requests.allKeys) { - [executedRequestTypes addObject:NSStringFromClass([requests[key] class])]; + [executedRequests addObject:requests[key]]; [OneSignalClient.sharedClient executeRequest:requests[key] onSuccess:^(NSDictionary *result) { results[key] = result; @@ -108,7 +107,7 @@ - (void)overrideExecuteRequest:(OneSignalRequest *)request onSuccess:(OSResultSu if (disableOverride) return [self overrideExecuteRequest:request onSuccess:successBlock onFailure:failureBlock]; - [executedRequestTypes addObject:NSStringFromClass([request class])]; + [executedRequests addObject:request]; if (executeInstantaneously) { [OneSignalClientOverrider finishExecutingRequest:request onSuccess:successBlock onFailure:failureBlock]; @@ -148,8 +147,8 @@ + (void)finishExecutingRequest:(OneSignalRequest *)request onSuccess:(OSResultSu } +(BOOL)hasExecutedRequestOfType:(Class)type { - for (id requestType in executedRequestTypes) - if ([requestType isEqualToString:NSStringFromClass(type)]) + for (OneSignalRequest *request in executedRequests) + if ([request isKindOfClass:type]) return true; return false; @@ -173,7 +172,7 @@ +(void)reset:(XCTestCase*)testInstance { networkRequestCount = 0; lastUrl = nil; lastHTTPRequest = nil; - [executedRequestTypes removeAllObjects]; + [executedRequests removeAllObjects]; } +(void)setLastHTTPRequest:(NSDictionary*)value { @@ -207,5 +206,9 @@ +(void)setShouldUseProvisionalAuth:(BOOL)provisional { shouldUseProvisionalAuthorization = provisional; } ++(NSArray *)executedRequests { + return executedRequests; +} + @end diff --git a/iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.h b/iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.h index 2f60d7fb6..dd35dce7f 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.h +++ b/iOS_SDK/OneSignalSDK/UnitTests/UnitTestCommonMethods.h @@ -29,6 +29,8 @@ #import #import "OneSignal.h" +#define TEST_EXTERNAL_USER_ID @"i_am_a_test_external_user_id" + NSString * serverUrlWithPath(NSString *path); @interface UnitTestCommonMethods : NSObject diff --git a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m index ded7db0ca..8f751af16 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m @@ -92,16 +92,12 @@ @interface UnitTests : XCTestCase @end -@implementation UnitTests { - NSString *testExternalUserId; -} +@implementation UnitTests // Called before each test. - (void)setUp { [super setUp]; - testExternalUserId = @"i_am_a_test_external_user_id"; - OneSignalHelperOverrider.mockIOSVersion = 10; [OneSignalUNUserNotificationCenter setUseiOS10_2_workaround:true]; @@ -2157,7 +2153,7 @@ - (void)testInvalidButtonFormat { } - (void)testSetExternalUserIdWithRegistration { - [OneSignal setExternalUserId:testExternalUserId]; + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil @@ -2165,7 +2161,7 @@ - (void)testSetExternalUserIdWithRegistration { [UnitTestCommonMethods runBackgroundThreads]; - XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalUserId); + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); } @@ -2177,17 +2173,17 @@ - (void)testSetExternalUserIdAfterRegistration { [UnitTestCommonMethods runBackgroundThreads]; - [OneSignal setExternalUserId:testExternalUserId]; + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [UnitTestCommonMethods runBackgroundThreads]; XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); - XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], testExternalUserId); + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID); } - (void)testRemoveExternalUserId { - [OneSignal setExternalUserId:testExternalUserId]; + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil @@ -2204,7 +2200,7 @@ - (void)testRemoveExternalUserId { // Tests to make sure that the SDK will not send an external ID if it already successfully sent the same ID - (void)testDoesntSendExistingExternalUserIdAfterRegistration { - [OneSignal setExternalUserId:testExternalUserId]; + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil @@ -2214,7 +2210,7 @@ - (void)testDoesntSendExistingExternalUserIdAfterRegistration { XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestRegisterUser class])); - [OneSignal setExternalUserId:testExternalUserId]; + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; // the PUT request to set external ID should not happen since the external ID // is the same as it was during registration @@ -2223,10 +2219,10 @@ - (void)testDoesntSendExistingExternalUserIdAfterRegistration { - (void)testDoesntSendExistingExternalUserIdBeforeRegistration { //mimics a previous session where the external user ID was set - [NSUserDefaults.standardUserDefaults setObject:testExternalUserId forKey:EXTERNAL_USER_ID]; + [NSUserDefaults.standardUserDefaults setObject:TEST_EXTERNAL_USER_ID forKey:EXTERNAL_USER_ID]; [NSUserDefaults.standardUserDefaults synchronize]; - [OneSignal setExternalUserId:testExternalUserId]; + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil