Skip to content

2.7.15#412

Merged
ianrumac merged 17 commits into
mainfrom
develop
May 19, 2026
Merged

2.7.15#412
ianrumac merged 17 commits into
mainfrom
develop

Conversation

@ianrumac
Copy link
Copy Markdown
Collaborator

@ianrumac ianrumac commented May 18, 2026

Changes in this pull request

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run ktlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

Greptile Summary

This release (2.7.15) introduces a fingerprint-based deduplication system for paywall preloading, fixes overenthusiastic subscriptionStatus_didChange emissions, repairs delegate event drops in SerialTaskManager, and corrects week↔day price calculation rounding errors.

  • Preload dedup: PaywallPreload gains an AtomicReference<String?> lastFingerprint that is CAS-committed before a preload starts; ConfigManager.recheckPreloadIfNeeded() is wired into subscription status changes, ComponentCallbacks2 locale/config events, and interface-style overrides to re-trigger preload only when device state actually changed. ConfigLogic is also revised to short-circuit after the first matching IF_TRUE rule per campaign rather than preloading all matches.
  • Subscription dedup: distinctUntilChanged with Entitlement.isDistinct() (which ignores product-ID-only enrichment changes) prevents spurious subscriptionStatus_didChange events.
  • Bug fixes: SerialTaskManager tasks are now wrapped in withErrorTracking so an exception no longer drops subsequent queued events; DependencyContainer.receipts() uses firstOrNull/filterNotNull to avoid NoSuchElementException on empty product lists; an unsafe as SuperwallPaywallActivity? cast is replaced with as?.

Confidence Score: 4/5

Safe to merge with awareness of one edge case in the fingerprint rollback path under task cancellation.

The fingerprint CAS rollback only fires when another task is already running at function entry. If the launched task itself is cancelled mid-flight (e.g., the 5-second delay is interrupted during scope teardown), neither lastFingerprint nor currentPreloadingTask is reset. On a subsequent device-state change the CAS to the new fingerprint succeeds, but currentPreloadingTask != null immediately rolls it back to the old value — so recheckPreloadIfNeeded() keeps seeing a mismatch and dispatching on every triggering event without ever starting a real preload. The rest of the changes (dedup logic, subscription-status filtering, price math, error-tracking in the task queue) are straightforward and well-tested.

superwall/src/main/java/com/superwall/sdk/config/PaywallPreload.kt — the CAS guard and task lifecycle around the new fingerprint field.

Important Files Changed

Filename Overview
superwall/src/main/java/com/superwall/sdk/config/PaywallPreload.kt Adds AtomicReference fingerprint dedup with CAS. Rollback only covers the "already-running at entry" case; a task cancelled mid-flight leaves both lastFingerprint and currentPreloadingTask permanently stuck.
superwall/src/main/java/com/superwall/sdk/config/ConfigManager.kt Adds recheckPreloadIfNeeded() with cheap IF_TRUE + fingerprint pre-checks before dispatching PreloadIfEnabled. Logic is sound; the pre-check is non-authoritative and the authoritative CAS lives in PaywallPreload.
superwall/src/main/java/com/superwall/sdk/config/ConfigLogic.kt Refactors IF_TRUE preload to only preload the first matching IF_TRUE rule per campaign (short-circuit). Logic and tests look correct; intentional behavior change per CHANGELOG.
superwall/src/main/java/com/superwall/sdk/Superwall.kt Adds distinctUntilChanged dedup on subscription status using Entitlement.isDistinct, ComponentCallbacks2 for locale/config changes, and recheckPreloadIfNeeded() call sites. Also fixes unsafe cast (as -> as?) and log message.
superwall/src/main/java/com/superwall/sdk/models/entitlements/Entitlement.kt Adds isDistinct() for dedup comparison (excludes productIds/latestProductId). Naming is clear and the fields compared are appropriate for detecting user-visible state changes.
superwall/src/main/java/com/superwall/sdk/store/abstractions/product/SubscriptionPeriod.kt Fixes day↔week price calculations to use exact BigDecimal(7) instead of 365/52 approximation; tests confirm correctness.
superwall/src/main/java/com/superwall/sdk/dependencies/DependencyContainer.kt Adds ActiveEntitlementsFactory, DateSerializer serializers module, and safe firstOrNull/filterNotNull in receipts(). All changes look correct.
superwall/src/main/java/com/superwall/sdk/misc/SerialTaskManager.kt Wraps task() in withErrorTracking so a failing delegate event no longer drops subsequent events from the queue.

Comments Outside Diff (1)

  1. superwall/src/main/java/com/superwall/sdk/config/PaywallPreload.kt, line 54-114 (link)

    P1 lastFingerprint permanently stuck after task cancellation or mid-flight failure

    The CAS rollback (lastFingerprint.compareAndSet(fingerprint, previous)) only fires when another task is already running at entry. If the task is successfully launched but later cancelled — e.g. delay(5000) receives a CancellationException during scope teardown, or any suspend call inside the coroutine throws before currentPreloadingTask = null on line 113 — both lastFingerprint and currentPreloadingTask remain permanently set with no reset path.

    This creates a compounding loop when device state subsequently changes: recheckPreloadIfNeeded() sees lastFingerprint.get() ("X") != newFingerprint ("Y") and keeps dispatching PreloadIfEnabled; each dispatch CAS-commits "Y", hits currentPreloadingTask != null, rolls back to "X", and returns — so lastFingerprint never advances and the dispatch fires again on every subsequent triggering event.

    A try/finally block inside the launched coroutine that resets lastFingerprint (and currentPreloadingTask) on abnormal exit would close this gap.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: superwall/src/main/java/com/superwall/sdk/config/PaywallPreload.kt
    Line: 54-114
    
    Comment:
    **`lastFingerprint` permanently stuck after task cancellation or mid-flight failure**
    
    The CAS rollback (`lastFingerprint.compareAndSet(fingerprint, previous)`) only fires when another task is *already running at entry*. If the task is successfully launched but later cancelled — e.g. `delay(5000)` receives a `CancellationException` during scope teardown, or any suspend call inside the coroutine throws before `currentPreloadingTask = null` on line 113 — both `lastFingerprint` and `currentPreloadingTask` remain permanently set with no reset path.
    
    This creates a compounding loop when device state subsequently changes: `recheckPreloadIfNeeded()` sees `lastFingerprint.get() ("X") != newFingerprint ("Y")` and keeps dispatching `PreloadIfEnabled`; each dispatch CAS-commits "Y", hits `currentPreloadingTask != null`, rolls back to "X", and returns — so `lastFingerprint` never advances and the dispatch fires again on every subsequent triggering event.
    
    A `try/finally` block inside the launched coroutine that resets `lastFingerprint` (and `currentPreloadingTask`) on abnormal exit would close this gap.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
superwall/src/main/java/com/superwall/sdk/config/PaywallPreload.kt:54-114
**`lastFingerprint` permanently stuck after task cancellation or mid-flight failure**

The CAS rollback (`lastFingerprint.compareAndSet(fingerprint, previous)`) only fires when another task is *already running at entry*. If the task is successfully launched but later cancelled — e.g. `delay(5000)` receives a `CancellationException` during scope teardown, or any suspend call inside the coroutine throws before `currentPreloadingTask = null` on line 113 — both `lastFingerprint` and `currentPreloadingTask` remain permanently set with no reset path.

This creates a compounding loop when device state subsequently changes: `recheckPreloadIfNeeded()` sees `lastFingerprint.get() ("X") != newFingerprint ("Y")` and keeps dispatching `PreloadIfEnabled`; each dispatch CAS-commits "Y", hits `currentPreloadingTask != null`, rolls back to "X", and returns — so `lastFingerprint` never advances and the dispatch fires again on every subsequent triggering event.

A `try/finally` block inside the launched coroutine that resets `lastFingerprint` (and `currentPreloadingTask`) on abnormal exit would close this gap.

Reviews (2): Last reviewed commit: "Fix stale preload fingerprint issues" | Re-trigger Greptile

yusuftor and others added 13 commits May 15, 2026 17:16
SubscriptionPeriod converted between days and weeks via a 52/365 factor.
But 52 weeks is only 364 days, so a 7-day period resolved to ~0.997
weeks instead of exactly 1 — inflating a 7-day product's weekly price
and skewing a week product's daily price (e.g. a $7.00/week product
reported $0.99/day instead of the exact $1.00).

7 days is exactly 1 week. Use the exact ratio in both directions:

- SubscriptionPeriod.pricePerDay .week: 365/52 -> 7
- SubscriptionPeriod.pricePerWeek .day: 52/365 -> 1/7
- RawStoreProduct.periodsPerUnit (trial path): the same two day↔week
  entries.

Month↔year was already exact (12); week/month and week/year stay as
conventional approximations — those aren't whole-number relationships.

Mirrors the same fix in the iOS SDK and the web editor so all three
agree. Adds two regression tests (a 7-day period's weekly price equals
the price; a weekly product's daily price divides by exactly 7) in the
active test region — the existing period-price tests are still inside
the disabled "TODO: Re-enable in CI" block.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two issues in the new regression tests:

- BigDecimal(6.99) calls the double constructor, which stores the
  inexact IEEE-754 value rather than exactly 6.99 — the two sides of
  the comparison could resolve to different values. Use the String
  constructor for exact decimals.
- Kotlin's assert() is compiled to a no-op unless the JVM runs with
  -ea, so the tests could silently pass even on a regression. Switch
  to JUnit's assertEquals, which always evaluates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread superwall/src/main/java/com/superwall/sdk/config/PaywallPreload.kt
@ianrumac
Copy link
Copy Markdown
Collaborator Author

@greptileai

@ianrumac ianrumac merged commit f4479d8 into main May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants