Repository navigation
feat: [SDK-5391] send a session end when a sessions API session ends - #1782
fadi-george wants to merge 6 commits into
Conversation
… starts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abdulraqeeb33
left a comment
There was a problem hiding this comment.
The end of the current user looks right. It is the record just replaced, queued before the next create, and a flag-off session is not ended.
Two ends are lost. The restored user does not survive a relaunch, and a record saved before lastUnfocusTime is ended at its start even when it already has foreground time.
Matches Android, which ends it at max(unfocus, focus) time. The focus that starts a new session is reported before the session starts, so that visit does not move the previous session's end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… across a kill Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… unfocus and duration Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abdulraqeeb33
left a comment
There was a problem hiding this comment.
The identity restore is right. identityModel(for:) re-registers the model the delta already holds, and the queued-end test now checks the real unfocus and duration.
onFocus still persists the next session's focus onto the current record. A kill before startNewSession ends the previous session at that focus.
| return | ||
| } | ||
| // Saved now, since a kill leaves no unfocus to record when the session was last active. | ||
| record?.lastFocusTime = wallNow |
There was a problem hiding this comment.
This writes lastFocusTime onto the loaded record and persists it. The previous focus stays only in openVisit, and startNewSession is what puts it back. The tracker calls onFocus and then startNewSession, which is dispatched onto the main queue (OneSignalTracker.m:83, OneSignal.m:379). A kill in that gap leaves the next session's focus on the previous record. The next launch has no openVisit, so endTime takes that focus over lastUnfocusTime.
Keep the boundary focus in openVisit until startNewSession writes it onto the new record. Add a test that focuses on one instance and starts the session on a fresh one.
There was a problem hiding this comment.
Fixed in 615c235. onFocus now only saves the focus on the record when it continues that session (record started in this process and back within 30 s of the last unfocus). Any other focus stays in memory until startNewSession puts it on the new record, which also covers a cold start and a skipped session start (e.g. consent pending). Added tests that focus on one instance and start the session on a fresh one, for both a resume past 30 s and a cold start.
| /// Seconds since 1970. A session killed in the foreground never unfocused, so it ends at its | ||
| /// last focus. The visit the kill cut short is not in the duration either. | ||
| public var endTime: TimeInterval { | ||
| max(startTime, lastUnfocusTime ?? startTime, lastFocusTime ?? startTime) |
There was a problem hiding this comment.
A record saved before these timestamps, with activeDuration already greater than 0 and both optionals nil, still ends at its start. When both are nil, end at startTime + activeDuration.
There was a problem hiding this comment.
Same as #1782 (comment): no saved record predates these fields since OSSessionRecord hasn't shipped. activeDuration also only grows in onUnfocus, which sets lastUnfocusTime in the same step, so duration > 0 with both nil can't happen. Leaving as is.
…t record Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Description
One Line Summary
When the sessions API flag is on, queue a session end with
end_timeand the final duration when the next session starts.Details
Motivation
Part of the iOS SDK Sessions stack (SDK-5391). Ended sessions need a
PATCH /apps/{app_id}/sessions/{session_id}withend_timeand the final cumulativeduration_seconds. Builds on #1768 and #1779. Replaces #1772, which was based on the earlier session queue. Android counterpart: OneSignal/OneSignal-Android-SDK#2781.Scope
OSSessionService.startNewSessionnow returnsOSSessionStart(started:ended:). The replaced record is filled with any IDs assigned since and handed back under the same lock that replaces it, so the session is not lost.OneSignalUserManagerImpl.startNewSessionqueues the ended session's update before the new session's create.end_timeis the later of the last unfocus and the last focus, saved in new optionalOSSessionRecord.lastUnfocusTimeandlastFocusTimefields (records stored by earlier versions still decode). Like Android, a session killed in the foreground ends at its last focus. The focus that starts a new session is reported before the session starts, so that visit counts toward the new session and does not move the previous one's end.onesignal_id. A session with no pinnedonesignal_idwas never created, so it is still dropped.OSSessionOperationExecutor. No executor changes.usesSessionsApiis true are ended this way. Flag off: nothing new is sent, and the legacy focus-time processors are unchanged.Things to note
end_timeandstart_timeare both wall clock, so a clock moved back mid-session could produceend_timebeforestart_time.Testing
Unit testing
OSSessionServiceTests: unfocus records the wall clock time; the first session ends nothing; a new session returns the previous one with its final duration and last unfocus; a session killed in the foreground ends at its start, or at its last focus after returning to the foreground; a resume past the threshold and a cold start do not move the previous session's end; a session from an earlier process ends with the IDs assigned since.SessionRecordUserTests: the previous session's end is queued before the next create; a legacy session queues no end; the end of a session whose user is no longer loaded is queued for its pinned user.OSSessionServiceTests,SessionRecordUserTests, andOSSessionOperationExecutorTestspass locally. One run failedtestNewSessionQueuesNoCreateWhenTheFlagIsOff, then the same 50 tests passed on 2 reruns, so it looks flaky. SwiftLint reports no new violation types.Manual testing
Not tested on device yet; end-to-end verification is planned in the iOS rollout and QA ticket.
Affected code checklist
Checklist
Overview
Testing
Final pass
🤖 Generated with Claude Code