diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt index da708c653..f7d1b943c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt @@ -288,8 +288,15 @@ internal class OperationRepo( if (externalId != null) { _jwtTokenStore.invalidateJwt(externalId) Logging.warn("Operation execution failed with 401 Unauthorized, JWT invalidated for user: $externalId. Operations re-queued.") + // Unblock any enqueueAndWait callers so loginSuspend doesn't hang. + ops.forEach { it.waiter?.wake(false) } + // Re-queue with waiter = null: the operation is preserved for retry + // (once a new JWT is provided via updateUserJwt), but the original + // waiter is detached since it was already woken above. synchronized(queue) { - ops.reversed().forEach { queue.add(0, it) } + ops.reversed().forEach { + queue.add(0, OperationQueueItem(it.operation, waiter = null, bucket = it.bucket, retries = it.retries)) + } } dispatchJwtInvalidatedToApp(externalId) } else { @@ -331,9 +338,15 @@ internal class OperationRepo( Logging.error("Operation execution failed with eventual retry, pausing the operation repo: $operations") // keep the failed operation and pause the operation repo from executing paused = true - // add back all operations to the front of the queue to be re-executed. + // Unblock any enqueueAndWait callers so loginSuspend doesn't hang. + ops.forEach { it.waiter?.wake(false) } + // Re-queue with waiter = null: the operation is preserved for retry + // on next cold start, but the original waiter is detached since it + // was already woken above. synchronized(queue) { - ops.reversed().forEach { queue.add(0, it) } + ops.reversed().forEach { + queue.add(0, OperationQueueItem(it.operation, waiter = null, bucket = it.bucket, retries = it.retries)) + } } } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index e4341abbf..e68535dd1 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -384,14 +384,20 @@ internal class OneSignalImp( if (isBackgroundThreadingEnabled) { waitForInit(operationName = "login") - suspendifyOnIO { loginHelper.login(externalId, jwtBearerToken) } } else { if (!isInitialized) { throw IllegalStateException("Must call 'initWithContext' before 'login'") } + } + + val context = loginHelper.switchUser(externalId, jwtBearerToken) ?: return + + if (isBackgroundThreadingEnabled) { + suspendifyOnIO { loginHelper.enqueueLogin(context) } + } else { Thread { runBlocking(runtimeIoDispatcher) { - loginHelper.login(externalId, jwtBearerToken) + loginHelper.enqueueLogin(context) } }.start() } @@ -695,7 +701,8 @@ internal class OneSignalImp( throw IllegalStateException("'initWithContext failed' before 'login'") } - loginHelper.login(externalId, jwtBearerToken) + val context = loginHelper.switchUser(externalId, jwtBearerToken) ?: return@withContext + loginHelper.enqueueLogin(context) } override suspend fun logoutSuspend() = diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt index cc9bb14da..ef516d06a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt @@ -15,22 +15,30 @@ class LoginHelper( private val jwtTokenStore: JwtTokenStore, private val lock: Any, ) { - suspend fun login( + internal data class LoginEnqueueContext( + val appId: String, + val newIdentityOneSignalId: String, + val externalId: String, + val existingOneSignalId: String?, + ) + + /** + * Synchronously switches local user models under the login/logout lock. + * Returns context needed for [enqueueLogin], or null if the user was + * already logged in with [externalId] (no switch needed). + */ + internal fun switchUser( externalId: String, jwtBearerToken: String? = null, - ) { - var currentIdentityExternalId: String? = null - var currentIdentityOneSignalId: String? = null - var newIdentityOneSignalId: String = "" - + ): LoginEnqueueContext? { synchronized(lock) { - currentIdentityExternalId = identityModelStore.model.externalId - currentIdentityOneSignalId = identityModelStore.model.onesignalId + val currentExternalId = identityModelStore.model.externalId + val currentOneSignalId = identityModelStore.model.onesignalId - if (currentIdentityExternalId == externalId) { + if (currentExternalId == externalId) { jwtTokenStore.putJwt(externalId, jwtBearerToken) operationRepo.forceExecuteOperations() - return + return null } jwtTokenStore.putJwt(externalId, jwtBearerToken) @@ -39,23 +47,30 @@ class LoginHelper( identityModel.externalId = externalId } - newIdentityOneSignalId = identityModelStore.model.onesignalId - } + val newOneSignalId = identityModelStore.model.onesignalId - val existingOneSignalId = - if (configModel.useIdentityVerification == true) { - null - } else { - if (currentIdentityExternalId == null) currentIdentityOneSignalId else null - } + val existingOneSignalId = + if (configModel.useIdentityVerification == true) { + null + } else { + if (currentExternalId == null) currentOneSignalId else null + } + + return LoginEnqueueContext(configModel.appId, newOneSignalId, externalId, existingOneSignalId) + } + } + /** + * Enqueues the [LoginUserOperation] and suspends until it completes. + */ + internal suspend fun enqueueLogin(context: LoginEnqueueContext) { val result = operationRepo.enqueueAndWait( LoginUserOperation( - configModel.appId, - newIdentityOneSignalId, - externalId, - existingOneSignalId, + context.appId, + context.newIdentityOneSignalId, + context.externalId, + context.existingOneSignalId, ), ) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt index 0f4367920..d1f129bc2 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt @@ -954,8 +954,9 @@ class OperationRepoTests : FunSpec({ operationRepo.start() val response = operationRepo.enqueueAndWait(operation) - // Then - response shouldBe true + // Then – waiter is woken with false immediately on FAIL_UNAUTHORIZED + // (operation is re-queued with waiter=null for retry when a new JWT is provided) + response shouldBe false verify { jwtTokenStore.invalidateJwt("test-user") } handlerCalledWith shouldBe "test-user" } @@ -1011,8 +1012,11 @@ class OperationRepoTests : FunSpec({ operationRepo.start() val response = operationRepo.enqueueAndWait(operation) - response shouldBe true + // Waiter is woken with false immediately; operation re-queued with waiter=null + response shouldBe false verify { jwtTokenStore.invalidateJwt("test-user") } + // The re-queued op (waiter=null) retries asynchronously; wait for it to complete + delay(3000) coVerify(exactly = 2) { executor.execute(any()) } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/LoginHelperTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/LoginHelperTests.kt index ec6415717..70d9cbd3b 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/LoginHelperTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/LoginHelperTests.kt @@ -11,6 +11,7 @@ import com.onesignal.user.internal.operations.LoginUserOperation import com.onesignal.user.internal.properties.PropertiesModel import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -19,15 +20,7 @@ import io.mockk.slot import io.mockk.verify import kotlinx.coroutines.runBlocking -/** - * Unit tests for the LoginHelper class - * - * These tests focus on the pure business logic of user login operations, - * complementing the integration tests in SDKInitTests.kt which test - * end-to-end SDK initialization and login behavior. - */ class LoginHelperTests : FunSpec({ - // Test constants - using consistent naming with SDKInitTests val appId = "appId" val currentExternalId = "current-user" val newExternalId = "new-user" @@ -38,8 +31,26 @@ class LoginHelperTests : FunSpec({ Logging.logLevel = LogLevel.NONE } - test("login with same external id returns early without creating user") { - // Given + fun createLoginHelper( + identityModelStore: com.onesignal.user.internal.identity.IdentityModelStore, + userSwitcher: UserSwitcher = mockk(relaxed = true), + operationRepo: IOperationRepo = mockk(relaxed = true), + configModel: ConfigModel = mockk().also { + every { it.appId } returns appId + every { it.useIdentityVerification } returns null + }, + jwtTokenStore: JwtTokenStore = mockk(relaxed = true), + lock: Any = Any(), + ) = LoginHelper( + identityModelStore = identityModelStore, + userSwitcher = userSwitcher, + operationRepo = operationRepo, + configModel = configModel, + jwtTokenStore = jwtTokenStore, + lock = lock, + ) + + test("switchUser with same external id returns null without creating user") { val mockIdentityModelStore = MockHelper.identityModelStore { model -> model.externalId = currentExternalId @@ -47,33 +58,22 @@ class LoginHelperTests : FunSpec({ } val mockUserSwitcher = mockk(relaxed = true) val mockOperationRepo = mockk(relaxed = true) - val mockConfigModel = mockk() - every { mockConfigModel.appId } returns appId - every { mockConfigModel.useIdentityVerification } returns false - val loginLock = Any() val loginHelper = - LoginHelper( + createLoginHelper( identityModelStore = mockIdentityModelStore, userSwitcher = mockUserSwitcher, operationRepo = mockOperationRepo, - configModel = mockConfigModel, - jwtTokenStore = mockk(relaxed = true), - lock = loginLock, ) - // When - runBlocking { - loginHelper.login(currentExternalId) - } + val context = loginHelper.switchUser(currentExternalId) - // Then - should return early without any operations + context shouldBe null verify(exactly = 0) { mockUserSwitcher.createAndSwitchToNewUser(suppressBackendOperation = any(), modify = any()) } coVerify(exactly = 0) { mockOperationRepo.enqueueAndWait(any()) } } - test("login with different external id creates and switches to new user") { - // Given + test("switchUser with different external id creates and switches to new user") { val mockIdentityModelStore = MockHelper.identityModelStore { model -> model.externalId = currentExternalId @@ -88,10 +88,6 @@ class LoginHelperTests : FunSpec({ val mockUserSwitcher = mockk() val mockOperationRepo = mockk() - val mockConfigModel = mockk() - every { mockConfigModel.appId } returns appId - every { mockConfigModel.useIdentityVerification } returns false - val loginLock = Any() val userSwitcherSlot = slot<(IdentityModel, PropertiesModel) -> Unit>() every { @@ -107,25 +103,65 @@ class LoginHelperTests : FunSpec({ coEvery { mockOperationRepo.enqueueAndWait(any()) } returns true val loginHelper = - LoginHelper( + createLoginHelper( identityModelStore = mockIdentityModelStore, userSwitcher = mockUserSwitcher, operationRepo = mockOperationRepo, - configModel = mockConfigModel, - jwtTokenStore = mockk(relaxed = true), - lock = loginLock, ) - // When - runBlocking { - loginHelper.login(newExternalId) - } + val context = loginHelper.switchUser(newExternalId) + + context shouldNotBe null + context!!.appId shouldBe appId + context.newIdentityOneSignalId shouldBe newOneSignalId + context.externalId shouldBe newExternalId - // Then - should switch users and enqueue login operation verify(exactly = 1) { mockUserSwitcher.createAndSwitchToNewUser(suppressBackendOperation = any(), modify = any()) } userSwitcherSlot.captured(newIdentityModel, PropertiesModel()) newIdentityModel.externalId shouldBe newExternalId + } + + test("enqueueLogin enqueues login operation and returns") { + val mockIdentityModelStore = + MockHelper.identityModelStore { model -> + model.externalId = currentExternalId + model.onesignalId = currentOneSignalId + } + + val newIdentityModel = + IdentityModel().apply { + externalId = newExternalId + onesignalId = newOneSignalId + } + + val mockUserSwitcher = mockk() + val mockOperationRepo = mockk() + + val userSwitcherSlot = slot<(IdentityModel, PropertiesModel) -> Unit>() + every { + mockUserSwitcher.createAndSwitchToNewUser( + suppressBackendOperation = any(), + modify = capture(userSwitcherSlot), + ) + } answers { + userSwitcherSlot.captured(newIdentityModel, PropertiesModel()) + every { mockIdentityModelStore.model } returns newIdentityModel + } + + coEvery { mockOperationRepo.enqueueAndWait(any()) } returns true + + val loginHelper = + createLoginHelper( + identityModelStore = mockIdentityModelStore, + userSwitcher = mockUserSwitcher, + operationRepo = mockOperationRepo, + ) + + val context = loginHelper.switchUser(newExternalId)!! + runBlocking { + loginHelper.enqueueLogin(context) + } coVerify(exactly = 1) { mockOperationRepo.enqueueAndWait( @@ -133,14 +169,13 @@ class LoginHelperTests : FunSpec({ operation.appId shouldBe appId operation.onesignalId shouldBe newOneSignalId operation.externalId shouldBe newExternalId - operation.existingOnesignalId shouldBe null // Current user already has external ID, so no existing OneSignal ID + operation.existingOnesignalId shouldBe null }, ) } } - test("login with null current external id provides existing onesignal id for conversion") { - // Given - anonymous user (no external ID) + test("switchUser with null current external id provides existing onesignal id for conversion") { val mockIdentityModelStore = MockHelper.identityModelStore { model -> model.externalId = null @@ -158,7 +193,6 @@ class LoginHelperTests : FunSpec({ val mockConfigModel = mockk() every { mockConfigModel.appId } returns appId every { mockConfigModel.useIdentityVerification } returns false - val loginLock = Any() val userSwitcherSlot = slot<(IdentityModel, PropertiesModel) -> Unit>() every { @@ -174,35 +208,31 @@ class LoginHelperTests : FunSpec({ coEvery { mockOperationRepo.enqueueAndWait(any()) } returns true val loginHelper = - LoginHelper( + createLoginHelper( identityModelStore = mockIdentityModelStore, userSwitcher = mockUserSwitcher, operationRepo = mockOperationRepo, configModel = mockConfigModel, - jwtTokenStore = mockk(relaxed = true), - lock = loginLock, ) - // When + val context = loginHelper.switchUser(newExternalId)!! runBlocking { - loginHelper.login(newExternalId) + loginHelper.enqueueLogin(context) } - // Then - should provide existing OneSignal ID for anonymous user conversion coVerify(exactly = 1) { mockOperationRepo.enqueueAndWait( withArg { operation -> operation.appId shouldBe appId operation.onesignalId shouldBe newOneSignalId operation.externalId shouldBe newExternalId - operation.existingOnesignalId shouldBe currentOneSignalId // For conversion + operation.existingOnesignalId shouldBe currentOneSignalId }, ) } } - test("login logs error when operation fails") { - // Given + test("enqueueLogin logs warning when operation fails") { val mockIdentityModelStore = MockHelper.identityModelStore { model -> model.externalId = currentExternalId @@ -217,10 +247,6 @@ class LoginHelperTests : FunSpec({ val mockUserSwitcher = mockk() val mockOperationRepo = mockk() - val mockConfigModel = mockk() - every { mockConfigModel.appId } returns appId - every { mockConfigModel.useIdentityVerification } returns false - val loginLock = Any() val userSwitcherSlot = slot<(IdentityModel, PropertiesModel) -> Unit>() every { @@ -233,25 +259,20 @@ class LoginHelperTests : FunSpec({ every { mockIdentityModelStore.model } returns newIdentityModel } - // Mock operation failure coEvery { mockOperationRepo.enqueueAndWait(any()) } returns false val loginHelper = - LoginHelper( + createLoginHelper( identityModelStore = mockIdentityModelStore, userSwitcher = mockUserSwitcher, operationRepo = mockOperationRepo, - configModel = mockConfigModel, - jwtTokenStore = mockk(relaxed = true), - lock = loginLock, ) - // When + val context = loginHelper.switchUser(newExternalId)!! runBlocking { - loginHelper.login(newExternalId) + loginHelper.enqueueLogin(context) } - // Then - should still switch users but operation fails verify(exactly = 1) { mockUserSwitcher.createAndSwitchToNewUser(suppressBackendOperation = any(), modify = any()) } coVerify(exactly = 1) { mockOperationRepo.enqueueAndWait(any()) } }