Skip to content

Conversation

@nan-li
Copy link
Contributor

@nan-li nan-li commented Jun 27, 2025

Description

One Line Summary

Fix push subscription observers not firing by preventing manual removal of a PushSubscription which allows correct transfer of existing observers.

Details

  • When the SDK hydrates users or changes users, it will trigger the subscription model store's replaceAll method. This will remove the current push subscription model and immediately add a new push subscription model. The Subscription Manager was removing the current push subscription and then adding a new push subscription that it manages. When the new one is added, the old one is removed and any observers are transferred over.
  • However, because the old one was already removed by the time the new one is added, observers are lost. The [onModelAdded] already triggers an add and remove, so we should not remove in the preceding [onModelRemoved] event.
  • These 2 events of [onModelAdded] and [onModelRemoved] are always together as they are always triggered via replaceAll. There is no current usage where only [onModelRemoved] is called for a push subscription model.
  • Even on 404s, push subscription models are not removed.

Motivation

Clients reported their push observers not firing in certain circumstances, and I was able to reproduce more generally as well.

Scope

  • Don't manually remove a PushSubscription instance managed by SubscriptionManager, it will be removed by an expected immediate call to remove and add a new PushSubscription instance.
  • There will only be 1 instance so the "add" call will do the replacement, which also transfers any observers.

Testing

Unit testing

OPTIONAL - Explain unit tests added, if not clear in the code.

Manual testing

Android emulator API 33

  • Tested to reproduce this issue easily: Add a push observer in Application onCreate and wait for the refresh-user call to complete. Toggle optIn and optOut and see your observer fires before the refresh-user returns and no longer fire after the call returns.

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

This change is Reviewable

* When the SDK hydrates users, it will trigger the subscription model store's `replaceAll` method. This will remove the current push subscription model and immediately add a new push subscription model. The Subscription Manager was removing the current push subscription and then adding a new push subscription that it manages. When the new one is added, the old one is removed and any observers are transferred over.
* However, because the old one was already removed by the time the new one is added, observers are lost. The [onModelAdded] already triggers an add and remove, so we should not remove in the [onModelRemoved] event.
* These 2 events are always together. There is no current usage where only [onModelRemoved] is called for a push subscription model. Even on 404s, push subscription models are not removed.
@nan-li nan-li force-pushed the fix/push_sub_observer_not_firing branch from fd0ecae to d120b52 Compare June 28, 2025 21:27
@nan-li nan-li merged commit f866e92 into main Jun 30, 2025
2 checks passed
@nan-li nan-li deleted the fix/push_sub_observer_not_firing branch June 30, 2025 18:43
@jkasten2 jkasten2 mentioned this pull request Jun 30, 2025
nan-li added a commit that referenced this pull request Jul 8, 2025
fix: push subscription observers not firing
@nan-li nan-li mentioned this pull request Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants