Skip to content

Commit ce80822

Browse files
committed
Combine user update deltas
* When processing the Delta queue and turning them into requests, we now combine deltas into one User Update request.
1 parent 6ddd65f commit ce80822

File tree

2 files changed

+106
-26
lines changed

2 files changed

+106
-26
lines changed

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

Lines changed: 104 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,38 @@
2828
import OneSignalOSCore
2929
import OneSignalCore
3030

31+
/// Helper struct to process and combine OSDeltas into one payload
32+
private struct OSCombinedProperties {
33+
var properties: [String: Any] = [:]
34+
var tags: [String: String] = [:]
35+
var location: OSLocationPoint?
36+
var refreshDeviceMetadata = false
37+
38+
// Items of Properties Deltas
39+
var sessionTime: Int = 0
40+
var sessionCount: Int = 0
41+
var purchases: [[String: AnyObject]] = []
42+
43+
func jsonRepresentation() -> [String: Any] {
44+
var propertiesObject = properties
45+
propertiesObject["tags"] = tags.isEmpty ? nil : tags
46+
propertiesObject["lat"] = location?.lat
47+
propertiesObject["long"] = location?.long
48+
49+
var deltas = [String: Any]()
50+
deltas["session_count"] = (sessionCount > 0) ? sessionCount : nil
51+
deltas["session_time"] = (sessionTime > 0) ? sessionTime : nil
52+
deltas["purchases"] = purchases.isEmpty ? nil : purchases
53+
54+
var params: [String: Any] = [:]
55+
params["properties"] = propertiesObject.isEmpty ? nil : propertiesObject
56+
params["refresh_device_metadata"] = refreshDeviceMetadata
57+
params["deltas"] = deltas.isEmpty ? nil : deltas
58+
59+
return params
60+
}
61+
}
62+
3163
class OSPropertyOperationExecutor: OSOperationExecutor {
3264
var supportedDeltas: [String] = [OS_UPDATE_PROPERTIES_DELTA]
3365
var deltaQueue: [OSDelta] = []
@@ -78,7 +110,7 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
78110

79111
func enqueueDelta(_ delta: OSDelta) {
80112
self.dispatchQueue.async {
81-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta\(delta)")
113+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta \(delta)")
82114
self.deltaQueue.append(delta)
83115
}
84116
}
@@ -89,38 +121,99 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
89121
}
90122
}
91123

124+
/// The `deltaQueue` should only contain updates for one user.
125+
/// Even when login -> addTag -> login -> addTag are called in immediate succession.
92126
func processDeltaQueue(inBackground: Bool) {
93127
self.dispatchQueue.async {
94-
if !self.deltaQueue.isEmpty {
95-
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)")
128+
if self.deltaQueue.isEmpty {
129+
// Delta queue is empty but there may be pending requests
130+
self.processRequestQueue(inBackground: inBackground)
131+
return
96132
}
133+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)")
134+
135+
// Holds mapping of identity model ID to the updates for it; there should only be one user
136+
var combinedProperties: [String: OSCombinedProperties] = [:]
137+
138+
// 1. Combined deltas into a single OSCombinedProperties for every user
97139
for delta in self.deltaQueue {
98140
guard let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(delta.identityModelId)
99141
else {
100-
// drop this delta
101142
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.processDeltaQueue dropped: \(delta)")
102143
continue
103144
}
145+
var combinedSoFar: OSCombinedProperties? = combinedProperties[identityModel.modelId]
146+
combinedSoFar = self.combineProperties(existing: combinedSoFar, delta: delta)
147+
combinedProperties[identityModel.modelId] = combinedSoFar
148+
}
104149

150+
if combinedProperties.count > 1 {
151+
OneSignalLog.onesignalLog(.LL_WARN, message: "OSPropertyOperationExecutor.combinedProperties contains \(combinedProperties.count) users")
152+
}
153+
154+
// 2. Turn each OSCombinedProperties' data into a Request
155+
for (modelId, properties) in combinedProperties {
156+
guard let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(modelId)
157+
else {
158+
// This should never happen as we already checked this during Deltas processing above
159+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.processDeltaQueue dropped: \(properties)")
160+
continue
161+
}
105162
let request = OSRequestUpdateProperties(
106-
properties: [delta.property: delta.value],
107-
deltas: nil,
108-
refreshDeviceMetadata: false, // Sort this out.
163+
params: properties.jsonRepresentation(),
109164
identityModel: identityModel
110165
)
111166
self.updateRequestQueue.append(request)
112167
}
113-
self.deltaQueue = [] // TODO: Check that we can simply clear all the deltas in the deltaQueue
114168

115-
// persist executor's requests (including new request) to storage
169+
self.deltaQueue.removeAll()
170+
171+
// Persist executor's requests (including new request) to storage
116172
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
173+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: [])
117174

118-
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue) // This should be empty, can remove instead?
119175
self.processRequestQueue(inBackground: inBackground)
120176
}
121177
}
122178

123-
// This method is called by `processDeltaQueue` only and does not need to be added to the dispatchQueue.
179+
/// Helper method to combine the information in an `OSDelta` to the existing `OSCombinedProperties` so far.
180+
private func combineProperties(existing: OSCombinedProperties?, delta: OSDelta) -> OSCombinedProperties {
181+
var combinedProperties = existing ?? OSCombinedProperties()
182+
183+
guard let property = OSPropertiesSupportedProperty(rawValue: delta.property) else {
184+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.combineProperties dropped unsupported property: \(delta.property)")
185+
return combinedProperties
186+
}
187+
188+
switch property {
189+
case .tags:
190+
if let tags = delta.value as? [String: String] {
191+
for (tag, value) in tags {
192+
combinedProperties.tags[tag] = value
193+
}
194+
}
195+
case .location:
196+
// Use the most recent location point
197+
combinedProperties.location = delta.value as? OSLocationPoint
198+
case .session_time:
199+
combinedProperties.sessionTime += (delta.value as? Int ?? 0)
200+
case .session_count:
201+
combinedProperties.refreshDeviceMetadata = true
202+
combinedProperties.sessionCount += (delta.value as? Int ?? 0)
203+
case .purchases:
204+
if let purchases = delta.value as? [[String: AnyObject]] {
205+
for purchase in purchases {
206+
combinedProperties.purchases.append(purchase)
207+
}
208+
}
209+
default:
210+
// First-level, un-nested properties as "language"
211+
combinedProperties.properties[delta.property] = delta.value
212+
}
213+
return combinedProperties
214+
}
215+
216+
/// This method is called by `processDeltaQueue` only and does not need to be added to the dispatchQueue.
124217
func processRequestQueue(inBackground: Bool) {
125218
if updateRequestQueue.isEmpty {
126219
return

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateProperties.swift

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,23 +53,10 @@ class OSRequestUpdateProperties: OneSignalRequest, OSUserRequest {
5353
}
5454
}
5555

56-
init(properties: [String: Any], deltas: [String: Any]?, refreshDeviceMetadata: Bool?, identityModel: OSIdentityModel) {
56+
init(params: [String: Any], identityModel: OSIdentityModel) {
5757
self.identityModel = identityModel
58-
self.stringDescription = "<OSRequestUpdateProperties with properties: \(properties) deltas: \(String(describing: deltas)) refreshDeviceMetadata: \(refreshDeviceMetadata ?? false)>"
58+
self.stringDescription = "<OSRequestUpdateProperties with parameters: \(params)>"
5959
super.init()
60-
61-
var propertiesObject = properties
62-
if let location = propertiesObject["location"] as? OSLocationPoint {
63-
propertiesObject["lat"] = location.lat
64-
propertiesObject["long"] = location.long
65-
propertiesObject.removeValue(forKey: "location")
66-
}
67-
var params: [String: Any] = [:]
68-
params["properties"] = propertiesObject
69-
params["refresh_device_metadata"] = refreshDeviceMetadata
70-
if let deltas = deltas {
71-
params["deltas"] = deltas
72-
}
7360
self.parameters = params
7461
self.method = PATCH
7562
_ = prepareForExecution() // sets the path property

0 commit comments

Comments
 (0)