Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,24 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
}

OneSignalCoreImpl.sharedClient().execute(request) { response in
// On success, remove request from cache, and we do need to hydrate
// TODO: We need to hydrate after all ? What why ?
// On success, remove request from cache
self.dispatchQueue.async {
self.updateRequestQueue.removeAll(where: { $0 == request})
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_PROPERTIES_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
if inBackground {
OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
}
}

// Re-assert the tags the server just confirmed by merging them back into the local model,
// to remedy a concurrent FetchUser whose response is missing the just-written tags
if OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel),
let properties = response?["properties"] as? [String: Any],
let confirmedTags = properties["tags"] as? [String: String],
!confirmedTags.isEmpty {
OneSignalUserManagerImpl.sharedInstance._user?.propertiesModel.mergeConfirmedTags(confirmedTags)
}

if let onesignalId = request.identityModel.onesignalId {
if let rywToken = response?["ryw_token"] as? String
{
Expand Down
19 changes: 19 additions & 0 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSPropertiesModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,25 @@ class OSPropertiesModel: OSModel {
self.set(property: "tags", newValue: tagsToSend)
}

/**
Merges server-confirmed tags into the local model, without enqueuing a new delta.
A value of `""` removes the tag.

Used to re-assert tags in order to remedy a concurrent FetchUser whose response
may be missing the just-written tags.
*/
func mergeConfirmedTags(_ serverTags: [String: String]) {
tagsLock.withLock {
for (key, value) in serverTags {
if value.isEmpty {
self.tags.removeValue(forKey: key)
} else {
self.tags[key] = value
}
}
}
}

public override func hydrateModel(_ response: [String: Any]) {
for property in response {
switch property.key {
Expand Down
59 changes: 59 additions & 0 deletions iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,63 @@ final class OneSignalUserTests: XCTestCase {
contains: expectedPayload)
)
}

/**
Unit test for the tag-merge primitive.

`mergeConfirmedTags` must overlay only the provided keys onto the local model: update existing
values, remove keys whose value is `""`, add new keys, and leave all other (e.g. backend-managed)
tags untouched. It must never wholesale-replace the tag dictionary.
*/
func testMergeConfirmedTags_mergesAndRemovesWithoutTouchingOtherTags() throws {
/* Setup */
let model = OSPropertiesModel(changeNotifier: OSEventProducer())
model.hydrate(["tags": ["keep": "1", "update": "old", "remove": "2"]])

/* When */
// "update" changes, "remove" is deleted (""), "add" is new; "keep" must be left untouched
model.mergeConfirmedTags(["update": "new", "remove": "", "add": "3"])

/* Then */
XCTAssertEqual(model.tags, ["keep": "1", "update": "new", "add": "3"])
}

/**
Regression test: `getTags()` returns `{}` after a concurrent, stale `FetchUser` overwrites the
local tag model.

A concurrent `FetchUser` whose response is missing a recently written tag clears the local tag
cache (`clearUserData()`) and hydrates without it. The `OSRequestUpdateProperties` 202 response
echoes the tags the server just confirmed, so merging those back into the local model must restore
the tags that the stale fetch cleared.
*/
func testUpdatePropertiesResponse_restoresTagsClearedByConcurrentFetch() throws {
/* Setup */
let client = MockOneSignalClient()
MockUserRequests.setDefaultCreateAnonUserResponses(with: client)
let tags = ["tag_a": "value_a", "tag_b": "value_b"]
MockUserRequests.setAddTagsResponse(with: client, tags: tags)
OneSignalCoreImpl.setSharedClient(client)

OneSignalUserManagerImpl.sharedInstance.start()

// Let the anonymous user be created so it has a OneSignal ID for the update request
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)

/* When */
// Tags are applied optimistically to the local model and queued as an update request
OneSignalUserManagerImpl.sharedInstance.addTags(tags)

// Simulate a concurrent, stale FetchUser clearing the local tag cache before the
// UpdateProperties 202 response is processed
OneSignalUserManagerImpl.sharedInstance.user.propertiesModel.clearData()
XCTAssertTrue(OneSignalUserManagerImpl.sharedInstance.getTags().isEmpty)

// Let the queued UpdateProperties request flush and its 202 echo be processed
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 1)

/* Then */
// The confirmed tags from the 202 response are merged back into the local model
XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance.getTags(), tags)
}
}
Loading