Merge pull request #1418 from OneSignal/use_central_identity_model_repo

[Bug] Some pending properties can be sent to new user incorrectly, when users change

iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/xcshareddata/xcschemes/OneSignalUserTests.xcscheme

@@ -7,7 +7,7 @@
       buildImplicitDependencies = "YES">
    </BuildAction>
    <TestAction
-      buildConfiguration = "Debug"
+      buildConfiguration = "Test"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
       shouldUseLaunchSchemeArgsEnv = "YES"
@@ -27,7 +27,7 @@
       </Testables>
    </TestAction>
    <LaunchAction
-      buildConfiguration = "Debug"
+      buildConfiguration = "Test"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
       launchStyle = "0"

iOS_SDK/OneSignalSDK/OneSignalCoreMocks/.swiftlint.yml

@@ -0,0 +1,4 @@
+# in tests, we may want to force cast and throw any errors
+disabled_rules:
+  - force_cast
+  - identifier_name

iOS_SDK/OneSignalSDK/OneSignalCoreMocks/Extensions/NSDictionary+UnitTests.swift

@@ -0,0 +1,24 @@
+extension NSDictionary {
+    func contains(key: String, value: Any) -> Bool {
+        guard let dictVal = self[key] else {
+            return false
+        }
+
+        return equals(dictVal, value)
+    }
+
+    func contains(_ dict: [String: Any]) -> Bool {
+        for (key, value) in dict {
+            if !contains(key: key, value: value) {
+                return false
+            }
+        }
+        return true
+    }
+
+    private func equals(_ x: Any, _ y: Any) -> Bool {
+        guard x is AnyHashable else { return false }
+        guard y is AnyHashable else { return false }
+        return (x as! AnyHashable) == (y as! AnyHashable)
+    }
+}

iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift

@@ -40,6 +40,8 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
     var shouldUseProvisionalAuthorization = false // new in iOS 12 (aka Direct to History)
     var remoteParamsOutcomes: [String: Any] = [:]
 
+    public var allRequestsHandled = true
+
     /** May add to or change this default remote params response*/
     public func getRemoteParamsResponse() -> [String: Any] {
         return remoteParamsResponse ?? [
@@ -70,7 +72,7 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
 
     // Temp. method to log info while building unit tests
     @objc public func logSelfInfo() {
-        print("🧪 MockOneSignalClient with executionQueue \(executionQueue)")
+        print("🧪 MockOneSignalClient with executedRequests \(executedRequests)")
     }
 
     public func reset() {
@@ -115,6 +117,7 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
         if (mockResponses[String(describing: request)]) != nil {
             successBlock(mockResponses[String(describing: request)])
         } else {
+            allRequestsHandled = false
             print("🧪 cannot find a mock response for request: \(request)")
         }
     }
@@ -137,3 +140,34 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
         mockResponses[request] = response
     }
 }
+
+// MARK: - Asserts
+
+extension MockOneSignalClient {
+    /**
+     Checks if there is only one executed request that contains the payload provided, and the url matches the path provided.
+     */
+    public func onlyOneRequest(contains path: String, contains payload: [String: Any]) -> Bool {
+        var found = false
+
+        for request in executedRequests {
+            guard let params = request.parameters as? NSDictionary  else {
+                continue
+            }
+
+            if params.contains(payload) {
+                if request.path == path {
+                    guard !found else {
+                        // False if more than 1 request satisfies both requirements
+                        return false
+                    }
+                    found = true
+                } else {
+                    return false
+                }
+            }
+        }
+
+        return found
+    }
+}

iOS_SDK/OneSignalSDK/OneSignalCoreMocks/OneSignalCoreMocks.swift

@@ -22,6 +22,7 @@
 
 import Foundation
 import OneSignalCore
+import XCTest
 
 @objc
 public class OneSignalCoreMocks: NSObject {
@@ -43,4 +44,10 @@ public class OneSignalCoreMocks: NSObject {
             sharedUserDefaults.removeObject(forKey: key)
         }
     }
+
+    /** Wait specified number of seconds for any async methods to run */
+    public static func waitForBackgroundThreads(seconds: Double) {
+        let expectation = XCTestExpectation(description: "Wait for \(seconds) seconds")
+        _ = XCTWaiter.wait(for: [expectation], timeout: seconds)
+    }
 }

iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSDelta.swift

@@ -26,6 +26,7 @@
  */
 
 import Foundation
+import OneSignalCore
 
 // TODO: Known Issue: Since these don't carry the app_id, it may have changed by the time Deltas become Requests, if app_id changes.
 // All requests requiring unique ID's will effectively be dropped.
@@ -34,6 +35,7 @@ open class OSDelta: NSObject, NSCoding {
     public let name: String
     public let deltaId: String
     public let timestamp: Date
+    public let identityModelId: String
     public var model: OSModel
     public let property: String
     public let value: Any
@@ -42,10 +44,11 @@ open class OSDelta: NSObject, NSCoding {
         return "<OSDelta \(name) with property: \(property) value: \(value)>"
     }
 
-    public init(name: String, model: OSModel, property: String, value: Any) {
+    public init(name: String, identityModelId: String, model: OSModel, property: String, value: Any) {
         self.name = name
         self.deltaId = UUID().uuidString
         self.timestamp = Date()
+        self.identityModelId = identityModelId
         self.model = model
         self.property = property
         self.value = value
@@ -55,6 +58,7 @@ open class OSDelta: NSObject, NSCoding {
         coder.encode(name, forKey: "name")
         coder.encode(deltaId, forKey: "deltaId")
         coder.encode(timestamp, forKey: "timestamp")
+        coder.encode(identityModelId, forKey: "identityModelId")
         coder.encode(model, forKey: "model")
         coder.encode(property, forKey: "property")
         coder.encode(value, forKey: "value")
@@ -64,17 +68,19 @@ open class OSDelta: NSObject, NSCoding {
         guard let name = coder.decodeObject(forKey: "name") as? String,
               let deltaId = coder.decodeObject(forKey: "deltaId") as? String,
               let timestamp = coder.decodeObject(forKey: "timestamp") as? Date,
+              let identityModelId = coder.decodeObject(forKey: "identityModelId") as? String,
               let model = coder.decodeObject(forKey: "model") as? OSModel,
               let property = coder.decodeObject(forKey: "property") as? String,
               let value = coder.decodeObject(forKey: "value")
         else {
-            // Log error
+            OneSignalLog.onesignalLog(.LL_ERROR, message: "Unable to init OSDelta from cache")
             return nil
         }
 
         self.name = name
         self.deltaId = deltaId
         self.timestamp = timestamp
+        self.identityModelId = identityModelId
         self.model = model
         self.property = property
         self.value = value

iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSModelStoreListener.swift

@@ -47,7 +47,7 @@ extension OSModelStoreListener {
         store.changeSubscription.subscribe(self)
     }
 
-    func close() {
+    public func close() {
         store.changeSubscription.unsubscribe(self)
     }
 

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift

@@ -40,11 +40,12 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
         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() {
-                if let modelInStore = OneSignalUserManagerImpl.sharedInstance.identityModelStore.getModel(modelId: delta.model.modelId) {
-                    // The model exists in the store, set it to be the Delta's model
+                if let modelInStore = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(delta.model.modelId) {
+                    // The model exists in the repo, set it to be the Delta's model
                     delta.model = modelInStore
                 } else {
                     // The model does not exist, drop this Delta
+                    OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor.init dropped \(delta)")
                     deltaQueue.remove(at: index)
                 }
             }
@@ -59,14 +60,15 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
         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() {
-                if let identityModel = OneSignalUserManagerImpl.sharedInstance.identityModelStore.getModel(modelId: request.identityModel.modelId) {
-                    // 1. The model exists in the store, so set it to be the Request's models
+                if let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(request.identityModel.modelId) {
+                    // 1. The model exists in the repo, so set it to be the Request's models
                     request.identityModel = identityModel
-                } else if let identityModel = OSUserExecutor.identityModels[request.identityModel.modelId] {
-                    // 2. The model exists in the user executor
-                    request.identityModel = identityModel
-                } else if !request.prepareForExecution() {
-                    // 3. The models do not exist AND this request cannot be sent, drop this Request
+                } else if request.prepareForExecution() {
+                    // 2. The request can be sent, add the model to the repo
+                      OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(request.identityModel)
+                } else {
+                    // 3. The model do not exist AND this request cannot be sent, drop this Request
+                    OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor.init dropped \(request)")
                     addRequestQueue.remove(at: index)
                 }
             }
@@ -79,14 +81,15 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
         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() {
-                if let identityModel = OneSignalUserManagerImpl.sharedInstance.identityModelStore.getModel(modelId: request.identityModel.modelId) {
-                    // 1. The model exists in the store, so set it to be the Request's model
+                if let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(request.identityModel.modelId) {
+                    // 1. The model exists in the repo, so set it to be the Request's model
                     request.identityModel = identityModel
-                } else if let identityModel = OSUserExecutor.identityModels[request.identityModel.modelId] {
-                    // 2. The model exists in the user executor
-                    request.identityModel = identityModel
-                } else if !request.prepareForExecution() {
+                } else if request.prepareForExecution() {
+                    // 2. The request can be sent, add the model to the repo
+                    OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(request.identityModel)
+                } else {
                     // 3. The model does not exist AND this request cannot be sent, drop this Request
+                    OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor.init dropped \(request)")
                     removeRequestQueue.remove(at: index)
                 }
             }

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift

@@ -40,13 +40,9 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
         // Read unfinished deltas from cache, if any...
         // Note that we should only have deltas for the current user as old ones are flushed..
         if var deltaQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, defaultValue: []) as? [OSDelta] {
-            // Hook each uncached Delta to the model in the store
             for (index, delta) in deltaQueue.enumerated().reversed() {
-                if let modelInStore = OneSignalUserManagerImpl.sharedInstance.propertiesModelStore.getModel(modelId: delta.model.modelId) {
-                    // 1. The model exists in the properties model store, set it to be the Delta's model
-                    delta.model = modelInStore
-                } else {
-                    // 2. The model does not exist, drop this Delta
+                if OneSignalUserManagerImpl.sharedInstance.getIdentityModel(delta.identityModelId) == nil {
+                    // The identity model does not exist, drop this Delta
                     OneSignalLog.onesignalLog(.LL_WARN, message: "OSPropertyOperationExecutor.init dropped: \(delta)")
                     deltaQueue.remove(at: index)
                 }
@@ -61,17 +57,13 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
         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() {
-                // 0. Hook up the properties model if its the current user's so it can hydrate
-                if let propertiesModel = OneSignalUserManagerImpl.sharedInstance.propertiesModelStore.getModel(modelId: request.modelToUpdate.modelId) {
-                    request.modelToUpdate = propertiesModel
-                }
-                if let identityModel = OneSignalUserManagerImpl.sharedInstance.identityModelStore.getModel(modelId: request.identityModel.modelId) {
-                    // 1. The identity model exist in the store, set it to be the Request's models
+                if let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(request.identityModel.modelId) {
+                    // 1. The identity model exist in the repo, set it to be the Request's model
                     request.identityModel = identityModel
-                } else if let identityModel = OSUserExecutor.identityModels[request.identityModel.modelId] {
-                    // 2. The model exists in the user executor
-                    request.identityModel = identityModel
-                } else if !request.prepareForExecution() {
+                } else if request.prepareForExecution() {
+                    // 2. The request can be sent, add the model to the repo
+                    OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(request.identityModel)
+                } else {
                     // 3. The identitymodel do not exist AND this request cannot be sent, drop this Request
                     OneSignalLog.onesignalLog(.LL_WARN, message: "OSPropertyOperationExecutor.init dropped: \(request)")
                     updateRequestQueue.remove(at: index)
@@ -103,17 +95,18 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
                 OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)")
             }
             for delta in self.deltaQueue {
-                guard let model = delta.model as? OSPropertiesModel else {
-                    // Log error
+                guard let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(delta.identityModelId)
+                else {
+                    // drop this delta
+                    OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.processDeltaQueue dropped: \(delta)")
                     continue
                 }
 
                 let request = OSRequestUpdateProperties(
                     properties: [delta.property: delta.value],
                     deltas: nil,
                     refreshDeviceMetadata: false, // Sort this out.
-                    modelToUpdate: model,
-                    identityModel: OneSignalUserManagerImpl.sharedInstance.user.identityModel // TODO: Make sure this is ok
+                    identityModel: identityModel
                 )
                 self.updateRequestQueue.append(request)
             }
@@ -204,7 +197,6 @@ extension OSPropertyOperationExecutor {
             properties: [:],
             deltas: propertiesDeltas.jsonRepresentation(),
             refreshDeviceMetadata: refreshDeviceMetadata,
-            modelToUpdate: propertiesModel,
             identityModel: identityModel)
 
         if sendImmediately {