Conversation
Operations carry a createdAt stamp. Tag, property, session and purchase operations are dropped after 90 days, custom events after 30 days, and any operation owned by a user other than the current one after 30 days, at load and at the top of every getNextOps pass. Login and subscription operations never age out. At 2000 queued operations an enqueue drops the oldest operation the limits cover, and a persisted store over 1 MB is reset instead of parsed.
📊 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)96/104 touched executable lines covered (92.3% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
|
UserRefreshService enqueues a RefreshUserOperation every session, and a queue parked for a token held one per session with nothing aging them out. The repo now keeps one per onesignalId, the way it keeps one login, transferring or waking the waiter. The load-time stamp given to an operation persisted before createdAt existed is written back once after load, so a launch that touches nothing else does not restart its clock.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Multi-model review
Reviewed with Claude Opus 5.5, GPT 5.6 Sol, and Grok 4.7. Aging limits, the 2000 cap, load-time stamps, and enqueue-time refresh dedupe match the writeup. Two holes cut against that intent.
Act on
- Null
externalIdskips the 30-day non-current-user rule (3/3). A tag with a null owner is kept at 31 days (the new test locks this in). An alias with a null owner never gets a type limit, so it never ages out. Pre-externalIdpersisted ops and leftover anonymous aliases after login (translateIdsdoes not setexternalId) stay on the long path. - Retry / rebuild inserts skip the new refresh/login dedupe (2/3). A 404 rebuild requeues the original refresh, then
queue.add(0)s anotherLoginUser+RefreshUserfor the same user. Those never age out. Route them throughinternalEnqueue.
Consider
getNextOpsreturns onJwtRequirement.UNKNOWNbeforedropStaleOperations()(1/3). Load already prunes.- One
persist()perremoveat load is O(n²) on a large upgrade queue (1/3). - Cap ranking uses raw
createdAt; a backward clock can evict the incoming op whileageOfclamps futures (2/3).
Noted
- Wiping a >1 MB store including login/subscription is the documented tradeoff.
- The load-time stamp write is async (~200ms prefs buffer), so a crash in that window restamps on the next launch.
Sent by Cursor Automation: PR Reviews
| val owner = op.externalId | ||
| if (owner != null && owner != _identityModelStore.model.externalId) { | ||
| limits.add(config.opRepoNonCurrentUserOpMaxAge) |
There was a problem hiding this comment.
Null externalId never takes the 30-day non-current-user limit. Combined with SetAlias/DeleteAlias having no type limit, an anonymous alias never expires. Pre-externalId persisted ops and leftover anonymous aliases after login (translateIds updates onesignalId only) stay on this path. Compare onesignalId, or treat a null owner as non-current when the identity store has one.
| synchronized(queue) { | ||
| for (op in response.operations.reversed()) { | ||
| op.id = UUID.randomUUID().toString() | ||
| op.createdAt = _time.currentTimeMillis |
There was a problem hiding this comment.
This insert (and the FAIL_RETRY requeue above) skips internalEnqueue, so the new one-refresh-per-onesignalId dedupe never runs. A 404 rebuild returns FAIL_RETRY plus LoginUser + RefreshUser while the original refresh is also put back — the parked-queue growth this PR is trying to stop. Route executor-returned ops through internalEnqueue.
1 participant
Description
One Line Summary
Drop queued operations past an age limit and cap the queue. SDK-5345.
Details
Motivation
An operation that cannot execute, such as one parked for a JWT under Identity Verification, stays in the persisted queue with no upper bound, and every persist rewrites the whole queue. A tag update or custom event executed months late overwrites newer values or is misattributed.
Scope
Operation.createdAtin unix milliseconds, set when the repo assigns an id and persisted with the operation. An operation persisted by an earlier SDK is stamped with the load time, and the store persists once after such a load so the clock does not restart on the next launch.ConfigModel: tag, property, session and purchase operations 90 days, custom events 30 days, any operation owned by a user other than the current one 30 days. Login and subscription operations never age out.OperationRepodrops stale operations at load and at the top of everygetNextOpspass, whether or not they could execute, removing them from the store and waking their waiters withfalse.opRepoMaxQueueSizeof 2000. At the cap an enqueue drops the oldest operation the limits cover. Login and subscription operations are never dropped, and with nothing droppable the new operation is added anyway.RefreshUserOperationnever ages out; instead the repo keeps one peronesignalId, sinceUserRefreshServiceenqueues one every session and a parked queue would hold them all.OperationModelStoreresets a persisted string over 1,048,576 characters instead of parsing it, login and subscription operations included. The cap keeps this SDK's own writes below that unless custom event payloads are very large; the tradeoff is accepted so a pathological store never reaches the parser.Testing
Unit testing
OperationRepoTestscovers each limit one day either side, the current-user, anonymous and future-createdAtexceptions, a parked operation crossing 90 days between two passes, the load-time stamp, the cap, and the refresh dedupe.OperationModelStoreTestscovers thecreatedAtround trip, the load-time stamp and its persistence across a second load, and the oversized-store reset.Manual testing
:OneSignal:core:testDebugUnitTestrun locally. Not run on a device.Affected code checklist
Checklist
Overview
Testing
Final pass
🤖 Generated with Claude Code