Skip to content

Commit 3f3dd09

Browse files
committed
Suppress anonymous ops at enqueue and add LogoutHelper IV test coverage
- Move anonymous operation suppression from individual model store listeners into OperationRepo.enqueue/enqueueAndWait so all enqueue sites (SessionListener, TrackGooglePurchase, CustomEventController, etc.) are covered. LoginUserOperation is exempt. - Remove redundant shouldSuppressForAnonymousUser() from IdentityModelStoreListener, PropertiesModelStoreListener, and SubscriptionModelStoreListener. - Add LogoutHelper tests for IV=true and IV=null (pre-HYDRATE) branches.
1 parent 13c11ed commit 3f3dd09

File tree

5 files changed

+96
-22
lines changed

5 files changed

+96
-22
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ internal class OperationRepo(
128128
operation: Operation,
129129
flush: Boolean,
130130
) {
131+
if (shouldSuppressAnonymousOp(operation)) return
132+
131133
Logging.log(LogLevel.DEBUG, "OperationRepo.enqueue(operation: $operation, flush: $flush)")
132134

133135
operation.id = UUID.randomUUID().toString()
@@ -140,6 +142,8 @@ internal class OperationRepo(
140142
operation: Operation,
141143
flush: Boolean,
142144
): Boolean {
145+
if (shouldSuppressAnonymousOp(operation)) return false
146+
143147
Logging.log(LogLevel.DEBUG, "OperationRepo.enqueueAndWait(operation: $operation, force: $flush)")
144148

145149
operation.id = UUID.randomUUID().toString()
@@ -437,6 +441,16 @@ internal class OperationRepo(
437441
}
438442
}
439443

444+
/**
445+
* Drop anonymous operations at enqueue time when IV is enabled.
446+
* LoginUserOperation is exempt — it's enqueued intentionally during logout
447+
* and purged later by [removeOperationsWithoutExternalId] if needed.
448+
*/
449+
private fun shouldSuppressAnonymousOp(op: Operation): Boolean {
450+
if (op is LoginUserOperation) return false
451+
return _configModelStore.model.useIdentityVerification == true && op.externalId == null
452+
}
453+
440454
/**
441455
* Determines whether [op] is allowed to execute given the current identity
442456
* verification (IV) state. Used by [getNextOps] to skip operations that

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/IdentityModelStoreListener.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ internal class IdentityModelStoreListener(
1414
opRepo: IOperationRepo,
1515
private val _configModelStore: ConfigModelStore,
1616
) : SingletonModelStoreListener<IdentityModel>(_identityModelStore, opRepo) {
17-
private fun shouldSuppressForAnonymousUser(): Boolean =
18-
_configModelStore.model.useIdentityVerification == true &&
19-
_identityModelStore.model.externalId == null
20-
2117
override fun getReplaceOperation(model: IdentityModel): Operation? {
2218
// when the identity model is replaced, nothing to do on the backend. Already handled via login process.
2319
return null
@@ -30,8 +26,6 @@ internal class IdentityModelStoreListener(
3026
oldValue: Any?,
3127
newValue: Any?,
3228
): Operation? {
33-
if (shouldSuppressForAnonymousUser()) return null
34-
3529
return if (newValue != null && newValue is String) {
3630
SetAliasOperation(_configModelStore.model.appId, model.onesignalId, model.externalId, property, newValue)
3731
} else {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/PropertiesModelStoreListener.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ internal class PropertiesModelStoreListener(
1717
private val _configModelStore: ConfigModelStore,
1818
private val _identityModelStore: IdentityModelStore,
1919
) : SingletonModelStoreListener<PropertiesModel>(store, opRepo) {
20-
private fun shouldSuppressForAnonymousUser(): Boolean =
21-
_configModelStore.model.useIdentityVerification == true &&
22-
_identityModelStore.model.externalId == null
23-
2420
override fun getReplaceOperation(model: PropertiesModel): Operation? {
2521
// when the property model is replaced, nothing to do on the backend. Already handled via login process.
2622
return null
@@ -33,8 +29,6 @@ internal class PropertiesModelStoreListener(
3329
oldValue: Any?,
3430
newValue: Any?,
3531
): Operation? {
36-
if (shouldSuppressForAnonymousUser()) return null
37-
3832
// for any of the property changes, we do not need to fire an operation.
3933
if (path.startsWith(PropertiesModel::locationTimestamp.name) ||
4034
path.startsWith(PropertiesModel::locationBackground.name) ||

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/listeners/SubscriptionModelStoreListener.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,7 @@ internal class SubscriptionModelStoreListener(
1818
private val _identityModelStore: IdentityModelStore,
1919
private val _configModelStore: ConfigModelStore,
2020
) : ModelStoreListener<SubscriptionModel>(store, opRepo) {
21-
private fun shouldSuppressForAnonymousUser(): Boolean =
22-
_configModelStore.model.useIdentityVerification == true &&
23-
_identityModelStore.model.externalId == null
24-
2521
override fun getAddOperation(model: SubscriptionModel): Operation? {
26-
if (shouldSuppressForAnonymousUser()) return null
27-
2822
val enabledAndStatus = getSubscriptionEnabledAndStatus(model)
2923
return CreateSubscriptionOperation(
3024
_configModelStore.model.appId,
@@ -39,8 +33,6 @@ internal class SubscriptionModelStoreListener(
3933
}
4034

4135
override fun getRemoveOperation(model: SubscriptionModel): Operation? {
42-
if (shouldSuppressForAnonymousUser()) return null
43-
4436
return DeleteSubscriptionOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId, _identityModelStore.model.externalId, model.id)
4537
}
4638

@@ -51,8 +43,6 @@ internal class SubscriptionModelStoreListener(
5143
oldValue: Any?,
5244
newValue: Any?,
5345
): Operation? {
54-
if (shouldSuppressForAnonymousUser()) return null
55-
5646
val enabledAndStatus = getSubscriptionEnabledAndStatus(model)
5747
return UpdateSubscriptionOperation(
5848
_configModelStore.model.appId,

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ 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
910
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
1011
import io.kotest.core.spec.style.FunSpec
1112
import io.kotest.matchers.shouldBe
@@ -177,4 +178,85 @@ class LogoutHelperTests : FunSpec({
177178
verify(atLeast = 1) { mockUserSwitcher.createAndSwitchToNewUser() }
178179
verify(atLeast = 1) { mockOperationRepo.enqueue(any()) }
179180
}
181+
182+
test("logout with IV=true disables push and suppresses backend operation") {
183+
// Given - identified user with IV enabled
184+
val pushSubId = "push-sub-id"
185+
val mockSubscriptionModel = mockk<SubscriptionModel>(relaxed = true)
186+
val mockIdentityModelStore =
187+
MockHelper.identityModelStore { model ->
188+
model.externalId = externalId
189+
model.onesignalId = onesignalId
190+
}
191+
val mockUserSwitcher = mockk<UserSwitcher>(relaxed = true)
192+
val mockOperationRepo = mockk<IOperationRepo>(relaxed = true)
193+
val mockConfigModel = mockk<ConfigModel>()
194+
every { mockConfigModel.appId } returns appId
195+
every { mockConfigModel.useIdentityVerification } returns true
196+
every { mockConfigModel.pushSubscriptionId } returns pushSubId
197+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>(relaxed = true)
198+
every { mockSubscriptionModelStore.get(pushSubId) } returns mockSubscriptionModel
199+
200+
val logoutHelper =
201+
LogoutHelper(
202+
identityModelStore = mockIdentityModelStore,
203+
userSwitcher = mockUserSwitcher,
204+
operationRepo = mockOperationRepo,
205+
configModel = mockConfigModel,
206+
subscriptionModelStore = mockSubscriptionModelStore,
207+
lock = Any(),
208+
)
209+
210+
// When
211+
logoutHelper.logout()
212+
213+
// Then
214+
verify { mockSubscriptionModel.isDisabledInternally = true }
215+
verify { mockUserSwitcher.createAndSwitchToNewUser(suppressBackendOperation = true) }
216+
verify(exactly = 0) { mockOperationRepo.enqueue(any()) }
217+
}
218+
219+
test("logout with IV=null (pre-HYDRATE) disables push and enqueues anonymous user") {
220+
// Given - identified user, IV state unknown
221+
val pushSubId = "push-sub-id"
222+
val mockSubscriptionModel = mockk<SubscriptionModel>(relaxed = true)
223+
val mockIdentityModelStore =
224+
MockHelper.identityModelStore { model ->
225+
model.externalId = externalId
226+
model.onesignalId = onesignalId
227+
}
228+
val mockUserSwitcher = mockk<UserSwitcher>(relaxed = true)
229+
val mockOperationRepo = mockk<IOperationRepo>(relaxed = true)
230+
val mockConfigModel = mockk<ConfigModel>()
231+
every { mockConfigModel.appId } returns appId
232+
every { mockConfigModel.useIdentityVerification } returns null
233+
every { mockConfigModel.pushSubscriptionId } returns pushSubId
234+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>(relaxed = true)
235+
every { mockSubscriptionModelStore.get(pushSubId) } returns mockSubscriptionModel
236+
237+
val logoutHelper =
238+
LogoutHelper(
239+
identityModelStore = mockIdentityModelStore,
240+
userSwitcher = mockUserSwitcher,
241+
operationRepo = mockOperationRepo,
242+
configModel = mockConfigModel,
243+
subscriptionModelStore = mockSubscriptionModelStore,
244+
lock = Any(),
245+
)
246+
247+
// When
248+
logoutHelper.logout()
249+
250+
// Then - push disabled, no suppression, anonymous LoginUserOperation enqueued
251+
verify { mockSubscriptionModel.isDisabledInternally = true }
252+
verify { mockUserSwitcher.createAndSwitchToNewUser() }
253+
verify {
254+
mockOperationRepo.enqueue(
255+
withArg<LoginUserOperation> { operation ->
256+
operation.appId shouldBe appId
257+
operation.externalId shouldBe null
258+
},
259+
)
260+
}
261+
}
180262
})

0 commit comments

Comments
 (0)