Skip to content

Commit a0f26fe

Browse files
abdulraqeeb33AR Abdul Azeezcursoragent
authored
fix: [SDK-5615] merge the anonymous user on composite login (#2816)
Co-authored-by: AR Abdul Azeez <abdul@onesignal.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 56826d4 commit a0f26fe

3 files changed

Lines changed: 221 additions & 21 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: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.onesignal.user.internal.operations.impl.executors
22

33
import android.os.Build
4+
import com.onesignal.OneSignalError
45
import com.onesignal.common.AndroidUtils
56
import com.onesignal.common.DeviceUtils
67
import com.onesignal.common.IDManager
@@ -91,15 +92,12 @@ internal class LoginUserOperationExecutor(
9192
return ExecutionResponse(ExecutionResult.FAIL_NORETRY)
9293
}
9394
if (shouldCreateUserDirectly(loginUserOp, alias)) {
94-
// When there is no existing user to attempt to associate with the externalId provided, we go right to
95-
// createUser. If there is no externalId provided this is an insert, if there is this will be an
96-
// "upsert with retrieval" as the user may already exist.
97-
//
98-
// Under IV, also skip the optimistic SetAliasOperation: that inline op identifies the
99-
// target user by `onesignal_id = existingOnesignalId`, but IV's alias-resolution would
100-
// rewrite the call to identify by the new (not-yet-registered) `external_id`, producing
101-
// a 404 or idempotent success against the wrong user. createUser's identities-map path
102-
// handles the merge correctly through backend upsert semantics.
95+
// A server anonymous user is merged by sending its onesignal_id. Under IV, skip
96+
// SetAlias: alias resolution would target the new external_id and 404 or hit the wrong user.
97+
anonymousUserToMerge(loginUserOp, alias)?.let { anonymousId ->
98+
val merged = createUser(loginUserOp, operations, anonymousId)
99+
if (!shouldAbandonMerge(merged)) return merged
100+
}
103101
return createUser(loginUserOp, operations)
104102
} else {
105103
// before we create a user we attempt to associate the user defined by existingOnesignalId with the
@@ -179,8 +177,10 @@ internal class LoginUserOperationExecutor(
179177
private suspend fun createUser(
180178
createUserOperation: LoginUserOperation,
181179
operations: List<Operation>,
180+
mergeIntoOnesignalId: String? = null,
182181
): ExecutionResponse {
183182
val identities = mutableMapOf<String, String>()
183+
mergeIntoOnesignalId?.let { identities[IdentityConstants.ONESIGNAL_ID] = it }
184184
var subscriptions = mapOf<String, SubscriptionObject>()
185185
val properties =
186186
PropertiesObject(
@@ -314,17 +314,29 @@ internal class LoginUserOperationExecutor(
314314
metadata = LoginWaitMetadata(backendOneSignalId, emailSubscriptionId, smsSubscriptionId),
315315
)
316316
} catch (ex: BackendException) {
317-
Logging.error("LoginUserOperationExecutor.createUser failed: HTTP ${ex.statusCode}")
318317
val responseType = NetworkUtils.getResponseStatusType(ex.statusCode)
319-
320-
return when (responseType) {
321-
NetworkUtils.ResponseStatusType.RETRYABLE ->
322-
backendExecutionResponse(ExecutionResult.FAIL_RETRY, ex)
323-
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
324-
backendExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED, ex)
325-
else ->
326-
backendExecutionResponse(ExecutionResult.FAIL_NORETRY, ex)
318+
val response =
319+
when (responseType) {
320+
NetworkUtils.ResponseStatusType.RETRYABLE ->
321+
backendExecutionResponse(ExecutionResult.FAIL_RETRY, ex)
322+
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
323+
backendExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED, ex)
324+
else ->
325+
backendExecutionResponse(ExecutionResult.FAIL_NORETRY, ex)
326+
}
327+
// The caller retries an abandoned merge, so this failure is not the outcome.
328+
if (mergeIntoOnesignalId != null && shouldAbandonMerge(response)) {
329+
Logging.debug("LoginUserOperationExecutor merge attempt failed: HTTP ${ex.statusCode}")
330+
if (responseType == NetworkUtils.ResponseStatusType.CONFLICT) {
331+
// Hardcode user-1. The 409 line above reads like a failure to the app.
332+
Logging.debug(
333+
"LoginUserOperationExecutor now handling 409 response with \"code\": \"user-1\" by switching to user with \"external_id\": \"${createUserOperation.externalId}\"",
334+
)
335+
}
336+
} else {
337+
Logging.error("LoginUserOperationExecutor.createUser failed: HTTP ${ex.statusCode}")
327338
}
339+
return response
328340
}
329341
}
330342

@@ -469,6 +481,37 @@ internal class LoginUserOperationExecutor(
469481
return op.existingOnesignalId == null || alias == null
470482
}
471483

484+
// A profile login sends the anonymous user's ID with the new external ID, so Create User attaches
485+
// the external ID to that user instead of creating a new one, like the SetAlias path does.
486+
private fun anonymousUserToMerge(op: LoginUserOperation, alias: String?): String? {
487+
if (!op.hasProfileFields() || _identityVerificationService.ivBehaviorActive || alias == null) return null
488+
return op.existingOnesignalId?.takeUnless { IDManager.isLocalId(it) }
489+
}
490+
491+
// Drop the anonymous id only when it can be why Create User failed.
492+
// A 400 is the payload, so a second request would fail the same way.
493+
private fun shouldAbandonMerge(response: ExecutionResponse): Boolean {
494+
if (response.result != ExecutionResult.FAIL_NORETRY) return false
495+
val status = response.httpStatusCode?.let { NetworkUtils.getResponseStatusType(it) }
496+
return status == NetworkUtils.ResponseStatusType.MISSING ||
497+
(status == NetworkUtils.ResponseStatusType.CONFLICT && onesignalIdConflict(response.httpResponse) != false)
498+
}
499+
500+
/** False when a parsed error does not name onesignal_id. Null when the body has none. */
501+
private fun onesignalIdConflict(body: String?): Boolean? =
502+
OneSignalError.fromBackendResponse(HTTP_CONFLICT, body)
503+
.details
504+
.filter { it.backendCode != null || conflictingAliases(it).isNotEmpty() }
505+
.takeIf { it.isNotEmpty() }
506+
?.any { namesMergedUser(it) }
507+
508+
// user-1 is any alias clash. Retry only when conflicting_aliases names onesignal_id.
509+
private fun namesMergedUser(detail: OneSignalError.Detail): Boolean =
510+
conflictingAliases(detail).containsKey(IdentityConstants.ONESIGNAL_ID)
511+
512+
private fun conflictingAliases(detail: OneSignalError.Detail): Map<*, *> =
513+
detail.meta?.get(CONFLICTING_ALIASES) as? Map<*, *> ?: emptyMap<String, Any?>()
514+
472515
private fun backendExecutionResponse(
473516
result: ExecutionResult,
474517
ex: BackendException,
@@ -481,6 +524,8 @@ internal class LoginUserOperationExecutor(
481524

482525
companion object {
483526
const val LOGIN_USER = "login-user"
527+
private const val HTTP_CONFLICT = 409
528+
private const val CONFLICTING_ALIASES = "conflicting_aliases"
484529
}
485530
}
486531

0 commit comments

Comments
 (0)