From 66b9515cb691ac54a8c4caa54568d0eb6219a3ef Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 18 Sep 2026 09:55:57 -0700 Subject: [PATCH 1/2] fix: [SDK-5279] recreate the push subscription when the fetched user has none The on-new-session Fetch User self-heal only ran when the response carried a "subscriptions" array. A user whose only subscription was deleted server-side comes back with no "subscriptions" key at all, so the check never fired and the device kept its stale subscription id until reinstall. - Treat an absent "subscriptions" key as an empty list in the self-heal, gated on the response carrying an identity object so a malformed 200 cannot trigger a duplicate create - Let OneSignalUserMocks.setUserManagerInternalUser take a push token - Add UserExecutorTests covering the missing key, a list without this device, a list that still has it, and a response with no identity --- .../Source/Executors/OSUserExecutor.swift | 15 +-- .../OneSignalUserMocks.swift | 8 +- .../Executors/UserExecutorTests.swift | 114 ++++++++++++++++++ 3 files changed, 126 insertions(+), 11 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index ee7d1a22b..9330b15d7 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -455,17 +455,14 @@ extension OSUserExecutor { OneSignalUserManagerImpl.sharedInstance.clearUserData(user) self.parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: OneSignalUserManagerImpl.sharedInstance.pushSubscriptionImpl.token) - // If this is a on-new-session's fetch user call, check that the subscription still exists + // If this is a on-new-session's fetch user call, check that the subscription still exists. + // A user with no subscriptions has no "subscriptions" key at all, so an absent list means empty, + // and only a response that parsed as a user (identity present) is trusted to say so. if request.onNewSession, let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId, - let subscriptionObjects = self.parseSubscriptionObjectResponse(response) { - var subscriptionExists = false - for subModel in subscriptionObjects { - if subModel["id"] as? String == subId { - subscriptionExists = true - break - } - } + self.parseIdentityObjectResponse(response) != nil { + let subscriptionObjects = self.parseSubscriptionObjectResponse(response) ?? [] + let subscriptionExists = subscriptionObjects.contains { $0["id"] as? String == subId } if !subscriptionExists { // This subscription probably has been deleted diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift index 1e0f513c2..15fb5073c 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserMocks/OneSignalUserMocks.swift @@ -48,10 +48,14 @@ public class OneSignalUserMocks: NSObject { OneSignalUserManagerImpl.sharedInstance.reset() } - public static func setUserManagerInternalUser(externalId: String = "test-external-id", onesignalId: String?) -> OSUserInternal { + public static func setUserManagerInternalUser( + externalId: String = "test-external-id", + onesignalId: String?, + pushToken: String = "" + ) -> OSUserInternal { let user = OneSignalUserManagerImpl.sharedInstance.setNewInternalUser( externalId: externalId, - pushSubscriptionModel: OSSubscriptionModel(type: .push, address: "", subscriptionId: testPushSubId, reachable: false, isDisabled: false, changeNotifier: OSEventProducer()) + pushSubscriptionModel: OSSubscriptionModel(type: .push, address: pushToken, subscriptionId: testPushSubId, reachable: false, isDisabled: false, changeNotifier: OSEventProducer()) ) if let onesignalId = onesignalId { user.identityModel.addAliases([OS_ONESIGNAL_ID: onesignalId]) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift index 36a2c712d..fd6e8cfd6 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift @@ -50,8 +50,17 @@ private class Mocks { let pushModel = OSSubscriptionModel(type: .push, address: "", subscriptionId: nil, reachable: false, isDisabled: false, changeNotifier: OSEventProducer()) return OSUserInternalImpl(identityModel: identityModel, propertiesModel: propertiesModel, pushSubscriptionModel: pushModel) } + + /// The on-new-session self-heal sends its Create through the user manager's subscription executor. + func installSubscriptionExecutor() -> OSSubscriptionOperationExecutor { + let executor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState) + OneSignalUserManagerImpl.sharedInstance.subscriptionExecutor = executor + return executor + } } +private let recreatedPushSubId = "recreated-push-sub-id" + final class UserExecutorTests: XCTestCase { override func setUpWithError() throws { @@ -263,4 +272,109 @@ final class UserExecutorTests: XCTestCase { XCTAssertNil(currentUser.identityModel.aliases["stale_label"]) XCTAssertEqual(currentUser.identityModel.externalId, userA_EUID) } + + // MARK: - On-new-session push subscription self-heal + + /// Installs a user whose push subscription already has a server id, as after an earlier session. + private func setUpUserWithPushSubscription() -> OSUserInternal { + return OneSignalUserMocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID, pushToken: "push-token") + } + + private func stubRecreatedPushSubscription(_ mocks: Mocks) { + mocks.client.setMockResponseForRequest( + request: "", + response: ["subscription": MockUserRequests.testDefaultPushSubPayload(id: recreatedPushSubId)] + ) + } + + private func fetchUserOnNewSession(_ mocks: Mocks, user: OSUserInternal, response: [String: Any]) { + mocks.client.setMockResponseForRequest( + request: "", + response: response + ) + mocks.userExecutor.fetchUser(aliasLabel: OS_ONESIGNAL_ID, aliasId: userA_OSID, identityModel: user.identityModel, onNewSession: true) + OneSignalCoreMocks.waitUntil("Fetch user request did not complete") { + mocks.client.hasCompletedRequestOfType(OSRequestFetchUser.self) + } + } + + /** + A user whose only subscription was deleted server-side comes back with no "subscriptions" key at all. + The self-heal must still notice this device's push subscription is gone and re-create it. + */ + func testFetchUser_onNewSession_recreatesPushSubscription_whenResponseHasNoSubscriptionsKey() { + /* Setup */ + let mocks = Mocks() + let subscriptionExecutor = mocks.installSubscriptionExecutor() + let user = setUpUserWithPushSubscription() + stubRecreatedPushSubscription(mocks) + + /* When */ + fetchUserOnNewSession(mocks, user: user, response: MockUserRequests.testIdentityPayload(onesignalId: userA_OSID, externalId: userA_EUID)) + subscriptionExecutor.processDeltaQueue(inBackground: false) + OneSignalCoreMocks.waitUntil("Push subscription was not re-created") { + user.pushSubscriptionModel.subscriptionId == recreatedPushSubId + } + + /* Then */ + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateSubscription.self, expectedCount: 1)) + XCTAssertEqual(user.pushSubscriptionModel.subscriptionId, recreatedPushSubId) + } + + /** + The response lists other subscriptions but not this device's push subscription, so it is re-created. + */ + func testFetchUser_onNewSession_recreatesPushSubscription_whenResponseOmitsThisDevice() { + /* Setup */ + let mocks = Mocks() + let subscriptionExecutor = mocks.installSubscriptionExecutor() + let user = setUpUserWithPushSubscription() + stubRecreatedPushSubscription(mocks) + var response: [String: Any] = MockUserRequests.testIdentityPayload(onesignalId: userA_OSID, externalId: userA_EUID) + response["subscriptions"] = [["type": "Email", "id": "remote_email_id", "token": "remote_email@example.com"]] + + /* When */ + fetchUserOnNewSession(mocks, user: user, response: response) + subscriptionExecutor.processDeltaQueue(inBackground: false) + OneSignalCoreMocks.waitUntil("Push subscription was not re-created") { + user.pushSubscriptionModel.subscriptionId == recreatedPushSubId + } + + /* Then */ + XCTAssertTrue(mocks.client.hasExecutedRequestOfType(OSRequestCreateSubscription.self, expectedCount: 1)) + XCTAssertEqual(user.pushSubscriptionModel.subscriptionId, recreatedPushSubId) + } + + /** + The response still lists this device's push subscription, so nothing is re-created. + */ + func testFetchUser_onNewSession_keepsPushSubscription_whenResponseContainsIt() { + /* Setup */ + let mocks = Mocks() + let user = setUpUserWithPushSubscription() + var response: [String: Any] = MockUserRequests.testIdentityPayload(onesignalId: userA_OSID, externalId: userA_EUID) + response["subscriptions"] = [MockUserRequests.testDefaultPushSubPayload(id: testPushSubId)] + + /* When */ + fetchUserOnNewSession(mocks, user: user, response: response) + + /* Then */ + // The self-heal clears the id before queuing its Create, so an unchanged id proves it did not run. + XCTAssertEqual(user.pushSubscriptionModel.subscriptionId, testPushSubId) + } + + /** + A response that did not parse as a user cannot vouch for a missing subscription list, so nothing is re-created. + */ + func testFetchUser_onNewSession_keepsPushSubscription_whenResponseHasNoIdentity() { + /* Setup */ + let mocks = Mocks() + let user = setUpUserWithPushSubscription() + + /* When */ + fetchUserOnNewSession(mocks, user: user, response: ["properties": ["language": "en"]]) + + /* Then */ + XCTAssertEqual(user.pushSubscriptionModel.subscriptionId, testPushSubId) + } } From bdd5eb5ee4f95ea2b638be4583a230e393a8778c Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 18 Sep 2026 14:21:05 -0700 Subject: [PATCH 2/2] chore: [SDK-5279] drop the identity gate from the self-heal Review follow-up on #1750. The gate only guarded against a fetch response without an identity object, which the server does not send, and hydration already trusts the same 200. The self-heal now reads the response the same way: an absent "subscriptions" key means the user has none. - Remove the no-identity test along with the gate - Restore the shared manager's subscription executor in tearDown --- .../Source/Executors/OSUserExecutor.swift | 6 ++--- .../Executors/UserExecutorTests.swift | 23 ++++++------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 9330b15d7..b9e72d850 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -456,11 +456,9 @@ extension OSUserExecutor { self.parseFetchUserResponse(response: response, identityModel: request.identityModel, originalPushToken: OneSignalUserManagerImpl.sharedInstance.pushSubscriptionImpl.token) // If this is a on-new-session's fetch user call, check that the subscription still exists. - // A user with no subscriptions has no "subscriptions" key at all, so an absent list means empty, - // and only a response that parsed as a user (identity present) is trusted to say so. + // A user with no subscriptions has no "subscriptions" key at all, so an absent key means none. if request.onNewSession, - let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId, - self.parseIdentityObjectResponse(response) != nil { + let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId { let subscriptionObjects = self.parseSubscriptionObjectResponse(response) ?? [] let subscriptionExists = subscriptionObjects.contains { $0["id"] as? String == subId } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift index fd6e8cfd6..204da8680 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/UserExecutorTests.swift @@ -63,6 +63,9 @@ private let recreatedPushSubId = "recreated-push-sub-id" final class UserExecutorTests: XCTestCase { + /// Whatever executor the shared manager had before a test installed its own, restored in tearDown. + private var previousSubscriptionExecutor: OSSubscriptionOperationExecutor? + override func setUpWithError() throws { OneSignalCoreMocks.clearUserDefaults() OneSignalUserMocks.reset() @@ -70,9 +73,12 @@ final class UserExecutorTests: XCTestCase { OneSignalIdentifiers.currentAppId = "test-app-id" // Temp. logging to help debug during testing OneSignalLog.setLogLevel(.LL_VERBOSE) + previousSubscriptionExecutor = OneSignalUserManagerImpl.sharedInstance.subscriptionExecutor } - override func tearDownWithError() throws { } + override func tearDownWithError() throws { + OneSignalUserManagerImpl.sharedInstance.subscriptionExecutor = previousSubscriptionExecutor + } func testCreateUser_withPushSubscription_addsToNewRecords() { /* Setup */ @@ -362,19 +368,4 @@ final class UserExecutorTests: XCTestCase { // The self-heal clears the id before queuing its Create, so an unchanged id proves it did not run. XCTAssertEqual(user.pushSubscriptionModel.subscriptionId, testPushSubId) } - - /** - A response that did not parse as a user cannot vouch for a missing subscription list, so nothing is re-created. - */ - func testFetchUser_onNewSession_keepsPushSubscription_whenResponseHasNoIdentity() { - /* Setup */ - let mocks = Mocks() - let user = setUpUserWithPushSubscription() - - /* When */ - fetchUserOnNewSession(mocks, user: user, response: ["properties": ["language": "en"]]) - - /* Then */ - XCTAssertEqual(user.pushSubscriptionModel.subscriptionId, testPushSubId) - } }