Skip to content

Commit 80ccd53

Browse files
nan-liclaude
andauthored
fix: prevent stale FetchUser from clobbering the current user's identity (#1672)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e9b42df commit 80ccd53

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,14 +448,19 @@ extension OSUserExecutor {
448448
OneSignalCoreImpl.sharedClient().execute(request) { response in
449449
self.removeFromQueue(request)
450450

451+
// A fetch for a user that is no longer current is stale
452+
guard OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel) else {
453+
self.executePendingRequests()
454+
return
455+
}
456+
451457
if let response = response {
452458
// Clear local data in preparation for hydration
453459
OneSignalUserManagerImpl.sharedInstance.clearUserData()
454460
self.parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: OneSignalUserManagerImpl.sharedInstance.pushSubscriptionImpl.token)
455461

456462
// If this is a on-new-session's fetch user call, check that the subscription still exists
457463
if request.onNewSession,
458-
OneSignalUserManagerImpl.sharedInstance.isCurrentUser(request.identityModel),
459464
let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId,
460465
let subscriptionObjects = self.parseSubscriptionObjectResponse(response) {
461466
var subscriptionExists = false

iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,61 @@ final class UserExecutorTests: XCTestCase {
187187
XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateUser.self))
188188
XCTAssertTrue(mocks.newRecordsState.records.isEmpty)
189189
}
190+
191+
/**
192+
Regression test for a login race that landed identity (and subsequent user updates) data on the wrong user.
193+
194+
When an on-new-session Fetch User request for a *previous* user (e.g. a cached anonymous user) is still
195+
pending and a `login()` switches the current user, the in-flight Fetch User must NOT clear the new current
196+
user's data.
197+
*/
198+
func testFetchUser_forNonCurrentUser_doesNotClearCurrentUserData() {
199+
/* Setup */
200+
let mocks = Mocks()
201+
202+
// The current user has just logged in with an external_id (userB).
203+
let currentUser = OneSignalUserMocks.setUserManagerInternalUser(externalId: userB_EUID, onesignalId: userB_OSID)
204+
205+
// A stale on-new-session Fetch User is in flight for a different, no-longer-current user (userA),
206+
// and its response only carries an onesignal_id (as an anonymous user's would).
207+
let staleIdentityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID], changeNotifier: OSEventProducer())
208+
mocks.client.setMockResponseForRequest(
209+
request: "<OSRequestFetchUser with onesignal_id: \(userA_OSID)>",
210+
response: MockUserRequests.testIdentityPayload(onesignalId: userA_OSID, externalId: nil)
211+
)
212+
213+
/* When */
214+
mocks.userExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: userA_OSID, identityModel: staleIdentityModel, onNewSession: true)
215+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
216+
217+
/* Then */
218+
XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestFetchUser.self))
219+
// The current user's external_id must be intact — the stale fetch must not have cleared it.
220+
XCTAssertEqual(currentUser.identityModel.externalId, userB_EUID)
221+
XCTAssertEqual(OneSignalUserManagerImpl.sharedInstance._user?.identityModel.externalId, userB_EUID)
222+
}
223+
224+
/**
225+
The normal new-session Fetch User for the *current* user must still clear stale local data before hydrating
226+
from the response, so the `isCurrentUser` guard added for the race above does not regress the common path.
227+
*/
228+
func testFetchUser_forCurrentUser_stillClearsStaleData() {
229+
/* Setup */
230+
let mocks = Mocks()
231+
let currentUser = OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
232+
// A stale local alias that is not present in the server response and should be cleared by the fetch.
233+
currentUser.identityModel.addAliases(["stale_label": "stale_value"])
234+
235+
MockUserRequests.setDefaultFetchUserResponseForHydration(with: mocks.client, externalId: userA_EUID)
236+
237+
/* When */
238+
mocks.userExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: userA_OSID, identityModel: currentUser.identityModel, onNewSession: false)
239+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
240+
241+
/* Then */
242+
XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestFetchUser.self))
243+
// clearUserData() ran for the current user: the stale alias is gone and server aliases are hydrated.
244+
XCTAssertNil(currentUser.identityModel.aliases["stale_label"])
245+
XCTAssertEqual(currentUser.identityModel.externalId, userA_EUID)
246+
}
190247
}

0 commit comments

Comments
 (0)