Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat] Combine user property updates for network call improvements, fix purchase bug #1444

Merged
merged 15 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,38 @@
import OneSignalOSCore
import OneSignalCore

/// Helper struct to process and combine OSDeltas into one payload
private struct OSCombinedProperties {
var properties: [String: Any] = [:]
var tags: [String: String] = [:]
var location: OSLocationPoint?
var refreshDeviceMetadata = false

// Items of Properties Deltas
var sessionTime: Int = 0
var sessionCount: Int = 0
var purchases: [[String: AnyObject]] = []

func jsonRepresentation() -> [String: Any] {
var propertiesObject = properties
propertiesObject["tags"] = tags.isEmpty ? nil : tags
propertiesObject["lat"] = location?.lat
propertiesObject["long"] = location?.long

var deltas = [String: Any]()
deltas["session_count"] = (sessionCount > 0) ? sessionCount : nil
deltas["session_time"] = (sessionTime > 0) ? sessionTime : nil
deltas["purchases"] = purchases.isEmpty ? nil : purchases

var params: [String: Any] = [:]
params["properties"] = propertiesObject.isEmpty ? nil : propertiesObject
params["refresh_device_metadata"] = refreshDeviceMetadata
params["deltas"] = deltas.isEmpty ? nil : deltas

return params
}
}

class OSPropertyOperationExecutor: OSOperationExecutor {
var supportedDeltas: [String] = [OS_UPDATE_PROPERTIES_DELTA]
var deltaQueue: [OSDelta] = []
Expand Down Expand Up @@ -78,7 +110,7 @@ class OSPropertyOperationExecutor: OSOperationExecutor {

func enqueueDelta(_ delta: OSDelta) {
self.dispatchQueue.async {
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta\(delta)")
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor enqueue delta \(delta)")
self.deltaQueue.append(delta)
}
}
Expand All @@ -89,38 +121,99 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
}
}

/// The `deltaQueue` should only contain updates for one user.
/// Even when login -> addTag -> login -> addTag are called in immediate succession.
func processDeltaQueue(inBackground: Bool) {
self.dispatchQueue.async {
if !self.deltaQueue.isEmpty {
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)")
if self.deltaQueue.isEmpty {
// Delta queue is empty but there may be pending requests
self.processRequestQueue(inBackground: inBackground)
return
}
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSPropertyOperationExecutor processDeltaQueue with queue: \(self.deltaQueue)")

// Holds mapping of identity model ID to the updates for it; there should only be one user
var combinedProperties: [String: OSCombinedProperties] = [:]

// 1. Combined deltas into a single OSCombinedProperties for every user
for delta in self.deltaQueue {
guard let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(delta.identityModelId)
else {
// drop this delta
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.processDeltaQueue dropped: \(delta)")
continue
}
var combinedSoFar: OSCombinedProperties? = combinedProperties[identityModel.modelId]
combinedSoFar = self.combineProperties(existing: combinedSoFar, delta: delta)
combinedProperties[identityModel.modelId] = combinedSoFar
nan-li marked this conversation as resolved.
Show resolved Hide resolved
}

if combinedProperties.count > 1 {
OneSignalLog.onesignalLog(.LL_WARN, message: "OSPropertyOperationExecutor.combinedProperties contains \(combinedProperties.count) users")
}

// 2. Turn each OSCombinedProperties' data into a Request
for (modelId, properties) in combinedProperties {
guard let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(modelId)
else {
// This should never happen as we already checked this during Deltas processing above
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.processDeltaQueue dropped: \(properties)")
continue
}
let request = OSRequestUpdateProperties(
properties: [delta.property: delta.value],
deltas: nil,
refreshDeviceMetadata: false, // Sort this out.
params: properties.jsonRepresentation(),
identityModel: identityModel
)
self.updateRequestQueue.append(request)
}
self.deltaQueue = [] // TODO: Check that we can simply clear all the deltas in the deltaQueue

// persist executor's requests (including new request) to storage
self.deltaQueue.removeAll()

// Persist executor's requests (including new request) to storage
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: [])

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

// This method is called by `processDeltaQueue` only and does not need to be added to the dispatchQueue.
/// Helper method to combine the information in an `OSDelta` to the existing `OSCombinedProperties` so far.
private func combineProperties(existing: OSCombinedProperties?, delta: OSDelta) -> OSCombinedProperties {
var combinedProperties = existing ?? OSCombinedProperties()

guard let property = OSPropertiesSupportedProperty(rawValue: delta.property) else {
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor.combineProperties dropped unsupported property: \(delta.property)")
return combinedProperties
}

switch property {
case .tags:
if let tags = delta.value as? [String: String] {
for (tag, value) in tags {
combinedProperties.tags[tag] = value
}
}
case .location:
// Use the most recent location point
combinedProperties.location = delta.value as? OSLocationPoint
case .session_time:
combinedProperties.sessionTime += (delta.value as? Int ?? 0)
case .session_count:
combinedProperties.refreshDeviceMetadata = true
combinedProperties.sessionCount += (delta.value as? Int ?? 0)
case .purchases:
if let purchases = delta.value as? [[String: AnyObject]] {
for purchase in purchases {
combinedProperties.purchases.append(purchase)
}
}
default:
// First-level, un-nested properties as "language"
combinedProperties.properties[delta.property] = delta.value
}
return combinedProperties
}

/// This method is called by `processDeltaQueue` only and does not need to be added to the dispatchQueue.
func processRequestQueue(inBackground: Bool) {
if updateRequestQueue.isEmpty {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,10 @@ class OSRequestUpdateProperties: OneSignalRequest, OSUserRequest {
}
}

init(properties: [String: Any], deltas: [String: Any]?, refreshDeviceMetadata: Bool?, identityModel: OSIdentityModel) {
init(params: [String: Any], identityModel: OSIdentityModel) {
self.identityModel = identityModel
self.stringDescription = "<OSRequestUpdateProperties with properties: \(properties) deltas: \(String(describing: deltas)) refreshDeviceMetadata: \(refreshDeviceMetadata ?? false)>"
self.stringDescription = "<OSRequestUpdateProperties with parameters: \(params)>"
super.init()

var propertiesObject = properties
if let location = propertiesObject["location"] as? OSLocationPoint {
propertiesObject["lat"] = location.lat
propertiesObject["long"] = location.long
propertiesObject.removeValue(forKey: "location")
}
var params: [String: Any] = [:]
params["properties"] = propertiesObject
params["refresh_device_metadata"] = refreshDeviceMetadata
if let deltas = deltas {
params["deltas"] = deltas
}
self.parameters = params
self.method = PATCH
_ = prepareForExecution() // sets the path property
Expand Down