fix: attributes thread safety issue

awslabs · Jul 18, 2024 · c34f633 · c34f633
1 parent 3ad7ee6
commit c34f633
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 1 deletion.
diff --git a/Sources/Clickstream/Dependency/Clickstream/Analytics/AnalyticsClient.swift b/Sources/Clickstream/Dependency/Clickstream/Analytics/AnalyticsClient.swift
@@ -29,6 +29,7 @@ class AnalyticsClient: AnalyticsClientBehaviour {
     private(set) var simpleUserAttributes: [String: Any] = [:]
     private let clickstream: ClickstreamContext
     private(set) var userId: String?
+    let attributeLock = NSLock()
     var autoRecordClient: AutoRecordEventClient
 
     init(clickstream: ClickstreamContext,
@@ -45,6 +46,7 @@ class AnalyticsClient: AnalyticsClientBehaviour {
     }
 
     func addGlobalAttribute(_ attribute: AttributeValue, forKey key: String) {
+        attributeLock.lock()
         let eventError = EventChecker.checkAttribute(
             currentNumber: globalAttributes.count,
             key: key,
@@ -54,9 +56,11 @@ class AnalyticsClient: AnalyticsClientBehaviour {
         } else {
             globalAttributes[key] = attribute
         }
+        attributeLock.unlock()
     }
 
     func addUserAttribute(_ attribute: AttributeValue, forKey key: String) {
+        attributeLock.lock()
         let eventError = EventChecker.checkUserAttribute(currentNumber: allUserAttributes.count,
                                                          key: key,
                                                          value: attribute)
@@ -72,22 +76,29 @@ class AnalyticsClient: AnalyticsClientBehaviour {
             userAttribute["set_timestamp"] = Date().millisecondsSince1970
             allUserAttributes[key] = userAttribute
         }
+        attributeLock.unlock()
     }
 
     func removeGlobalAttribute(forKey key: String) {
+        attributeLock.lock()
         globalAttributes[key] = nil
+        attributeLock.unlock()
     }
 
     func removeUserAttribute(forKey key: String) {
+        attributeLock.lock()
         allUserAttributes[key] = nil
+        attributeLock.unlock()
     }
 
     func updateUserId(_ id: String?) {
         if userId != id {
             userId = id
             UserDefaultsUtil.saveCurrentUserId(storage: clickstream.storage, userId: userId)
             if let newUserId = id, !newUserId.isEmpty {
+                attributeLock.lock()
                 allUserAttributes = JsonObject()
+                attributeLock.unlock()
                 let userInfo = UserDefaultsUtil.getNewUserInfo(storage: clickstream.storage, userId: newUserId)
                 // swiftlint:disable force_cast
                 clickstream.userUniqueId = userInfo["user_unique_id"] as! String
@@ -105,7 +116,9 @@ class AnalyticsClient: AnalyticsClientBehaviour {
     }
 
     func updateUserAttributes() {
+        attributeLock.lock()
         UserDefaultsUtil.updateUserAttributes(storage: clickstream.storage, userAttributes: allUserAttributes)
+        attributeLock.unlock()
     }
 
     // MARK: - Event recording
@@ -130,6 +143,9 @@ class AnalyticsClient: AnalyticsClientBehaviour {
     }
 
     func record(_ event: ClickstreamEvent) throws {
+        if event.eventType != Event.PresetEvent.CLICKSTREAM_ERROR{
+            attributeLock.lock()
+        }
         for (key, attribute) in globalAttributes {
             event.addGlobalAttribute(attribute, forKey: key)
         }
@@ -147,6 +163,9 @@ class AnalyticsClient: AnalyticsClientBehaviour {
             event.setUserAttribute(simpleUserAttributes)
         }
         try eventRecorder.save(event)
+        if event.eventType != Event.PresetEvent.CLICKSTREAM_ERROR{
+            attributeLock.unlock()
+        }
     }
 
     func recordEventError(_ eventError: EventChecker.EventError) {
@@ -167,13 +186,15 @@ class AnalyticsClient: AnalyticsClientBehaviour {
     }
 
     func getSimpleUserAttributes() -> [String: Any] {
+        attributeLock.lock()
         simpleUserAttributes = [:]
         simpleUserAttributes[Event.ReservedAttribute.USER_FIRST_TOUCH_TIMESTAMP]
             = allUserAttributes[Event.ReservedAttribute.USER_FIRST_TOUCH_TIMESTAMP]
         if allUserAttributes.keys.contains(Event.ReservedAttribute.USER_ID) {
             simpleUserAttributes[Event.ReservedAttribute.USER_ID]
                 = allUserAttributes[Event.ReservedAttribute.USER_ID]
         }
+        attributeLock.unlock()
         return simpleUserAttributes
     }
 }

diff --git a/Sources/Clickstream/Dependency/Clickstream/Event/ClickstreamEvent.swift b/Sources/Clickstream/Dependency/Clickstream/Event/ClickstreamEvent.swift
@@ -21,6 +21,7 @@ class ClickstreamEvent: AnalyticsPropertiesModel {
     private(set) lazy var attributes: [String: AttributeValue] = [:]
     private(set) lazy var items: [ClickstreamAttribute] = []
     private(set) lazy var userAttributes: [String: Any] = [:]
+    let attributeLock = NSLock()
     let systemInfo: SystemInfo
     let netWorkType: String
 
@@ -72,7 +73,11 @@ class ClickstreamEvent: AnalyticsPropertiesModel {
     }
 
     func setUserAttribute(_ attributes: [String: Any]) {
-        userAttributes = attributes
+        attributeLock.lock()
+        for attr in attributes {
+            userAttributes[attr.key] = attr.value
+        }
+        attributeLock.unlock()
     }
 
     func attribute(forKey key: String) -> AttributeValue? {
@@ -109,9 +114,11 @@ class ClickstreamEvent: AnalyticsPropertiesModel {
         if !items.isEmpty {
             event["items"] = items
         }
+        attributeLock.lock()
         if !userAttributes.isEmpty {
             event["user"] = userAttributes
         }
+        attributeLock.unlock()
         event["attributes"] = getAttributeObject(from: attributes)
         return event
     }

diff --git a/Tests/ClickstreamTests/Clickstream/EventRecorderTest.swift b/Tests/ClickstreamTests/Clickstream/EventRecorderTest.swift
@@ -182,6 +182,20 @@ class EventRecorderTest: XCTestCase {
         XCTAssertEqual(85.5, (userAttributes["score"] as! JsonObject)["value"] as! Double)
         XCTAssertEqual("carl", (userAttributes["_user_name"] as! JsonObject)["value"] as! String)
     }
+
+    func testModifyGlobalUserAttributesWillNotAffectTheEventUserAttributes() throws {
+        var userInfo = JsonObject()
+        let currentTimeStamp = Date().millisecondsSince1970
+        var userNameInfo = JsonObject()
+        userNameInfo["value"] = "carl"
+        userNameInfo["set_timestamp"] = currentTimeStamp
+        userInfo["_user_name"] = userNameInfo
+        clickstreamEvent.setUserAttribute(userInfo)
+        userInfo = JsonObject()
+        userNameInfo["value"] = "mike"
+        userInfo["_user_name"] = userNameInfo
+        XCTAssertEqual((clickstreamEvent.userAttributes["_user_name"] as! JsonObject)["value"] as! String, "carl")
+    }
 
     func testRecordEventWithInvalidAttribute() throws {
         clickstreamEvent.addGlobalAttribute(Decimal.nan, forKey: "invalidDecimal")