Conversation
33135b4 to
0e7fb6b
Compare
nan-li
changed the title
nan-li
changed the title
Update Properties Requests are dropped after 90 days, Custom Events after 30 days, and any Request owned by a user other than the current one after 30 days. Each executor checks at uncache and at the start of every flush pass, before prepareForExecution, so a Request that ages past its limit goes even if it could send by then. Create User, Fetch User and the subscription Requests never age out.
A custom event stays a Delta until its user has an onesignal_id, and the Request built later started at age zero, so events parked for a token skipped both limits. The custom events executor now ages its Deltas by their timestamp at uncache and on every pass, and a Request built from a Delta keeps the Delta's timestamp. Tests cover the identity and custom events flush paths, a user switch between flushes, and the Delta case. The age clamp is gone since a negative age never passes a positive limit, and the drop comments say dropped rather than goes.
f7ba56c to
89025a3
Compare
nan-li
changed the title
nan-li
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Multi-model review (Opus 5.5, GPT 5.6 Sol, Grok 4.7) of the 609-line aging change.
Core checks look right: limits, uncache rewrite, Create/Subscription exemptions, and future timestamps having no age.
Act on (3/3)
- Property and identity Deltas reset their age when they become Requests. Custom events copy
delta.timestamp; the other two keepDate()fromOneSignalRequest.init.OSOperationRepocan hold Deltas whilerequirement == .unknownand across launches, so a months-old tag, session, or alias Delta is sent as age 0 — the late-overwrite case this PR is meant to stop. - A nil owner never takes the 30-day cap (
if let owner = owner). Alias Requests also passtypeLimit: nil, so they never age. That covers leftover anonymous work after login and pre-ownerExternalIdcache blobs.
Consider (3/3) Tests miss Identify User, Fetch Identity By Subscription, a current-user alias well past 30 days, nil-owner vs an identified current user, and an old property Delta becoming a Request.
Noted Future timestamps stay unbounded until wall time catches up (documented). currentExternalId can briefly be nil during logout. An in-flight sentToClient Request can be logged as dropped while HTTP still completes.
Sent by Cursor Automation: PR Reviews
| static func maxAge(owner: String?, typeLimit: TimeInterval?, currentExternalId: String?) -> TimeInterval? { | ||
| var limit = typeLimit | ||
| if let owner = owner, owner != currentExternalId { | ||
| limit = min(limit ?? .infinity, nonCurrentUserRequestMaxAge) | ||
| } | ||
| return limit |
There was a problem hiding this comment.
Act on (3/3): a nil owner never takes the 30-day cap. Alias Requests also pass typeLimit: nil, so they never age — leftover anonymous work after login, and cache blobs from before ownerExternalId (documented as nil). Properties still get 90 days instead of 30.
owner != currentExternalId without the if let would apply the cap when the current user is identified and the stamp is nil, while both-nil (anonymous still current) would stay uncapped.
| ownerExternalId: combined.ownerExternalId | ||
| ) | ||
| // Aged from the oldest event, not from when its user got an onesignal_id. | ||
| request.timestamp = combined.timestamp |
There was a problem hiding this comment.
Act on (3/3): this stamp is what property and identity skip. They build OSRequestUpdateProperties / alias Requests from Deltas and leave timestamp as Date(), so a Delta that sat in OSOperationRepo (paused, or requirement == .unknown) or in the executor Delta cache restarts at age 0. Age those Deltas and copy the oldest timestamp the way this executor does.
1 participant
Description
One Line Summary
Drop queued user Requests past an age limit. SDK-5345.
Details
Motivation
Under Identity Verification a Request that cannot be signed waits in its executor queue with no upper bound, and a property update or custom event delivered months late overwrites newer values or is misattributed.
Scope
OSRequestAgingholds the limits: Update Properties 90 days, Custom Events 30 days, any Request owned by a user other than the current one 30 days. A current user's alias changes, Create User, Identify User, Fetch Identity By Subscription and the subscription Requests never age out.prepareForExecution, and rewrite their cache entry when anything is dropped.onesignal_id, so the custom events executor ages its Deltas too, and a Request built from a Delta keeps the Delta's timestamp.nowProviderso tests can move the clock.Testing
Unit testing
ExecutorRequestAgingTestscovers each limit one day either side, the current-user and future-timestamp exceptions, the Requests that never age out, a Request that crosses 90 days between two flushes, the identity and custom events flush paths, a user switch between flushes, and a custom event Delta that waits for anonesignal_id.Manual testing
Full OneSignalUserTests bundle run serially on an iPhone 17 Pro simulator. Run on device.
Affected code checklist
Checklist
Overview
Testing
Final pass
🤖 Generated with Claude Code