Skip to content

Commit 1a087af

Browse files
nan-licursoragent
andcommitted
fix: restore local tags cleared by a concurrent FetchUser (#1678)
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c112f46 commit 1a087af

3 files changed

Lines changed: 89 additions & 2 deletions

File tree

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,15 +318,24 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
318318
}
319319

320320
OneSignalCoreImpl.sharedClient().execute(request) { response in
321-
// On success, remove request from cache, and we do need to hydrate
322-
// TODO: We need to hydrate after all ? What why ?
321+
// On success, remove request from cache
323322
self.dispatchQueue.async {
324323
self.updateRequestQueue.removeAll(where: { $0 == request})
325324
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
326325
if inBackground {
327326
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
328327
}
329328
}
329+
330+
// Re-assert the tags the server just confirmed by merging them back into the local model,
331+
// to remedy a concurrent FetchUser whose response is missing the just-written tags
332+
if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel),
333+
let properties = response?["properties"] as? [String: Any],
334+
let confirmedTags = properties["tags"] as? [String: String],
335+
!confirmedTags.isEmpty {
336+
OneSignalUserManagerImpl.sharedInstance._user?.propertiesModel.mergeConfirmedTags(confirmedTags)
337+
}
338+
330339
if let onesignalId = request.identityModel.onesignalId {
331340
if let rywToken = response?["ryw_token"] as? String
332341
{

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Modeling/OSPropertiesModel.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,25 @@ class OSPropertiesModel: OSModel {
151151
self.set(property: "tags", newValue: tagsToSend)
152152
}
153153

154+
/**
155+
Merges server-confirmed tags into the local model, without enqueuing a new delta.
156+
A value of `""` removes the tag.
157+
158+
Used to re-assert tags in order to remedy a concurrent FetchUser whose response
159+
may be missing the just-written tags.
160+
*/
161+
func mergeConfirmedTags(_ serverTags: [String: String]) {
162+
tagsLock.withLock {
163+
for (key, value) in serverTags {
164+
if value.isEmpty {
165+
self.tags.removeValue(forKey: key)
166+
} else {
167+
self.tags[key] = value
168+
}
169+
}
170+
}
171+
}
172+
154173
public override func hydrateModel(_ response: [String: Any]) {
155174
for property in response {
156175
switch property.key {

iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,63 @@ final class OneSignalUserTests: XCTestCase {
192192
contains: expectedPayload)
193193
)
194194
}
195+
196+
/**
197+
Unit test for the tag-merge primitive.
198+
199+
`mergeConfirmedTags` must overlay only the provided keys onto the local model: update existing
200+
values, remove keys whose value is `""`, add new keys, and leave all other (e.g. backend-managed)
201+
tags untouched. It must never wholesale-replace the tag dictionary.
202+
*/
203+
func testMergeConfirmedTags_mergesAndRemovesWithoutTouchingOtherTags() throws {
204+
/* Setup */
205+
let model = OSPropertiesModel(changeNotifier: OSEventProducer())
206+
model.hydrate(["tags": ["keep": "1", "update": "old", "remove": "2"]])
207+
208+
/* When */
209+
// "update" changes, "remove" is deleted (""), "add" is new; "keep" must be left untouched
210+
model.mergeConfirmedTags(["update": "new", "remove": "", "add": "3"])
211+
212+
/* Then */
213+
XCTAssertEqual(model.tags, ["keep": "1", "update": "new", "add": "3"])
214+
}
215+
216+
/**
217+
Regression test: `getTags()` returns `{}` after a concurrent, stale `FetchUser` overwrites the
218+
local tag model.
219+
220+
A concurrent `FetchUser` whose response is missing a recently written tag clears the local tag
221+
cache (`clearUserData()`) and hydrates without it. The `OSRequestUpdateProperties` 202 response
222+
echoes the tags the server just confirmed, so merging those back into the local model must restore
223+
the tags that the stale fetch cleared.
224+
*/
225+
func testUpdatePropertiesResponse_restoresTagsClearedByConcurrentFetch() throws {
226+
/* Setup */
227+
let client = MockOneSignalClient()
228+
MockUserRequests.setDefaultCreateAnonUserResponses(with: client)
229+
let tags = ["tag_a": "value_a", "tag_b": "value_b"]
230+
MockUserRequests.setAddTagsResponse(with: client, tags: tags)
231+
OneSignalCoreImpl.setSharedClient(client)
232+
233+
OneSignalUserManagerImpl.sharedInstance.start()
234+
235+
// Let the anonymous user be created so it has a OneSignal ID for the update request
236+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
237+
238+
/* When */
239+
// Tags are applied optimistically to the local model and queued as an update request
240+
OneSignalUserManagerImpl.sharedInstance.addTags(tags)
241+
242+
// Simulate a concurrent, stale FetchUser clearing the local tag cache before the
243+
// UpdateProperties 202 response is processed
244+
OneSignalUserManagerImpl.sharedInstance.user.propertiesModel.clearData()
245+
XCTAssertTrue(OneSignalUserManagerImpl.sharedInstance.getTags().isEmpty)
246+
247+
// Let the queued UpdateProperties request flush and its 202 echo be processed
248+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 1)
249+
250+
/* Then */
251+
// The confirmed tags from the 202 response are merged back into the local model
252+
XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance.getTags(), tags)
253+
}
195254
}

0 commit comments

Comments
 (0)