Skip to content

Commit fc0197e

Browse files
nan-liclaude
andcommitted
fix(iv): reorder switchUserIv to avoid unsubscribing the OLD user
Setting isDisabledInternally = true on the OLD push sub model BEFORE createAndSwitchToNewUser fires a NORMAL-tagged change that propagates through SubscriptionModelStoreListener.getUpdateOperation. The listener reads the still-current OLD identity (alice / OS-A) and enqueues an UpdateSubscriptionOperation(externalId = "alice", enabled = false, status = UNSUBSCRIBE) — which then dispatches with alice's still-valid JWT and unsubscribes the just-departed user server-side. Reorder so the user-switch happens first (suppressBackendOperation = true so the new model's add doesn't fire), then set the flag on the NEW push sub with NO_PROPOGATE so the listener filters it. The new push sub reuses the old id, so configModel.pushSubscriptionId after the switch points at the new model. Also fixes the secondary issue from the same review: previously the new SubscriptionModel was created without isDisabledInternally, so the flag never landed on the post-logout anonymous user. With the new ordering, the flag is set directly on the new model. Add tests for the ordering and the Phase 3 short-circuit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6e2eac7 commit fc0197e

2 files changed

Lines changed: 70 additions & 6 deletions

File tree

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LogoutHelperIvExtensions.kt

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.onesignal.user.internal
22

3+
import com.onesignal.common.modeling.ModelChangeTags
34
import com.onesignal.core.internal.config.ConfigModel
5+
import com.onesignal.user.internal.subscriptions.SubscriptionModel
46
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
57

68
/**
@@ -16,10 +18,18 @@ import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
1618
* Under IV-required, the new device-scoped (anonymous) user can't authenticate against
1719
* the backend without a JWT. To prevent the model-store listener from generating create-
1820
* subscription ops that would 401/permanently-block the queue:
19-
* 1. Mark the current push subscription as internally disabled.
20-
* 2. Switch users with [UserSwitcher.createAndSwitchToNewUser] in `suppressBackendOperation`
21-
* mode so subscription replacement does NOT propagate to listeners that would enqueue
22-
* backend ops.
21+
* 1. Switch to the anonymous user with [UserSwitcher.createAndSwitchToNewUser] in
22+
* `suppressBackendOperation` mode so subscription replacement does not propagate to
23+
* listeners that would enqueue backend ops.
24+
* 2. Mark the new push subscription as internally disabled with [ModelChangeTags.NO_PROPOGATE]
25+
* so subsequent property mutations (FCM token refresh, permission change, etc.)
26+
* short-circuit through [com.onesignal.user.internal.operations.impl.listeners.SubscriptionModelStoreListener.getSubscriptionEnabledAndStatus]
27+
* instead of enqueueing real ops.
28+
*
29+
* Order matters: setting the flag on the OLD model first (with default NORMAL tag) would
30+
* fire `getUpdateOperation` against the OLD user with their still-valid JWT — the listener
31+
* would build an `UpdateSubscriptionOperation(externalId = OLD)` carrying `(false, UNSUBSCRIBE)`,
32+
* dispatch it, and unsubscribe the just-departed user server-side.
2333
*
2434
* Returns `true` when IV-specific handling was applied (caller skips legacy enqueue),
2535
* or `false` when IV behavior is inactive (caller falls through to the legacy logout).
@@ -32,9 +42,13 @@ internal fun switchUserIv(
3242
): Boolean {
3343
if (!ivBehaviorActive) return false
3444

45+
userSwitcher.createAndSwitchToNewUser(suppressBackendOperation = true)
3546
configModel.pushSubscriptionId?.let { pushSubId ->
36-
subscriptionModelStore.get(pushSubId)?.let { it.isDisabledInternally = true }
47+
subscriptionModelStore.get(pushSubId)?.setBooleanProperty(
48+
SubscriptionModel::isDisabledInternally.name,
49+
true,
50+
ModelChangeTags.NO_PROPOGATE,
51+
)
3752
}
38-
userSwitcher.createAndSwitchToNewUser(suppressBackendOperation = true)
3953
return true
4054
}

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/LogoutHelperTests.kt

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import com.onesignal.debug.LogLevel
66
import com.onesignal.debug.internal.logging.Logging
77
import com.onesignal.mocks.MockHelper
88
import com.onesignal.user.internal.operations.LoginUserOperation
9+
import com.onesignal.user.internal.subscriptions.SubscriptionModel
10+
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
11+
import com.onesignal.user.internal.subscriptions.SubscriptionType
912
import io.kotest.core.spec.style.FunSpec
1013
import io.kotest.matchers.shouldBe
1114
import io.mockk.every
@@ -192,4 +195,51 @@ class LogoutHelperTests : FunSpec({
192195
verify(atLeast = 1) { mockUserSwitcher.createAndSwitchToNewUser() }
193196
verify(atLeast = 1) { mockOperationRepo.enqueue(any()) }
194197
}
198+
199+
test("switchUserIv switches user before marking new push sub as internally disabled") {
200+
// Given - IV active, push sub model in store
201+
val pushSubId = "push-sub-id"
202+
val newPushSubModel =
203+
SubscriptionModel().apply {
204+
id = pushSubId
205+
type = SubscriptionType.PUSH
206+
}
207+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>(relaxed = true)
208+
every { mockSubscriptionModelStore.get(pushSubId) } returns newPushSubModel
209+
val mockUserSwitcher = mockk<UserSwitcher>(relaxed = true)
210+
val mockConfigModel = mockk<ConfigModel>()
211+
every { mockConfigModel.pushSubscriptionId } returns pushSubId
212+
213+
// When
214+
val handled = switchUserIv(mockUserSwitcher, mockSubscriptionModelStore, mockConfigModel, ivBehaviorActive = true)
215+
216+
// Then
217+
handled shouldBe true
218+
219+
// Order: user-switch must happen BEFORE the flag is set on the new push sub.
220+
// Setting the flag on the OLD model first would fire an UpdateSubscriptionOperation
221+
// against the OLD user with their valid JWT and unsubscribe them server-side.
222+
verifyOrder {
223+
mockUserSwitcher.createAndSwitchToNewUser(suppressBackendOperation = true)
224+
mockSubscriptionModelStore.get(pushSubId)
225+
}
226+
227+
// The flag is set on the model (verified via the underlying property).
228+
newPushSubModel.isDisabledInternally shouldBe true
229+
}
230+
231+
test("switchUserIv returns false when IV behavior is inactive") {
232+
// Given - Phase 3: new code path on, IV behavior off
233+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>(relaxed = true)
234+
val mockUserSwitcher = mockk<UserSwitcher>(relaxed = true)
235+
val mockConfigModel = mockk<ConfigModel>(relaxed = true)
236+
237+
// When
238+
val handled = switchUserIv(mockUserSwitcher, mockSubscriptionModelStore, mockConfigModel, ivBehaviorActive = false)
239+
240+
// Then - falls through to legacy logout flow; no IV-specific calls.
241+
handled shouldBe false
242+
verify(exactly = 0) { mockUserSwitcher.createAndSwitchToNewUser(suppressBackendOperation = any()) }
243+
verify(exactly = 0) { mockSubscriptionModelStore.get(any()) }
244+
}
195245
})

0 commit comments

Comments
 (0)