feat: [SDK-5024] add composite login(externalId, profile) as one Create User POST - #2743
feat: [SDK-5024] add composite login(externalId, profile) as one Create User POST#2743abdulraqeeb33 wants to merge 5 commits into
Conversation
…te User POST Apply email, SMS, tags, and aliases in a single upsert so identity-only login stays unchanged, and surface HTTP 4xx on the waiting OneSignalResult instead of pausing the op repo. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)196/218 touched executable lines covered (89.9% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Multi-model review (Claude Opus 5, GPT 5.6 Sol, Cursor Grok 4.6) on the composite login(externalId, profile) change. Substantive diff is +1,331/−64 (1,333 after excluding generated core.api).
Act on
- Composite 400/409 drops the whole grouped batch (3/3).
FAIL_NORETRY→dropAndWake(ops)removes login and grouped push create/transfer ops. AfterswitchUserhas already replaced local identity, a bad email/SMS can strand the user on a local OneSignal ID with no push create. Fail the waiter without dropping followers, or revert the switch. - Same-user composite login can return success before HTTP (3/3). Dedupe by
onesignalIdwakes a second waiter withOperationWaitResult(true)when the queued op already has a waiter. Combined with first-winsmergeProfileFromon email/phone, a laterlogin(id, profile)reports success while its address is discarded, and a later 400 never reaches that caller. - Same-user context is captured off the login lock (2/3).
contextForCurrentUserandloginDataFromStoresread live identity/subscription stores. Concurrent login/logout can pair profile A with user B, or return{externalId: A, onesignalId: B}.
Consider
- Full profile plus raw HTTP bodies are logged (3/3).
- Reserved-only aliases still count as profile fields, so same-user login is no longer a no-op and 4xx uses
FAIL_NORETRY(2/3). - Any profile field skips the anonymous→identified
SetAliasmerge (Opus). persistProfileSubscriptionhardcodesSUBSCRIBED/optedIn=true; match-by-type can bind the wrong email/SMS (Opus/Grok).backendCodeis the HTTP status, not a catalog code; 404/410 still pauses the repo (Opus).
Noted / dismissed
OneSignalUserProfile as a public data class (copy/componentN API surface), constructor binary breaks on ExecutionResponse/LoginUserOperation, hydrateProfile writing the request not the response, and catalog backendCode parsing — last is explicitly out of scope.
Sent by Cursor Automation: PR Reviews
| NetworkUtils.ResponseStatusType.INVALID, | ||
| NetworkUtils.ResponseStatusType.CONFLICT, | ||
| -> | ||
| if (createUserOperation.hasProfileFields()) { | ||
| backendExecutionResponse(ExecutionResult.FAIL_NORETRY, ex) |
There was a problem hiding this comment.
Act on (3/3): Composite 400/409 returns FAIL_NORETRY, and OperationRepo then dropAndWakes the entire grouped batch — not just this login.
LoginUserOperation, CreateSubscriptionOperation, and TransferSubscriptionOperation share createComparisonKey = "$appId.User.$onesignalId". After a new-user switchUser, the push create is in that group. A malformed SMS/email therefore drops the device subscription create while local identity already shows the new externalId and a still-local onesignalId.
A retry takes the same-user path and POSTs profile-only, so push never reattaches. Identity-only 4xx still pause and keep the ops; this path should fail the waiter without dropping grouped followers, or roll the local switch back.
| existingOp.mergeProfileFrom(op) | ||
| // Transfer the waiter so enqueueAndWait callers see the queued op's real execution result. | ||
| if (queueItem.waiter != null && existing.waiter == null) { | ||
| existing.waiter = queueItem.waiter | ||
| } else { | ||
| queueItem.waiter?.wake(true) | ||
| queueItem.waiter?.wake(OperationWaitResult(true)) |
There was a problem hiding this comment.
Act on (3/3): When a second composite login(id, profile) dedupes onto a queued LoginUserOperation that already has a waiter, this wakes the incoming waiter with success immediately.
Every login path now uses enqueueAndAwaitResult, so the second caller always hits this else. OneSignalImp.login then returns OneSignalResult.success(loginDataFromStores(...)) before createUser runs — often with a local onesignalId and null subscription IDs. If the surviving op later 400s, only the first caller sees failure.
Attach/chain waiters (or wait on the surviving op). Do not wake(true) for a composite login that has not executed.
| internal fun mergeProfileFrom(other: LoginUserOperation) { | ||
| if (email.isNullOrEmpty()) email = other.email | ||
| if (phoneNumber.isNullOrEmpty()) phoneNumber = other.phoneNumber | ||
| if (other.tags.isNotEmpty()) tags = tags + other.tags | ||
| if (other.aliases.isNotEmpty()) aliases = aliases + other.aliases | ||
| } |
There was a problem hiding this comment.
Act on (2/3, bundled with the dedupe success above): Email/phone are first-write-wins; tags/aliases are last-write-wins.
login("alice", profile(email="old@x.com")) then login("alice", profile(email="new@x.com")) while the first is queued sends old@x.com and drops the new address — and the second call already returned success. Make addresses last-write-wins, or do not merge ops whose email/phone differ.
| internal fun contextForCurrentUser(externalId: String): LoginEnqueueContext = | ||
| LoginEnqueueContext( | ||
| appId = configModel.appId, | ||
| newIdentityOneSignalId = identityModelStore.model.onesignalId, | ||
| externalId = externalId, | ||
| existingOneSignalId = null, | ||
| ) |
There was a problem hiding this comment.
Act on (2/3): Same-user composite login snapshots onesignalId here with no loginLogoutLock, after switchUser already released it.
A concurrent logout/login can enqueue LoginUserOperation(onesignalId=otherUser, externalId=original, profile). Combined with loginDataFromStores reading the live identity/subscription stores after await, caller A can get {externalId: A, onesignalId: B} or attach profile fields to the wrong local user.
Capture {appId, onesignalId, externalId} under the same lock, and abort if externalId no longer matches.
…Repo LoginData lookup lives on LoginHelper, profile emptiness on OneSignalUserProfile, nested properties use JSONUtils.mapToJson, and wait results are an ExecutionResponse extension. Co-authored-by: Cursor <cursoragent@cursor.com>
| ) | ||
|
|
||
| OutlineButton( | ||
| text = "LOGIN WITH PROFILE", |
There was a problem hiding this comment.
this needs to be added to iOS and all the other wrappers
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Description
One Line Summary
Add
login(externalId, profile)so email, SMS, tags, and aliases go out on a single Create User upsert, and return HTTP 4xx on the waitingOneSignalResult.Details
Motivation
Identity-only
login(externalId)cannot attach profile fields. Composite login needs those fields on one Create User POST (upsert byexternal_id) so a bad SMS/email cannot freeze the op repo, and the suspend caller gets the backend body instead of a generic "did not complete".Stacked on #2710 (
ar/sdk-4988). Parent: SDK-5023. Types/signature: SDK-5024. One POST: SDK-5025.Scope
OneSignalUserProfile(email,phoneNumberin E.164,tags,aliases) plus JavaBuilder. JWT stays a separate argument.externalIdstays the login key.login/loginSuspendare unchanged.externalIdwith no profile fields is still a no-op. SameexternalIdwith profile fields upserts.FAIL_NORETRY) instead of pausing the repo. Identity-only 4xx still pause.enqueueAndAwaitResultcarries HTTP status/body so compositeloginreturnsOneSignalResult.failure(BACKEND_ERROR, message=body, backendCode=HTTP status). Catalog codes are not parsed yet.Not in this PR: persistence (SDK-5026), telemetry (SDK-5027), a login callback, catalog
backendCode.Testing
Unit testing
Executor tests cover identity-only payload unchanged, profile aliases/email/SMS/tags on Create User, reserved alias skip, 400/409
FAIL_NORETRYwith HTTP fields, 401, and hydration. OperationRepo wakesenqueueAndAwaitResultwith status/body. LoginHelper, UserBackendService nested tags, and profile builder tests added.Manual testing
Demo app on a physical device. Composite login with a non-E.164 SMS (
4129089471) returns HTTP 400Invalid token format for device type SMS, logs the body, returnsOneSignalResult.failure, and does not pause the op repo. Retry with+14129089471succeeds.Affected code checklist
Checklist
Overview
Testing
Final pass
Made with Cursor