Skip to content

Commit edfe844

Browse files
AR Abdul Azeezcursoragent
andcommitted
fix: merge the anonymous user on composite login
Composite login creates the user directly, so data set on the anonymous user before login was left behind. It now sends the anonymous user's onesignal_id with the external ID, and Create User attaches the external ID to that user, matching what login(externalId) does through SetAlias. A 409 means the external ID already belongs to another user, so it retries without the onesignal_id. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e5ba296 commit edfe844

3 files changed

Lines changed: 76 additions & 3 deletions

File tree

‎OneSignalSDK/detekt/detekt-baseline-core.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<ID>ComplexMethod:ConfigModelStoreListener.kt$ConfigModelStoreListener$private fun fetchParams()</ID>
99
<ID>ComplexMethod:HttpClient.kt$HttpClient$private suspend fun makeRequestIODispatcher( url: String, method: String?, jsonBody: JSONObject?, timeout: Int, headers: OptionalHeaders?, ): HttpResponse</ID>
1010
<ID>ComplexMethod:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List&lt;Operation>): ExecutionResponse</ID>
11-
<ID>ComplexMethod:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List&lt;Operation>, ): ExecutionResponse</ID>
11+
<ID>ComplexMethod:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List&lt;Operation>, mergeIntoOnesignalId: String? = null, ): ExecutionResponse</ID>
1212
<ID>ComplexMethod:OSDatabase.kt$OSDatabase$@Synchronized private fun internalOnUpgrade( db: SQLiteDatabase, oldVersion: Int, newVersion: Int, )</ID>
1313
<ID>ComplexMethod:OperationModelStore.kt$OperationModelStore$override fun create(jsonObject: JSONObject?): Operation?</ID>
1414
<ID>ComplexMethod:OperationRepo.kt$OperationRepo$internal suspend fun executeOperations(ops: List&lt;OperationQueueItem>)</ID>
@@ -194,7 +194,7 @@
194194
<ID>LongMethod:FeatureFlagsBackendService.kt$FeatureFlagsBackendService$override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome</ID>
195195
<ID>LongMethod:HttpClient.kt$HttpClient$private suspend fun makeRequestIODispatcher( url: String, method: String?, jsonBody: JSONObject?, timeout: Int, headers: OptionalHeaders?, ): HttpResponse</ID>
196196
<ID>LongMethod:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List&lt;Operation>): ExecutionResponse</ID>
197-
<ID>LongMethod:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List&lt;Operation>, ): ExecutionResponse</ID>
197+
<ID>LongMethod:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List&lt;Operation>, mergeIntoOnesignalId: String? = null, ): ExecutionResponse</ID>
198198
<ID>LongMethod:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun loginUser( loginUserOp: LoginUserOperation, operations: List&lt;Operation>, ): ExecutionResponse</ID>
199199
<ID>LongMethod:OperationRepo.kt$OperationRepo$internal suspend fun executeOperations(ops: List&lt;OperationQueueItem>)</ID>
200200
<ID>LongMethod:OperationRepo.kt$OperationRepo$private fun internalEnqueue( queueItem: OperationQueueItem, flush: Boolean, addToStore: Boolean, index: Int? = null, )</ID>
@@ -293,7 +293,7 @@
293293
<ID>NestedBlockDepth:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List&lt;Operation>): ExecutionResponse</ID>
294294
<ID>NestedBlockDepth:InfluenceManager.kt$InfluenceManager$private fun attemptSessionUpgrade( entryAction: AppEntryAction, directId: String? = null, )</ID>
295295
<ID>NestedBlockDepth:JSONUtils.kt$JSONUtils$fun compareJSONArrays( jsonArray1: JSONArray?, jsonArray2: JSONArray?, ): Boolean</ID>
296-
<ID>NestedBlockDepth:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List&lt;Operation>, ): ExecutionResponse</ID>
296+
<ID>NestedBlockDepth:LoginUserOperationExecutor.kt$LoginUserOperationExecutor$private suspend fun createUser( createUserOperation: LoginUserOperation, operations: List&lt;Operation>, mergeIntoOnesignalId: String? = null, ): ExecutionResponse</ID>
297297
<ID>NestedBlockDepth:RefreshUserOperationExecutor.kt$RefreshUserOperationExecutor$private suspend fun getUser(op: RefreshUserOperation): ExecutionResponse</ID>
298298
<ID>NestedBlockDepth:ServiceRegistration.kt$ServiceRegistrationReflection$override fun resolve(provider: IServiceProvider): Any?</ID>
299299
<ID>NestedBlockDepth:ServiceRegistration.kt$ServiceRegistrationReflection$private fun doesHaveAllParameters( constructor: Constructor&lt;*>, provider: IServiceProvider, ): Boolean</ID>

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,11 @@ internal class LoginUserOperationExecutor(
9999
// rewrite the call to identify by the new (not-yet-registered) `external_id`, producing
100100
// a 404 or idempotent success against the wrong user. createUser's identities-map path
101101
// handles the merge correctly through backend upsert semantics.
102+
anonymousUserToMerge(loginUserOp, alias)?.let { anonymousId ->
103+
val merged = createUser(loginUserOp, operations, anonymousId)
104+
// 409: the external ID already belongs to another user, so there is nothing to merge into.
105+
if (merged.httpStatusCode != HTTP_CONFLICT) return merged
106+
}
102107
return createUser(loginUserOp, operations)
103108
} else {
104109
// before we create a user we attempt to associate the user defined by existingOnesignalId with the
@@ -178,8 +183,10 @@ internal class LoginUserOperationExecutor(
178183
private suspend fun createUser(
179184
createUserOperation: LoginUserOperation,
180185
operations: List<Operation>,
186+
mergeIntoOnesignalId: String? = null,
181187
): ExecutionResponse {
182188
val identities = mutableMapOf<String, String>()
189+
mergeIntoOnesignalId?.let { identities[IdentityConstants.ONESIGNAL_ID] = it }
183190
var subscriptions = mapOf<String, SubscriptionObject>()
184191
val properties =
185192
PropertiesObject(
@@ -447,6 +454,13 @@ internal class LoginUserOperationExecutor(
447454
return op.existingOnesignalId == null || alias == null
448455
}
449456

457+
// A profile login sends the anonymous user's ID with the new external ID, so Create User attaches
458+
// the external ID to that user instead of creating a new one, like the SetAlias path does.
459+
private fun anonymousUserToMerge(op: LoginUserOperation, alias: String?): String? {
460+
if (!op.hasProfileFields() || _identityVerificationService.ivBehaviorActive || alias == null) return null
461+
return op.existingOnesignalId?.takeUnless { IDManager.isLocalId(it) }
462+
}
463+
450464
private fun backendExecutionResponse(
451465
result: ExecutionResult,
452466
ex: BackendException,
@@ -459,6 +473,7 @@ internal class LoginUserOperationExecutor(
459473

460474
companion object {
461475
const val LOGIN_USER = "login-user"
476+
private const val HTTP_CONFLICT = 409
462477
}
463478
}
464479

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest
66
import com.onesignal.OneSignalUserProfile
77
import com.onesignal.common.AndroidUtils
88
import com.onesignal.common.DeviceUtils
9+
import com.onesignal.common.IDManager
910
import com.onesignal.common.OneSignalUtils
1011
import com.onesignal.common.PIIHasher
1112
import com.onesignal.common.RootToolsInternalMethods
@@ -1579,6 +1580,63 @@ class LoginUserOperationExecutorTests : FunSpec({
15791580
coVerify(exactly = 0) { identityExecutor.execute(any()) }
15801581
}
15811582

1583+
test("composite login from an anonymous user sends its onesignal_id so the user is merged") {
1584+
val mockUserBackendService = mockk<IUserBackendService>()
1585+
coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
1586+
CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to "existing-osid"), PropertiesObject(), listOf())
1587+
1588+
val response =
1589+
profileExecutor(mockUserBackendService).execute(
1590+
listOf(LoginUserOperation(appId, localOneSignalId, "externalId", "existing-osid", OneSignalUserProfile(tags = mapOf("plan" to "pro")))),
1591+
)
1592+
1593+
response.result shouldBe ExecutionResult.SUCCESS
1594+
response.idTranslations shouldBe mapOf(localOneSignalId to "existing-osid")
1595+
coVerify(exactly = 1) {
1596+
mockUserBackendService.createUser(
1597+
appId,
1598+
mapOf(IdentityConstants.EXTERNAL_ID to "externalId", IdentityConstants.ONESIGNAL_ID to "existing-osid"),
1599+
any(),
1600+
any(),
1601+
)
1602+
}
1603+
}
1604+
1605+
test("composite login retries without onesignal_id when the external ID belongs to another user") {
1606+
val mockUserBackendService = mockk<IUserBackendService>()
1607+
val mergeIdentities = mapOf(IdentityConstants.EXTERNAL_ID to "externalId", IdentityConstants.ONESIGNAL_ID to "existing-osid")
1608+
val plainIdentities = mapOf(IdentityConstants.EXTERNAL_ID to "externalId")
1609+
coEvery { mockUserBackendService.createUser(appId, mergeIdentities, any(), any()) } throws BackendException(409, "user-1")
1610+
coEvery { mockUserBackendService.createUser(appId, plainIdentities, any(), any()) } returns
1611+
CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
1612+
1613+
val response =
1614+
profileExecutor(mockUserBackendService).execute(
1615+
listOf(LoginUserOperation(appId, localOneSignalId, "externalId", "existing-osid", OneSignalUserProfile(tags = mapOf("plan" to "pro")))),
1616+
)
1617+
1618+
response.result shouldBe ExecutionResult.SUCCESS
1619+
response.idTranslations shouldBe mapOf(localOneSignalId to remoteOneSignalId)
1620+
coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mergeIdentities, any(), any()) }
1621+
coVerify(exactly = 1) { mockUserBackendService.createUser(appId, plainIdentities, any(), any()) }
1622+
}
1623+
1624+
test("composite login does not merge when the anonymous user is still local, or under IV") {
1625+
for ((existingId, ivActive) in listOf(IDManager.createLocalId() to false, "existing-osid" to true)) {
1626+
val mockUserBackendService = mockk<IUserBackendService>()
1627+
coEvery { mockUserBackendService.createUser(any(), any(), any(), any(), any()) } returns
1628+
CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
1629+
1630+
profileExecutor(mockUserBackendService, ivActive = ivActive).execute(
1631+
listOf(LoginUserOperation(appId, localOneSignalId, "externalId", existingId, OneSignalUserProfile(tags = mapOf("plan" to "pro")))),
1632+
)
1633+
1634+
coVerify(exactly = 1) {
1635+
mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any(), any(), any())
1636+
}
1637+
}
1638+
}
1639+
15821640
test("identity-only login still associates via SetAlias when existingOnesignalId is set") {
15831641
val mockUserBackendService = mockk<IUserBackendService>()
15841642
val identityExecutor = mockk<IdentityOperationExecutor>()

0 commit comments

Comments
 (0)