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 5 commits
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
12 changes: 12 additions & 0 deletions iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
3CE8CC582911B2B2000DB0D3 /* SystemConfiguration.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3CE8CC572911B2B2000DB0D3 /* SystemConfiguration.framework */; };
3CE8CC5B29143F4B000DB0D3 /* NSDateFormatter+OneSignal.m in Sources */ = {isa = PBXBuildFile; fileRef = DE98772A2591655800DE07D5 /* NSDateFormatter+OneSignal.m */; };
3CE9227A289FA88B001B1062 /* OSIdentityModelStoreListener.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CE92279289FA88B001B1062 /* OSIdentityModelStoreListener.swift */; };
3CEE90A72BFE6ABD00B0FB5B /* SupportedProperty.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CEE90A62BFE6ABD00B0FB5B /* SupportedProperty.swift */; };
3CEE93422B7C4174008440BD /* OneSignalUserMocks.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3CC063DD2B6D7F2A002BB07F /* OneSignalUserMocks.framework */; };
3CEE93432B7C4174008440BD /* OneSignalUserMocks.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 3CC063DD2B6D7F2A002BB07F /* OneSignalUserMocks.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
3CEE93462B7C73AB008440BD /* OneSignalCoreMocks.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3CC0639A2B6D7A8C002BB07F /* OneSignalCoreMocks.framework */; };
Expand Down Expand Up @@ -1184,6 +1185,7 @@
3CE8CC552911B1E0000DB0D3 /* UIKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = UIKit.framework; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk/System/iOSSupport/System/Library/Frameworks/UIKit.framework; sourceTree = DEVELOPER_DIR; };
3CE8CC572911B2B2000DB0D3 /* SystemConfiguration.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = SystemConfiguration.framework; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk/System/Library/Frameworks/SystemConfiguration.framework; sourceTree = DEVELOPER_DIR; };
3CE92279289FA88B001B1062 /* OSIdentityModelStoreListener.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIdentityModelStoreListener.swift; sourceTree = "<group>"; };
3CEE90A62BFE6ABD00B0FB5B /* SupportedProperty.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SupportedProperty.swift; sourceTree = "<group>"; };
3CF8629D28A183F900776CA4 /* OSIdentityModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIdentityModel.swift; sourceTree = "<group>"; };
3CF8629F28A1964F00776CA4 /* OSPropertiesModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSPropertiesModel.swift; sourceTree = "<group>"; };
3CF862A128A197D200776CA4 /* OSPropertiesModelStoreListener.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSPropertiesModelStoreListener.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1914,6 +1916,7 @@
3C9AD6BA2B2284AB00BC1540 /* Executors */ = {
isa = PBXGroup;
children = (
3CEE90A52BFE6A7700B0FB5B /* Support */,
3C8E6E0028AC0BA10031E48A /* OSIdentityOperationExecutor.swift */,
3C8E6DFE28AB09AE0031E48A /* OSPropertyOperationExecutor.swift */,
3CE795FA28DBDCE700736BD4 /* OSSubscriptionOperationExecutor.swift */,
Expand Down Expand Up @@ -1983,6 +1986,14 @@
path = OneSignalUserTests;
sourceTree = "<group>";
};
3CEE90A52BFE6A7700B0FB5B /* Support */ = {
isa = PBXGroup;
children = (
3CEE90A62BFE6ABD00B0FB5B /* SupportedProperty.swift */,
);
path = Support;
sourceTree = "<group>";
};
3E2400391D4FFC31008BDE70 /* OneSignalFramework */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -3926,6 +3937,7 @@
3C8E6E0128AC0BA10031E48A /* OSIdentityOperationExecutor.swift in Sources */,
3CF862A228A197D200776CA4 /* OSPropertiesModelStoreListener.swift in Sources */,
3C277D7E2BD76E0000857606 /* OSIdentityModelRepo.swift in Sources */,
3CEE90A72BFE6ABD00B0FB5B /* SupportedProperty.swift in Sources */,
3C9AD6C12B22886600BC1540 /* OSRequestUpdateSubscription.swift in Sources */,
3C0EF49E28A1DBCB00E5434B /* OSUserInternalImpl.swift in Sources */,
3C8E6DFF28AB09AE0031E48A /* OSPropertyOperationExecutor.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,6 @@ typedef enum {ATTRIBUTED, NOT_ATTRIBUTED} FocusAttributionState;

// OneSignal Background Task Identifiers
#define ATTRIBUTED_FOCUS_TASK @"ATTRIBUTED_FOCUS_TASK"
#define UNATTRIBUTED_FOCUS_TASK @"UNATTRIBUTED_FOCUS_TASK"
#define SEND_SESSION_TIME_TO_USER_TASK @"SEND_SESSION_TIME_TO_USER_TASK"
#define OPERATION_REPO_BACKGROUND_TASK @"OPERATION_REPO_BACKGROUND_TASK"
#define IDENTITY_EXECUTOR_BACKGROUND_TASK @"IDENTITY_EXECUTOR_BACKGROUND_TASK_"
#define PROPERTIES_EXECUTOR_BACKGROUND_TASK @"PROPERTIES_EXECUTOR_BACKGROUND_TASK_"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ public class OSOperationRepo: NSObject {
}
}

func enqueueDelta(_ delta: OSDelta) {
/**
Enqueueing is driven by model changes and called manually by the User Manager to
add session time, session count and purchase data.

// TODO: We can make this method internal once there is no manual adding of a Delta except through stores.
This can happen when session data and purchase data use the model / store / listener infrastructure.
*/
public func enqueueDelta(_ delta: OSDelta) {
guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: nil) else {
return
}
Expand Down
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 Expand Up @@ -188,33 +281,3 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
}
}
}

extension OSPropertyOperationExecutor {
// TODO: We can make this go through the operation repo
func updateProperties(propertiesDeltas: OSPropertiesDeltas, refreshDeviceMetadata: Bool, propertiesModel: OSPropertiesModel, identityModel: OSIdentityModel, sendImmediately: Bool = false, onSuccess: (() -> Void)? = nil, onFailure: (() -> Void)? = nil) {

let request = OSRequestUpdateProperties(
properties: [:],
deltas: propertiesDeltas.jsonRepresentation(),
refreshDeviceMetadata: refreshDeviceMetadata,
identityModel: identityModel)

if sendImmediately {
// Bypass the request queues
OneSignalCoreImpl.sharedClient().execute(request) { _ in
if let onSuccess = onSuccess {
onSuccess()
}
} onFailure: { _ in
if let onFailure = onFailure {
onFailure()
}
}
} else {
self.dispatchQueue.async {
self.updateRequestQueue.append(request)
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
Modified MIT License

Copyright 2024 OneSignal

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

1. The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

2. All copies of substantial portions of the Software may only be used in connection
with services provided by OneSignal.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
*/

/**
These are supported properties for updating a user's properties.
The `OSDelta` `property` field for user updates must be one of the following.
The `OSPropertyOperationExecutor` will only process the following updates.
*/
// swiftlint:disable identifier_name
enum OSPropertiesSupportedProperty: String {
// Driven by Properties Model changes
case language
case location
case tags
// Created manually by User Manager, not through Models
case session_count
case session_time
case purchases
}
// swiftlint:enable identifier_name
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ class OSPropertiesModelStoreListener: OSModelStoreListener {
}

func getUpdateModelDelta(_ args: OSModelChangedArgs) -> OSDelta? {
guard let _ = OSPropertiesSupportedProperty(rawValue: args.property) else {
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertiesModelStoreListener.getUpdateModelDelta encountered unsupported property: \(args.property)")
return nil
}

return OSDelta(
name: OS_UPDATE_PROPERTIES_DELTA,
identityModelId: OneSignalUserManagerImpl.sharedInstance.user.identityModel.modelId,
Expand Down
Loading