Skip to content

Commit 215b888

Browse files
nan-linan-li
committed
refactor(iv): move IUserJwtInvalidatedListener notifier into JwtTokenStore
JwtTokenStore is the source of truth for invalidation events (its invalidateJwt method is what fires both internal and developer-facing notifications). It already implements IEventNotifier<IJwtUpdateListener> and dispatches to internal listeners. Adding a second EventProducer for the developer-facing IUserJwtInvalidatedListener is consistent with the class's existing notifier shape — just a different audience. This eliminates the bridge that lived in UserManager: - UserManager loses IJwtUpdateListener, _jwtTokenStore ctor param, jwtInvalidatedNotifier, addJwtInvalidatedListener, removeJwtInvalidatedListener, fireJwtInvalidated, onJwtInvalidated. - OneSignalImp.{add,remove}UserJwtInvalidatedListener now route directly to JwtTokenStore. Side benefits: - No eager-construction problem: JwtTokenStore is already pulled in by OperationRepo (an IStartableService), so the bridge is live before any 401 can dispatch — no IBootstrapService registration needed for any class. - UserManager goes back to being purely about user state. - Improves on reference branches #2599 / #2613 which kept the bridge in UserManager and accepted an eager-construction-via-IAM dependency. Tests: - Move the 3 JWT-listener tests from UserManagerTests to JwtTokenStoreTests (invalidation fires listener, late subscriber doesn't get replay, removeListener stops notifications). - Drop _jwtTokenStore param from UserManager construction sites.
1 parent 46d525f commit 215b888

9 files changed

Lines changed: 136 additions & 261 deletions

File tree

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IUserJwtInvalidatedListener.kt‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ package com.onesignal
66
* detected that the JWT for a user is no longer valid (typically a 401 from
77
* the OneSignal backend on a request signed with that JWT).
88
*
9-
* Threading: regular fire delivery happens on a background dispatcher. Replay
10-
* delivery (when an invalidation occurred before any listener was subscribed)
11-
* happens synchronously on the thread that calls
12-
* [IOneSignal.addUserJwtInvalidatedListener]. Implementations should not assume
13-
* a specific thread.
9+
* Threading: delivered on a background dispatcher
10+
* (`OneSignalDispatchers.launchOnDefault`). Implementations should not assume a
11+
* specific thread and should re-dispatch to the UI thread if needed.
12+
*
13+
* Pure pub/sub: only listeners subscribed at the time of the invalidation
14+
* receive the event. Subscribe early (e.g. in `Application.onCreate`) to avoid
15+
* missing cold-start 401s.
1416
*/
1517
fun interface IUserJwtInvalidatedListener {
1618
/**

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepoIvExtensions.kt‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,8 @@ internal fun OperationRepo.hasValidJwtIfRequired(
3030

3131
/**
3232
* Handles a [com.onesignal.core.internal.operations.ExecutionResult.FAIL_UNAUTHORIZED] response
33-
* when IV behavior is active. Invalidates the JWT for the failing op's externalId (which fires
34-
* `IJwtUpdateListener.onJwtInvalidated` to subscribers, surfacing to the developer via the
35-
* public-API layer), and re-queues the ops (waiter wake with `false` so `enqueueAndWait`
33+
* when IV behavior is active. Invalidates the JWT for the failing op's externalId
34+
* and re-queues the ops (waiter wake with `false` so `enqueueAndWait`
3635
* callers don't hang).
3736
*
3837
* Returns `true` if IV-specific handling was applied (caller should stop processing this result),
@@ -48,8 +47,10 @@ internal fun OperationRepo.handleFailUnauthorized(
4847
if (!ivBehaviorActive) return false
4948
val externalId = startingOp.operation.externalId ?: return false
5049

51-
// Fires onJwtInvalidated to subscribers BEFORE we wake waiters — otherwise an
52-
// `enqueueAndWait` caller could return before the developer-facing event propagates.
50+
// Schedules an async fire of onUserJwtInvalidated to subscribers via
51+
// OneSignalDispatchers.launchOnDefault — the developer-facing listener invocation is
52+
// NOT ordered with respect to the waiter.wake below; awaiting `enqueueAndWait` callers
53+
// may resume before, after, or concurrent with the listener.
5354
jwtTokenStore.invalidateJwt(externalId)
5455
Logging.info(
5556
"Operation execution failed with 401 Unauthorized, JWT invalidated for user: $externalId. " +

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import com.onesignal.user.IUserManager
3636
import com.onesignal.user.UserModule
3737
import com.onesignal.user.internal.LoginHelper
3838
import com.onesignal.user.internal.LogoutHelper
39-
import com.onesignal.user.internal.UserManager
4039
import com.onesignal.user.internal.UserSwitcher
4140
import com.onesignal.user.internal.identity.IdentityModelStore
4241
import com.onesignal.user.internal.jwt.JwtTokenStore
@@ -460,7 +459,7 @@ internal class OneSignalImp(
460459
throw IllegalStateException("Must call 'initWithContext' before 'addUserJwtInvalidatedListener'")
461460
}
462461
}
463-
services.getService<UserManager>().addJwtInvalidatedListener(listener)
462+
jwtTokenStore.addUserJwtInvalidatedListener(listener)
464463
}
465464

466465
override fun removeUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) {
@@ -471,7 +470,7 @@ internal class OneSignalImp(
471470
throw IllegalStateException("Must call 'initWithContext' before 'removeUserJwtInvalidatedListener'")
472471
}
473472
}
474-
services.getService<UserManager>().removeJwtInvalidatedListener(listener)
473+
jwtTokenStore.removeUserJwtInvalidatedListener(listener)
475474
}
476475

477476
override fun <T> hasService(c: Class<T>): Boolean = services.hasService(c)

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

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.onesignal.user.internal
22

3-
import com.onesignal.IUserJwtInvalidatedListener
4-
import com.onesignal.UserJwtInvalidatedEvent
53
import com.onesignal.common.IDManager
64
import com.onesignal.common.JSONUtils
75
import com.onesignal.common.OneSignalUtils
@@ -16,16 +14,13 @@ import com.onesignal.user.internal.backend.IdentityConstants
1614
import com.onesignal.user.internal.customEvents.ICustomEventController
1715
import com.onesignal.user.internal.identity.IdentityModel
1816
import com.onesignal.user.internal.identity.IdentityModelStore
19-
import com.onesignal.user.internal.jwt.IJwtUpdateListener
20-
import com.onesignal.user.internal.jwt.JwtTokenStore
2117
import com.onesignal.user.internal.properties.PropertiesModel
2218
import com.onesignal.user.internal.properties.PropertiesModelStore
2319
import com.onesignal.user.internal.subscriptions.ISubscriptionManager
2420
import com.onesignal.user.internal.subscriptions.SubscriptionList
2521
import com.onesignal.user.state.IUserStateObserver
2622
import com.onesignal.user.state.UserChangedState
2723
import com.onesignal.user.state.UserState
28-
import com.onesignal.common.threading.OneSignalDispatchers
2924
import com.onesignal.user.subscriptions.IPushSubscription
3025

3126
internal open class UserManager(
@@ -34,8 +29,7 @@ internal open class UserManager(
3429
private val _propertiesModelStore: PropertiesModelStore,
3530
private val _customEventController: ICustomEventController,
3631
private val _languageContext: ILanguageContext,
37-
private val _jwtTokenStore: JwtTokenStore,
38-
) : IUserManager, ISingletonModelStoreChangeHandler<IdentityModel>, IJwtUpdateListener {
32+
) : IUserManager, ISingletonModelStoreChangeHandler<IdentityModel> {
3933
override val onesignalId: String
4034
get() = if (IDManager.isLocalId(_identityModel.onesignalId)) "" else _identityModel.onesignalId
4135

@@ -50,8 +44,6 @@ internal open class UserManager(
5044

5145
val changeHandlersNotifier = EventProducer<IUserStateObserver>()
5246

53-
private val jwtInvalidatedNotifier = EventProducer<IUserJwtInvalidatedListener>()
54-
5547
override val pushSubscription: IPushSubscription
5648
get() = _subscriptionManager.subscriptions.push
5749

@@ -67,37 +59,6 @@ internal open class UserManager(
6759

6860
init {
6961
_identityModelStore.subscribe(this)
70-
// Subscribe to JwtTokenStore so 401-driven invalidations from JwtTokenStore.invalidateJwt
71-
// surface to developer-facing IUserJwtInvalidatedListener subscribers.
72-
_jwtTokenStore.subscribe(this)
73-
}
74-
75-
fun addJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) {
76-
jwtInvalidatedNotifier.subscribe(listener)
77-
}
78-
79-
fun removeJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) {
80-
jwtInvalidatedNotifier.unsubscribe(listener)
81-
}
82-
83-
/**
84-
* Fire [IUserJwtInvalidatedListener.onUserJwtInvalidated] to all currently-subscribed
85-
* listeners on a background dispatcher (so HYDRATE / op-repo paths that synchronously
86-
* trigger invalidation don't run app code on the SDK's internal thread). Pure pub/sub —
87-
* matches iOS: late subscribers don't get a replay of earlier events.
88-
*/
89-
fun fireJwtInvalidated(externalId: String) {
90-
OneSignalDispatchers.launchOnDefault {
91-
jwtInvalidatedNotifier.fire { listener ->
92-
runCatching { listener.onUserJwtInvalidated(UserJwtInvalidatedEvent(externalId)) }
93-
.onFailure { ex -> Logging.warn("UserManager: jwt-invalidated listener threw", ex) }
94-
}
95-
}
96-
}
97-
98-
// IJwtUpdateListener — JwtTokenStore -> developer-facing event bridge.
99-
override fun onJwtInvalidated(externalId: String) {
100-
fireJwtInvalidated(externalId)
10162
}
10263

10364
override fun addAlias(

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IJwtUpdateListener.kt‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
package com.onesignal.user.internal.jwt
22

33
/**
4-
* Notifications from [JwtTokenStore] about JWT state changes for an [externalId].
5-
* Listeners should call [JwtTokenStore.getJwt] for the current value — event delivery
6-
* order is not guaranteed to match mutation order across concurrent writers.
4+
* SDK-internal notifications from [JwtTokenStore] about JWT state changes for an [externalId].
5+
* Fires when a JWT is added/refreshed via `putJwt` or stale entries are pruned. The
6+
* developer-facing 401-invalidation event is delivered separately via
7+
* [com.onesignal.IUserJwtInvalidatedListener] (see [JwtTokenStore.addUserJwtInvalidatedListener]).
78
*/
89
internal interface IJwtUpdateListener {
910
/** Fired when a JWT was added or refreshed (`putJwt`), or when stale entries are pruned. */
10-
fun onJwtUpdated(externalId: String) {}
11-
12-
/**
13-
* Fired when a JWT is explicitly invalidated (`invalidateJwt`), e.g. on a 401 response.
14-
* Surfaced to the developer as "the JWT for this user is no longer valid; please refresh."
15-
* Don't trigger from internal cleanup paths (logout, user switch) where notifying the
16-
* app is undesirable.
17-
*/
18-
fun onJwtInvalidated(externalId: String) {}
11+
fun onJwtUpdated(externalId: String)
1912
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/JwtTokenStore.kt‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package com.onesignal.user.internal.jwt
22

3+
import com.onesignal.IUserJwtInvalidatedListener
4+
import com.onesignal.UserJwtInvalidatedEvent
35
import com.onesignal.common.events.EventProducer
4-
import com.onesignal.common.events.IEventNotifier
6+
import com.onesignal.common.threading.OneSignalDispatchers
57
import com.onesignal.core.internal.preferences.IPreferencesService
68
import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys
79
import com.onesignal.core.internal.preferences.PreferenceStores
@@ -13,20 +15,30 @@ import org.json.JSONObject
1315
* Persistent store mapping externalId -> JWT. Multi-user so ops queued under a previous user
1416
* can still resolve their JWT at execution time. Storage is unconditional; *usage* of JWTs is
1517
* gated on `IdentityVerificationService.ivBehaviorActive`.
18+
*
19+
* Notifies two distinct audiences on JWT changes:
20+
* - SDK-internal subscribers via [IJwtUpdateListener] ([addInternalUpdateListener]).
21+
* - Developer-facing subscribers via [IUserJwtInvalidatedListener]
22+
* ([addUserJwtInvalidatedListener]). Pure pub/sub: only listeners subscribed at the time
23+
* of [invalidateJwt] receive the event. Matches iOS — no buffering for late subscribers.
1624
*/
1725
internal class JwtTokenStore(
1826
private val _prefs: IPreferencesService,
19-
) : IEventNotifier<IJwtUpdateListener> {
27+
) {
2028
private val tokens: MutableMap<String, String> = mutableMapOf()
2129
private var isLoaded: Boolean = false
22-
private val updates = EventProducer<IJwtUpdateListener>()
30+
private val internalUpdateListeners = EventProducer<IJwtUpdateListener>()
31+
private val publicInvalidatedListeners = EventProducer<IUserJwtInvalidatedListener>()
2332

24-
override val hasSubscribers: Boolean
25-
get() = updates.hasSubscribers
33+
fun addInternalUpdateListener(listener: IJwtUpdateListener) = internalUpdateListeners.subscribe(listener)
2634

27-
override fun subscribe(handler: IJwtUpdateListener) = updates.subscribe(handler)
35+
fun removeInternalUpdateListener(listener: IJwtUpdateListener) = internalUpdateListeners.unsubscribe(listener)
2836

29-
override fun unsubscribe(handler: IJwtUpdateListener) = updates.unsubscribe(handler)
37+
fun addUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) =
38+
publicInvalidatedListeners.subscribe(listener)
39+
40+
fun removeUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) =
41+
publicInvalidatedListeners.unsubscribe(listener)
3042

3143
fun getJwt(externalId: String): String? {
3244
synchronized(tokens) {
@@ -51,13 +63,13 @@ internal class JwtTokenStore(
5163
}
5264
}
5365
if (changed) {
54-
updates.fire { it.onJwtUpdated(externalId) }
66+
internalUpdateListeners.fire { it.onJwtUpdated(externalId) }
5567
}
5668
}
5769

5870
/**
59-
* Removes the JWT for [externalId] and notifies subscribers via
60-
* [IJwtUpdateListener.onJwtInvalidated]. Surfaced to the developer as "your JWT is no
71+
* Removes the JWT for [externalId] and notifies developer-facing subscribers via
72+
* [IUserJwtInvalidatedListener]. Surfaced to the developer as "your JWT is no
6173
* longer valid; please refresh." Don't call from internal cleanup paths (logout, user
6274
* switch) — use a different mechanism if you need to clear without notifying the app.
6375
*/
@@ -71,13 +83,17 @@ internal class JwtTokenStore(
7183
}
7284
}
7385
if (existed) {
86+
// Dispatch developer-facing event on a background thread so the SDK's internal
87+
// thread (op-repo / HYDRATE paths) doesn't run app code synchronously.
7488
// Per-subscriber try/catch so one throwing listener doesn't break others or
7589
// propagate up into the operation queue (would otherwise drop the failing op).
76-
updates.fire { listener ->
77-
runCatching { listener.onJwtInvalidated(externalId) }
78-
.onFailure { ex ->
79-
Logging.warn("JwtTokenStore: subscriber threw on onJwtInvalidated for externalId=$externalId", ex)
80-
}
90+
OneSignalDispatchers.launchOnDefault {
91+
publicInvalidatedListeners.fire { listener ->
92+
runCatching { listener.onUserJwtInvalidated(UserJwtInvalidatedEvent(externalId)) }
93+
.onFailure { ex ->
94+
Logging.warn("JwtTokenStore: IUserJwtInvalidatedListener threw for externalId=$externalId", ex)
95+
}
96+
}
8197
}
8298
}
8399
}
@@ -95,7 +111,7 @@ internal class JwtTokenStore(
95111
}
96112
}
97113
for (externalId in removed) {
98-
updates.fire { it.onJwtUpdated(externalId) }
114+
internalUpdateListeners.fire { it.onJwtUpdated(externalId) }
99115
}
100116
}
101117

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt‎

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import com.onesignal.debug.internal.logging.Logging
1313
import com.onesignal.mocks.CoreInternalMocks
1414
import com.onesignal.mocks.MockHelper
1515
import com.onesignal.mocks.MockPreferencesService
16-
import com.onesignal.user.internal.jwt.IJwtUpdateListener
1716
import com.onesignal.user.internal.jwt.JwtRequirement
1817
import com.onesignal.user.internal.jwt.JwtTokenStore
1918
import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState
@@ -1088,7 +1087,7 @@ class OperationRepoTests : FunSpec({
10881087
verify(exactly = 1) { mocks.operationRepo.forceExecuteOperations() }
10891088
}
10901089

1091-
test("FAIL_UNAUTHORIZED with IV active invalidates JWT, re-queues ops, and fires onJwtInvalidated") {
1090+
test("FAIL_UNAUTHORIZED with IV active invalidates JWT, re-queues ops, and fires IUserJwtInvalidatedListener") {
10921091
val mocks = Mocks()
10931092
mocks.identityVerificationService = CoreInternalMocks.identityVerificationService(
10941093
newCodePathsRun = true,
@@ -1104,13 +1103,11 @@ class OperationRepoTests : FunSpec({
11041103
mocks.jwtTokenStore.putJwt("alice", "stale-token")
11051104

11061105
var invalidatedId: String? = null
1107-
mocks.jwtTokenStore.subscribe(
1108-
object : IJwtUpdateListener {
1109-
override fun onJwtInvalidated(externalId: String) {
1110-
invalidatedId = externalId
1111-
}
1112-
},
1113-
)
1106+
val listenerWaiter = com.onesignal.common.threading.Waiter()
1107+
mocks.jwtTokenStore.addUserJwtInvalidatedListener { event ->
1108+
invalidatedId = event.externalId
1109+
listenerWaiter.wake()
1110+
}
11141111

11151112
mocks.operationRepo.start()
11161113
// enqueueAndWait with failure should wake waiter with false.
@@ -1120,6 +1117,7 @@ class OperationRepoTests : FunSpec({
11201117
mocks.operationRepo.enqueueAndWait(op)
11211118
}
11221119
}
1120+
listenerWaiter.waitForWake()
11231121

11241122
waitResult shouldBe false
11251123
invalidatedId shouldBe "alice"
@@ -1128,43 +1126,6 @@ class OperationRepoTests : FunSpec({
11281126
verify(exactly = 0) { mocks.operationModelStore.remove(opId) }
11291127
}
11301128

1131-
test("FAIL_UNAUTHORIZED with throwing onJwtInvalidated subscriber does not drop ops") {
1132-
// Regression: a misbehaving subscriber must not propagate an exception up into
1133-
// executeOperations' catch, which would route the op through dropAndWake (op lost).
1134-
val mocks = Mocks()
1135-
mocks.identityVerificationService = CoreInternalMocks.identityVerificationService(
1136-
newCodePathsRun = true,
1137-
ivBehaviorActive = true,
1138-
)
1139-
mocks.configModelStore.model.useIdentityVerification = JwtRequirement.REQUIRED
1140-
1141-
val op = mockOperation(externalId = "alice")
1142-
val opId = op.id
1143-
coEvery { mocks.executor.execute(any()) } returns ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
1144-
mocks.jwtTokenStore.putJwt("alice", "stale-token")
1145-
mocks.jwtTokenStore.subscribe(
1146-
object : IJwtUpdateListener {
1147-
override fun onJwtInvalidated(externalId: String) {
1148-
throw RuntimeException("boom from subscriber")
1149-
}
1150-
},
1151-
)
1152-
1153-
mocks.operationRepo.start()
1154-
val waitResult =
1155-
runBlocking {
1156-
withTimeout(2_000) {
1157-
mocks.operationRepo.enqueueAndWait(op)
1158-
}
1159-
}
1160-
1161-
waitResult shouldBe false
1162-
// JWT was still invalidated despite the subscriber throw.
1163-
mocks.jwtTokenStore.getJwt("alice") shouldBe null
1164-
// Op was re-queued (not dropped) — proving the throw didn't escape into executeOperations.
1165-
verify(exactly = 0) { mocks.operationModelStore.remove(opId) }
1166-
}
1167-
11681129
test("FAIL_UNAUTHORIZED with IV inactive falls back to default drop-on-fail") {
11691130
val mocks = Mocks()
11701131
mocks.identityVerificationService = CoreInternalMocks.identityVerificationService(
@@ -1177,13 +1138,7 @@ class OperationRepoTests : FunSpec({
11771138
coEvery { mocks.executor.execute(any()) } returns ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
11781139

11791140
var invalidatedFired = false
1180-
mocks.jwtTokenStore.subscribe(
1181-
object : IJwtUpdateListener {
1182-
override fun onJwtInvalidated(externalId: String) {
1183-
invalidatedFired = true
1184-
}
1185-
},
1186-
)
1141+
mocks.jwtTokenStore.addUserJwtInvalidatedListener { invalidatedFired = true }
11871142

11881143
mocks.operationRepo.start()
11891144
val waitResult =

0 commit comments

Comments
 (0)