From b968181a2a71a24ddc18ffffd1ec746517bd6960 Mon Sep 17 00:00:00 2001 From: jinliu9508 Date: Mon, 13 Jan 2025 13:41:00 -0500 Subject: [PATCH] Fix: bug that causes onesignalId to be local when setting alias while identity verification is off Also removed backend operation suppression; fixed test units using identity operation executor --- .../com/onesignal/internal/OneSignalImp.kt | 6 +- .../executors/IdentityOperationExecutor.kt | 10 ++- .../IdentityOperationExecutorTests.kt | 80 ++++++++++++++++--- 3 files changed, 82 insertions(+), 14 deletions(-) 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 eee48bc89b..f9b05960b3 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 @@ -257,7 +257,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { ) if (legacyPlayerId == null) { Logging.debug("initWithContext: creating new device-scoped user") - createAndSwitchToNewUser(suppressBackendOperation = true) + createAndSwitchToNewUser() } else { Logging.debug("initWithContext: creating user linked to subscription $legacyPlayerId") @@ -306,7 +306,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { suppressBackendOperation = true } - createAndSwitchToNewUser(suppressBackendOperation = true) + createAndSwitchToNewUser(suppressBackendOperation = suppressBackendOperation) // This operation will be dropped if identity verification is on at the time the operation // is being processed @@ -471,7 +471,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { modify(identityModel, propertiesModel) } - if (identityModel.jwtToken != null) { + if (!useIdentityVerification || identityModel.jwtToken != null) { setupNewSubscription(identityModel, propertiesModel, suppressBackendOperation, sdkId) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt index a2c59553fc..0884f107cc 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt @@ -3,6 +3,7 @@ package com.onesignal.user.internal.operations.impl.executors import com.onesignal.common.NetworkUtils import com.onesignal.common.exceptions.BackendException import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.operations.ExecutionResponse import com.onesignal.core.internal.operations.ExecutionResult import com.onesignal.core.internal.operations.IOperationExecutor @@ -19,6 +20,7 @@ import com.onesignal.user.internal.operations.impl.states.NewRecordsState internal class IdentityOperationExecutor( private val _identityBackend: IIdentityBackendService, private val _identityModelStore: IdentityModelStore, + private val _configModelStore: ConfigModelStore, private val _buildUserService: IRebuildUserService, private val _newRecordState: NewRecordsState, ) : IOperationExecutor { @@ -45,7 +47,13 @@ internal class IdentityOperationExecutor( if (lastOperation is SetAliasOperation) { try { - val identityAlias = _identityModelStore.getIdentityAlias() + var identityAlias: Pair + // use onesignalId from the operation if identity verification is turned off + if (_configModelStore.model.useIdentityVerification) { + identityAlias = _identityModelStore.getIdentityAlias() + } else { + identityAlias = Pair(IdentityConstants.ONESIGNAL_ID, lastOperation.onesignalId) + } _identityBackend.setAlias( lastOperation.appId, identityAlias.first, diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt index d2ae66bf5d..3888cd1e06 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt @@ -41,7 +41,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -77,7 +83,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -98,7 +110,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -119,7 +137,13 @@ class IdentityOperationExecutorTests : FunSpec({ every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -142,7 +166,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add("onesignalId") } val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, newRecordState) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + newRecordState, + ) val operations = listOf(SetAliasOperation("appId", "onesignalId", "aliasKey1", "aliasValue1")) // When @@ -169,7 +199,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -192,7 +228,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -212,7 +254,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -234,7 +282,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, getNewRecordState()) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + getNewRecordState(), + ) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When @@ -259,7 +313,13 @@ class IdentityOperationExecutorTests : FunSpec({ val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add("onesignalId") } val identityOperationExecutor = - IdentityOperationExecutor(mockIdentityBackendService, mockIdentityModelStore, mockBuildUserService, newRecordState) + IdentityOperationExecutor( + mockIdentityBackendService, + mockIdentityModelStore, + MockHelper.configModelStore(), + mockBuildUserService, + newRecordState, + ) val operations = listOf(DeleteAliasOperation("appId", "onesignalId", "aliasKey1")) // When