Skip to content

Commit 21bda18

Browse files
nan-linan-li
authored
Merge pull request #1398 from OneSignal/fix_user_executor_cache_from_blocking
Fix user executor's requests from being blocked
2 parents 5ff232e + efef827 commit 21bda18

12 files changed

+43
-21
lines changed

‎iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSDelta.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ open class OSDelta: NSObject, NSCoding {
3939
public let value: Any
4040

4141
override open var description: String {
42-
return "OSDelta \(name) with property: \(property) value: \(value)"
42+
return "<OSDelta \(name) with property: \(property) value: \(value)>"
4343
}
4444

4545
public init(name: String, model: OSModel, property: String, value: Any) {

‎iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSOperationExecutor.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@
2727

2828
import OneSignalCore
2929

30-
// TODO: Concrete executors drop OSDeltas and Requests when initializing from the cache, when they cannot be connected to their respective models anymore. Revisit this behavior of dropping.
31-
30+
/**
31+
Concrete executors drop OSDeltas and Requests when initializing from the cache, when they cannot be connected to their respective models anymore. These cannot be sent, so they are dropped..
32+
*/
3233
public protocol OSOperationExecutor {
3334
var supportedDeltas: [String] { get }
3435
var deltaQueue: [OSDelta] { get }

‎iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSOperationRepo.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ public class OSOperationRepo: NSObject {
7171
// Read the Deltas from cache, if any...
7272
if let deltaQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_OPERATION_REPO_DELTA_QUEUE_KEY, defaultValue: []) as? [OSDelta] {
7373
self.deltaQueue = deltaQueue
74+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSOperationRepo.start() with deltaQueue: \(deltaQueue)")
7475
} else {
75-
// log error
76+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSOperationRepo.start() is unable to uncache the OSDelta queue.")
7677
}
7778

7879
pollFlushQueue()

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
4949
}
5050
}
5151
self.deltaQueue = deltaQueue
52+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_IDENTITY_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue)
5253
} else {
5354
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor error encountered reading from cache for \(OS_IDENTITY_EXECUTOR_DELTA_QUEUE_KEY)")
5455
}
@@ -70,6 +71,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
7071
}
7172
}
7273
self.addRequestQueue = addRequestQueue
74+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_IDENTITY_EXECUTOR_ADD_REQUEST_QUEUE_KEY, withValue: self.addRequestQueue)
7375
} else {
7476
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor error encountered reading from cache for \(OS_IDENTITY_EXECUTOR_ADD_REQUEST_QUEUE_KEY)")
7577
}
@@ -89,6 +91,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
8991
}
9092
}
9193
self.removeRequestQueue = removeRequestQueue
94+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_IDENTITY_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY, withValue: self.removeRequestQueue)
9295
} else {
9396
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSIdentityOperationExecutor error encountered reading from cache for \(OS_IDENTITY_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY)")
9497
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,12 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
4747
delta.model = modelInStore
4848
} else {
4949
// 2. The model does not exist, drop this Delta
50+
OneSignalLog.onesignalLog(.LL_WARN, message: "OSPropertyOperationExecutor.init dropped: \(delta)")
5051
deltaQueue.remove(at: index)
5152
}
5253
}
5354
self.deltaQueue = deltaQueue
55+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue)
5456
} else {
5557
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor error encountered reading from cache for \(OS_PROPERTIES_EXECUTOR_DELTA_QUEUE_KEY)")
5658
}
@@ -71,10 +73,12 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
7173
request.identityModel = identityModel
7274
} else if !request.prepareForExecution() {
7375
// 3. The identitymodel do not exist AND this request cannot be sent, drop this Request
76+
OneSignalLog.onesignalLog(.LL_WARN, message: "OSPropertyOperationExecutor.init dropped: \(request)")
7477
updateRequestQueue.remove(at: index)
7578
}
7679
}
7780
self.updateRequestQueue = updateRequestQueue
81+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
7882
} else {
7983
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor error encountered reading from cache for \(OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY)")
8084
}

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
5151
}
5252
}
5353
self.deltaQueue = deltaQueue
54+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_DELTA_QUEUE_KEY, withValue: self.deltaQueue)
5455
} else {
5556
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_DELTA_QUEUE_KEY)")
5657
}
@@ -87,6 +88,7 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
8788
requestQueue.append(request)
8889
}
8990
self.addRequestQueue = requestQueue
91+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_ADD_REQUEST_QUEUE_KEY, withValue: self.addRequestQueue)
9092
} else {
9193
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_ADD_REQUEST_QUEUE_KEY)")
9294
}
@@ -106,6 +108,7 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
106108
}
107109
}
108110
self.removeRequestQueue = removeRequestQueue
111+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY, withValue: self.removeRequestQueue)
109112
} else {
110113
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_REMOVE_REQUEST_QUEUE_KEY)")
111114
}
@@ -125,6 +128,7 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
125128
}
126129
}
127130
self.updateRequestQueue = updateRequestQueue
131+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
128132
} else {
129133
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionOperationExecutor error encountered reading from cache for \(OS_SUBSCRIPTION_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY)")
130134
}

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ class OSUserExecutor {
9191
identityModels[req.identityModelToUpdate.modelId] = req.identityModelToUpdate
9292
} else {
9393
// 4. Both models don't exist yet
94+
// Drop the request if the identityModelToIdentify does not already exist AND the request is missing OSID
95+
// Otherwise, this request will forever fail `prepareForExecution` and block pending requests such as recovery calls to `logout` or `login`
96+
guard request.prepareForExecution() else {
97+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor.start() dropped: \(request)")
98+
continue
99+
}
94100
identityModels[req.identityModelToIdentify.modelId] = req.identityModelToIdentify
95101
identityModels[req.identityModelToUpdate.modelId] = req.identityModelToUpdate
96102
}
@@ -99,7 +105,7 @@ class OSUserExecutor {
99105
}
100106
}
101107
self.userRequestQueue = userRequestQueue
102-
108+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue)
103109
// Read unfinished Transfer Subscription requests from cache, if any...
104110
if let transferSubscriptionRequestQueue = OneSignalUserDefaults.initShared().getSavedCodeableData(forKey: OS_USER_EXECUTOR_TRANSFER_SUBSCRIPTION_REQUEST_QUEUE_KEY, defaultValue: []) as? [OSRequestTransferSubscription] {
105111
// We only care about the last transfer subscription request
@@ -111,14 +117,14 @@ class OSUserExecutor {
111117
self.transferSubscriptionRequestQueue = [request]
112118
} else if !request.prepareForExecution() {
113119
// The model do not exist AND this request cannot be sent, drop this Request
114-
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor.start() reading request \(request) from cache failed. Dropping request.")
120+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor.start() dropped: \(request)")
115121
self.transferSubscriptionRequestQueue = []
116122
}
117123
}
118124
} else {
119125
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor error encountered reading from cache for \(OS_USER_EXECUTOR_TRANSFER_SUBSCRIPTION_REQUEST_QUEUE_KEY)")
120126
}
121-
127+
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_TRANSFER_SUBSCRIPTION_REQUEST_QUEUE_KEY, withValue: self.transferSubscriptionRequestQueue)
122128
executePendingRequests()
123129
}
124130

@@ -144,6 +150,7 @@ class OSUserExecutor {
144150

145151
static func executePendingRequests() {
146152
let requestQueue: [OSUserRequest] = userRequestQueue + transferSubscriptionRequestQueue
153+
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OSUserExecutor.executePendingRequests called with queue \(requestQueue)")
147154

148155
if requestQueue.isEmpty {
149156
return
@@ -155,6 +162,7 @@ class OSUserExecutor {
155162
}) {
156163
// Return as soon as we reach an un-executable request
157164
if !request.prepareForExecution() {
165+
OneSignalLog.onesignalLog(.LL_WARN, message: "OSUserExecutor.executePendingRequests() is blocked by unexecutable request \(request)")
158166
return
159167
}
160168

@@ -388,10 +396,7 @@ class OSUserExecutor {
388396
request.sentToClient = true
389397
OneSignalCoreImpl.sharedClient().execute(request) { _ in
390398
removeFromQueue(request)
391-
392-
// TODO: ... hydrate with returned identity object?
393399
executePendingRequests()
394-
395400
} onFailure: { error in
396401
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor executeTransferPushSubscriptionRequest failed with error: \(error.debugDescription)")
397402
if let nsError = error as? NSError {

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ public class OneSignalUserManagerImpl: NSObject, OneSignalUserManager {
260260
}
261261
start()
262262
guard externalId != "" else {
263-
// Log error
263+
OneSignalLog.onesignalLog(.LL_ERROR, message: "OneSignal.User login called with empty externalId. This is not allowed.")
264264
return
265265
}
266266
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignal.User login called with externalId: \(externalId)")
@@ -539,7 +539,7 @@ extension OneSignalUserManagerImpl {
539539
@objc
540540
public func startNewSession() {
541541
OneSignalLog.onesignalLog(.LL_VERBOSE, message: "OneSignalUserManagerImpl starting new session")
542-
guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: nil) else {
542+
guard !OneSignalConfigManager.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: "_startNewSession") else {
543543
return
544544
}
545545
start()
@@ -549,9 +549,12 @@ extension OneSignalUserManagerImpl {
549549
updateSession(sessionCount: 1, sessionTime: nil, refreshDeviceMetadata: true)
550550

551551
// Fetch the user's data if there is a onesignal_id
552-
// TODO: What if onesignal_id is missing, because we may init a user from cache but it may be missing onesignal_id. Is this ok.
553552
if let onesignalId = onesignalId {
554553
OSUserExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: onesignalId, identityModel: user.identityModel, onNewSession: true)
554+
} else {
555+
// It is possible to init a user from cache who is missing the onesignalId
556+
// This can happen if any createUser or identifyUser requests are cached
557+
OneSignalLog.onesignalLog(.LL_WARN, message: "OneSignalUserManagerImpl.startNewSession() is unable to fetch user with External ID \(externalId ?? "nil") due to null OneSignal ID")
555558
}
556559
}
557560

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
6666
self.identityModel = identityModel
6767
self.pushSubscriptionModel = pushSubscriptionModel
6868
self.originalPushToken = originalPushToken
69-
self.stringDescription = "OSRequestCreateUser"
69+
self.stringDescription = "<OSRequestCreateUser with externalId: \(identityModel.externalId ?? "nil")>"
7070
super.init()
7171

7272
var params: [String: Any] = [:]
@@ -114,7 +114,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
114114
self.identityModel = identityModel
115115
self.pushSubscriptionModel = pushSubscriptionModel
116116
self.originalPushToken = coder.decodeObject(forKey: "originalPushToken") as? String
117-
self.stringDescription = "OSRequestCreateUser"
117+
self.stringDescription = "<OSRequestCreateUser with externalId: \(identityModel.externalId ?? "nil")>"
118118
super.init()
119119
self.parameters = parameters
120120
self.method = HTTPMethod(rawValue: rawMethod)

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestIdentifyUser.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class OSRequestIdentifyUser: OneSignalRequest, OSUserRequest {
5555
} else {
5656
// self.path is non-nil, so set to empty string
5757
self.path = ""
58+
OneSignalLog.onesignalLog(.LL_DEBUG, message: "Cannot generate the Identify User request due to null app ID or null OneSignal ID.")
5859
return false
5960
}
6061
}
@@ -71,7 +72,7 @@ class OSRequestIdentifyUser: OneSignalRequest, OSUserRequest {
7172
self.identityModelToUpdate = identityModelToUpdate
7273
self.aliasLabel = aliasLabel
7374
self.aliasId = aliasId
74-
self.stringDescription = "OSRequestIdentifyUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)"
75+
self.stringDescription = "<OSRequestIdentifyUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)>"
7576
super.init()
7677
self.parameters = ["identity": [aliasLabel: aliasId]]
7778
self.method = PATCH
@@ -105,7 +106,7 @@ class OSRequestIdentifyUser: OneSignalRequest, OSUserRequest {
105106
self.identityModelToUpdate = identityModelToUpdate
106107
self.aliasLabel = aliasLabel
107108
self.aliasId = aliasId
108-
self.stringDescription = "OSRequestIdentifyUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)"
109+
self.stringDescription = "<OSRequestIdentifyUser with aliasLabel: \(aliasLabel) aliasId: \(aliasId)>"
109110
super.init()
110111
self.timestamp = timestamp
111112
self.parameters = parameters

0 commit comments

Comments
 (0)