Conversation
Fix PIFlushWorkInstrumentationTest compile error.
* Inbox v2 SDK-5709 (#995) * task(SDK-5709): add Inbox V2 constants (T0.1) Adds the 5 shared constants (fetch type, response keys, prefs key, throttle window) that every Inbox V2 task reads from, so literal strings stay in one place and can't silently drift across files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add sendInboxFetch and sendInboxDelete to CtApi (T0.2) Routes the V2 inbox to its dedicated endpoint so fetch/delete don't get batched into /a1 alongside regular events. Also teaches getUriForPath to split multi-segment relativeUrl on '/' before appending so paths like inbox/v2/getMessages aren't URL-encoded into a single %2F-laden segment; single-segment callers are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): parse isRead from V2 inbox JSON in CTMessageDAO (T0.3) Reads the new isRead boolean from V2 responses so cross-device read state survives into the DAO. Missing field defaults to unread, preserving V1 behaviour unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add CallResult sealed class for V2 network calls (DF1) Unifies the five possible outcomes of any single-shot V2 network call (success, throttled, disabled, HTTP error, transport failure) so callers pattern-match once and the compiler enforces exhaustive handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add EndpointCall interface for V2 network calls (DF2) Defines the single-method contract that every V2 endpoint implements, so orchestration, error mapping, and testing all plug into one shape regardless of whether the call is a fetch, delete, or future event push. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add NetworkScope coroutine owner per SDK instance (DF3) Introduces one CoroutineScope per CleverTap instance, backed by a SupervisorJob and an injectable dispatcher (default Dispatchers.IO), so V2 network coroutines can be launched with structured lifetimes and isolated failures without blocking the existing CTExecutorFactory threads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add EventRequestBody helper for V2 call payloads (DF4) Centralizes the [header, event] JSON array layout every direct V2 call needs, so each EndpointCall builds only its own event object and never hand-rolls the surrounding array. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add FetchThrottle generic per-account rate limiter (DF5) Adds a reusable persistent throttle keyed by account id and pref name, so V2 fetch callers (public fetchInbox and pull-to-refresh) enforce the 5-min window across app restarts without baking feature-specific state into the class. Uses the existing Clock interface and TestClock for determinism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add InboxFetchCall — first V2 EndpointCall implementor (DF6) Wires the V2 inbox fetch end-to-end: builds the wzrk_fetch event, wraps it in EventRequestBody, hits sendInboxFetch on the injected dispatcher, and maps each HTTP outcome to a CallResult (200→Success, 403→Disabled, else →HttpError, empty/parse/IO→NetworkFailure). Sets the per-endpoint error matrix pattern every future EndpointCall will follow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add InboxV2Merger pure dual-filter functions (T1.4b) Extracts the V2 response merge math into a zero-dependency Kotlin object so the controller method can orchestrate DB and lock while the filter logic stays unit-testable with no mocks. Single predicate definition is shared by the pre-write and post-read passes so they can't drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add processV2Response orchestration to CTInboxController (T1.4) Threads the InboxV2Merger dual-filter through the controller's DB and lock: pre-write filter → upsert → re-read under messagesLock → post-read cleanup → batch delete of stale rows → in-memory swap. Returns whether anything changed so the response handler can decide when to fire the UI callback, matching the V1 updateMessages contract. Pending sets are empty today; T3.3 wires them to the pending_deletes/pending_reads tables. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add InboxV2Response mirroring V1 guards (T1.4a) Routes V2 fetch payloads through the same four safety checks V1 uses (analytics-only, key presence, parse try/catch, null-controller init) so the active-fetch path can't bypass guards the passive path enforces. Bridges to CTInboxController.processV2Response and fires inboxMessagesDidUpdate on real changes. Opens the minimum accessors the cross-package response handler needs: CTMessageDAO.initWithJSON becomes public and CTInboxController gains a getUserId() getter. Both classes remain @RestrictTo(LIBRARY) so consumer visibility is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add InboxV2Fetcher orchestrator (DF7) Puts the inbox-specific glue between the generic EndpointCall and the response handler: session-scoped disable flag, throttle gate (honoured for user-initiated triggers, bypassed for app-launch and onUserLogin), recordFetch on the allowed path, and hand-off to InboxV2Response on success so the lock, controller-init, and UI callback stay where T1.4a owns them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add InboxV2Bridge Java-interop adapter and wire the V2 chain (DF8) Lets Java callers fire-and-forget an Inbox V2 fetch: the bridge launches the suspend fetcher on NetworkScope and delivers success/failure to an optional FetchInboxCallback. The factory now constructs the full InboxV2Response → InboxFetchCall → FetchThrottle → InboxV2Fetcher → InboxV2Bridge chain once per SDK instance and exposes the bridge through CoreState so the trigger wirings (T1.5/T1.6/T1.7) become one-liners. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): fetch Inbox V2 on cold app launch (T1.5) Drops a single bridge.submit(false, null) into ActivityLifeCycleManager's cold-launch block so the inbox is actively pulled once per app launch without a throttle. The factory now constructs the V2 chain before the lifecycle manager so the bridge is ready at wire time; activityLifeCycle- Manager and loginController keep their original relative order. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): fetch Inbox V2 after onUserLogin identity switch (T1.6) Hooks InboxV2Bridge.submit(false, null) into asyncProfileSwitchUser right after resetInbox() so the new identity's inbox loads immediately from the server instead of waiting on the next /a1 push. Throttle is bypassed — every identity switch is a fresh fetch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): expose public fetchInbox() API on CleverTapAPI (T1.7) Gives customers with custom inbox UIs a way to force an on-demand refresh from the CleverTap servers. Two overloads (no-arg and callback-taking) route through InboxV2Bridge.submit with respectThrottle=true, so rapid user-initiated calls can't spam the endpoint. Null inbox controller is handled defensively: the callback fires with false, no network call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): pull-to-refresh on built-in inbox via V2 fetch (T1.9) Wraps CTInboxListViewFragment's list in a SwipeRefreshLayout and routes the refresh gesture through the public fetchInbox API so the throttle applies and existing inboxMessagesDidUpdate path drives the adapter refresh. The outer wrapper delegates canChildScrollUp to whichever RecyclerView is actually on screen — mid-scroll upward drags scroll the list instead of falsely firing a refresh, which the default mTarget.canScrollVertically(-1) check on a LinearLayout target would otherwise do. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add message _id to Viewed and Clicked inbox events (T2.1) Inserts data.getMessageId() under evtData._id in pushInboxMessageStateEvent so backend can map Viewed/Clicked events to a specific inbox message and update server-side isRead. Without this field, cross-device read-state sync has no way to land. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): suppress rapid repeat inbox events on the same device (T2.2) Introduces a generic per-key sliding-window EventSuppressor backed by ConcurrentHashMap.put so two suppressors (Viewed 2s, Clicked 5s) can gate pushInboxMessageStateEvent without new locks. Closes the public-API dedup gap: customers building custom inboxes can't inflate analytics by firing Viewed on every scroll-into-view or double-tapping a message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): skip Viewed events for already-read inbox messages (T2.3) Adds an early-return guard at the top of pushInboxMessageStateEvent: if the message is already read (V2 delivered isRead=true from another device), suppress the Viewed event to avoid cross-device duplicate view counts. Clicked semantics are unchanged — distinct taps on separate devices remain distinct engagement events. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): add pending-actions tables and DAO for offline resilience (T3.1) Adds two SQLite tables — inbox_pending_deletes and inbox_pending_reads — keyed by composite (USER_ID, ID) so a local delete/read intent survives app kill and a later V2 fetch can't resurrect a deleted message or overwrite a locally-read message until the server confirms the action. The DB migrates from v6 to v7 additively (no alteration of inboxMessages). The DAO mirrors existing conventions: belowMemThreshold guard, transaction- wrapped batch insert with CONFLICT_IGNORE, parameterized IN batch delete, cursor.use for reads, SQLiteException catches on writes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): wire pending-reads into V2 fetch path (T3.3) Records a row in inbox_pending_reads whenever the user marks a message read, so a V2 fetch with a stale isRead=false can't un-read the message across app restart. processV2Response now reads both pending sets for real (replacing the T1.4 empty-set placeholder) and, once a server echo confirms a pending read, batch-removes the row. The cleanup runs BEFORE preWriteFilter because the merger's override mutates incoming DAOs in place and would otherwise make every pending id look server-confirmed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): sync inbox deletes to the server with retry on init (T3.2) Adds an action-only EndpointCall<Unit> (InboxDeleteCall) and a coordinator that batches N deletes into one HTTP call, clears inbox_pending_deletes rows atomically on 2xx, and drains leftover pending rows at inbox-init time so a delete that failed offline eventually lands. Local intent is recorded before the server call so an app kill mid-sync can't lose it. Three literals are captured behind working assumptions, each flippable with a one-line edit once backend confirms: delete URL path, event name ("Message Deleted"), and the messages array container key. DBAdapter is fetched via a supplier so the coordinator's first DB load happens on the NetworkScope's IO dispatcher instead of the factory's main-thread construction path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): make FetchThrottle in-memory (T4.1) Cross-session persistence was redundant. Every process start runs an un-throttled V2 fetch (cold launch and onUserLogin submit with respectThrottle=false), and InboxV2Fetcher calls recordFetch() before the endpoint runs regardless of the caller. The stored timestamp is therefore overwritten before any throttled caller can read it, so a per-instance AtomicLong yields identical observable behavior without the disk I/O, Context, config, and prefKey plumbing. Drops INBOX_V2_LAST_FETCH_TS_KEY, removes Robolectric from FetchThrottleTest, and simplifies construction in CleverTapFactory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): tag inbox messages with V1/V2 source (T5.1) V1 and V2 inbox messages live in the same `inboxMessages` table but have divergent backend contracts: V2 events must carry `_id` and V2 deletes must sync to the v2 endpoint, while V1 must do neither. The new `InboxMessageSource` discriminator persists per-message on the DAO and a `source TEXT NOT NULL DEFAULT 'V1'` column (folded into the existing v7 migration — no version bump, branch is unreleased). `InboxResponse` tags parsed DAOs V1, `InboxV2Response` tags V2; `CTInboxController` exposes an `isV2Message(id)` helper so the gate sites in T5.2 / T5.3 can branch without reading source off the public `CTInboxMessage`. `CTInboxMessage` is intentionally untouched and `CTMessageDAO.toJSON()` does not include source, so the V1/V2 tag never reaches the public model via `getData()`. A regression test in `CTMessageDAOTest` asserts the JSON stays clean. No behavioral change yet; T5.2 and T5.3 consume the tag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): include message _id only for V2 inbox events (T5.2) Backend rejects `_id` on Notification Viewed / Clicked events for V1 inbox messages. Gate the `_id` put in `AnalyticsManager.pushInboxMessageStateEvent()` on the T5.1 source tag via a private `isV2InboxMessage(msgId)` helper that consults `CTInboxController.isV2Message`. Null controller, null message id, and unknown ids all fall through to V1 behavior (no `_id` emitted). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): gate delete sync and pending tables on V2 source (T5.3) Backend does not handle V1 deletes, and V1 inbox messages have no server-side state to sync. Four sites in `CTInboxController` now gate on the T5.1 source tag: - `deleteInboxMessage` / `deleteInboxMessagesForIDs` only add to `inbox_pending_deletes` and invoke `InboxDeleteCoordinator.syncDelete` for V2 messages. V1 paths are local-only. - `_markReadForMessageWithId` / `_markReadForMessagesWithIds` only add to `inbox_pending_reads` for V2 messages. V1 markRead stays local. Batch paths (`deleteInboxMessagesForIDs`, `_markReadForMessagesWithIds`) iterate `messages` once under a single `messagesLock` with an id-set membership check — O(N+M) instead of O(N*M) per-id scans. Single-item paths read source directly off the DAO returned by `findMessageById`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): QA polish — V2 event metadata fields and sync controller init - Stamp standard event metadata (s, pg, ep, f, lsl, pai, n) onto every V2 endpoint event body via a new buildInboxV2Event helper, so the V2 fetch and Message Deleted events match the shape of regular events the server expects. - Add ControllerManager.initializeInboxSync() and have InboxV2Response call it instead of the async initializeInbox(), so the very first V2 response on cold launch / post-login is no longer silently dropped while the controller is still initialising. - Verbose logging across the V2 call, fetcher, and coordinator paths to make the QA flow auditable. - Tests updated/added for all of the above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): rename V2 inbox event id key from _id to wzrk_mid Backend updated the contract for V2 inbox Notification Viewed and Notification Clicked events: the message identifier now travels under the key wzrk_mid instead of _id. Plain key rename — V1 messages still emit no id field; V2 gating in isV2InboxMessage() unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): align V2 inbox delete with confirmed backend contract Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): persist full delete payload per pending row (T6.1) When a delete-sync fails, retryPending re-sends the deletes on the next inbox init. The retry payload was id-only — the local message was gone, so wzrk_id / wzrk_pivot / etc. weren't carried, and backend couldn't correlate the retried delete with the original campaign attribution. Adds a wzrkParams TEXT column to inbox_pending_deletes (folded into the unreleased v7 migration; no DATABASE_VERSION bump). The DAO now exposes two reads — getPendingDeleteIds(userId) for the merger's set-membership filter and getPendingDeletes(userId) returning a typed List<PendingDelete> for retry reconstruction. Single and batch insert methods take a wzrkParams JSONObject? alongside the id; addPendingDeletes operates on List<PendingDelete> so each row carries its own params. CTInboxController forwards dao.getWzrkParams() at delete time on both the single-message and batch-IDs paths, all within the existing single pass under messagesLock so source resolution still happens before the local cache wipe. InboxDeleteCoordinator.retryPending rebuilds each CTInboxMessage with id + wzrkParams so the retry POST body matches the initial sync body byte-for-byte. The merger is unchanged — it keeps reading getPendingDeleteIds for filtering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): two-phase pending delete with TTL-driven cleanup (T6.2) Backend confirmed in QA that the V2 delete API has a 1-2 hour delete-propagation window: a 200 from /inbox/v2/deleteMessages only acknowledges the request was accepted; the message can still come back in V2 fetch responses for a couple of hours, and absence from a fetch is also non-authoritative. Treating 200 as "done" therefore lets a follow-up fetch resurrect the locally-deleted message into cache. Promotes inbox_pending_deletes through a two-state machine. New rows default to PENDING_SEND. On HTTP 200 the coordinator transitions the targeted rows to AWAITING_CONFIRM via markPendingDeletesAwaitingConfirm instead of deleting them, so InboxV2Merger keeps filtering the message out of incoming fetches via getPendingDeleteIds (both states). retryPending reads only PENDING_SEND rows. Final cleanup is TTL-driven: each pending row stores the message's wzrk_ttl as expires (folded into the v7 schema, no DATABASE_VERSION bump). When a V2 message lacks a usable TTL, CTInboxController falls back to now + 1 day via resolvePendingDeleteExpiry — comfortably past the propagation window without lingering forever. removeExpiredAwaitingConfirm sweeps state=AWAITING_CONFIRM AND expires<=now; it runs at the top of processV2Response (before reading the pending-delete id set so an expired row's id stops filtering the now-stale message) and at the start of retryPending (so devices that rarely fetch still self-clean). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): wire InboxV2Response into /a1 decorator chain (T7.1) Live-behaviour V2 inbox campaigns deliver inbox_notifs_v2 inside the /a1 event response. The decorator chain only knew about the V1 InboxResponse, so V2 payloads riding /a1 were silently dropped and live campaigns never reached the controller. InboxV2Response now extends CleverTapResponseDecorator with a 3-arg override that delegates to the existing single-arg processResponse, so the same parser handles both the direct fetch path and the /a1 decorator path. CleverTapFactory builds the instance once and splices it into cleverTapResponses next to InboxResponse; the V2 fetch pipeline keeps using the same instance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): TTL-driven cleanup of pending_reads (T7.2) A pending_read row is cleared by the existing server-caught-up loop inside processV2Response — but only when the server eventually echoes the message back with isRead=1. Several paths starve that echo: user deletes the message after marking read, message TTL elapses on the backend, campaign retargeted, identity switch, etc. The row strands forever, overriding isRead on any future incoming message that happens to share the id. Mirroring the T6.2 pattern for pending_deletes: - inbox_pending_reads gains an `expires` column (folded into v7, no DATABASE_VERSION bump on this unreleased branch). - Captures wzrk_ttl at markRead time via the shared resolvePendingActionExpiry helper (renamed from resolvePendingDeleteExpiry now that both paths use it). 1-day fallback when the DAO has no usable TTL. - DAO grows addPendingRead(messageId, userId, expiresAt), batch addPendingReads(rows, userId), and removeExpiredPendingReads sweep. DBAdapter facade updated. - processV2Response sweeps expired pending_reads next to the existing AWAITING_CONFIRM sweep, before reading the pending sets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): use controller-fresh DAO for inbox Viewed event CTInboxBaseMessageViewHolder.markItemAsRead synchronously mutates inboxMessage.setRead(true) on the UI thread before the async messageDidShow body runs. By the time pushInboxMessageStateEvent inspects data.isRead(), the UI mirror already reads true, which trips the T2.3 cross-device gate (skip Viewed if isRead) and drops the legitimate first Viewed event for the local mark. Switch the analytics path to use the freshly-fetched `message` from getInboxMessageForId (controller-backed, still read=false) and fire the event before markReadInboxMessage flips the controller state. Same external behavior, but the gate now sees the correct pre-mark state and the Viewed event reliably emits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * task(SDK-5709): cross-device delete sync via index_state sweep (T7.3) Adds index_state column (PENDING_INDEXING | INDEXED) to inboxMessages. After each V2 fetch, incoming ids are bulk-promoted to INDEXED; V2 messages that are INDEXED (or stale PENDING_INDEXING older than the 6 h grace window) but absent from the fetch response are swept locally as cross-device deletes. Fresh PENDING_INDEXING rows are never swept, preventing false-positive removal of live-behaviour messages not yet indexed by the fetch backend. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * task(SDK-5709): wire FETCH sweep into processV2Response (T7.3 complete) Completes the cross-device delete sync feature started in the previous T7.3 commit (which added the DAO layer). Adds the controller-level sweep logic that runs on every V2 fetch response to remove locally-cached V2 messages that have been deleted on another device. Key changes: - InboxV2DeliverySource enum (FETCH | A1) disambiguates how a V2 payload arrived; the sweep is FETCH-only because only a fetch is a complete authoritative inbox snapshot. - InboxMessageDAO/Impl: findSweepableV2Ids(userId, staleCutoff) returns ids of INDEXED V2 rows and stale PENDING_INDEXING rows (older than the 6 h grace window) — both treated as reliable cross-device delete signals. - DBAdapter: wrappers for markIndexed and findSweepableV2Ids. - CTInboxController.processV2Response gains an InboxV2DeliverySource param and INDEXING_GRACE_SECONDS = 6 h constant. FETCH path: (1) bulk-promotes incoming ids to INDEXED, (2) sweeps absent sweepable ids from DB before postReadCleanup re-reads the full table. - InboxV2Response routes FETCH vs A1 source through to the controller. - 16 new tests across three test classes (DAO, controller, response). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(SDK-5709): infinite-TTL guard, sweep verbose logs, and PBS INDEXED fix - Infinite TTL (ttl=0): resolvePendingActionExpiry now returns 0 (never expires) when the message TTL is 0, preventing premature expiry of fire-and-forget LBS non-persistent messages. DAO queries for removeExpiredAwaitingConfirm and removeExpiredPendingReads now include `expires > 0` so infinite-TTL rows are never purged. findSweepableV2Ids adds `expires != 0` to exclude infinite-TTL messages from cross-device sweep. - Sweep verbose logging: processV2Response now emits verbose logcat lines for markIndexed, sweep removals (or nothing-to-remove), and expired AWAITING_CONFIRM / pending-read row cleanup, enabling dev QA verification without a debugger. - PBS INDEXED bug fix: brand-new PBS rows delivered for the first time via FETCH were stored as PENDING_INDEXING because markIndexed ran before the row existed in the DB. processV2Response(FETCH) now stamps all toUpsert DAOs with indexState=INDEXED before calling upsertMessages, so the INSERT writes INDEXED directly. The ON CONFLICT clause intentionally omits index_state, so existing INDEXED rows are unaffected. Fixes PP-001 and PP-006 (cross-device sweep was blind until the second FETCH). Tests added: infinite-TTL guards in InboxMessageDAOImplTest, InboxPendingActionsDAOImplTest, and CTInboxControllerTest; regression test for the PBS INDEXED fix in CTInboxControllerTest. * fix(SDK-5709): record throttle only on Success/HttpError, not NetworkFailure A transport failure (timeout, no network) no longer stamps the 5-minute throttle window, so a pull-to-refresh or fetchInbox() retry is never silently swallowed after a failure where no server contact was made. HttpError still records the fetch — the server responded, so the throttle applies. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(SDK-5709): fix linting * fix(SDK-5709): dispatch fetchInbox failure callback on network thread The not-initialized early-exit in fetchInbox() was calling the callback synchronously on the caller's thread, violating the documented contract that callbacks fire on the SDK's network dispatcher. Routes it through InboxV2Bridge.submitFailure() so all callback paths use the same thread. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(SDK-5709): make EndpointCall a fun interface Single-abstract-method contract is now explicit in the type. Verified that fun interface with a suspend abstract method compiles on Kotlin 2.0.10. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(SDK-5709): remove dead networkScope and inboxDeleteCoordinator from CoreState Both fields were stored on CoreState but never accessed via it — each consumer already held its own injected reference (InboxV2Bridge and InboxDeleteCoordinator via constructor params, ControllerManager via setter). Removing the redundant fields keeps CoreState as a registry of things actually needed through it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * task(SDK-5709): inject Clock into CTInboxController for testable time Replace raw System.currentTimeMillis() calls with Clock.currentTimeSeconds() via constructor injection; existing callers get Clock.SYSTEM through the delegating constructor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(SDK-5709): replace Logger with ILogger * fix(SDK-5709): fix failing tests * task(SDK-5709): replace AtomicLong with @volatile for FetchThrottle timestamp Plain read in shouldThrottle and plain write in recordFetch need only volatile visibility, not atomic compound ops — @volatile is sufficient. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(SDK-5709): add DBAdapter V2 method tests and InboxV2Bridge submitFailure tests. Cover markIndexed, findSweepableV2Ids, all pending-delete/read CRUD methods, TTL-expiry sweeps, batch operations, and null/empty guards in DBAdapterTest. Add submitFailure happy-path and scope-cancellation tests in InboxV2BridgeTest. * fix(SDK-5709): when new user logs in, call fetch after arp is received * fix(SDK-5709): throttle only user-initiated inbox fetches; introduce FetchTrigger enum System calls (app-launch, onUserLogin) no longer record the throttle timestamp, so they cannot block a subsequent pull-to-refresh or fetchInbox() call. Only USER_INITIATED calls both check and record the throttle. Replaces the boolean respectThrottle flag with an enum class FetchTrigger for type-safe, self-documenting call sites. * inbox v2 pull-to-refresh disabled on 403 and restore V1 viewed event behaviour SDK-5709 (#1007) * fix(SDK-5709): hide pull-to-refresh on V2 403 and restore V1 viewed event behaviour Two behavioural fixes for the Inbox V2 rollout: 1. Pull-to-refresh (SwipeRefreshLayout) is now hidden — both eagerly on fragment creation and after the first fetch that returns 403 — so the gesture affordance disappears once the V2 endpoint is session-disabled. Exposes disabledForSession through InboxV2Fetcher → InboxV2Bridge → CleverTapAPI (@RestrictTo LIBRARY) for the fragment to query. 2. The cross-device read gate ("skip Viewed if isRead=true") is now guarded by isV2InboxMessage(), restoring pre-feature behaviour for V1 messages. Clients upgrading with V2 disabled (403) continue to receive Viewed events for already-read V1 messages, avoiding a silent regression. * fix(SDK-5709): disable swipe gesture only; setVisibility(GONE) hid the message list * fix(SDK-5709): reset throttle window on user login and consider non 200 status code as disable fetch * fix(SDK-5709): replace activity context with application context as per PR comment --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> * docs changes and version bump SDK-5709 (#1005) * chore(SDK-5709): update docs System calls (app-launch, onUserLogin) no longer record the throttle timestamp, so they cannot block a subsequent pull-to-refresh or fetchInbox() call. Only USER_INITIATED calls both check and record the throttle. Replaces the boolean respectThrottle flag with an enum class FetchTrigger for type-safe, self-documenting call sites. * chore(SDK-5709): improve docs System calls (app-launch, onUserLogin) no longer record the throttle timestamp, so they cannot block a subsequent pull-to-refresh or fetchInbox() call. Only USER_INITIATED calls both check and record the throttle. Replaces the boolean respectThrottle flag with an enum class FetchTrigger for type-safe, self-documenting call sites. * chore(SDK-5709): bump version and run copyTemplates System calls (app-launch, onUserLogin) no longer record the throttle timestamp, so they cannot block a subsequent pull-to-refresh or fetchInbox() call. Only USER_INITIATED calls both check and record the throttle. Replaces the boolean respectThrottle flag with an enum class FetchTrigger for type-safe, self-documenting call sites. * chore(SDK-5709): update CTCORECHANGELOG.md regarding accounts not using v2 and uses 8.2.0 * chore(SDK-5709): update release date * chore(SDK-5709): update sample app core version --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…on CleverTapAPI (#999) * feat(SDK-5763): add DisplayUnitCache interface + setter on CleverTapAPI Extracts the public surface of CTDisplayUnitController into a new DisplayUnitCache interface and exposes CleverTapAPI.setDisplayUnitCache() so external SDKs (e.g. Native Display) can install their own cache implementation. Default behaviour preserved when the setter is not called — the SDK lazily installs CTDisplayUnitController on first adUnit_notifs response. - displayunits/DisplayUnitCache.java: new public interface - displayunits/CTDisplayUnitController.java: implements DisplayUnitCache - ControllerManager.java: holds DisplayUnitCache reference; new getDisplayUnitCache()/setDisplayUnitCache(); deprecated old getters - CleverTapAPI.java: setDisplayUnitCache() public; lookup methods route via interface - AnalyticsManager, DisplayUnitResponse, LoginController: route via getDisplayUnitCache() - AnalyticsManagerTest: mock points updated to displayUnitCache * SDK-5763: add pushDisplayUnitElementClickedEventForID(unitID, elementID, additionalProperties) Adds a dedicated Core SDK method for tracking element-level clicks within a Native Display unit, separate from the existing unit-level pushDisplayUnitClickedEventForID. The new method: - Carries the campaign's wzrk_* fields from the cached unit JSON (same enrichment as the unit-level method). - Adds wzrk_element_id = elementID to evtData from the dedicated parameter. - Merges additionalProperties into evtData after wzrk_* enrichment. Keys starting with wzrk_ are stripped defensively — the prefix is reserved for server-controlled attribution. The existing pushDisplayUnitClickedEventForID(unitId) stays untouched for unit-level clicks (graceful degradation for older ND SDK clients). Why a new method (vs an overload of the existing one): - Method name encodes the semantic — readers/dashboard query authors don't need tribal knowledge to interpret "click + additionalProperties". - ND SDK PR #32 wires its reflective probe to this new signature; old Core SDK callers continue to call the legacy single-arg method unchanged. No silent attribution semantics drift. Tests in AnalyticsManagerTest cover: - Happy path: elementID lands as wzrk_element_id; additionalProperties merge as first-class evtData fields. - wzrk_* keys in additionalProperties are filtered out. - coreMetaData.setWzrkParams (which feeds wzrk_ref batch headers) receives only wzrk_* keys — caller-supplied non-wzrk extras don't ride along on subsequent batched events. - displayUnitCache null short-circuit preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * SDK-5763: type updateDisplayUnits with parsed models, hoist parsing to response Refactor the DisplayUnitCache interface so updateDisplayUnits takes List<CleverTapDisplayUnit> instead of JSONArray and is void. The cache contract is now pure-model — JSON parsing lives in DisplayUnitResponse, not in cache implementations. - DisplayUnitCache: parameter is List<CleverTapDisplayUnit>; return removed (caller already has the list). - CTDisplayUnitController: resets and populates the items map from the supplied list, skipping null/empty-id entries. - DisplayUnitResponse: new parseDisplayUnitsFromJson(JSONArray) filters malformed entries (error field) and per-item JSONException, then hands the model list to the cache and notifies the listener. - CTDisplayUnitControllerTest: rewritten against the list-based API. Host-installed DisplayUnitCache implementations no longer need to know about the SDK's JSON shape; they only deal with model objects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * SDK-5763: address review — TOCTOU + cross-thread visibility for display-unit cache - ControllerManager.displayUnitCache is now volatile so writes from setDisplayUnitCache() (any thread, including a public-API caller outside the synchronized init block in DisplayUnitResponse) are safely visible to subsequent reads. - CleverTapAPI.getAllDisplayUnits() and getDisplayUnitForId() now capture the cache reference into a local variable once, then null-check and dereference that local. Closes the TOCTOU window where a concurrent setDisplayUnitCache(null) could let the second getDisplayUnitCache() return null after the guard had passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * SDK-5763: import DisplayUnitCache and use the short name throughout CleverTapAPI Adds the DisplayUnitCache import in CleverTapAPI.java, switches the setDisplayUnitCache(...) parameter to the short name, and replaces the fully-qualified DisplayUnitCache references inside getAllDisplayUnits() and getDisplayUnitForId() (introduced when the import wasn't yet present) with the short name as well. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: fixes test case. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughImplements App Inbox V2 (fetch/delete, throttling, DB schema v7, controller processing, delete coordinator, bridge, tests), introduces DisplayUnitCache and element-level click analytics, adds swipe-to-refresh in inbox UI, wires lifecycle/login triggers, updates docs/examples, and bumps SDK to 8.2.0. ChangesInbox V2 sync and Display Unit cache integration
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App/Fragment
participant API as CleverTapAPI
participant Bridge as InboxV2Bridge
participant Fetcher as InboxV2Fetcher
participant Endpoint as CtApi
participant Resp as InboxV2Response
participant DB as DBAdapter/DAOs
participant Cntrl as CTInboxController
App->>API: fetchInbox(callback)
API->>Bridge: submit(USER_INITIATED, callback)
Bridge->>Fetcher: fetch(USER_INITIATED)
Fetcher->>Endpoint: POST /inbox/v2/getMessages
Endpoint-->>Fetcher: 200 JSON
Fetcher->>Resp: processResponse(JSONObject)
Resp->>DB: upsert/sweep/update
Resp->>Cntrl: processV2Response(FETCH)
Fetcher-->>Bridge: Success
Bridge-->>App: callback(true)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clevertap-core/src/main/java/com/clevertap/android/sdk/db/CtDatabase.kt (1)
67-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard v1→v7 migration from duplicate-column ALTER.
When
oldVersion == 1, Line 73 recreatesinboxMessageswithsourceandindex_statealready present, then Line 107 and Line 108 try to add those columns again, which can fail upgrade with a duplicate-column SQL error.💡 Suggested fix
- if (oldVersion < 7) { + if (oldVersion < 7) { executeStatement(db, CREATE_INBOX_PENDING_DELETES_TABLE) executeStatement(db, CREATE_INBOX_PENDING_READS_TABLE) - executeStatement(db, ALTER_INBOX_MESSAGES_ADD_SOURCE) - executeStatement(db, ALTER_INBOX_MESSAGES_ADD_INDEX_STATE) + if (oldVersion != 1) { + executeStatement(db, ALTER_INBOX_MESSAGES_ADD_SOURCE) + executeStatement(db, ALTER_INBOX_MESSAGES_ADD_INDEX_STATE) + } }Also applies to: 104-109, 309-333
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/db/CtDatabase.kt` around lines 67 - 83, The migration for oldVersion == 1 recreates inboxMessages via CREATE_INBOX_MESSAGES_TABLE but later runs ALTER TABLE ADD COLUMN for "source" and "index_state", causing duplicate-column errors; update CtDatabase.kt to check whether those columns already exist before running the ALTERs (e.g. query PRAGMA table_info('inboxMessages') or similar helper) and only call executeStatement to add columns if they are absent; apply the same guard where the same ALTERs occur (the blocks referencing CREATE_INBOX_MESSAGES_TABLE, CREATE_INBOX_MESSAGES_TABLE-related ALTERs and the code areas around the other occurrences noted) so the v1→v7 path skips duplicate ADD COLUMN statements.clevertap-core/src/main/java/com/clevertap/android/sdk/response/DisplayUnitResponse.java (1)
85-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmpty payload should clear cache instead of returning early.
Line 85 currently returns for
messages.length() == 0, socache.updateDisplayUnits(...)and callback notification never run. If server sends[], stale display units can persist.💡 Proposed fix
- if (messages == null || messages.length() == 0) { + if (messages == null) { logger.verbose(config.getAccountId(), - Constants.FEATURE_DISPLAY_UNIT + "Can't parse Display Units, jsonArray is either empty or null"); + Constants.FEATURE_DISPLAY_UNIT + "Can't parse Display Units, jsonArray is null"); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/response/DisplayUnitResponse.java` around lines 85 - 89, The handler in DisplayUnitResponse currently returns early when messages is empty, leaving stale data; instead, detect messages == null versus messages.length()==0 and for the empty-array case call cache.updateDisplayUnits(...) with an empty list (or equivalent clear), invoke the display-units callback/notification path just as with a populated payload, and log the empty payload event via logger.verbose; keep the null check returning early but ensure messages.length()==0 triggers cache clearing and callback invocation so stale units are removed.
🧹 Nitpick comments (5)
clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxController.java (2)
601-605: 💤 Low valueClarify the negative TTL branch semantics.
The condition
ttl > 0L ? ttl : nowSeconds + PENDING_DELETE_DEFAULT_TTL_SECONDSapplies the default TTL whenttlis negative, which seems like an unexpected edge case. A brief comment explaining when this could occur (e.g., legacy data, parsing errors) would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxController.java` around lines 601 - 605, The negative-TTL branch in resolvePendingActionExpiry (method) is unclear: add a concise comment above the ternary that explains why dao.getExpires() might be negative (e.g., legacy/formatting/parsing errors or sentinel values) and that in such cases we intentionally apply PENDING_DELETE_DEFAULT_TTL_SECONDS to avoid immediate expiry; reference CTMessageDAO.getExpires() and the PENDING_DELETE_DEFAULT_TTL_SECONDS constant in the comment so future readers understand the fallback semantics.
141-171: ⚖️ Poor tradeoffPotential TOCTOU between V2 partitioning and the actual delete.
The code reads messages under
messagesLock(lines 147-158), then releases the lock to write pending deletes to DB (line 161), then acquiresinboxControllerLockto perform the delete (line 163). If another thread modifiesmessagesbetween these locks, the V2 partition could be stale.This is likely safe in practice because inbox operations run through the async executor, but the window exists. Consider documenting the serialization assumption or restructuring to hold the lock continuously.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxController.java` around lines 141 - 171, Partitioning V2 messages currently happens under messagesLock then releases it before writing pendingRows and calling _deleteMessagesForIds, allowing a TOCTOU; to fix, perform the V2 partitioning, pendingRows construction (resolvePendingActionExpiry), the dbAdapter.addPendingDeletes(...) call, and the _deleteMessagesForIds(messageIDs) invocation while holding the same higher-level inbox controller lock (ctLockManager.getInboxControllerLock()) so the sequence (v2Messages/pendingRows creation -> db write -> deletion -> callbackManager._notifyInboxMessagesDidUpdate()) is atomic; adjust synchronization so messagesLock work happens inside that controller lock or acquire controller lock first, then iterate messages to build v2Messages/pendingRows, call dbAdapter.addPendingDeletes, call _deleteMessagesForIds, and finally call inboxDeleteCoordinator.syncDelete if needed.clevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxPendingActionsDAO.kt (2)
167-174: 💤 Low valueReturn value check may incorrectly report failure for duplicate inserts.
insertWithOnConflictwithCONFLICT_IGNOREreturns-1when a constraint violation causes the insert to be skipped. The check>= 0will returnfalsein this case, but for idempotent semantics (the row already exists), this should arguably be considered success.If intentional (only report success for newly inserted rows), consider documenting this behavior. Otherwise, for true idempotency:
Proposed fix for idempotent semantics
return try { - dbHelper.writableDatabase.insertWithOnConflict( + val result = dbHelper.writableDatabase.insertWithOnConflict( Table.INBOX_PENDING_DELETES.tableName, null, cv, SQLiteDatabase.CONFLICT_IGNORE - ) >= 0 + ) + // -1 means either error or conflict-ignored; for idempotency, we only + // care that no exception was thrown. + true } catch (e: SQLiteException) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxPendingActionsDAO.kt` around lines 167 - 174, The current insert in InboxPendingActionsDAO uses dbHelper.writableDatabase.insertWithOnConflict(..., SQLiteDatabase.CONFLICT_IGNORE) and treats any -1 result as failure; change the success check to treat -1 (constraint-ignored duplicate) as success for idempotent semantics: capture the insertWithOnConflict return into a variable (e.g., result) and return true if result >= 0 OR result == -1; keep the SQLiteException catch/log using logger.verbose for Table.INBOX_PENDING_DELETES to preserve error handling.
193-200: 💤 Low valueSame return value consideration applies to
addPendingRead.This has the same
>= 0check pattern. If the idempotent semantics fix is applied toaddPendingDelete, apply it here too for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxPendingActionsDAO.kt` around lines 193 - 200, The addPendingRead method currently treats insert result as successful with ">= 0"—make it consistent with the idempotent semantics applied to addPendingDelete by changing the success check to use "> 0" instead; update the return expression in the insert block that references Table.INBOX_PENDING_READS.tableName so the method returns true only for a positive rowId while preserving the existing SQLiteException catch and logging.clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/EventSuppressor.kt (1)
24-30: ⚖️ Poor tradeoffMemory consideration: unbounded key growth.
The
ConcurrentHashMaphas no eviction mechanism. If unique keys accumulate over time (e.g., unique message IDs across sessions), the map will grow indefinitely. For inbox event suppression where keys are bounded by message count, this is likely acceptable. If this class is reused for other purposes with unbounded key spaces, consider adding TTL-based eviction or a size cap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/EventSuppressor.kt` around lines 24 - 30, The ConcurrentHashMap lastSeen used by shouldSuppress can grow unbounded for unbounded key spaces; update the implementation to prevent memory leak by adding eviction: either replace lastSeen with a bounded cache with TTL (e.g., use a ConcurrentLinkedHashMap/Guava Cache or an LRU with maximumSize and expireAfterWrite using the same windowMs) or implement periodic cleanup of entries older than windowMs when calling shouldSuppress; ensure symbols referenced are lastSeen, shouldSuppress, windowMs and preserve thread-safety for concurrent access.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java`:
- Around line 1114-1120: The code dereferences data via data.getMessageId()
before verifying data is non-null, which can throw an NPE; update the logic in
AnalyticsManager so you first check if data == null (or otherwise guard) and
return early, then call data.getMessageId(); ensure the null check happens
before using data in isV2InboxMessage(...) and before calling
config.getLogger().verbose(...) so all references to data (getMessageId,
isV2InboxMessage) occur only after the null guard.
In `@clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java`:
- Around line 2372-2374: The async path in messageDidShow calls
getInboxMessageForId(inboxMessage.getMessageId()) and immediately uses the
result (CTInboxMessage message) without null-checking, causing an NPE if the
message was removed; update the code in the messageDidShow handling to
null-check the returned CTInboxMessage from getInboxMessageForId(...) and if
it's null either return early (skip analytics) or fall back to using the
original inboxMessage for analytics, and add a small debug log mentioning the
missing messageId; reference CTInboxMessage, getInboxMessageForId(...),
inboxMessage, and messageDidShow when locating the fix.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxDeleteCall.kt`:
- Around line 53-61: The current when-block in InboxDeleteCall treats every
non-200 response as CallResult.Disabled which is incorrect; update the response
handling in InboxDeleteCall (the method processing the HTTP response) to return
CallResult.Success(Unit) for 200, return CallResult.Disabled only for the
explicit disable status (e.g., 403), and for all other non-200 statuses return a
CallResult.HttpError containing the response code (and optional body/message)
instead of flipping to Disabled; keep logger calls but log different messages
per branch (200, 403, other) so transient 5xx/429 responses are surfaced as
errors rather than disabling the V2 inbox for the session.
- Around line 45-47: The current execute() implementation in InboxDeleteCall
wraps work with withContext(dispatcher) and uses catch (e: Exception) which
swallows CancellationException; update the exception handling so
CancellationException is re-thrown and not converted to
CallResult.NetworkFailure. Concretely, in the execute() method surrounding
withContext(dispatcher) change the single catch to first detect/rethrow
CancellationException (either via a specific catch (e: CancellationException) {
throw e } or by checking if (e is CancellationException) throw e) and only then
return CallResult.NetworkFailure for other Exceptions, making the same change
for both catch sites that currently return CallResult.NetworkFailure.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxFetchCall.kt`:
- Around line 60-70: The current when(response.code) branch treats every non-200
response as CallResult.Disabled which incorrectly disables Inbox V2 on
transient/server errors; change the logic in InboxFetchCall.kt (the
response.code handling) to only return CallResult.Disabled for the explicit
disable status (403) and for all other non-200 codes return a
CallResult.HttpError (include the HTTP status and response body or message) so
failures remain observable; ensure you still read the body safely (like
response.readBody() ?: "") when constructing the HttpError to include useful
context.
- Around line 52-54: The catch blocks currently catch Exception (in the
withContext scope and the later block) which swallows CancellationException;
update each catch to rethrow CancellationException immediately (e.g., if (e is
CancellationException) throw e) and only convert other exceptions into
CallResult.NetworkFailure(e) so coroutine cancellation propagates correctly;
adjust the catch around withContext and the later catch (e: Exception) that
returns CallResult.NetworkFailure to follow this pattern, keeping references to
CallResult.NetworkFailure and the withContext scope.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/response/InboxV2Response.kt`:
- Around line 78-80: The verbose log prints the raw messages JSON (messages)
which can leak inbox content; update the logger call in InboxV2Response (around
the logger.verbose lines and before _processInboxMessages) to avoid logging full
payloads — log only safe metadata such as messages.length(), source, and
optionally a list of message IDs or counts per type, and remove or redact the
"$messages" interpolation so the raw JSON is never written to logs.
In
`@clevertap-core/src/test/java/com/clevertap/android/sdk/ActivityLifeCycleManagerTest.kt`:
- Around line 167-175: The test currently sets
coreState.coreMetaData.isAppLaunchPushed = true so both
activityLifeCycleManager.activityResumed(mockActivity) calls are suppressed and
the "submit once then suppress" path isn't exercised; change the setup to start
with isAppLaunchPushed = false (or remove that assignment), call
activityLifeCycleManager.activityResumed(mockActivity) twice, and then assert
inboxV2Bridge.submit was invoked exactly once to verify the first call triggers
submit and the second is suppressed; references:
coreState.coreMetaData.isAppLaunchPushed,
activityLifeCycleManager.activityResumed(...), and inboxV2Bridge.submit(...).
In
`@clevertap-core/src/test/java/com/clevertap/android/sdk/AnalyticsManagerTest.kt`:
- Around line 40-43: Tests fail to compile due to ambiguous assertions: remove
the duplicate imports from kotlin.test so only org.junit.Assert versions are
imported; specifically delete any imports of assertEquals and assertFalse from
kotlin.test (leave import lines for org.junit.Assert.assertEquals, assertFalse,
assertNotNull, assertTrue intact) so calls in AnalyticsManagerTest.kt resolve
unambiguously.
In
`@clevertap-core/src/test/java/com/clevertap/android/sdk/db/dao/InboxMessageDAOImplTest.kt`:
- Around line 392-404: The test currently uses Long.MAX_VALUE which lets
PENDING_INDEXING rows pass the staleness cutoff; change the test to prove
INDEXED status drives inclusion by (a) inserting two V2 messages for the same
user via inboxMessageDAO.upsertMessages using getCtMsgDao(..., source =
InboxMessageSource.V2) — one left as PENDING_INDEXING and another marked INDEXED
via inboxMessageDAO.markIndexed(...) — and (b) call
inboxMessageDAO.findSweepableV2Ids(userId, a cutoff that excludes the
PENDING_INDEXING row but includes the INDEXED row) and assert the result
contains only the ID you marked INDEXED. This ensures findSweepableV2Ids is
validated for INDEXED vs PENDING_INDEXING behavior.
In `@clevertap-core/src/test/java/com/clevertap/android/sdk/db/DBAdapterTest.kt`:
- Around line 321-334: The test currently uses Long.MAX_VALUE as the cutoff so a
PENDING_INDEXING row (date=100L) is sweepable regardless of markIndexed; change
the assertion to use a cutoff that would NOT make the original pending row
sweepable unless markIndexed changed its state—e.g. call
dbAdapter.findSweepableV2Ids(userId, 99L) or another value < 100L—and assert
that the returned list contains msgId after calling
dbAdapter.markIndexed(listOf(msgId), userId); reference the test helpers/getter
getV2MsgDao, the InboxIndexState.PENDING_INDEXING initial state, and the methods
markIndexed and findSweepableV2Ids when applying this change.
In
`@clevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/InboxFetchCallTest.kt`:
- Around line 147-154: In the test `null header from builder returns
NetworkFailure without hitting network` after calling `newCall(http, header =
null).execute()` add an assertion that the provided MockHttpClient recorded zero
requests to ensure the short-circuit prevented any network activity; locate the
MockHttpClient instance (variable `http`) and use its request-count or
equivalent inspection API (e.g., verify zero calls, no lastRequest, or
requestCount == 0) to assert no HTTP request was executed in addition to the
existing NetworkFailure assertion.
In `@docs/EXAMPLES.md`:
- Around line 349-350: Update the docs to clarify fetchInbox callback semantics:
change the sentence around "success is true if the fetch completed and new
messages were received" to state that success indicates the fetch completed
successfully (regardless of whether new messages arrived) and that empty results
are valid; note that inbox listener callbacks (inboxMessagesDidUpdate) will
still fire automatically when new messages are present. Reference the fetchInbox
callback and inboxMessagesDidUpdate listener so integrators understand success
means completion, not necessarily new-message arrival.
In `@sample/src/main/java/com/clevertap/demo/ui/main/HomeScreenViewModel.kt`:
- Around line 393-397: setProfileLocation() is currently a no-op because it
reassigns cleverTapAPI.location to itself; replace that with assigning an actual
location value (e.g., a real Location object or a stored/current location
variable) to cleverTapAPI?.location and ensure you handle nullability before
calling printVar. Specifically, update the method so cleverTapAPI?.location =
<actualLocation> (for example a currentLocation or profileLocation variable or a
new Location(lat, lon)), then call printVar("Location",
cleverTapAPI?.location?.toString() ?: "null") so the code sets a real profile
location and avoids NPEs when printing.
---
Outside diff comments:
In `@clevertap-core/src/main/java/com/clevertap/android/sdk/db/CtDatabase.kt`:
- Around line 67-83: The migration for oldVersion == 1 recreates inboxMessages
via CREATE_INBOX_MESSAGES_TABLE but later runs ALTER TABLE ADD COLUMN for
"source" and "index_state", causing duplicate-column errors; update
CtDatabase.kt to check whether those columns already exist before running the
ALTERs (e.g. query PRAGMA table_info('inboxMessages') or similar helper) and
only call executeStatement to add columns if they are absent; apply the same
guard where the same ALTERs occur (the blocks referencing
CREATE_INBOX_MESSAGES_TABLE, CREATE_INBOX_MESSAGES_TABLE-related ALTERs and the
code areas around the other occurrences noted) so the v1→v7 path skips duplicate
ADD COLUMN statements.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/response/DisplayUnitResponse.java`:
- Around line 85-89: The handler in DisplayUnitResponse currently returns early
when messages is empty, leaving stale data; instead, detect messages == null
versus messages.length()==0 and for the empty-array case call
cache.updateDisplayUnits(...) with an empty list (or equivalent clear), invoke
the display-units callback/notification path just as with a populated payload,
and log the empty payload event via logger.verbose; keep the null check
returning early but ensure messages.length()==0 triggers cache clearing and
callback invocation so stale units are removed.
---
Nitpick comments:
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxPendingActionsDAO.kt`:
- Around line 167-174: The current insert in InboxPendingActionsDAO uses
dbHelper.writableDatabase.insertWithOnConflict(...,
SQLiteDatabase.CONFLICT_IGNORE) and treats any -1 result as failure; change the
success check to treat -1 (constraint-ignored duplicate) as success for
idempotent semantics: capture the insertWithOnConflict return into a variable
(e.g., result) and return true if result >= 0 OR result == -1; keep the
SQLiteException catch/log using logger.verbose for Table.INBOX_PENDING_DELETES
to preserve error handling.
- Around line 193-200: The addPendingRead method currently treats insert result
as successful with ">= 0"—make it consistent with the idempotent semantics
applied to addPendingDelete by changing the success check to use "> 0" instead;
update the return expression in the insert block that references
Table.INBOX_PENDING_READS.tableName so the method returns true only for a
positive rowId while preserving the existing SQLiteException catch and logging.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxController.java`:
- Around line 601-605: The negative-TTL branch in resolvePendingActionExpiry
(method) is unclear: add a concise comment above the ternary that explains why
dao.getExpires() might be negative (e.g., legacy/formatting/parsing errors or
sentinel values) and that in such cases we intentionally apply
PENDING_DELETE_DEFAULT_TTL_SECONDS to avoid immediate expiry; reference
CTMessageDAO.getExpires() and the PENDING_DELETE_DEFAULT_TTL_SECONDS constant in
the comment so future readers understand the fallback semantics.
- Around line 141-171: Partitioning V2 messages currently happens under
messagesLock then releases it before writing pendingRows and calling
_deleteMessagesForIds, allowing a TOCTOU; to fix, perform the V2 partitioning,
pendingRows construction (resolvePendingActionExpiry), the
dbAdapter.addPendingDeletes(...) call, and the _deleteMessagesForIds(messageIDs)
invocation while holding the same higher-level inbox controller lock
(ctLockManager.getInboxControllerLock()) so the sequence (v2Messages/pendingRows
creation -> db write -> deletion ->
callbackManager._notifyInboxMessagesDidUpdate()) is atomic; adjust
synchronization so messagesLock work happens inside that controller lock or
acquire controller lock first, then iterate messages to build
v2Messages/pendingRows, call dbAdapter.addPendingDeletes, call
_deleteMessagesForIds, and finally call inboxDeleteCoordinator.syncDelete if
needed.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/EventSuppressor.kt`:
- Around line 24-30: The ConcurrentHashMap lastSeen used by shouldSuppress can
grow unbounded for unbounded key spaces; update the implementation to prevent
memory leak by adding eviction: either replace lastSeen with a bounded cache
with TTL (e.g., use a ConcurrentLinkedHashMap/Guava Cache or an LRU with
maximumSize and expireAfterWrite using the same windowMs) or implement periodic
cleanup of entries older than windowMs when calling shouldSuppress; ensure
symbols referenced are lastSeen, shouldSuppress, windowMs and preserve
thread-safety for concurrent access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5d408f2a-68af-4e34-99e7-1561ac95af2e
📒 Files selected for processing (86)
CHANGELOG.mdREADME.mdclevertap-core/build.gradleclevertap-core/src/androidTest/kotlin/PIFlushWorkInstrumentationTest.ktclevertap-core/src/main/java/com/clevertap/android/sdk/ActivityLifeCycleManager.javaclevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.javaclevertap-core/src/main/java/com/clevertap/android/sdk/BaseAnalyticsManager.javaclevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.javaclevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/main/java/com/clevertap/android/sdk/Constants.javaclevertap-core/src/main/java/com/clevertap/android/sdk/ControllerManager.javaclevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.ktclevertap-core/src/main/java/com/clevertap/android/sdk/FetchInboxCallback.javaclevertap-core/src/main/java/com/clevertap/android/sdk/db/CtDatabase.ktclevertap-core/src/main/java/com/clevertap/android/sdk/db/DBAdapter.ktclevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxMessageDAO.ktclevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxMessageDAOImpl.ktclevertap-core/src/main/java/com/clevertap/android/sdk/db/dao/InboxPendingActionsDAO.ktclevertap-core/src/main/java/com/clevertap/android/sdk/displayunits/CTDisplayUnitController.javaclevertap-core/src/main/java/com/clevertap/android/sdk/displayunits/DisplayUnitCache.javaclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxController.javaclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxListViewFragment.javaclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTMessageDAO.javaclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/EventSuppressor.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/InboxDeleteCoordinator.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/InboxIndexState.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/InboxMessageSource.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/InboxV2Bridge.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/InboxV2Fetcher.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inbox/InboxV2Merger.ktclevertap-core/src/main/java/com/clevertap/android/sdk/login/LoginController.javaclevertap-core/src/main/java/com/clevertap/android/sdk/network/api/CtApi.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/CallResult.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/EndpointCall.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/EventRequestBody.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/FetchThrottle.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/FetchTrigger.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxDeleteCall.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxFetchCall.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxV2EventBuilder.ktclevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/NetworkScope.ktclevertap-core/src/main/java/com/clevertap/android/sdk/response/DisplayUnitResponse.javaclevertap-core/src/main/java/com/clevertap/android/sdk/response/InboxV2DeliverySource.ktclevertap-core/src/main/java/com/clevertap/android/sdk/response/InboxV2Response.ktclevertap-core/src/main/res/layout/inbox_list_view.xmlclevertap-core/src/test/java/com/clevertap/android/sdk/ActivityLifeCycleManagerTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/AnalyticsManagerTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/CleverTapAPITest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/ConstantsTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/MockAnalyticsManager.ktclevertap-core/src/test/java/com/clevertap/android/sdk/MockCoreState.ktclevertap-core/src/test/java/com/clevertap/android/sdk/db/DBAdapterTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/db/DatabaseHelperMigrationTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/db/dao/InboxMessageDAOImplTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/db/dao/InboxPendingActionsDAOImplTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/displayunits/CTDisplayUnitControllerTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/CTInboxControllerTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/CTMessageDAOTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/EventSuppressorTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/InboxDeleteCoordinatorTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/InboxV2BridgeTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/InboxV2FetcherTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inbox/InboxV2MergerTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/login/LoginControllerTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/api/CtApiInboxFetchTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/CallResultTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/EventRequestBodyTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/FetchThrottleTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/InboxDeleteCallTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/InboxFetchCallTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/InboxV2EventBuilderTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/fetch/NetworkScopeTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/network/http/MockHttpClient.ktclevertap-core/src/test/java/com/clevertap/android/sdk/response/InboxV2ResponseTest.ktclevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/content/VerticalImageBigContentView.ktdocs/CTCORECHANGELOG.mddocs/CTGEOFENCE.mddocs/CTPUSHTEMPLATES.mddocs/EXAMPLES.mdgradle-scripts/jacoco_root.gradlegradle/libs.versions.tomlsample/build.gradlesample/src/main/java/com/clevertap/demo/ui/main/HomeScreenModel.ktsample/src/main/java/com/clevertap/demo/ui/main/HomeScreenViewModel.kttemplates/CTCORECHANGELOG.mdtemplates/EXAMPLES.md
| } catch (e: Exception) { | ||
| return@withContext CallResult.NetworkFailure(e) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "InboxDeleteCall.kt"Repository: CleverTap/clevertap-android-sdk
Length of output: 163
🏁 Script executed:
wc -l clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxDeleteCall.ktRepository: CleverTap/clevertap-android-sdk
Length of output: 166
🏁 Script executed:
cat -n clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxDeleteCall.ktRepository: CleverTap/clevertap-android-sdk
Length of output: 4170
🏁 Script executed:
# Check for CancellationException imports and usage patterns in the codebase
rg "CancellationException" --type kotlin -B 3 -A 3Repository: CleverTap/clevertap-android-sdk
Length of output: 6671
🏁 Script executed:
# Check what Exception types are caught in similar coroutine patterns
rg "catch\s*\(\s*e:\s*Exception\s*\)" --type kotlin -B 2 -A 2 | head -60Repository: CleverTap/clevertap-android-sdk
Length of output: 6607
Replace catch (e: Exception) with explicit CancellationException re-throw on lines 45–47 and 64–67.
The execute() method wraps code in withContext(dispatcher) but catches Exception, which also catches CancellationException. This swallows the cancellation signal and converts it to NetworkFailure, breaking the coroutine cancellation contract. The interface EndpointCall<T> and CallResult explicitly document: "don't swallow CancellationException."
Suggested fix
+import kotlinx.coroutines.CancellationException
@@
- } catch (e: Exception) {
+ } catch (e: CancellationException) {
+ throw e
+ } catch (e: Exception) {
return@withContext CallResult.NetworkFailure(e)
}
@@
- } catch (e: Exception) {
+ } catch (e: CancellationException) {
+ throw e
+ } catch (e: Exception) {
logger.verbose("InboxV2", "delete failed: $e")
CallResult.NetworkFailure(e)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxDeleteCall.kt`
around lines 45 - 47, The current execute() implementation in InboxDeleteCall
wraps work with withContext(dispatcher) and uses catch (e: Exception) which
swallows CancellationException; update the exception handling so
CancellationException is re-thrown and not converted to
CallResult.NetworkFailure. Concretely, in the execute() method surrounding
withContext(dispatcher) change the single catch to first detect/rethrow
CancellationException (either via a specific catch (e: CancellationException) {
throw e } or by checking if (e is CancellationException) throw e) and only then
return CallResult.NetworkFailure for other Exceptions, making the same change
for both catch sites that currently return CallResult.NetworkFailure.
| } catch (e: Exception) { | ||
| return@withContext CallResult.NetworkFailure(e) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "InboxFetchCall.kt" -type fRepository: CleverTap/clevertap-android-sdk
Length of output: 164
🏁 Script executed:
cat -n clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxFetchCall.ktRepository: CleverTap/clevertap-android-sdk
Length of output: 3909
Preserve coroutine cancellation instead of catching it as a generic failure.
Lines 52-54 and 73-76 catch Exception, which includes CancellationException. When a coroutine is cancelled, swallowing the CancellationException prevents proper cancellation propagation and can cause resource leaks or unexpected behavior. Cancellation should be rethrown separately.
Suggested fix
+import kotlinx.coroutines.CancellationException
@@
- } catch (e: Exception) {
+ } catch (e: CancellationException) {
+ throw e
+ } catch (e: Exception) {
return@withContext CallResult.NetworkFailure(e)
}
@@
- } catch (e: Exception) {
+ } catch (e: CancellationException) {
+ throw e
+ } catch (e: Exception) {
logger.verbose("InboxV2", "fetch failed: $e")
CallResult.NetworkFailure(e)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e: Exception) { | |
| return@withContext CallResult.NetworkFailure(e) | |
| } | |
| } catch (e: CancellationException) { | |
| throw e | |
| } catch (e: Exception) { | |
| return@withContext CallResult.NetworkFailure(e) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/network/fetch/InboxFetchCall.kt`
around lines 52 - 54, The catch blocks currently catch Exception (in the
withContext scope and the later block) which swallows CancellationException;
update each catch to rethrow CancellationException immediately (e.g., if (e is
CancellationException) throw e) and only convert other exceptions into
CallResult.NetworkFailure(e) so coroutine cancellation propagates correctly;
adjust the catch around withContext and the later catch (e: Exception) that
returns CallResult.NetworkFailure to follow this pattern, keeping references to
CallResult.NetworkFailure and the withContext scope.
Summary by CodeRabbit
Release Notes
New Features
fetchInbox()method with 5-minute throttlingDependencies