Skip to content

Commit 10725ac

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 10725ac

3 files changed

Lines changed: 28 additions & 18 deletions

File tree

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

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,23 +164,33 @@ internal class OperationRepo(
164164

165165
// Dedupe LoginUserOperation for the same user — prevents RecoverFromDroppedLoginBug
166166
// 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.
167+
// If the incoming op carries an existingOnesignalId (for anonymous-user conversion)
168+
// that the queued op lacks, merge it in before dropping the duplicate so we don't
169+
// lose the anon merge link. The queued op's waiter will receive the real result when
170+
// the op executes; the incoming op's waiter is woken optimistically with `true` since
171+
// only LoginHelper.enqueueLogin uses the result and it just logs on failure.
172+
// If the incoming op came from loadSavedOperations (addToStore = false) it's a stale
173+
// duplicate in the store; remove it to avoid an orphan that lingers until cold start.
174174
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)
175+
if (op is LoginUserOperation) {
176+
val existing =
177+
queue.firstOrNull {
178+
it.operation is LoginUserOperation && it.operation.onesignalId == op.onesignalId
179+
}
180+
if (existing != null) {
181+
val existingOp = existing.operation as LoginUserOperation
182+
if (op.existingOnesignalId != null && existingOp.existingOnesignalId == null) {
183+
Logging.debug("OperationRepo: internalEnqueue - merged existingOnesignalId=${op.existingOnesignalId} into queued LoginUserOperation for onesignalId: ${op.onesignalId}.")
184+
existingOp.existingOnesignalId = op.existingOnesignalId
185+
} else {
186+
Logging.debug("OperationRepo: internalEnqueue - LoginUserOperation for onesignalId: ${op.onesignalId} already exists in the queue.")
187+
}
188+
if (!addToStore) {
189+
_operationModelStore.remove(queueItem.operation.id)
190+
}
191+
queueItem.waiter?.wake(true)
192+
return
181193
}
182-
queueItem.waiter?.wake(true)
183-
return
184194
}
185195

186196
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)