Skip to content

Commit 2d9f1f3

Browse files
nan-liclaude
andcommitted
Preserve existingOnesignalId when deduping LoginUserOperation
When dedupe fires, the existing queued op came from RecoverFromDroppedLoginBug (always has existingOnesignalId=null) and the incoming op from enqueueLogin may carry the anonymous user's UUID needed to merge their tags/subscriptions into the newly- identified user. Previously the dedupe kept whichever op was first, silently dropping the merge info when the recovery op won the race. Merge the incoming op's existingOnesignalId into the queued op in place before dropping the duplicate. LoginUserOperation.existingOnesignalId setter relaxed from private to internal to allow the in-place update. Also initialize the LoginUserOperations in the loadSavedOperations test properly — the old dedupe only compared ids so uninitialized ops worked incidentally; the new dedupe reads onesignalId. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7570a43 commit 2d9f1f3

3 files changed

Lines changed: 24 additions & 20 deletions

File tree

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

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -162,25 +162,29 @@ internal class OperationRepo(
162162
return
163163
}
164164

165-
// Dedupe LoginUserOperation for the same user — prevents RecoverFromDroppedLoginBug
166-
// and the real login() call from both enqueuing a login op during the timing window.
167-
// Wake the waiter optimistically with `true`: the already-queued op will do the work,
168-
// but we don't await its real result (that would require waiter chaining). enqueueAndWait
169-
// callers like loginSuspend would otherwise hang forever. The only caller today is
170-
// LoginHelper.enqueueLogin which just logs on failure, so the lost signal is acceptable.
171-
// If the op came from loadSavedOperations (addToStore = false) it's also a stale
172-
// duplicate in the store; remove it to avoid an orphan entry that lingers until the
173-
// next cold start.
165+
// Dedupe LoginUserOperation for the same user. Merge the incoming
166+
// existingOnesignalId in so the anon-user conversion isn't lost when
167+
// RecoverFromDroppedLoginBug wins the race (it enqueues with null).
174168
val op = queueItem.operation
175-
if (op is LoginUserOperation &&
176-
queue.any { it.operation is LoginUserOperation && it.operation.onesignalId == op.onesignalId }
177-
) {
178-
Logging.debug("OperationRepo: internalEnqueue - LoginUserOperation for onesignalId: ${op.onesignalId} already exists in the queue.")
179-
if (!addToStore) {
180-
_operationModelStore.remove(queueItem.operation.id)
169+
if (op is LoginUserOperation) {
170+
val existing =
171+
queue.firstOrNull {
172+
it.operation is LoginUserOperation && it.operation.onesignalId == op.onesignalId
173+
}
174+
if (existing != null) {
175+
val existingOp = existing.operation as LoginUserOperation
176+
if (op.existingOnesignalId != null && existingOp.existingOnesignalId == null) {
177+
Logging.debug("OperationRepo: internalEnqueue - merged existingOnesignalId=${op.existingOnesignalId} into queued LoginUserOperation for onesignalId: ${op.onesignalId}.")
178+
existingOp.existingOnesignalId = op.existingOnesignalId
179+
} else {
180+
Logging.debug("OperationRepo: internalEnqueue - LoginUserOperation for onesignalId: ${op.onesignalId} already exists in the queue.")
181+
}
182+
if (!addToStore) {
183+
_operationModelStore.remove(queueItem.operation.id)
184+
}
185+
queueItem.waiter?.wake(true)
186+
return
181187
}
182-
queueItem.waiter?.wake(true)
183-
return
184188
}
185189

186190
if (index != null) {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class LoginUserOperation() : Operation(LoginUserOperationExecutor.LOGIN_USER) {
4848
*/
4949
var existingOnesignalId: String?
5050
get() = getOptStringProperty(::existingOnesignalId.name)
51-
private set(value) {
51+
internal set(value) {
5252
setOptStringProperty(::existingOnesignalId.name, value)
5353
}
5454

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ class OperationRepoTests : FunSpec({
100100
),
101101
)
102102

103-
val cachedOperation = LoginUserOperation()
104-
val newOperation = LoginUserOperation()
103+
val cachedOperation = LoginUserOperation("appId", "cached-onesignal-id", null, null)
104+
val newOperation = LoginUserOperation("appId", "new-onesignal-id", null, null)
105105
val jsonArray = JSONArray()
106106

107107
// cache the operation

0 commit comments

Comments
 (0)