Skip to content

Commit 8ca9bcc

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

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
@@ -246,15 +246,24 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
246246
}
247247

248248
OneSignalCoreImpl.sharedClient().execute(request) { response in
249-
// On success, remove request from cache, and we do need to hydrate
250-
// TODO: We need to hydrate after all ? What why ?
249+
// On success, remove request from cache
251250
self.dispatchQueue.async {
252251
self.updateRequestQueue.removeAll(where: { $0 == request})
253252
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
254253
if inBackground {
255254
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
256255
}
257256
}
257+
258+
// Re-assert the tags the server just confirmed by merging them back into the local model,
259+
// to remedy a concurrent FetchUser whose response is missing the just-written tags
260+
if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel),
261+
let properties = response?["properties"] as? [String: Any],
262+
let confirmedTags = properties["tags"] as? [String: String],
263+
!confirmedTags.isEmpty {
264+
OneSignalUserManagerImpl.sharedInstance._user?.propertiesModel.mergeConfirmedTags(confirmedTags)
265+
}
266+
258267
if let onesignalId = request.identityModel.onesignalId {
259268
if let rywToken = response?["ryw_token"] as? String
260269
{

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

0 commit comments

Comments
 (0)