fix: [SDK-5279] recreate the push subscription when the fetched user has none

[nan-li]

Description

One Line Summary

Recreate the push subscription on a new session when the fetched user has no subscriptions. SDK-5279.

Details

Motivation

Deleting a user's only push subscription from the dashboard left the device without push until reinstall. The fetch-user response has no subscriptions key when the user has none, and the on-new-session self-heal in OSUserExecutor required that key, so it never ran for the case it exists for. Fixes #1745.

Scope

• The self-heal treats an absent subscriptions key as an empty list, then clears the stale id and sends the Create as it already does when the list omits this device.
• A PATCH to a deleted subscription still fails with a 404 and is not re-created here. That is a separate follow-up.

Testing

Unit testing

Three UserExecutorTests cases for the on-new-session fetch: no subscriptions key, a list without this device, and a list that still has it. The first fails without the fix. The first fails without the fix. OneSignalUserMocks.setUserManagerInternalUser takes an optional push token so the Create request has a readable mock key.

Manual testing

Reproduced on a device before the fix. With the fix, the next launch logs found this device's push subscription gone, sends the Create, and the IAM fetch that follows uses the new subscription id. Unit tests run on an iPhone 17 Pro simulator.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

[nan-li]

Ran this through three reviewers on different models (Claude Opus 5, GPT-5.6 Sol, Grok 4.6). The fix is right for the bug it targets, and the ordering details that could have sunk it all check out: the existence check runs after hydratePushSubscription rather than before, clearUserData never touches push state, and the id is cleared before the Create is built so the payload can't carry a dead id. One consensus finding I'd close before merging.

?? [] also swallows unparseable payloads

All three models flagged this independently. parseSubscriptionObjectResponse returns nil for a missing subscriptions key, but equally for "subscriptions": null, for a dictionary or string under that key, and for an array where any single element isn't a dictionary, since as? [[String: Any]] checks every element. ?? [] turns all of those into "this user has zero subscriptions", so a response with a valid identity and a malformed subscriptions value clears the local id and POSTs a Create. Every one of those shapes was a silent no-op before this PR. It also narrows the claim in the description, because a malformed 200 can still trigger a spurious Create whenever identity itself happens to parse.

The blast radius is more than a duplicate row. The subscriptionId setter clears remoteDisabledReason when the id goes nil (OSSubscriptionModel.swift:215-217), so a device the app owner disabled with -22 or -31 comes back enabled. A genuinely disabled subscription still appears in subscriptions, so that can only happen down this malformed path, which is itself an argument for closing it.

Folding it into the existing condition list keeps it minimal and avoids an early return, which would skip the executePendingRequests() at the end of the closure:

// An absent key means the user has none. A key that is present but unparseable
// can't tell us this device's subscription is gone.
let subscriptionObjects = self.parseSubscriptionObjectResponse(response)
let subscriptionsAreKnown = subscriptionObjects != nil || response["subscriptions"] == nil

if request.onNewSession,
   let subId = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId,
   self.parseIdentityObjectResponse(response) != nil,
   subscriptionsAreKnown {
    let subscriptionExists = (subscriptionObjects ?? []).contains { $0["id"] as? String == subId }

The negative tests can't observe a Create

..._whenResponseContainsIt and ..._whenResponseHasNoIdentity assert only that subscriptionId is unchanged. That does hold today, and only because neither test installs the executor, so a spurious createPushSubscriptionRequest() hits the nil-executor branch and leaves the cleared id as its one observable trace. The proof rests entirely on the clear happening before the queue inside the implementation. Reorder those two lines, or stop clearing the id so the server can dedupe by token, and both tests keep passing while a Create goes out on every session. Installing the executor, draining it, and asserting expectedCount: 0 makes the assertion independent of that ordering.

Smaller notes

• The identity gate is weaker than the comment reads. {"identity": {}} passes it, and it never checks that the response is for the user you asked about. executeFetchIdentityBySubscription already requires identityObject[OS_ONESIGNAL_ID], and matching that against request.aliasId here would be tighter. The [String: String] cast failing on a non-string alias value fails closed, which is the right direction, and identity hydration already leans on the same cast.
• Missing cases, roughly by value. onNewSession == false with no subscriptions key, since that flag is the only thing keeping the self-heal off the post-login fetch path and nothing in the suite pins it. Then a wrong-typed subscriptions value, "subscriptions": [], and subscriptionId == nil.
• installSubscriptionExecutor leaves the executor on the shared manager, and neither OneSignalUserMocks.reset() nor tearDown restores it. No blast radius today, since executors are only registered with OSOperationRepo inside start(), but it's a trap in a suite that already carries an apology for singleton leakage.
• Only the branch where Create returns a subscription object is stubbed. Per the note on MockUserRequests.setAddEmailResponse, the server can also return {} when the subscription already exists on the user, and OSSubscriptionOperationExecutor logs and returns without hydrating there, leaving the id nil until a later fetch. Probably out of scope, just not covered.
• Two of the three raised the window between currentUser(matching:) and createPushSubscriptionRequest(), which reads whoever is current at that instant rather than the request's identity model, so a login landing in between can attribute the Create to the new user. It predates this PR, and the let subId binding defuses most of the duplicate-Create variants. Worth a follow-up mainly because this change makes the branch reachable in the common case instead of never.

[fadi-george]

Potential issues:

• parseSubscriptionObjectResponse(response) ?? [] treats both a missing subscriptions key and a present-but-malformed value as empty. For example, subscriptions: null, an object, or a mixed array could clear a valid local subscription ID and create a duplicate. Could we only use [] when the key is genuinely absent, and skip self-healing when it is present but cannot be parsed?
• Checking only that identity parses accepts {} or an external-ID-only identity. Since the existing aliases were already cleared, this can clear the subscription ID and then fail to create its replacement because onesignal_id is unavailable. Could we require the returned onesignal_id to match the requested user and add tests for both malformed cases?

[nan-li]

Ran this through three reviewers on different models (Claude Opus 5, GPT-5.6 Sol, Grok 4.6). The fix is right for the bug it targets, and the ordering details that could have sunk it all check out: the existence check runs after hydratePushSubscription rather than before, clearUserData never touches push state, and the id is cleared before the Create is built so the payload can't carry a dead id. One consensus finding I'd close before merging.

...

Went through these with the follow-up commit (bdd5eb5). Kept the PR to the reported behavior: a fetch either carries a parseable subscriptions array or omits the key, which is what a user with no subscriptions looks like, and hydration trusts the same 200 without checks, so the self-heal now does too. That is why the identity gate came out rather than getting tightened, and why there is no guard for a value that does not parse.

Negative tests stay on the id assertion, since the self-heal clears the id before it queues anything and I did not want a production seam for the drain. Executor leak fixed with a tearDown restore. Tests stay at the three cases the bug needs.

The {} Create response is #1748's. The currentUser window predates this PR and stays a separate follow-up.

[nan-li]

Potential issues:

• parseSubscriptionObjectResponse(response) ?? [] treats both a missing subscriptions key and a present-but-malformed value as empty. For example, subscriptions: null, an object, or a mixed array could clear a valid local subscription ID and create a duplicate. Could we only use [] when the key is genuinely absent, and skip self-healing when it is present but cannot be parsed?
• Checking only that identity parses accepts {} or an external-ID-only identity. Since the existing aliases were already cleared, this can clear the subscription ID and then fail to create its replacement because onesignal_id is unavailable. Could we require the returned onesignal_id to match the requested user and add tests for both malformed cases?

Thanks for the look. I decided not to guard against either shape here. After going through these I kept the PR to the reported behavior rather than adding guards for response shapes the server does not send. The fetch response has exactly two forms in practice: a parseable subscriptions array, or no key at all when the user has no subscriptions (the case this PR fixes). Hydration already trusts the same 200 with no checks on identity or subscriptions, so guarding only the self-heal would not buy much and hides the fact that a malformed response has bigger problems by then.

The follow-up commit (bdd5eb5) removes the weak identity gate rather than tightening it, so the self-heal reads the response the same way hydration does. If you have a real path where the server returns one of those shapes, I'll add the guard and a test.