From 6eda3bfad13989b1a44e1149ac4729ec51154656 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 25 Jul 2024 18:19:47 -0700 Subject: [PATCH 1/3] User executor is no longer a static class * Primary Motivation: After a dispatch queue was added to the user executor, it's uncaching can happen on a background thread. However, when the User Manager starts, the user executor needs to finish uncaching first before other executors begin uncaching. This is because their uncached requests rely on the identity models organized by the user executor. By happening on a background thread, this order is not guaranteed. By making the User Executor an instance instead of a static class, it can uncache in its initializer without concurrency dangers. * Additional motivations include Identity Verification work that allows the user executor to conform to the same listener protocol as other executors. * All other executors are instances owned by the User Manager. * Make the User Executor also an instance owned by the User Manager, as it is the only class that calls it. * Access to user executor is force unwrapped because everything is guarded behind the User Manager's `start()` call. This will surface any issues or wrong usages early. --- .../Source/Executors/OSUserExecutor.swift | 232 +++++++++--------- .../Source/OneSignalUserManagerImpl.swift | 13 +- .../OneSignalUserMocks.swift | 5 - .../OneSignalUserTests.swift | 10 +- 4 files changed, 127 insertions(+), 133 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 7577310d3..d6832505e 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -34,81 +34,79 @@ import OneSignalOSCore Can execute `OSRequestCreateUser`, `OSRequestIdentifyUser`, `OSRequestFetchUser`, `OSRequestFetchIdentityBySubscription`. */ class OSUserExecutor { - static var userRequestQueue: [OSUserRequest] = [] + var userRequestQueue: [OSUserRequest] = [] // The User executor dispatch queue, serial. This synchronizes access to the request queues. - private static let dispatchQueue = DispatchQueue(label: "OneSignal.OSUserExecutor", target: .global()) + private let dispatchQueue = DispatchQueue(label: "OneSignal.OSUserExecutor", target: .global()) // Read in requests from the cache, do not read in FetchUser requests as this is not needed. - static func start() { - self.dispatchQueue.async { - var userRequestQueue: [OSUserRequest] = [] - - // Read unfinished Create User + Identify User + Get Identity By Subscription requests from cache, if any... - if let cachedRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSUserRequest] { - // Hook each uncached Request to the right model reference - for request in cachedRequestQueue { - if request.isKind(of: OSRequestFetchIdentityBySubscription.self), let req = request as? OSRequestFetchIdentityBySubscription { - if let identityModel = getIdentityModel(req.identityModel.modelId) { - // 1. The model exist in the repo, set it to be the Request's model - // It is the current user or the model has already been processed - req.identityModel = identityModel - } else { - // 2. The model do not exist, use the model on the request, and add to repo. - addIdentityModel(req.identityModel) - } - userRequestQueue.append(req) - - } else if request.isKind(of: OSRequestCreateUser.self), let req = request as? OSRequestCreateUser { - if let identityModel = getIdentityModel(req.identityModel.modelId) { - // 1. The model exist in the repo, set it to be the Request's model - req.identityModel = identityModel - } else { - // 2. The models do not exist, use the model on the request, and add to repo. - addIdentityModel(req.identityModel) - } - userRequestQueue.append(req) - - } else if request.isKind(of: OSRequestIdentifyUser.self), let req = request as? OSRequestIdentifyUser { - - if let identityModelToIdentify = getIdentityModel(req.identityModelToIdentify.modelId), - let identityModelToUpdate = getIdentityModel(req.identityModelToUpdate.modelId) { - // 1. Both models exist in the repo, set it to be the Request's models - req.identityModelToIdentify = identityModelToIdentify - req.identityModelToUpdate = identityModelToUpdate - } else if let identityModelToIdentify = getIdentityModel(req.identityModelToIdentify.modelId), - getIdentityModel(req.identityModelToUpdate.modelId) == nil { - // 2. A model is in the repo, the other model does not exist - req.identityModelToIdentify = identityModelToIdentify - addIdentityModel(req.identityModelToUpdate) - } else { - // 3. Both models don't exist yet - // Drop the request if the identityModelToIdentify does not already exist AND the request is missing OSID - // Otherwise, this request will forever fail `prepareForExecution` and block pending requests such as recovery calls to `logout` or `login` - guard request.prepareForExecution() else { - OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor.start() dropped: \(request)") - continue - } - addIdentityModel(req.identityModelToIdentify) - addIdentityModel(req.identityModelToUpdate) + init() { + var userRequestQueue: [OSUserRequest] = [] + + // Read unfinished Create User + Identify User + Get Identity By Subscription requests from cache, if any... + if let cachedRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSUserRequest] { + // Hook each uncached Request to the right model reference + for request in cachedRequestQueue { + if request.isKind(of: OSRequestFetchIdentityBySubscription.self), let req = request as? OSRequestFetchIdentityBySubscription { + if let identityModel = getIdentityModel(req.identityModel.modelId) { + // 1. The model exist in the repo, set it to be the Request's model + // It is the current user or the model has already been processed + req.identityModel = identityModel + } else { + // 2. The model do not exist, use the model on the request, and add to repo. + addIdentityModel(req.identityModel) + } + userRequestQueue.append(req) + + } else if request.isKind(of: OSRequestCreateUser.self), let req = request as? OSRequestCreateUser { + if let identityModel = getIdentityModel(req.identityModel.modelId) { + // 1. The model exist in the repo, set it to be the Request's model + req.identityModel = identityModel + } else { + // 2. The models do not exist, use the model on the request, and add to repo. + addIdentityModel(req.identityModel) + } + userRequestQueue.append(req) + + } else if request.isKind(of: OSRequestIdentifyUser.self), let req = request as? OSRequestIdentifyUser { + + if let identityModelToIdentify = getIdentityModel(req.identityModelToIdentify.modelId), + let identityModelToUpdate = getIdentityModel(req.identityModelToUpdate.modelId) { + // 1. Both models exist in the repo, set it to be the Request's models + req.identityModelToIdentify = identityModelToIdentify + req.identityModelToUpdate = identityModelToUpdate + } else if let identityModelToIdentify = getIdentityModel(req.identityModelToIdentify.modelId), + getIdentityModel(req.identityModelToUpdate.modelId) == nil { + // 2. A model is in the repo, the other model does not exist + req.identityModelToIdentify = identityModelToIdentify + addIdentityModel(req.identityModelToUpdate) + } else { + // 3. Both models don't exist yet + // Drop the request if the identityModelToIdentify does not already exist AND the request is missing OSID + // Otherwise, this request will forever fail `prepareForExecution` and block pending requests such as recovery calls to `logout` or `login` + guard request.prepareForExecution() else { + OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor.start() dropped: \(request)") + continue } - userRequestQueue.append(req) + addIdentityModel(req.identityModelToIdentify) + addIdentityModel(req.identityModelToUpdate) } + userRequestQueue.append(req) } } - self.userRequestQueue = userRequestQueue - OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue) - - migrateTransferSubscriptionRequests() - executePendingRequests() } + self.userRequestQueue = userRequestQueue + OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue) + + migrateTransferSubscriptionRequests() + executePendingRequests() } /** Read Transfer Subscription requests from cache, if any. As of `5.2.3`, the SDK will no longer send Transfer Subscription requests, so migrate the request into an equivalent Create User request. */ - static private func migrateTransferSubscriptionRequests() { + private func migrateTransferSubscriptionRequests() { if let transferSubscriptionRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_USER_EXECUTOR_TRANSFER_SUBSCRIPTION_REQUEST_QUEUE_KEY, defaultValue: nil) as? [OSRequestTransferSubscription] { OneSignalUserDefaults.initShared().removeValue(forKey: OS_USER_EXECUTOR_TRANSFER_SUBSCRIPTION_REQUEST_QUEUE_KEY) @@ -120,37 +118,37 @@ class OSUserExecutor { } } - static private func getIdentityModel(_ modelId: String) -> OSIdentityModel? { + private func getIdentityModel(_ modelId: String) -> OSIdentityModel? { return OneSignalUserManagerImpl.sharedInstance.getIdentityModel(modelId) } - static private func addIdentityModel(_ model: OSIdentityModel) { + private func addIdentityModel(_ model: OSIdentityModel) { OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(model) } - static func appendToQueue(_ request: OSUserRequest) { + func appendToQueue(_ request: OSUserRequest) { self.dispatchQueue.async { self.userRequestQueue.append(request) OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue) } } - static func removeFromQueue(_ request: OSUserRequest) { + func removeFromQueue(_ request: OSUserRequest) { self.dispatchQueue.async { - userRequestQueue.removeAll(where: { $0 == request}) + self.userRequestQueue.removeAll(where: { $0 == request}) OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue) } } - static func executePendingRequests() { + func executePendingRequests() { self.dispatchQueue.async { - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSUserExecutor.executePendingRequests called with queue \(userRequestQueue)") + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSUserExecutor.executePendingRequests called with queue \(self.userRequestQueue)") - if userRequestQueue.isEmpty { + if self.userRequestQueue.isEmpty { return } - for request in userRequestQueue { + for request in self.userRequestQueue { // Return as soon as we reach an un-executable request if !request.prepareForExecution() { OneSignalLog.onesignalLog(.LL_WARN, message: "OSUserExecutor.executePendingRequests() is blocked by unexecutable request \(request)") @@ -158,16 +156,16 @@ class OSUserExecutor { } if request.isKind(of: OSRequestFetchIdentityBySubscription.self), let fetchIdentityRequest = request as? OSRequestFetchIdentityBySubscription { - executeFetchIdentityBySubscriptionRequest(fetchIdentityRequest) + self.executeFetchIdentityBySubscriptionRequest(fetchIdentityRequest) return } else if request.isKind(of: OSRequestCreateUser.self), let createUserRequest = request as? OSRequestCreateUser { - executeCreateUserRequest(createUserRequest) + self.executeCreateUserRequest(createUserRequest) return } else if request.isKind(of: OSRequestIdentifyUser.self), let identifyUserRequest = request as? OSRequestIdentifyUser { - executeIdentifyUserRequest(identifyUserRequest) + self.executeIdentifyUserRequest(identifyUserRequest) return } else if request.isKind(of: OSRequestFetchUser.self), let fetchUserRequest = request as? OSRequestFetchUser { - executeFetchUserRequest(fetchUserRequest) + self.executeFetchUserRequest(fetchUserRequest) return } else { // Log Error @@ -181,7 +179,7 @@ class OSUserExecutor { // MARK: - Execution extension OSUserExecutor { // We will pass minimal properties to this request - static func createUser(_ user: OSUserInternal) { + func createUser(_ user: OSUserInternal) { let originalPushToken = user.pushSubscriptionModel.address let request = OSRequestCreateUser(identityModel: user.identityModel, propertiesModel: user.propertiesModel, pushSubscriptionModel: user.pushSubscriptionModel, originalPushToken: originalPushToken) @@ -193,13 +191,13 @@ extension OSUserExecutor { /** This Create User call expects an external ID and the Identity Model to hydrate with the OneSignal ID */ - static func createUser(aliasLabel: String, aliasId: String, identityModel: OSIdentityModel) { + func createUser(aliasLabel: String, aliasId: String, identityModel: OSIdentityModel) { let request = OSRequestCreateUser(aliasLabel: aliasLabel, aliasId: aliasId, identityModel: identityModel) appendToQueue(request) executePendingRequests() } - static func executeCreateUserRequest(_ request: OSRequestCreateUser) { + func executeCreateUserRequest(_ request: OSRequestCreateUser) { guard !request.sentToClient else { return } @@ -217,13 +215,13 @@ extension OSUserExecutor { } OneSignalCoreImpl.sharedClient().execute(request) { response in - removeFromQueue(request) + self.removeFromQueue(request) // TODO: Differentiate if we need to fetch the user based on response code of 200, 201, 202 // Create User's response won't send us the user's complete info if this user already exists if let response = response { // Parse the response for any data we need to update - parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: request.originalPushToken) + self.parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: request.originalPushToken) // If this user already exists and we logged into an external_id, fetch the user data // TODO: Only do this if response code is 200 or 202 @@ -232,9 +230,9 @@ extension OSUserExecutor { let identity = request.parameters?["identity"] as? [String: String], let onesignalId = request.identityModel.onesignalId, identity[OS_EXTERNAL_ID] != nil { - fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModel) + self.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModel) } else { - executePendingRequests() + self.executePendingRequests() } } OSOperationRepo.sharedInstance.paused = false @@ -250,12 +248,12 @@ extension OSUserExecutor { request.sentToClient = false } } else { - executePendingRequests() + self.executePendingRequests() } } } - static func fetchIdentityBySubscription(_ user: OSUserInternal) { + func fetchIdentityBySubscription(_ user: OSUserInternal) { let request = OSRequestFetchIdentityBySubscription(identityModel: user.identityModel, pushSubscriptionModel: user.pushSubscriptionModel) appendToQueue(request) @@ -265,7 +263,7 @@ extension OSUserExecutor { /** For migrating legacy players from 3.x to 5.x. This request will fetch the identity object for a subscription ID, and we will use the returned onesignalId to fetch and hydrate the local user. */ - static func executeFetchIdentityBySubscriptionRequest(_ request: OSRequestFetchIdentityBySubscription) { + func executeFetchIdentityBySubscriptionRequest(_ request: OSRequestFetchIdentityBySubscription) { guard !request.sentToClient else { return } @@ -275,20 +273,20 @@ extension OSUserExecutor { request.sentToClient = true OneSignalCoreImpl.sharedClient().execute(request) { response in - removeFromQueue(request) + self.removeFromQueue(request) - if let identityObject = parseIdentityObjectResponse(response), + if let identityObject = self.parseIdentityObjectResponse(response), let onesignalId = identityObject[OS_ONESIGNAL_ID] { request.identityModel.hydrate(identityObject) // Fetch this user's data if it is the current user guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) else { - executePendingRequests() + self.executePendingRequests() return } - fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModel) + self.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModel) } } onFailure: { error in OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor executeFetchIdentityBySubscriptionRequest failed with error: \(error.debugDescription)") @@ -297,14 +295,14 @@ extension OSUserExecutor { if responseType != .retryable { // Fail, no retry, remove the subscription_id but keep the same push subscription model OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId = nil - removeFromQueue(request) + self.removeFromQueue(request) } } - executePendingRequests() + self.executePendingRequests() } } - static func identifyUser(externalId: String, identityModelToIdentify: OSIdentityModel, identityModelToUpdate: OSIdentityModel) { + func identifyUser(externalId: String, identityModelToIdentify: OSIdentityModel, identityModelToUpdate: OSIdentityModel) { let request = OSRequestIdentifyUser( aliasLabel: OS_EXTERNAL_ID, aliasId: externalId, @@ -317,7 +315,7 @@ extension OSUserExecutor { executePendingRequests() } - static func executeIdentifyUserRequest(_ request: OSRequestIdentifyUser) { + func executeIdentifyUserRequest(_ request: OSRequestIdentifyUser) { guard !request.sentToClient else { return } @@ -328,11 +326,11 @@ extension OSUserExecutor { request.sentToClient = true OneSignalCoreImpl.sharedClient().execute(request) { _ in - removeFromQueue(request) + self.removeFromQueue(request) guard let onesignalId = request.identityModelToIdentify.onesignalId else { OneSignalLog.onesignalLog(.LL_ERROR, message: "executeIdentifyUserRequest succeeded but is now missing OneSignal ID!") - executePendingRequests() + self.executePendingRequests() return } @@ -346,9 +344,9 @@ extension OSUserExecutor { // the anonymous user has been identified, still need to Fetch User as we cleared local data // Fetch the user only if its the current user if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) { - fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModelToUpdate) + self.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: request.identityModelToUpdate) } else { - executePendingRequests() + self.executePendingRequests() } } onFailure: { error in if let nsError = error as? NSError { @@ -357,22 +355,22 @@ extension OSUserExecutor { // Returns 409 if any provided (label, id) pair exists on another User, so the SDK will switch to this user. OneSignalLog.onesignalLog(.LL_DEBUG, message: "executeIdentifyUserRequest returned error code user-2. Now handling user-2 error response... switch to this user.") - removeFromQueue(request) + self.removeFromQueue(request) if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) { // Generate a Create User request, if it's still the current user - createUser(OneSignalUserManagerImpl.sharedInstance.user) + self.createUser(OneSignalUserManagerImpl.sharedInstance.user) } else { // This will hydrate the OneSignal ID for any pending requests - createUser(aliasLabel: request.aliasLabel, aliasId: request.aliasId, identityModel: request.identityModelToUpdate) + self.createUser(aliasLabel: request.aliasLabel, aliasId: request.aliasId, identityModel: request.identityModelToUpdate) } } else if responseType == .invalid || responseType == .unauthorized { // Failed, no retry - removeFromQueue(request) - executePendingRequests() + self.removeFromQueue(request) + self.executePendingRequests() } else if responseType == .missing { - removeFromQueue(request) - executePendingRequests() + self.removeFromQueue(request) + self.executePendingRequests() // Logout if the user in the SDK is the same guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModelToUpdate) else { @@ -383,12 +381,12 @@ extension OSUserExecutor { OneSignalUserManagerImpl.sharedInstance._logout() } } else { - executePendingRequests() + self.executePendingRequests() } } } - static func fetchUser(aliasLabel: String, aliasId: String, identityModel: OSIdentityModel, onNewSession: Bool = false) { + func fetchUser(aliasLabel: String, aliasId: String, identityModel: OSIdentityModel, onNewSession: Bool = false) { let request = OSRequestFetchUser(identityModel: identityModel, aliasLabel: aliasLabel, aliasId: aliasId, onNewSession: onNewSession) appendToQueue(request) @@ -396,7 +394,7 @@ extension OSUserExecutor { executePendingRequests() } - static func executeFetchUserRequest(_ request: OSRequestFetchUser) { + func executeFetchUserRequest(_ request: OSRequestFetchUser) { guard !request.sentToClient else { return } @@ -406,18 +404,18 @@ extension OSUserExecutor { } request.sentToClient = true OneSignalCoreImpl.sharedClient().execute(request) { response in - removeFromQueue(request) + self.removeFromQueue(request) if let response = response { // Clear local data in preparation for hydration OneSignalUserManagerImpl.sharedInstance.clearUserData() - parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: OneSignalUserManagerImpl.sharedInstance.pushSubscriptionImpl.token) + self.parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: OneSignalUserManagerImpl.sharedInstance.pushSubscriptionImpl.token) // If this is a on-new-session's fetch user call, check that the subscription still exists if request.onNewSession, OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel), let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId, - let subscriptionObjects = parseSubscriptionObjectResponse(response) { + let subscriptionObjects = self.parseSubscriptionObjectResponse(response) { var subscriptionExists = false for subModel in subscriptionObjects { if subModel["id"] as? String == subId { @@ -434,13 +432,13 @@ extension OSUserExecutor { } } } - executePendingRequests() + self.executePendingRequests() } onFailure: { error in OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor executeFetchUserRequest failed with error: \(error.debugDescription)") if let nsError = error as? NSError { let responseType = OSNetworkingUtils.getResponseStatusType(nsError.code) if responseType == .missing { - removeFromQueue(request) + self.removeFromQueue(request) // Logout if the user in the SDK is the same guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) else { @@ -451,10 +449,10 @@ extension OSUserExecutor { OneSignalUserManagerImpl.sharedInstance._logout() } else if responseType != .retryable { // If the error is not retryable, remove from cache and queue - removeFromQueue(request) + self.removeFromQueue(request) } } - executePendingRequests() + self.executePendingRequests() } } } @@ -464,7 +462,7 @@ extension OSUserExecutor { /** Used to parse Create User and Fetch User responses. The `originalPushToken` is the push token when the request was created, which may be different from the push token currently in the SDK. For example, when the request was created, there may be no push token yet, but soon after, the SDK receives a push token. This is used to determine whether or not to hydrate the push subscription. */ - static func parseFetchUserResponse(response: [AnyHashable: Any], identityModel: OSIdentityModel, originalPushToken: String?) { + func parseFetchUserResponse(response: [AnyHashable: Any], identityModel: OSIdentityModel, originalPushToken: String?) { // If this was a create user, it hydrates the onesignal_id of the request's identityModel // The model in the store may be different, and it may be waiting on the onesignal_id of this previous model @@ -533,7 +531,7 @@ extension OSUserExecutor { /** Returns if 2 tokens are equal. This is needed as a nil token is equal to the empty string "". */ - static func areTokensEqual(tokenA: String?, tokenB: String?) -> Bool { + func areTokensEqual(tokenA: String?, tokenB: String?) -> Bool { // They are both strings or both nil if tokenA == tokenB { return true @@ -545,15 +543,15 @@ extension OSUserExecutor { return false } - static func parseSubscriptionObjectResponse(_ response: [AnyHashable: Any]?) -> [[String: Any]]? { + func parseSubscriptionObjectResponse(_ response: [AnyHashable: Any]?) -> [[String: Any]]? { return response?["subscriptions"] as? [[String: Any]] } - static func parsePropertiesObjectResponse(_ response: [AnyHashable: Any]?) -> [String: Any]? { + func parsePropertiesObjectResponse(_ response: [AnyHashable: Any]?) -> [String: Any]? { return response?["properties"] as? [String: Any] } - static func parseIdentityObjectResponse(_ response: [AnyHashable: Any]?) -> [String: String]? { + func parseIdentityObjectResponse(_ response: [AnyHashable: Any]?) -> [String: String]? { return response?["identity"] as? [String: String] } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index e5e7f8ead..9a2973d74 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -177,6 +177,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { let pushSubscriptionModelStoreListener: OSSubscriptionModelStoreListener // Executors must be initialize after sharedInstance is initialized + var userExecutor: OSUserExecutor? var propertyExecutor: OSPropertyOperationExecutor? var identityExecutor: OSIdentityOperationExecutor? var subscriptionExecutor: OSSubscriptionOperationExecutor? @@ -221,7 +222,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { // Setup the executors // The OSUserExecutor has to run first, before other executors - OSUserExecutor.start() + self.userExecutor = OSUserExecutor() OSOperationRepo.sharedInstance.start() // Cannot initialize these executors in `init` as they reference the sharedInstance @@ -290,7 +291,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { let newUser = setNewInternalUser(externalId: nil, pushSubscriptionModel: pushSubscriptionModel) // 3. Make the request - OSUserExecutor.fetchIdentityBySubscription(newUser) + userExecutor!.fetchIdentityBySubscription(newUser) } private func createNewUser(externalId: String?, token: String?) -> OSUserInternal { @@ -315,7 +316,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { let newUser = setNewInternalUser(externalId: externalId, pushSubscriptionModel: pushSubscriptionModel) newUser.identityModel.jwtBearerToken = token - OSUserExecutor.createUser(newUser) + userExecutor!.createUser(newUser) return self.user } @@ -340,7 +341,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { let newUser = setNewInternalUser(externalId: externalId, pushSubscriptionModel: pushSubscriptionModel) // Now proceed to identify the previous user - OSUserExecutor.identifyUser( + userExecutor!.identifyUser( externalId: externalId, identityModelToIdentify: identityModelToIdentify, identityModelToUpdate: newUser.identityModel @@ -546,13 +547,13 @@ extension OneSignalUserManagerImpl { } start() - OSUserExecutor.executePendingRequests() + userExecutor!.executePendingRequests() OSOperationRepo.sharedInstance.paused = false updatePropertiesDeltas(property: .session_count, value: 1) // Fetch the user's data if there is a onesignal_id if let onesignalId = onesignalId { - OSUserExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: user.identityModel, onNewSession: true) + userExecutor!.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: user.identityModel, onNewSession: true) } else { // It is possible to init a user from cache who is missing the onesignalId // This can happen if any createUser or identifyUser requests are cached diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift index ae30a48c1..5e7b48f4b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift @@ -35,15 +35,10 @@ public class OneSignalUserMocks: NSObject { // TODO: create mocked server responses to user requests @objc public static func reset() { - resetStaticUserExecutor() // TODO: Reset Operation Repo first // OSCoreMocks.resetOperationRepo() OneSignalUserManagerImpl.sharedInstance.reset() } - - public static func resetStaticUserExecutor() { - OSUserExecutor.userRequestQueue.removeAll() - } } extension OSIdentityModelRepo { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift index 8769dc3cf..4ec494e70 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift @@ -230,7 +230,7 @@ final class OneSignalUserTests: XCTestCase { let identityModel1 = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: UUID().uuidString], changeNotifier: OSEventProducer()) let identityModel2 = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: UUID().uuidString], changeNotifier: OSEventProducer()) - OSUserExecutor.start() + let userExecutor = OSUserExecutor() /* When */ @@ -239,10 +239,10 @@ final class OneSignalUserTests: XCTestCase { let fetchRequest = OSRequestFetchUser(identityModel: identityModel1, aliasLabel: OS_ONESIGNAL_ID, aliasId: UUID().uuidString, onNewSession: false) // Append and execute requests simultaneously - OSUserExecutor.appendToQueue(identifyRequest) - OSUserExecutor.appendToQueue(fetchRequest) - OSUserExecutor.executeIdentifyUserRequest(identifyRequest) - OSUserExecutor.executeFetchUserRequest(fetchRequest) + userExecutor.appendToQueue(identifyRequest) + userExecutor.appendToQueue(fetchRequest) + userExecutor.executeIdentifyUserRequest(identifyRequest) + userExecutor.executeFetchUserRequest(fetchRequest) } // Run background threads From eede113a9ded8f60b35163aad52d6634c1fbfd59 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 25 Jul 2024 18:25:29 -0700 Subject: [PATCH 2/3] nit: Fix a string description to simplify tests * Fix the string description of OSRequestCreateUser to be uniform --- .../OneSignalUser/Source/Requests/OSRequestCreateUser.swift | 6 +++--- .../OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift index 4532f44e5..d88bde925 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift @@ -70,7 +70,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest { self.identityModel = identityModel self.pushSubscriptionModel = pushSubscriptionModel self.originalPushToken = originalPushToken - self.stringDescription = "" + self.stringDescription = "" super.init() var params: [String: Any] = [:] @@ -95,7 +95,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest { init(aliasLabel: String, aliasId: String, identityModel: OSIdentityModel) { self.identityModel = identityModel - self.stringDescription = "" + self.stringDescription = "" super.init() self.parameters = [ "identity": [aliasLabel: aliasId], @@ -126,7 +126,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest { self.identityModel = identityModel self.pushSubscriptionModel = coder.decodeObject(forKey: "pushSubscriptionModel") as? OSSubscriptionModel self.originalPushToken = coder.decodeObject(forKey: "originalPushToken") as? String - self.stringDescription = "" + self.stringDescription = "" super.init() self.parameters = parameters self.method = HTTPMethod(rawValue: rawMethod) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift index 403fe3dd4..e79ceea8e 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserRequests.swift @@ -80,7 +80,7 @@ extension MockUserRequests { let anonCreateResponse = testDefaultFullCreateUserResponse(onesignalId: anonUserOSID, externalId: nil, subscriptionId: testPushSubId) client.setMockResponseForRequest( - request: "", + request: "", response: anonCreateResponse) } @@ -90,7 +90,7 @@ extension MockUserRequests { let userResponse = MockUserRequests.testIdentityPayload(onesignalId: osid, externalId: externalId) client.setMockResponseForRequest( - request: "", + request: "", response: userResponse ) client.setMockResponseForRequest( @@ -114,7 +114,7 @@ extension MockUserRequests { // 2. Set the response for the subsequent Create User request let userResponse = MockUserRequests.testIdentityPayload(onesignalId: osid, externalId: externalId) client.setMockResponseForRequest( - request: "", + request: "", response: userResponse) // 3. Set the response for the subsequent Fetch User request client.setMockResponseForRequest( From eb58e4f69a9f41e7e5d40b0b996fb8ecfb9d8b91 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 26 Jul 2024 09:25:47 -0700 Subject: [PATCH 3/3] no logic change - organize executor startup code * No logic changes here, just break up executor code into digestible functions --- .../Executors/OSIdentityOperationExecutor.swift | 14 +++++++++++--- .../Executors/OSPropertyOperationExecutor.swift | 10 ++++++++-- .../OSSubscriptionOperationExecutor.swift | 17 ++++++++++++++--- .../Source/Executors/OSUserExecutor.swift | 11 +++++++---- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift index d195f390d..0a428abaa 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift @@ -39,7 +39,13 @@ class OSIdentityOperationExecutor: OSOperationExecutor { private let dispatchQueue = DispatchQueue(label: "OneSignal.OSIdentityOperationExecutor", target: .global()) init() { - // Read unfinished deltas from cache, if any... + // Read unfinished deltas and requests from cache, if any... + uncacheDeltas() + uncacheAddAliasRequests() + uncacheRemoveAliasRequests() + } + + private func uncacheDeltas() { if var deltaQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_IDENTITY_EXECUTOR_DELTA_QUEUE_KEY, defaultValue: []) as? [OSDelta] { // Hook each uncached Delta to the model in the store for (index, delta) in deltaQueue.enumerated().reversed() { @@ -57,9 +63,9 @@ class OSIdentityOperationExecutor: OSOperationExecutor { } else { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor error encountered reading from cache for \(OS_IDENTITY_EXECUTOR_DELTA_QUEUE_KEY)") } + } - // Read unfinished requests from cache, if any... - + private func uncacheAddAliasRequests() { if var addRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_IDENTITY_EXECUTOR_ADD_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestAddAliases] { // Hook each uncached Request to the model in the store for (index, request) in addRequestQueue.enumerated().reversed() { @@ -80,7 +86,9 @@ class OSIdentityOperationExecutor: OSOperationExecutor { } else { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor error encountered reading from cache for \(OS_IDENTITY_EXECUTOR_ADD_REQUEST_QUEUE_KEY)") } + } + private func uncacheRemoveAliasRequests() { if var removeRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_IDENTITY_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestRemoveAlias] { // Hook each uncached Request to the model in the store for (index, request) in removeRequestQueue.enumerated().reversed() { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift index 27dec7298..5a0625f67 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift @@ -69,8 +69,13 @@ class OSPropertyOperationExecutor: OSOperationExecutor { private let dispatchQueue = DispatchQueue(label: "OneSignal.OSPropertyOperationExecutor", target: .global()) init() { - // Read unfinished deltas from cache, if any... + // Read unfinished deltas and requests from cache, if any... // Note that we should only have deltas for the current user as old ones are flushed.. + uncacheDeltas() + uncacheUpdateRequests() + } + + private func uncacheDeltas() { if var deltaQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, defaultValue: []) as? [OSDelta] { for (index, delta) in deltaQueue.enumerated().reversed() { if OneSignalUserManagerImpl.sharedInstance.getIdentityModel(delta.identityModelId) == nil { @@ -84,8 +89,9 @@ class OSPropertyOperationExecutor: OSOperationExecutor { } else { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor error encountered reading from cache for \(OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY)") } + } - // Read unfinished requests from cache, if any... + private func uncacheUpdateRequests() { if var updateRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestUpdateProperties] { // Hook each uncached Request to the model in the store for (index, request) in updateRequestQueue.enumerated().reversed() { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift index 4fd23f296..837a49eb1 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift @@ -41,7 +41,14 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor { private let dispatchQueue = DispatchQueue(label: "OneSignal.OSSubscriptionOperationExecutor", target: .global()) init() { - // Read unfinished deltas from cache, if any... + // Read unfinished deltas and requests from cache, if any... + uncacheDeltas() + uncacheCreateSubscriptionRequests() + uncacheDeleteSubscriptionRequests() + uncacheUpdateSubscriptionRequests() + } + + private func uncacheDeltas() { if var deltaQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_DELTA_QUEUE_KEY, defaultValue: []) as? [OSDelta] { // Hook each uncached Delta to the model in the store for (index, delta) in deltaQueue.enumerated().reversed() { @@ -59,9 +66,9 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor { } else { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_DELTA_QUEUE_KEY)") } + } - // Read unfinished requests from cache, if any... - + private func uncacheCreateSubscriptionRequests() { var requestQueue: [OSRequestCreateSubscription] = [] if let cachedAddRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_ADD_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestCreateSubscription] { @@ -97,7 +104,9 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor { } else { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_ADD_REQUEST_QUEUE_KEY)") } + } + private func uncacheDeleteSubscriptionRequests() { if var removeRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestDeleteSubscription] { // Hook each uncached Request to the model in the store for (index, request) in removeRequestQueue.enumerated().reversed() { @@ -118,7 +127,9 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor { } else { OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY)") } + } + private func uncacheUpdateSubscriptionRequests() { if var updateRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestUpdateSubscription] { // Hook each uncached Request to the model in the store for (index, request) in updateRequestQueue.enumerated().reversed() { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index d6832505e..52e5d0534 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -39,8 +39,14 @@ class OSUserExecutor { // The User executor dispatch queue, serial. This synchronizes access to the request queues. private let dispatchQueue = DispatchQueue(label: "OneSignal.OSUserExecutor", target: .global()) - // Read in requests from the cache, do not read in FetchUser requests as this is not needed. init() { + uncacheUserRequests() + migrateTransferSubscriptionRequests() + executePendingRequests() + } + + // Read in requests from the cache, do not read in FetchUser requests as this is not needed. + private func uncacheUserRequests() { var userRequestQueue: [OSUserRequest] = [] // Read unfinished Create User + Identify User + Get Identity By Subscription requests from cache, if any... @@ -97,9 +103,6 @@ class OSUserExecutor { } self.userRequestQueue = userRequestQueue OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue) - - migrateTransferSubscriptionRequests() - executePendingRequests() } /**