diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModel.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModel.swift index 96f2249f2..eb65a8167 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModel.swift @@ -52,11 +52,11 @@ open class OSModel: NSObject, NSCoding { } // We can add operation name to this... , such as enum of "updated", "deleted", "added" - public func set(property: String, newValue: T) { + public func set(property: String, newValue: T, preventServerUpdate: Bool = false) { let changeArgs = OSModelChangedArgs(model: self, property: property, newValue: newValue) changeNotifier.fire { modelChangeHandler in - modelChangeHandler.onModelUpdated(args: changeArgs, hydrating: self.hydrating) + modelChangeHandler.onModelUpdated(args: changeArgs, hydrating: self.hydrating || preventServerUpdate) } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift index 64a2f832c..701d34ec2 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStore.swift @@ -31,6 +31,7 @@ open class OSModelStore: NSObject { let storeKey: String let changeSubscription: OSEventProducer var models: [String: TModel] + let lock = NSLock() public init(changeSubscription: OSEventProducer, storeKey: String) { self.storeKey = storeKey @@ -67,67 +68,76 @@ open class OSModelStore: NSObject { Examples: "person@example.com" for a subscription model or `OS_IDENTITY_MODEL_KEY` for an identity model. */ public func getModel(key: String) -> TModel? { - return self.models[key] + lock.withLock { + return self.models[key] + } } /** Uses the `modelId` to get the corresponding model in the store's models dictionary. */ public func getModel(modelId: String) -> TModel? { - for model in models.values { - if model.modelId == modelId { - return model + lock.withLock { + for model in models.values { + if model.modelId == modelId { + return model + } } + return nil } - return nil } public func getModels() -> [String: TModel] { - return self.models + lock.withLock { + return self.models + } } public func add(id: String, model: TModel, hydrating: Bool) { // TODO: Check if we are adding the same model? Do we replace? // For example, calling addEmail multiple times with the same email // Check API endpoint for behavior - models[id] = model + lock.withLock { + models[id] = model - // persist the models (including new model) to storage - OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models) + // persist the models (including new model) to storage + OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models) - // listen for changes to this model - model.changeNotifier.subscribe(self) + // listen for changes to this model + model.changeNotifier.subscribe(self) - guard !hydrating else { - return - } + guard !hydrating else { + return + } - self.changeSubscription.fire { modelStoreListener in - modelStoreListener.onAdded(model) + self.changeSubscription.fire { modelStoreListener in + modelStoreListener.onAdded(model) + } } } /** - Returns false if this model does not exist in the store. + Nothing will happen if this model does not exist in the store. This can happen if remove email or SMS is called and it doesn't exist in the store. */ public func remove(_ id: String) { - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSModelStore remove() called with model \(id)") - // TODO: Nothing will happen if model doesn't exist in the store - if let model = models[id] { - models.removeValue(forKey: id) - - // persist the models (with removed model) to storage - OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models) - - // no longer listen for changes to this model - model.changeNotifier.unsubscribe(self) - - self.changeSubscription.fire { modelStoreListener in - modelStoreListener.onRemoved(model) + lock.withLock { + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSModelStore remove() called with model \(id)") + if let model = models[id] { + models.removeValue(forKey: id) + + // persist the models (with removed model) to storage + OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models) + + // no longer listen for changes to this model + model.changeNotifier.unsubscribe(self) + + self.changeSubscription.fire { modelStoreListener in + modelStoreListener.onRemoved(model) + } + } else { + OneSignalLog.onesignalLog(.LL_ERROR, message: "OSModelStore cannot remove \(id) because it doesn't exist in the store.") } - } else { - OneSignalLog.onesignalLog(.LL_ERROR, message: "OSModelStore cannot remove \(id) because it doesn't exist in the store.") } } @@ -146,20 +156,24 @@ open class OSModelStore: NSObject { In contrast, it is not necessary for the Identity or Properties Model Stores to do so. */ public func clearModelsFromStore() { - self.models = [:] + lock.withLock { + self.models = [:] + } } } extension OSModelStore: OSModelChangedHandler { public func onModelUpdated(args: OSModelChangedArgs, hydrating: Bool) { // persist the changed models to storage - OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models) + lock.withLock { + OneSignalUserDefaults.initShared().saveCodeableData(forKey: self.storeKey, withValue: self.models) - guard !hydrating else { - return - } - self.changeSubscription.fire { modelStoreListener in - modelStoreListener.onUpdated(args) + guard !hydrating else { + return + } + self.changeSubscription.fire { modelStoreListener in + modelStoreListener.onUpdated(args) + } } } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift index 0a930320a..6e4799cfb 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift @@ -117,13 +117,19 @@ class OSSubscriptionModel: OSModel { } } - // Set via server response + /** + Typically, the subscription ID is set via server response, so don't trigger a server update call when it changes. + It can also be set to null by the SDK when the user or subscription is detected as missing. + Setting the subscription ID to null will serve as a "reset" and will later hydrate a value from a user create rquest. + */ var subscriptionId: String? { didSet { guard subscriptionId != oldValue else { return } - self.set(property: "subscriptionId", newValue: subscriptionId) + + // If the ID has changed, don't trigger a server call, since it can be set to null + self.set(property: "subscriptionId", newValue: subscriptionId, preventServerUpdate: true) guard self.type == .push else { return diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index 5d22dc858..1a11f4d5d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -122,6 +122,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { let newRecordsState = OSNewRecordsState() + private let startQueue = DispatchQueue(label: "com.onesignal.user.start") var hasCalledStart = false private var jwtExpiredHandler: OSJwtExpiredHandler? @@ -136,9 +137,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { } // There is no user instance, initialize a "guest user" - let user = _login(externalId: nil, token: nil) - _user = user - return user + return createNewUser(externalId: nil, token: nil) } var _user: OSUserInternal? @@ -192,71 +191,77 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { self.pushSubscriptionImpl = OSPushSubscriptionImpl(pushSubscriptionModelStore: pushSubscriptionModelStore) } - // TODO: This method is called A LOT, check if all calls are needed. @objc public func start() { guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: nil) else { return } - guard !hasCalledStart else { - return - } - hasCalledStart = true - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager calling start") - - OSNotificationsManager.delegate = self - - var hasCachedUser = false - - // Path 1. Load user from cache, if any - // Corrupted state if any of these models exist without the others - if let identityModel = identityModelStore.getModels()[OS_IDENTITY_MODEL_KEY], - let propertiesModel = propertiesModelStore.getModels()[OS_PROPERTIES_MODEL_KEY], - let pushSubscription = pushSubscriptionModelStore.getModels()[OS_PUSH_SUBSCRIPTION_MODEL_KEY] { - hasCachedUser = true - _user = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription) - addIdentityModelToRepo(identityModel) - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager.start called, loaded the user from cache.") - } - - // TODO: Update the push sub model with any new state from NotificationsManager - - // Setup the executors - // The OSUserExecutor has to run first, before other executors - self.userExecutor = OSUserExecutor(newRecordsState: newRecordsState) - OSOperationRepo.sharedInstance.start() - - // Cannot initialize these executors in `init` as they reference the sharedInstance - let propertyExecutor = OSPropertyOperationExecutor(newRecordsState: newRecordsState) - let identityExecutor = OSIdentityOperationExecutor(newRecordsState: newRecordsState) - let subscriptionExecutor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState) - self.propertyExecutor = propertyExecutor - self.identityExecutor = identityExecutor - self.subscriptionExecutor = subscriptionExecutor - OSOperationRepo.sharedInstance.addExecutor(identityExecutor) - OSOperationRepo.sharedInstance.addExecutor(propertyExecutor) - OSOperationRepo.sharedInstance.addExecutor(subscriptionExecutor) - - // Path 2. There is a legacy player to migrate - if let legacyPlayerId = OneSignalUserDefaults.initShared().getSavedString(forKey: OSUD_LEGACY_PLAYER_ID, defaultValue: nil) { - OneSignalLog.onesignalLog(.LL_DEBUG, message: "OneSignalUserManager: creating user linked to legacy subscription \(legacyPlayerId)") - createUserFromLegacyPlayer(legacyPlayerId) - OneSignalUserDefaults.initShared().saveString(forKey: OSUD_PUSH_SUBSCRIPTION_ID, withValue: legacyPlayerId) - OneSignalUserDefaults.initStandard().removeValue(forKey: OSUD_LEGACY_PLAYER_ID) - OneSignalUserDefaults.initShared().removeValue(forKey: OSUD_LEGACY_PLAYER_ID) - } else { - // Path 3. Creates an anonymous user if there isn't one in the cache nor a legacy player - createUserIfNil() - } + // Use a serial queue to make the start process atomic + startQueue.sync { + guard !hasCalledStart else { + return + } + + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager calling start") + + OSNotificationsManager.delegate = self - // Model store listeners subscribe to their models - identityModelStoreListener.start() - propertiesModelStoreListener.start() - subscriptionModelStoreListener.start() - pushSubscriptionModelStoreListener.start() - if hasCachedUser { - _user?.update() + var hasCachedUser = false + + // Path 1. Load user from cache, if any + // Corrupted state if any of these models exist without the others + if let identityModel = identityModelStore.getModels()[OS_IDENTITY_MODEL_KEY], + let propertiesModel = propertiesModelStore.getModels()[OS_PROPERTIES_MODEL_KEY], + let pushSubscription = pushSubscriptionModelStore.getModels()[OS_PUSH_SUBSCRIPTION_MODEL_KEY] { + hasCachedUser = true + _user = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription) + addIdentityModelToRepo(identityModel) + OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager.start called, loaded the user from cache.") + } + + // TODO: Update the push sub model with any new state from NotificationsManager + + // Setup the executors + // The OSUserExecutor has to run first, before other executors + self.userExecutor = OSUserExecutor(newRecordsState: newRecordsState) + OSOperationRepo.sharedInstance.start() + + // Cannot initialize these executors in `init` as they reference the sharedInstance + let propertyExecutor = OSPropertyOperationExecutor(newRecordsState: newRecordsState) + let identityExecutor = OSIdentityOperationExecutor(newRecordsState: newRecordsState) + let subscriptionExecutor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState) + self.propertyExecutor = propertyExecutor + self.identityExecutor = identityExecutor + self.subscriptionExecutor = subscriptionExecutor + OSOperationRepo.sharedInstance.addExecutor(identityExecutor) + OSOperationRepo.sharedInstance.addExecutor(propertyExecutor) + OSOperationRepo.sharedInstance.addExecutor(subscriptionExecutor) + + // Path 2. There is a legacy player to migrate + if let legacyPlayerId = OneSignalUserDefaults.initShared().getSavedString(forKey: OSUD_LEGACY_PLAYER_ID, defaultValue: nil) { + OneSignalLog.onesignalLog(.LL_DEBUG, message: "OneSignalUserManager: creating user linked to legacy subscription \(legacyPlayerId)") + createUserFromLegacyPlayer(legacyPlayerId) + OneSignalUserDefaults.initShared().saveString(forKey: OSUD_PUSH_SUBSCRIPTION_ID, withValue: legacyPlayerId) + OneSignalUserDefaults.initStandard().removeValue(forKey: OSUD_LEGACY_PLAYER_ID) + OneSignalUserDefaults.initShared().removeValue(forKey: OSUD_LEGACY_PLAYER_ID) + } else { + // Path 3. Creates an anonymous user if there isn't one in the cache nor a legacy player + if _user == nil { + // There is no user instance, initialize a "guest user" + _ = createNewUser(externalId: nil, token: nil) + } + } + + // Model store listeners subscribe to their models + identityModelStoreListener.start() + propertiesModelStoreListener.start() + subscriptionModelStoreListener.start() + pushSubscriptionModelStoreListener.start() + if hasCachedUser { + _user?.update() + } + hasCalledStart = true } } @@ -279,7 +284,16 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { return } OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignal.User login called with externalId: \(externalId)") - _ = _login(externalId: externalId, token: token) + + // Logging into an identified user from an anonymous user + if let user = _user, user.isAnonymous { + user.identityModel.jwtBearerToken = token + identifyUser(externalId: externalId, currentUser: user) + } else { + // Logging into identified -> anon, identified -> identified, or nil -> identified + _ = createNewUser(externalId: externalId, token: token) + } + } /** @@ -303,7 +317,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { Migrating a legacy player does not result in a Create User call, but certain local data should be sent manually. */ private func updateLegacyPlayer(_ user: OSUserInternal) { - if let timezoneId = user.propertiesModel.timezoneId { + if let timezoneId = _user?.propertiesModel.timezoneId { updatePropertiesDeltas(property: .timezone_id, value: timezoneId) } } @@ -331,7 +345,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { let newUser = setNewInternalUser(externalId: externalId, pushSubscriptionModel: pushSubscriptionModel) newUser.identityModel.jwtBearerToken = token userExecutor!.createUser(newUser) - return self.user + return newUser } /** @@ -389,31 +403,14 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { OneSignalUserManagerImpl.sharedInstance.subscriptionModelStore.clearModelsFromStore() } - private func _login(externalId: String?, token: String?) -> OSUserInternal { - guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: nil) else { - return _mockUser - } - OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManager internal _login called with externalId: \(externalId ?? "nil")") - - // If have token, validate token. Account for this being a requirement. - // Logging into an identified user from an anonymous user - if let externalId = externalId, - let user = _user, - user.isAnonymous { - user.identityModel.jwtBearerToken = token - identifyUser(externalId: externalId, currentUser: user) - return self.user - } - - // Logging into anon -> anon, identified -> anon, identified -> identified, or nil -> any user - return createNewUser(externalId: externalId, token: token) - } - /** The SDK needs to have a user at all times, so this method will create a new anonymous user. If the current user is already anonymous, calling `logout` results in a no-op. */ @objc public func logout() { + guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: "logout") else { + return + } guard user.identityModel.externalId != nil else { OneSignalLog.onesignalLog(.LL_DEBUG, message: "OneSignal.User logout called, but the user is currently anonymous, so not logging out.") return @@ -478,8 +475,9 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager { pushSubscriptionModelStore.add(id: OS_PUSH_SUBSCRIPTION_MODEL_KEY, model: pushSubscription, hydrating: false) } - _user = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription) - return self.user + let newUser = OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushSubscription) + _user = newUser + return newUser } /** @@ -585,6 +583,10 @@ extension OneSignalUserManagerImpl { return } + guard let user = _user else { + return + } + // Get the identity and properties model of the current user let identityModel = user.identityModel let propertiesModel = user.propertiesModel diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserObjcTests.m b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserObjcTests.m index b367edbea..68c413f0b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserObjcTests.m +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserObjcTests.m @@ -33,6 +33,9 @@ - (void)testSendPurchases { MockOneSignalClient* client = [MockOneSignalClient new]; + // 0. Purchases will be dropped if there is no user instance. + [OneSignalUserManagerImpl.sharedInstance start]; + // 1. Set up mock responses for the anonymous user [MockUserRequests setDefaultCreateAnonUserResponsesWith:client]; [OneSignalCoreImpl setSharedClient:client]; diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift index 9165e31d7..3e292deae 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift @@ -74,6 +74,8 @@ final class OneSignalUserTests: XCTestCase { func testBasicCombiningUserUpdateDeltas_resultsInOneRequest() throws { /* Setup */ + OneSignalUserManagerImpl.sharedInstance.start() + let client = MockOneSignalClient() MockUserRequests.setDefaultCreateAnonUserResponses(with: client) OneSignalCoreImpl.setSharedClient(client) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserConcurrencyTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserConcurrencyTests.swift index bcf076a74..5d5252a2d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserConcurrencyTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/UserConcurrencyTests.swift @@ -278,4 +278,13 @@ final class UserConcurrencyTests: XCTestCase { identityModel.clearData() } } + + func testConcurrentStart() throws { + DispatchQueue.concurrentPerform(iterations: 5_000) { _ in + OneSignalUserManagerImpl.sharedInstance.start() + } + DispatchQueue.concurrentPerform(iterations: 5_000) { _ in + _ = OneSignalUserManagerImpl.sharedInstance.user + } + } }