-
Notifications
You must be signed in to change notification settings - Fork 60
Publish config before the StoreKit read for cached subscribers #519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,11 @@ class ConfigManager { | |
|
|
||
| var configRetryCount = 0 | ||
|
|
||
| /// The purchases load that `fetchConfiguration` starts before it publishes | ||
| /// `configState` on the cached-config path. Trial eligibility awaits it so a | ||
| /// paywall opened during the load still sees the active subscription groups. | ||
| private(set) var initialPurchasesLoad: Task<Void, Never>? | ||
|
|
||
| private unowned let storeKitManager: StoreKitManager | ||
| unowned let storage: Storage | ||
| private unowned let network: Network | ||
|
|
@@ -179,15 +184,36 @@ class ConfigManager { | |
| ) | ||
| } | ||
|
|
||
| // Step 5: Track device attributes | ||
| // Step 5: Process config and set state. | ||
| // | ||
| // On the cached-config path the user is a known subscriber and the | ||
| // config is already on disk, so nothing below needs the network. But | ||
| // `processConfig` reads StoreKit, which can take 10 to 25 seconds on a | ||
| // weak connection, and every `register` call waits on `configState`. | ||
| // Publishing before that read keeps gated features from stalling at | ||
| // cold launch. The sync path keeps its order: with no known subscriber, | ||
| // purchases are read before a paywall can be shown. | ||
| var didPublishConfig = false | ||
| if shouldFetchAsync { | ||
| didPublishConfig = await processConfig( | ||
| config, | ||
| isFirstTime: true, | ||
| publishBeforeLoadingPurchases: true | ||
| ) | ||
| } | ||
|
|
||
| // Step 6: Track device attributes | ||
| let deviceAttributes = await factory.makeSessionDeviceAttributes() | ||
| await Superwall.shared.track( | ||
| InternalSuperwallEvent.DeviceAttributes(deviceAttributes: deviceAttributes) | ||
| ) | ||
|
|
||
| // Step 6: Process config and set state | ||
| await processConfig(config, isFirstTime: true) | ||
| configState.send(.retrieved(config)) | ||
| if !shouldFetchAsync { | ||
| await processConfig(config, isFirstTime: true) | ||
| } | ||
| if !didPublishConfig { | ||
| configState.send(.retrieved(config)) | ||
| } | ||
|
|
||
| // Step 7: Schedule background tasks | ||
| scheduleBackgroundTasks( | ||
|
|
@@ -387,10 +413,20 @@ class ConfigManager { | |
| ) | ||
| } | ||
|
|
||
| /// Applies `config` and loads purchases from StoreKit. | ||
| /// | ||
| /// - Parameter publishBeforeLoadingPurchases: When `true`, sends | ||
| /// `configState` as soon as the config is applied and before the StoreKit | ||
| /// read starts. Only safe when the subscription status on disk is already | ||
| /// known, which is why `fetchConfiguration` passes it on the cached-config | ||
| /// path alone. Ignored in test mode, where products come from the API. | ||
| /// - Returns: Whether `configState` was published here. | ||
| @discardableResult | ||
| private func processConfig( | ||
| _ config: Config, | ||
| isFirstTime: Bool | ||
| ) async { | ||
| isFirstTime: Bool, | ||
| publishBeforeLoadingPurchases: Bool = false | ||
| ) async -> Bool { | ||
| storage.save( | ||
| config.featureFlags.disableVerbosePlacements, forType: DisableVerbosePlacements.self) | ||
| storage.save(config, forType: LatestConfig.self) | ||
|
|
@@ -404,6 +440,7 @@ class ConfigManager { | |
| let testModeJustActivated = !wasTestMode && testModeManager.isTestMode | ||
| let testModeJustDeactivated = wasTestMode && !testModeManager.isTestMode | ||
|
|
||
| var didPublishConfig = false | ||
| if testModeManager.isTestMode { | ||
| // In test mode, fetch products from API instead of StoreKit | ||
| await fetchTestModeProducts(testModeManager: testModeManager) | ||
|
|
@@ -423,7 +460,20 @@ class ConfigManager { | |
| entitlements: [] | ||
| ).merging(with: .blank(), granting: entitlementsInfo.granted) | ||
| } | ||
| await factory.loadPurchasedProducts(config: config) | ||
| if publishBeforeLoadingPurchases { | ||
| // The task handle is stored before the publish so anything that | ||
| // presents on this config can await the load through | ||
| // `initialPurchasesLoad`. | ||
| let purchasesLoad = Task { [factory] in | ||
| await factory.loadPurchasedProducts(config: config) | ||
| } | ||
| initialPurchasesLoad = purchasesLoad | ||
| configState.send(.retrieved(config)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Technical details# Post-load state readable while empty/stale on the cached-config path
## Affected sites
- `Sources/SuperwallKit/Config/ConfigManager.swift:471` — `configState.send(.retrieved(config))` now precedes `await purchasesLoad.value`, unblocking every `register`/`track` awaiter.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:272` — `getActiveProductIds()` reads `manager.purchases`, which is in-memory only and populated solely by `loadPurchases` inside the deferred task.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:1018` — `activeProducts:` in `getTemplateDevice()`, which feeds `DependencyContainer.makeAudienceFilterAttributes` (`:471`, the `device` dict handed to CEL) and `makeJsonVariables` (`:359`, paywall template variables).
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:264` — `isSubscribed(to:)`, same backing, surfaced as the per-product `isSubscribed` template variable via `TemplateLogic.swift:29`.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:216` — `syncSubscriptionStatus(purchases:)`, the correction that deactivates refunded/expired App Store subscriptions, now runs concurrently with presentation instead of before it.
## Required outcome
- Audience-filter evaluation during the window must not silently report `device.activeProducts` as empty for a user who owns products; either the value is restored/derived from disk, or the attribute is omitted rather than reported as an empty set, or the evaluation awaits the load.
- The staleness accepted for `device.subscriptionStatus` / `activeEntitlements` during the window should be a stated decision, since the neighbouring CHANGELOG entry promises that refunded and expired App Store subscriptions "still deactivate immediately".
## Open questions for the human
- How widely is `device.activeProducts` used in customer audience filters? If it is rare, documenting the window may be enough; if it is common, an empty set is a silent wrong-audience match for the whole cold-launch window.
- Is a lapsed subscriber getting gated features unlocked for the duration of the StoreKit read an accepted trade-off for the latency win? |
||
| didPublishConfig = true | ||
| await purchasesLoad.value | ||
| } else { | ||
| await factory.loadPurchasedProducts(config: config) | ||
| } | ||
| } | ||
|
|
||
| if !testModeManager.isTestMode { | ||
|
|
@@ -442,6 +492,8 @@ class ConfigManager { | |
| let reason = testModeManager.testModeReason { | ||
| await presentTestModeModal(reason: reason, config: config) | ||
| } | ||
|
|
||
| return didPublishConfig | ||
| } | ||
|
|
||
| /// Reassigns variants and preloads paywalls again. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,9 +252,11 @@ actor ReceiptManager { | |
| return true | ||
| } | ||
|
|
||
| // `activeSubscriptionGroupIds` is populated in `loadPurchasedProducts`, which always | ||
| // completes before a paywall opens (config is only marked retrieved after it runs, | ||
| // and presentation waits for config), so this reflects current subscription state. | ||
| // `activeSubscriptionGroupIds` is populated in `loadPurchasedProducts`. On the | ||
| // sync config path that load completes before config is published. On the | ||
| // cached-config path config is published first, so paywall callers go through | ||
| // `DependencyContainer.isFreeTrialAvailable`, which awaits the load before | ||
| // reaching here. | ||
|
Comment on lines
+255
to
+259
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The claim that paywall callers all go through Technical details# `ReceiptManager.isFreeTrialAvailable` has a caller that bypasses `initialPurchasesLoad`
## Affected sites
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:255-259` — the new comment asserts a universal guarantee.
- `Sources/SuperwallKit/StoreKit/Transactions/TransactionManager.swift:734` — `return await receiptManager.isFreeTrialAvailable(for: product)`, via a directly held `receiptManager`, not `factory`. `TransactionManager.Factory` is `OptionsFactory` only (`:21`), so it currently cannot reach the awaited wrapper.
- Downstream: `TransactionManager.swift:655` → `:725` → `PurchasingCoordinator.isFreeTrialAvailable` → `TransactionManager.swift:1092` `didStartOffer` → `:1103` `TransactionType.freeTrialStart` vs `.subscriptionStart`.
- Secondary, lower severity: `Sources/SuperwallKit/Paywall/Request/Operators/AddPaywallProducts.swift:311` — the `.eligible` branch calls `hasActiveIntroOffer`, which reads `Superwall.shared.customerInfo.subscriptions` (`:336-338`) with no `isPlaceholder` guard, unlike `hasEverHadEntitlement` at `:368`. That value is disk-restored so it is stale rather than empty, but it drives the user-facing `paywall.isFreeTrialAvailable` template value.
## Required outcome
- Either the purchase path observes the same `activeSubscriptionGroupIds` guarantee as the paywall path, or the comment is corrected to say which callers are unprotected and why that is acceptable.
## Suggested approach
- Widen `TransactionManager.Factory` to `OptionsFactory & ReceiptFactory` and route `:734` through `factory.isFreeTrialAvailable(for:)`. `DependencyContainer` already conforms, so every production and test call site is unchanged. |
||
| return !activeSubscriptionGroupIds.contains(subscriptionGroupId) | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hoisting
processConfigabove this block means thedevice_attributestrack now happens after the StoreKit read on the async path, sinceprocessConfigstillawaitspurchasesLoad.valuebefore returning. That delays the event by the same 10–25 s the PR is removing fromregister, on the exact launches the PR description measures. Was that intended, or should the track stay ahead of the load and onlyconfigStatemove?