Merge pull request #1427 from OneSignal/fix/identify_user_bug_when_in_the_middle

nan-li · web-flow · commit 445b5d20ec79 · 2024-05-10T13:44:13.000-07:00
[Bug] Fix processing of Identify User response if the user has changed since then
diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj
@@ -116,6 +116,8 @@
 		3CA283A92B86A30400097465 /* OneSignalCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DE7D17E627026B95002D3A5D /* OneSignalCore.framework */; };
 		3CA283AA2B86A30400097465 /* OneSignalCore.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DE7D17E627026B95002D3A5D /* OneSignalCore.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		3CA6CE0A28E4F19B00CA0585 /* OSUserRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA6CE0928E4F19B00CA0585 /* OSUserRequest.swift */; };
+		3CA8B8822BEC2FCB0010ADA1 /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3C7A39D42B7C18EE0082665E /* XCTest.framework */; };
+		3CA8B8832BEC2FCB0010ADA1 /* XCTest.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 3C7A39D42B7C18EE0082665E /* XCTest.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		3CC063942B6D6B6B002BB07F /* OneSignalCore.m in Sources */ = {isa = PBXBuildFile; fileRef = 3CC063932B6D6B6B002BB07F /* OneSignalCore.m */; };
 		3CC063A22B6D7A8E002BB07F /* OneSignalCoreMocks.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3CC0639A2B6D7A8C002BB07F /* OneSignalCoreMocks.framework */; };
 		3CC063A72B6D7A8E002BB07F /* OneSignalCoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CC063A62B6D7A8E002BB07F /* OneSignalCoreTests.swift */; };
@@ -1022,6 +1024,7 @@
 			dstSubfolderSpec = 10;
 			files = (
 				3CEE93542B7C78EC008440BD /* OneSignalUser.framework in Embed Frameworks */,
+				3CA8B8832BEC2FCB0010ADA1 /* XCTest.framework in Embed Frameworks */,
 				3CEE934F2B7C787B008440BD /* OneSignalOSCore.framework in Embed Frameworks */,
 			);
 			name = "Embed Frameworks";
@@ -1569,6 +1572,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				3CEE93532B7C78EC008440BD /* OneSignalUser.framework in Frameworks */,
+				3CA8B8822BEC2FCB0010ADA1 /* XCTest.framework in Frameworks */,
 				3CEE934E2B7C787B008440BD /* OneSignalOSCore.framework in Frameworks */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
diff --git a/iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OneSignalClient.m b/iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OneSignalClient.m
@@ -203,7 +203,7 @@ - (void)handleJSONNSURLResponse:(NSURLResponse*)response data:(NSData*)data erro
     if (data != nil && [data length] > 0) {
         innerJson = [NSJSONSerialization JSONObjectWithData:data options:NSJSONReadingMutableContainers error:&jsonError];
         innerJson[@"httpStatusCode"] = [NSNumber numberWithLong:statusCode];
-        [OneSignalLog onesignalLog:ONE_S_LL_VERBOSE message:[NSString stringWithFormat:@"network response (%@): %@", NSStringFromClass([request class]), innerJson]];
+        [OneSignalLog onesignalLog:ONE_S_LL_VERBOSE message:[NSString stringWithFormat:@"network response (%@) with URL %@: %@", NSStringFromClass([request class]), request.urlRequest.URL.absoluteString, innerJson]];
         if (jsonError) {
             if (failureBlock != nil)
                 failureBlock([NSError errorWithDomain:@"OneSignal Error" code:statusCode userInfo:@{@"returned" : jsonError}]);
diff --git a/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift b/iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift
@@ -35,7 +35,7 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
     public var lastHTTPRequest: OneSignalRequest?
     public var networkRequestCount = 0
     public var executedRequests: [OneSignalRequest] = []
-    public var executeInstantaneously = true
+    public var executeInstantaneously = false
 
     var remoteParamsResponse: [String: Any]?
     var shouldUseProvisionalAuthorization = false // new in iOS 12 (aka Direct to History)
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift
@@ -329,6 +329,10 @@ class OSUserExecutor {
             if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {
                 fetchUser(aliasLabel: OS_EXTERNAL_ID, aliasId: request.aliasId, identityModel: request.identityModelToUpdate)
             } else {
+                // Need to hydrate the identity model for any pending requests
+                if let osid = request.identityModelToIdentify.onesignalId {
+                    request.identityModelToUpdate.hydrate([OS_ONESIGNAL_ID: osid])
+                }
                 executePendingRequests()
             }
         } onFailure: { error in
@@ -340,12 +344,15 @@ class OSUserExecutor {
                     OneSignalLog.onesignalLog(.LL_DEBUG, message: "executeIdentifyUserRequest returned error code user-2. Now handling user-2 error response... switch to this user.")
 
                     removeFromQueue(request)
-                    // Fetch the user only if its the current user
+                    // Transfer the push subscription, and fetch only if it's the current user
                     if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) {
                         fetchUser(aliasLabel: OS_EXTERNAL_ID, aliasId: request.aliasId, identityModel: request.identityModelToUpdate)
-                        // TODO: Link ^ to the new user... what was this todo for?
+                        transferPushSubscriptionTo(aliasLabel: request.aliasLabel, aliasId: request.aliasId)
+                    } else {
+                        // Use external_id for any pending requests, avoiding a fetch to hydrate onesignal_id
+                        request.identityModelToUpdate.primaryAliasLabel = .external_id
+                        executePendingRequests()
                     }
-                    transferPushSubscriptionTo(aliasLabel: request.aliasLabel, aliasId: request.aliasId)
                 } else if responseType == .invalid || responseType == .unauthorized {
                     // Failed, no retry
                     removeFromQueue(request)
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSIdentityModel.swift
@@ -29,7 +29,23 @@ import Foundation
 import OneSignalCore
 import OneSignalOSCore
 
+// By matching the enum name to the raw value, it will always stringify correctly
+enum OSDefaultAlias: String {
+    // swiftlint:disable identifier_name
+    case onesignal_id = "onesignal_id"
+    case external_id = "external_id"
+    // swiftlint:enable identifier_name
+}
+
 class OSIdentityModel: OSModel {
+    /**
+     Set either `onesignal_id` or `external_id`, representing the alias that will be used in requests.
+     */
+    var primaryAliasLabel: OSDefaultAlias = .onesignal_id
+    var primaryAliasId: String? {
+        return if primaryAliasLabel == .external_id { externalId } else { onesignalId }
+    }
+
     var onesignalId: String? {
         return internalGetAlias(OS_ONESIGNAL_ID)
     }
@@ -57,6 +73,7 @@ class OSIdentityModel: OSModel {
         aliasesLock.withLock {
             super.encode(with: coder)
             coder.encode(aliases, forKey: "aliases")
+            coder.encode(primaryAliasLabel.rawValue, forKey: "primaryAliasLabel") // Encodes as String
         }
     }
 
@@ -66,6 +83,12 @@ class OSIdentityModel: OSModel {
             // log error
             return nil
         }
+        if let rawType = coder.decodeObject(forKey: "primaryAliasLabel") as? String,
+           let label = OSDefaultAlias(rawValue: rawType) {
+            self.primaryAliasLabel = label
+        } else {
+            self.primaryAliasLabel = .onesignal_id
+        }
         self.aliases = aliases
     }
 
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestAddAliases.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestAddAliases.swift
@@ -39,9 +39,10 @@ class OSRequestAddAliases: OneSignalRequest, OSUserRequest {
 
     // requires a `onesignal_id` to send this request
     func prepareForExecution() -> Bool {
-        if let onesignalId = identityModel.onesignalId, let appId = OneSignalConfigManager.getAppId() {
+        let aliasLabel = identityModel.primaryAliasLabel
+        if let aliasId = identityModel.primaryAliasId, let appId = OneSignalConfigManager.getAppId() {
             self.addJWTHeader(identityModel: identityModel)
-            self.path = "apps/\(appId)/users/by/\(OS_ONESIGNAL_ID)/\(onesignalId)/identity"
+            self.path = "apps/\(appId)/users/by/\(aliasLabel)/\(aliasId)/identity"
             return true
         } else {
             // self.path is non-nil, so set to empty string
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateSubscription.swift
@@ -44,9 +44,10 @@ class OSRequestCreateSubscription: OneSignalRequest, OSUserRequest {
 
     // Need the onesignal_id of the user
     func prepareForExecution() -> Bool {
-        if let onesignalId = identityModel.onesignalId, let appId = OneSignalConfigManager.getAppId() {
+        let aliasLabel = identityModel.primaryAliasLabel
+        if let aliasId = identityModel.primaryAliasId, let appId = OneSignalConfigManager.getAppId() {
             self.addJWTHeader(identityModel: identityModel)
-            self.path = "apps/\(appId)/users/by/\(OS_ONESIGNAL_ID)/\(onesignalId)/subscriptions"
+            self.path = "apps/\(appId)/users/by/\(aliasLabel)/\(aliasId)/subscriptions"
             return true
         } else {
             self.path = "" // self.path is non-nil, so set to empty string
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestFetchUser.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestFetchUser.swift
@@ -58,7 +58,7 @@ class OSRequestFetchUser: OneSignalRequest, OSUserRequest {
         self.aliasLabel = aliasLabel
         self.aliasId = aliasId
         self.onNewSession = onNewSession
-        self.stringDescription = "<OSRequestFetchUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)>"
+        self.stringDescription = "<OSRequestFetchUser with \(aliasLabel): \(aliasId)>"
         super.init()
         self.method = GET
         _ = prepareForExecution() // sets the path property
@@ -88,7 +88,7 @@ class OSRequestFetchUser: OneSignalRequest, OSUserRequest {
         self.aliasLabel = aliasLabel
         self.aliasId = aliasId
         self.onNewSession = coder.decodeBool(forKey: "onNewSession")
-        self.stringDescription = "<OSRequestFetchUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)>"
+        self.stringDescription = "<OSRequestFetchUser with \(aliasLabel): \(aliasId)>"
         super.init()
         self.method = HTTPMethod(rawValue: rawMethod)
         self.timestamp = timestamp
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestIdentifyUser.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestIdentifyUser.swift
@@ -48,14 +48,15 @@ class OSRequestIdentifyUser: OneSignalRequest, OSUserRequest {
 
     // requires a onesignal_id to send this request
     func prepareForExecution() -> Bool {
-        if let onesignalId = identityModelToIdentify.onesignalId, let appId = OneSignalConfigManager.getAppId() {
+        let aliasLabel = identityModelToIdentify.primaryAliasLabel
+        if let aliasId = identityModelToIdentify.primaryAliasId, let appId = OneSignalConfigManager.getAppId() {
             self.addJWTHeader(identityModel: identityModelToIdentify)
-            self.path = "apps/\(appId)/users/by/\(OS_ONESIGNAL_ID)/\(onesignalId)/identity"
+            self.path = "apps/\(appId)/users/by/\(aliasLabel)/\(aliasId)/identity"
             return true
         } else {
             // self.path is non-nil, so set to empty string
             self.path = ""
-            OneSignalLog.onesignalLog(.LL_DEBUG, message: "Cannot generate the Identify User request due to null app ID or null OneSignal ID.")
+            OneSignalLog.onesignalLog(.LL_DEBUG, message: "Cannot generate the Identify User request due to null app ID or null \(aliasLabel) ID.")
             return false
         }
     }
@@ -72,7 +73,7 @@ class OSRequestIdentifyUser: OneSignalRequest, OSUserRequest {
         self.identityModelToUpdate = identityModelToUpdate
         self.aliasLabel = aliasLabel
         self.aliasId = aliasId
-        self.stringDescription = "<OSRequestIdentifyUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)>"
+        self.stringDescription = "<OSRequestIdentifyUser with \(aliasLabel): \(aliasId)>"
         super.init()
         self.parameters = ["identity": [aliasLabel: aliasId]]
         self.method = PATCH
@@ -106,7 +107,7 @@ class OSRequestIdentifyUser: OneSignalRequest, OSUserRequest {
         self.identityModelToUpdate = identityModelToUpdate
         self.aliasLabel = aliasLabel
         self.aliasId = aliasId
-        self.stringDescription = "<OSRequestIdentifyUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)>"
+        self.stringDescription = "<OSRequestIdentifyUser with \(aliasLabel): \(aliasId)>"
         super.init()
         self.timestamp = timestamp
         self.parameters = parameters
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestRemoveAlias.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestRemoveAlias.swift
@@ -38,9 +38,10 @@ class OSRequestRemoveAlias: OneSignalRequest, OSUserRequest {
     var identityModel: OSIdentityModel
 
     func prepareForExecution() -> Bool {
-        if let onesignalId = identityModel.onesignalId, let appId = OneSignalConfigManager.getAppId() {
+        let aliasLabel = identityModel.primaryAliasLabel
+        if let aliasId = identityModel.primaryAliasId, let appId = OneSignalConfigManager.getAppId() {
             self.addJWTHeader(identityModel: identityModel)
-            self.path = "apps/\(appId)/users/by/\(OS_ONESIGNAL_ID)/\(onesignalId)/identity/\(labelToRemove)"
+            self.path = "apps/\(appId)/users/by/\(aliasLabel)/\(aliasId)/identity/\(labelToRemove)"
             return true
         } else {
             // self.path is non-nil, so set to empty string
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestTransferSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestTransferSubscription.swift
@@ -66,7 +66,7 @@ class OSRequestTransferSubscription: OneSignalRequest, OSUserRequest {
         self.subscriptionModel = subscriptionModel
         self.aliasLabel = aliasLabel
         self.aliasId = aliasId
-        self.stringDescription = "OSRequestTransferSubscription"
+        self.stringDescription = "<OSRequestTransferSubscription to \(aliasLabel): \(aliasId)>"
         super.init()
         self.parameters = ["identity": [aliasLabel: aliasId]]
         self.method = PATCH
@@ -97,7 +97,7 @@ class OSRequestTransferSubscription: OneSignalRequest, OSUserRequest {
         self.subscriptionModel = subscriptionModel
         self.aliasLabel = aliasLabel
         self.aliasId = aliasId
-        self.stringDescription = "OSRequestTransferSubscription"
+        self.stringDescription = "<OSRequestTransferSubscription to \(aliasLabel): \(aliasId)>"
         super.init()
         self.parameters = parameters
         self.method = HTTPMethod(rawValue: rawMethod)
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateProperties.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateProperties.swift
@@ -39,11 +39,12 @@ class OSRequestUpdateProperties: OneSignalRequest, OSUserRequest {
     // TODO: Decide if addPushSubscriptionIdToAdditionalHeadersIfNeeded should block.
     // Note Android adds it to requests, if the push sub ID exists
     func prepareForExecution() -> Bool {
-        if let onesignalId = identityModel.onesignalId,
+        let aliasLabel = identityModel.primaryAliasLabel
+        if let aliasId = identityModel.primaryAliasId,
             let appId = OneSignalConfigManager.getAppId() {
             _ = self.addPushSubscriptionIdToAdditionalHeaders()
             self.addJWTHeader(identityModel: identityModel)
-            self.path = "apps/\(appId)/users/by/\(OS_ONESIGNAL_ID)/\(onesignalId)"
+            self.path = "apps/\(appId)/users/by/\(aliasLabel)/\(aliasId)"
             return true
         } else {
             // self.path is non-nil, so set to empty string
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserDefines.swift b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserDefines.swift
@@ -3,3 +3,5 @@ public let userA_OSID = "test_user_a_onesignal_id"
 public let userA_EUID = "test_user_a_external_id"
 public let userB_OSID = "test_user_b_onesignal_id"
 public let userB_EUID = "test_user_b_external_id"
+
+public let testPushSubId = "test_push_subscription_id"
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift
@@ -1,4 +1,5 @@
 import OneSignalCore
+import OneSignalCoreMocks
 
 public class MockUserRequests {
 
@@ -15,4 +16,121 @@ public class MockUserRequests {
             "properties": properties
         ]
     }
+
+    public static func testDefaultPushSubPayload(id: String) -> [String: Any] {
+        return [
+            "id": testPushSubId,
+            "app_id": "test-app-id",
+            "type": "iOSPush",
+            "token": "",
+            "enabled": true,
+            "notification_types": 80,
+            "session_time": 0,
+            "session_count": 1,
+            "sdk": "test_sdk_version",
+            "device_model": "iPhone14,3",
+            "device_os": "17.4.1",
+            "rooted": false,
+            "test_type": 1,
+            "app_version": "1.4.4",
+            "net_type": 0,
+            "carrier": "",
+            "web_auth": "",
+            "web_p256": ""
+        ]
+    }
+
+    public static func testDefaultFullCreateUserResponse(onesignalId: String, externalId: String?, subscriptionId: String?) -> [String: Any] {
+        let identity = testIdentityPayload(onesignalId: onesignalId, externalId: externalId)
+        let subscription = testDefaultPushSubPayload(id: testPushSubId)
+        let properties = [
+            "language": "en",
+            "timezone_id": "America/Los_Angeles",
+            "country": "US",
+            "first_active": 1714860182,
+            "last_active": 1714860182,
+            "ip": "xxx.xx.xxx.xxx"
+        ] as [String: Any]
+
+        return [
+            "subscriptions": [subscription],
+            "identity": identity["identity"]!,
+            "properties": properties
+        ]
+    }
+}
+
+// MARK: - Set Up Default Client Responses
+
+extension MockUserRequests {
+    private static func getOneSignalId(for externalId: String) -> String {
+        switch externalId {
+        case userA_EUID:
+            return userA_OSID
+        case userB_EUID:
+            return userB_OSID
+        default:
+            return UUID().uuidString
+        }
+    }
+
+    public static func setDefaultCreateAnonUserResponses(with client: MockOneSignalClient) {
+        let anonCreateResponse = testDefaultFullCreateUserResponse(onesignalId: anonUserOSID, externalId: nil, subscriptionId: testPushSubId)
+
+        client.setMockResponseForRequest(
+            request: "<OSRequestCreateUser with externalId: nil>",
+            response: anonCreateResponse)
+    }
+
+    public static func setDefaultCreateUserResponses(with client: MockOneSignalClient, externalId: String) {
+        let osid = getOneSignalId(for: externalId)
+
+        let userResponse = MockUserRequests.testIdentityPayload(onesignalId: osid, externalId: externalId)
+
+        client.setMockResponseForRequest(
+            request: "<OSRequestCreateUser with externalId: \(externalId)>",
+            response: userResponse
+        )
+        client.setMockResponseForRequest(
+            request: "<OSRequestFetchUser with external_id: \(externalId)>",
+            response: userResponse
+        )
+    }
+
+    public static func setDefaultIdentifyUserResponses(with client: MockOneSignalClient, externalId: String, conflicted: Bool = false) {
+        var osid: String
+        var fetchResponse: [String: [String: String]]
+
+        // 1. Set the response for the Identify User request
+        if conflicted {
+            osid = getOneSignalId(for: externalId)
+            fetchResponse = MockUserRequests.testIdentityPayload(onesignalId: osid, externalId: externalId)
+            client.setMockFailureResponseForRequest(
+                request: "<OSRequestIdentifyUser with external_id: \(externalId)>",
+                error: NSError(domain: "not-important", code: 409)
+            )
+        } else {
+            // The Identify User is successful, the OSID is unchanged
+            osid = anonUserOSID
+            fetchResponse = MockUserRequests.testIdentityPayload(onesignalId: osid, externalId: externalId)
+            client.setMockResponseForRequest(
+                request: "<OSRequestIdentifyUser with external_id: \(externalId)>",
+                response: fetchResponse
+            )
+        }
+        // 2. Set the response for the subsequent Fetch User request
+        client.setMockResponseForRequest(
+            request: "<OSRequestFetchUser with external_id: \(externalId)>",
+            response: fetchResponse
+        )
+    }
+
+    public static func setAddTagsResponse(with client: MockOneSignalClient, tags: [String: String]) {
+        let tagsResponse = MockUserRequests.testPropertiesPayload(properties: ["tags": tags])
+
+        client.setMockResponseForRequest(
+            request: "<OSRequestUpdateProperties with properties: [\"tags\": \(tags)] deltas: nil refreshDeviceMetadata: false>",
+            response: tagsResponse
+        )
+    }
 }
diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift
