Skip to content

Commit bd3a062

Browse files
zhu-xiaoweizhu-xiaowei
authored
fix: attributes thread safety issue (#73)
1 parent 3ad7ee6 commit bd3a062

File tree

8 files changed

+67
-2
lines changed

8 files changed

+67
-2
lines changed

‎.swiftformat

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
--enable semicolons
3636
--semicolons never
3737
--disable sortedImports
38+
--disable sortImports
3839
--importgrouping testable-bottom
3940
--enable spaceAroundOperators
4041
--operatorfunc spaced

‎.swiftlint.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ opt_in_rules:
1616

1717
disabled_rules:
1818
- opening_brace
19+
- non_optional_string_data_conversion
1920

2021
# configurable rules can be customized from this configuration file
2122
closing_brace: error

‎Sources/Clickstream/AWSClickstreamPlugin+ClientBehavior.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ extension AWSClickstreamPlugin {
6969
}
7070

7171
func registerGlobalProperties(_ properties: AnalyticsProperties) {
72-
properties.forEach { key, value in
72+
for (key, value) in properties {
7373
analyticsClient.addGlobalAttribute(value, forKey: key)
7474
}
7575
}

‎Sources/Clickstream/Dependency/Clickstream/Analytics/AnalyticsClient.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class AnalyticsClient: AnalyticsClientBehaviour {
2929
private(set) var simpleUserAttributes: [String: Any] = [:]
3030
private let clickstream: ClickstreamContext
3131
private(set) var userId: String?
32+
let attributeLock = NSLock()
3233
var autoRecordClient: AutoRecordEventClient
3334

3435
init(clickstream: ClickstreamContext,
@@ -45,6 +46,7 @@ class AnalyticsClient: AnalyticsClientBehaviour {
4546
}
4647

4748
func addGlobalAttribute(_ attribute: AttributeValue, forKey key: String) {
49+
attributeLock.lock()
4850
let eventError = EventChecker.checkAttribute(
4951
currentNumber: globalAttributes.count,
5052
key: key,
@@ -54,9 +56,11 @@ class AnalyticsClient: AnalyticsClientBehaviour {
5456
} else {
5557
globalAttributes[key] = attribute
5658
}
59+
attributeLock.unlock()
5760
}
5861

5962
func addUserAttribute(_ attribute: AttributeValue, forKey key: String) {
63+
attributeLock.lock()
6064
let eventError = EventChecker.checkUserAttribute(currentNumber: allUserAttributes.count,
6165
key: key,
6266
value: attribute)
@@ -72,22 +76,29 @@ class AnalyticsClient: AnalyticsClientBehaviour {
7276
userAttribute["set_timestamp"] = Date().millisecondsSince1970
7377
allUserAttributes[key] = userAttribute
7478
}
79+
attributeLock.unlock()
7580
}
7681

7782
func removeGlobalAttribute(forKey key: String) {
83+
attributeLock.lock()
7884
globalAttributes[key] = nil
85+
attributeLock.unlock()
7986
}
8087

8188
func removeUserAttribute(forKey key: String) {
89+
attributeLock.lock()
8290
allUserAttributes[key] = nil
91+
attributeLock.unlock()
8392
}
8493

8594
func updateUserId(_ id: String?) {
8695
if userId != id {
8796
userId = id
8897
UserDefaultsUtil.saveCurrentUserId(storage: clickstream.storage, userId: userId)
8998
if let newUserId = id, !newUserId.isEmpty {
99+
attributeLock.lock()
90100
allUserAttributes = JsonObject()
101+
attributeLock.unlock()
91102
let userInfo = UserDefaultsUtil.getNewUserInfo(storage: clickstream.storage, userId: newUserId)
92103
// swiftlint:disable force_cast
93104
clickstream.userUniqueId = userInfo["user_unique_id"] as! String
@@ -105,7 +116,9 @@ class AnalyticsClient: AnalyticsClientBehaviour {
105116
}
106117

107118
func updateUserAttributes() {
119+
attributeLock.lock()
108120
UserDefaultsUtil.updateUserAttributes(storage: clickstream.storage, userAttributes: allUserAttributes)
121+
attributeLock.unlock()
109122
}
110123

111124
// MARK: - Event recording
@@ -130,6 +143,9 @@ class AnalyticsClient: AnalyticsClientBehaviour {
130143
}
131144

132145
func record(_ event: ClickstreamEvent) throws {
146+
if event.eventType != Event.PresetEvent.CLICKSTREAM_ERROR{
147+
attributeLock.lock()
148+
}
133149
for (key, attribute) in globalAttributes {
134150
event.addGlobalAttribute(attribute, forKey: key)
135151
}
@@ -147,6 +163,9 @@ class AnalyticsClient: AnalyticsClientBehaviour {
147163
event.setUserAttribute(simpleUserAttributes)
148164
}
149165
try eventRecorder.save(event)
166+
if event.eventType != Event.PresetEvent.CLICKSTREAM_ERROR{
167+
attributeLock.unlock()
168+
}
150169
}
151170

152171
func recordEventError(_ eventError: EventChecker.EventError) {
@@ -167,13 +186,15 @@ class AnalyticsClient: AnalyticsClientBehaviour {
167186
}
168187

169188
func getSimpleUserAttributes() -> [String: Any] {
189+
attributeLock.lock()
170190
simpleUserAttributes = [:]
171191
simpleUserAttributes[Event.ReservedAttribute.USER_FIRST_TOUCH_TIMESTAMP]
172192
= allUserAttributes[Event.ReservedAttribute.USER_FIRST_TOUCH_TIMESTAMP]
173193
if allUserAttributes.keys.contains(Event.ReservedAttribute.USER_ID) {
174194
simpleUserAttributes[Event.ReservedAttribute.USER_ID]
175195
= allUserAttributes[Event.ReservedAttribute.USER_ID]
176196
}
197+
attributeLock.unlock()
177198
return simpleUserAttributes
178199
}
179200
}

‎Sources/Clickstream/Dependency/Clickstream/Event/ClickstreamEvent.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class ClickstreamEvent: AnalyticsPropertiesModel {
2121
private(set) lazy var attributes: [String: AttributeValue] = [:]
2222
private(set) lazy var items: [ClickstreamAttribute] = []
2323
private(set) lazy var userAttributes: [String: Any] = [:]
24+
let attributeLock = NSLock()
2425
let systemInfo: SystemInfo
2526
let netWorkType: String
2627

@@ -72,7 +73,11 @@ class ClickstreamEvent: AnalyticsPropertiesModel {
7273
}
7374

7475
func setUserAttribute(_ attributes: [String: Any]) {
75-
userAttributes = attributes
76+
attributeLock.lock()
77+
for attr in attributes {
78+
userAttributes[attr.key] = attr.value
79+
}
80+
attributeLock.unlock()
7681
}
7782

7883
func attribute(forKey key: String) -> AttributeValue? {
@@ -109,9 +114,11 @@ class ClickstreamEvent: AnalyticsPropertiesModel {
109114
if !items.isEmpty {
110115
event["items"] = items
111116
}
117+
attributeLock.lock()
112118
if !userAttributes.isEmpty {
113119
event["user"] = userAttributes
114120
}
121+
attributeLock.unlock()
115122
event["attributes"] = getAttributeObject(from: attributes)
116123
return event
117124
}

‎Tests/ClickstreamTests/Clickstream/ClickstreamEventTest.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
@testable import Clickstream
99
import Foundation
1010
import XCTest
11+
1112
class ClickstreamEventTest: XCTestCase {
1213
let testAppId = "testAppId"
1314
let storage = ClickstreamContextStorage(userDefaults: UserDefaults.standard)

‎Tests/ClickstreamTests/Clickstream/EventRecorderTest.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,20 @@ class EventRecorderTest: XCTestCase {
183183
XCTAssertEqual("carl", (userAttributes["_user_name"] as! JsonObject)["value"] as! String)
184184
}
185185

186+
func testModifyGlobalUserAttributesWillNotAffectTheEventUserAttributes() throws {
187+
var userInfo = JsonObject()
188+
let currentTimeStamp = Date().millisecondsSince1970
189+
var userNameInfo = JsonObject()
190+
userNameInfo["value"] = "carl"
191+
userNameInfo["set_timestamp"] = currentTimeStamp
192+
userInfo["_user_name"] = userNameInfo
193+
clickstreamEvent.setUserAttribute(userInfo)
194+
userInfo = JsonObject()
195+
userNameInfo["value"] = "mike"
196+
userInfo["_user_name"] = userNameInfo
197+
XCTAssertEqual((clickstreamEvent.userAttributes["_user_name"] as! JsonObject)["value"] as! String, "carl")
198+
}
199+
186200
func testRecordEventWithInvalidAttribute() throws {
187201
clickstreamEvent.addGlobalAttribute(Decimal.nan, forKey: "invalidDecimal")
188202
try eventRecorder.save(clickstreamEvent)

‎Tests/ClickstreamTests/IntegrationTest.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,26 @@ class IntegrationTest: XCTestCase {
333333
XCTAssertEqual(1, eventCount)
334334
}
335335

336+
func testModifyUserAttributesInMulitThread() throws {
337+
for number in 1 ... 100 {
338+
Task {
339+
ClickstreamAnalytics.addUserAttributes([
340+
"_user_age": 21,
341+
"isFirstOpen": true,
342+
"score": 85.2,
343+
"_user_name": "name\(number)"
344+
])
345+
}
346+
Task {
347+
ClickstreamAnalytics.setUserId("userId\(number)")
348+
}
349+
Task {
350+
ClickstreamAnalytics.recordEvent("testEvent\(number)")
351+
}
352+
}
353+
Thread.sleep(forTimeInterval: 1)
354+
}
355+
336356
// MARK: - Objc test
337357

338358
func testRecordEventForObjc() throws {

0 commit comments

Comments
 (0)