feat: [SDK-5088] add device gesture that copies the push subscription ID to the clipboard - #2727
feat: [SDK-5088] add device gesture that copies the push subscription ID to the clipboard#2727nan-li wants to merge 5 commits into
Conversation
📊 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)78/81 touched executable lines covered (96.3% — requires ≥ 80%) |
nan-li
changed the title
60c2bf2 to
d06b7f7
Compare
87f6063 to
5fa9ed9
Compare
… ID to the clipboard Backgrounding and foregrounding the app 6 times within 30 seconds copies the push subscription ID to the clipboard, ready to paste into the dashboard. The clip is os: <id>. The short prefix marks the value as a OneSignal ID for the dashboard's paste target and for anyone who copied it by accident. Cycles are counted on a monotonic clock. A cycle needs a real background phase of at least 250ms, which filters the synthetic rotation unfocus/focus pair from ApplicationService.onOrientationChanged, and the 30s sliding window is the only rate rule. Each counted cycle logs at verbose so manual testing can watch progress. The gesture skips when privacy consent is withheld or the push subscription does not exist yet, and adding sdk_device_gesture_disabled to an app's enabled feature keys turns it off remotely. The raw ConfigModel.sdkRemoteFeatureFlags list is checked instead of IFeatureManager because the KMP catalog hides unregistered keys. CoreModule.register moved its misconfigured-fallback block into a helper to stay under detekt's LongMethod cap after the new registration.
DeviceGestureDetector now takes the IObservabilityEventRecorder from the container and records ObservabilityEvent.DEVICE_GESTURE each time the gesture is recognised. gesture.result is copied, no_id or disabled, and a copied event also carries gesture.push_subscription_id, the value that went on the clipboard. The copied result is recorded from the main-thread block after the clip is set, so it never overstates. A gesture under withheld privacy consent records nothing, because the event ships to the backend. Tests drive the detector with a recorder spy and pin the attribute names and values, since the log backend is queried by them.
The gesture is going into the public docs, so a person who performs it and gets nothing cannot tell whether the SDK missed it or had no ID to give. Without a push subscription the clip is now "os: no subscription ID yet", which says which one it was. The event result stays no_id and is recorded after the clip is set, like copied. Withheld consent, the remote kill switch and a gesture that was not recognised still copy nothing.
The detector scanned the raw fetched flag list because the kill switch key had no catalog entry. Now that it has one, DeviceGestureDetector takes IFeatureManager and asks it for SDK_DEVICE_GESTURE_DISABLED like every other flag, which also drops the hand-rolled case handling. The switch still reads present-means-off, and absent still means on.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Multi-model review
Models: Claude Opus 5, GPT 5.6 Sol, Cursor Grok 4.6.
Act on (3/3): monotonicMillis defaults to SystemClock.uptimeMillis(), which freezes in deep sleep. The 30s window and 250ms dwell are specified as elapsed time. Six cycles can accumulate across a doze, and a real lock-screen background can shrink below 250ms of uptime and get dropped as a rotation blip. elapsedRealtime() is still monotonic and matches the rules.
Consider
- (Opus) No minimum gesture duration. Six ≥250ms backgrounds can fire in a couple of seconds; Recents / Custom Tabs / OAuth hops can hit it. The KDoc’s “sustained five-second round trips” is the slowest qualifying pace, not a floor.
- (Opus+Grok, Sol related)
setPrimaryClipthenrecordGesture(copied|no_id)does not prove the clip landed (API 29+ can drop a no-focus write silently). The IO→Main hop insuspendifyOnMaincan lose foreground before the write. - (Grok) The 250ms rotation filter includes other sync work on
ApplicationService.onOrientationChanged(IAMWebViewManager.removeAllViewssits between unfocus and focus). Tests only inject a 1ms blip.
Noted: consent not re-checked after the async hop (Sol, narrow race); no local integrator off-switch (Opus, matches default-on design); EXTRA_IS_SENSITIVE does not suppress the Android 13 preview (Opus); logging while holding the detector lock (Opus).
Dismissed: unmerged KMP pin (already a merge gate); registerMisconfiguredFallbacks extract (behavior-preserving).
Sent by Cursor Automation: PR Reviews
| * reflection-based resolver still picks the only constructor (see the class KDoc on | ||
| * [com.onesignal.core.internal.config.impl.FeatureFlagsRefreshService]). | ||
| */ | ||
| internal var monotonicMillis: () -> Long = { SystemClock.uptimeMillis() } |
There was a problem hiding this comment.
Act on (3/3): uptimeMillis() pauses in deep sleep, so WINDOW_MS and MIN_BACKGROUND_DWELL_MS are uptime, not elapsed time. Six cycles can accumulate across a doze, and a lock-screen background can look like a <250ms rotation blip.
Use SystemClock.elapsedRealtime() — still monotonic / immune to NTP, and it matches the 30s / 250ms rules.
There was a problem hiding this comment.
Switched to elapsedRealtime() in 03042cd, and iOS moved to a sleep-inclusive clock in the same round, so the 30-second window means real time on both platforms.
| Logging.warn("DeviceGestureDetector: clipboard service unavailable, nothing copied") | ||
| } else { | ||
| // No EXTRA_IS_SENSITIVE: the Android 13+ copy preview is the person's confirmation. | ||
| clipboard.setPrimaryClip(ClipData.newPlainText(CLIP_LABEL, text)) |
There was a problem hiding this comment.
Consider (Opus+Grok; Sol on the hop): setPrimaryClip is void and can be a silent no-op on API 29+ if the app has lost focus. Recording copied/no_id here overstates the KDoc guarantee that a result never claims a clipboard change that did not happen.
suspendifyOnMain goes IO→Main, so the 6th focus can unfocus again before this runs. Read primaryClip back before recording, or dispatch while still on the focus path.
There was a problem hiding this comment.
The IO to Main hop is gone in 03042cd: the write now runs on the focus callback itself, so it happens while the app still has focus. We deliberately never read the clipboard back, since reads are what Android restricts and shows to the user; the write is a synchronous, guarded call, and nothing is recorded if it throws.
| // Cold start or first focus after start(); nothing to pair with. | ||
| backgroundedAt == null -> false | ||
| // Faster than any human app switch; rotation produces synthetic pairs like this. | ||
| now - backgroundedAt < MIN_BACKGROUND_DWELL_MS -> { |
There was a problem hiding this comment.
Consider (Grok): This dwell is wall-clock between onUnfocused and onFocus, not “time the app was backgrounded.” onOrientationChanged fires those back-to-back but with other sync handlers in between (WebViewManager.onActivityStopped → removeAllViews()). If that work exceeds 250ms, a configChanges rotation counts as a cycle. Tests only inject a 1ms blip.
There was a problem hiding this comment.
Left as is. The floor is a time filter by design, and there is no measurement of that in-between work exceeding 250 ms; if a rotation ever counts as a cycle it shows up as accidental copies in the event data, and the floor can be raised then.
…side the lock Three review findings and two wording fixes on the detector. The clock is now SystemClock.elapsedRealtime(), which keeps counting through deep sleep. uptimeMillis() stopped at every lock, so six brief visits spread over an afternoon could add up to one "30 second" window and copy the ID to a clipboard nobody asked about, and a lock that put the phone to sleep could read as a sub-250ms blip and be dropped. The window now means what the docs say. The clipboard write runs on the focus callback itself instead of hopping through the IO dispatcher and back to main. The callback already arrives on main and the clipboard needs no particular thread, so the hop only added a gap between recognising the gesture and writing. The write is guarded so a misbehaving clipboard service cannot take the lifecycle callback down, and the tests no longer need the dispatcher mocks. The verbose progress lines are built inside the detector lock and logged after it, since Logging calls app listeners synchronously. Wording: the class doc described the slowest qualifying pace as if it were a floor, and the recordGesture doc claimed no result is recorded without a clipboard change, which disabled contradicts. Tests add the 249ms versus 250ms dwell edge, the 30 second window edge, and the missing clipboard service path.
|
Replies on the three inline threads cover the clock, the clipboard write and the rotation filter. On the rest of the review:
|
1 participant
Description
One Line Summary
Six background/foreground cycles within 30 seconds copy the push subscription ID to the clipboard, and each recognised gesture records an observability event.
Details
Motivation
Finding your own device in the dashboard is the slowest part of sending a first test push. Today that means digging a subscription ID out of verbose logs or searching by external ID. With this, anyone can background and foreground the app six times within 30 seconds on a production build and the SDK copies the push subscription ID to the clipboard, ready to paste into the dashboard's subscription ID search. No app changes, no new permissions.
Ticket SDK-5088. Stacked on #2732 (SDK-5156), which this PR targets. GitHub retargets it to
mainonce that merges. The catalog entry it reads arrives through #2732's KMP pin (OneSignal-KMP-SDK#24); this PR does not move the submodule.Scope
DeviceGestureDetectorcounts background-to-foreground pairs on a monotonic clock that keeps counting through deep sleep, so the window means real time and brief visits spread over a day cannot add up to one. A cycle needs a background phase of at least 250ms, which filters the synthetic unfocus/focus pair that rotation fires when an activity handles orientation itself. The 30 second window is the only rate rule, so six cycles fit inside it at round trips of five seconds or faster, and rapid switching between two apps can reach it; the event below measures how often that happens. Each counted cycle logs at verbose, outside the detector lock. The clipboard write runs on the focus callback itself, with no dispatcher hop.The clip reads
os: <id>. The short prefix marks the value as a OneSignal ID for the dashboard's paste target and for anyone who copied it by accident. It is not marked sensitive, so the Android 13+ copy preview doubles as confirmation.Nothing is copied when privacy consent is withheld or when
sdk_device_gesture_disabledis on for the app. That key is an inverted kill switch in the KMP catalog (FeatureFlag.SDK_DEVICE_GESTURE_DISABLED), read throughIFeatureManagerlike every other flag, so a device that has never fetched flags still has the gesture and a customer can be opted out per app. Without a push subscription yet the clip isos: no subscription ID yetinstead, so someone following the docs can tell the gesture worked.Each recognised gesture also records
ObservabilityEvent.DEVICE_GESTUREthrough the recorder from #2732, so we can see how often the gesture is used without the gesture making a request of its own.gesture.resultiscopied,no_idordisabled. A copied event also carriesgesture.push_subscription_id, the value that went on the clipboard.copiedandno_idare recorded after the clip is set. Nothing is recorded under withheld consent. No event ships untilsdk_event_device_gestureis on for the app.CoreModule.registermoved its misconfigured-fallback registrations into a helper to stay under detekt's method length cap. No behavior change.Other
The dashboard paste target that recognizes the clip and extracts the ID is separate work. Until then the person pastes the ID portion by hand.
Testing
Unit testing
DeviceGestureDetectorTests, 21 tests, drives synthetic focus/unfocus sequences against a fake clock and reads back the Robolectric clipboard; the write is synchronous, so no dispatcher mocks are involved. Sixteen cover the counting rules, both edges of the 250ms floor and of the 30 second window, every skip condition and a missing clipboard service, with the feature manager as a strict mock that answers only the kill switch flag. Five use a recorder spy and pin the exact attribute names and values, since the log queries match on them.LoggerLifecycleManagerTest,LoggerLifecycleManagerFaultTestandspotlessCheckpass on the rebased branch.Manual testing
Run on an Android 14 emulator with
sdk_event_device_gestureforced on through the local feature override: the gesture copied the ID and the event arrived in Cloud Logging with the expected attributes. Not yet run on a physical device. The iOS detection was hand-tested on the simulator, which is what settled the timing on a single window rule.Affected code checklist
Checklist
Overview
Testing
Final pass