From 2b908be9b190d1dabb8f805300519738263cfc4f Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Thu, 9 Jul 2026 17:13:45 +0200 Subject: [PATCH 1/9] fix(offerings): gate getOfferings delivery on ui_config and stale-cache paths getOfferings (with remote config enabled) must not return until the paywall config data it depends on is queryable. Two gaps closed: Delivery already waited for the workflows topic and its prefetch blobs; it now also resolves the ui_config body, concurrently, so a paywall render right after getOfferings has its styling in hand without a further round trip. Both steps remain best-effort: either failing resolves nil rather than stranding delivery. And the stale-memory-cache path previously bypassed the gate entirely, returning before config readiness. Stale delivery now goes through the same gate, with the background refresh kicked first so the gate cannot delay it. The gate is a no-op once config has synced, so stale cache hits still return fast everywhere but the very first launch. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 39 +++++++---- .../Purchasing/OfferingsManagerTests.swift | 66 +++++++++++++++++-- .../Purchases/BasePurchasesTests.swift | 31 ++++++++- 3 files changed, 114 insertions(+), 22 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index e7145af426..2b5fac7628 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -28,6 +28,9 @@ class OfferingsManager { private let dateProvider: DateProvider // Nil when remote config is disabled, in which case offerings delivery is unchanged. private let remoteConfigManager: RemoteConfigManagerType? + // Derived from `remoteConfigManager`; resolves the `ui_config` body the delivery gate + // waits on. Nil exactly when the manager is. + private let uiConfigProvider: UiConfigProvider? init(deviceCache: DeviceCache, operationDispatcher: OperationDispatcher, @@ -47,6 +50,7 @@ class OfferingsManager { self.diagnosticsTracker = diagnosticsTracker self.dateProvider = dateProvider self.remoteConfigManager = remoteConfigManager + self.uiConfigProvider = remoteConfigManager.map { UiConfigProvider(manager: $0) } } func offerings( @@ -99,23 +103,19 @@ class OfferingsManager { notFoundProductIds: nil) if cacheStatus == .stale { - // Serve the cached offerings immediately and refresh in the background, preserving the - // existing "return fast, update later" behavior. The background update goes through the - // same workflows-aware delivery path, so it refreshes the workflows list too. Gating - // here as well would both block delivery and double-fetch the list. - self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(memoryCachedOfferings)) + // Kick the background refresh before the gated delivery below; the readiness + // gate must not delay it. self.updateOfferingsCache(appUserID: appUserID, isAppBackgrounded: isAppBackgrounded, fetchPolicy: fetchPolicy, completion: nil) - } else { - // Fresh offerings (no background refresh coming): ensure remote config has synced at - // least once before delivering, so a caller resolving a workflow right after - // `getOfferings` succeeds isn't racing the first config sync. This is a no-op once the - // `workflows` topic is already populated, which it normally is on this path. - self.deliverWhenConfigReady { - self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(memoryCachedOfferings)) - } + } + // Cached delivery (fresh or stale) goes through the readiness gate: `getOfferings` + // returning means the paywall config data is queryable. The gate is a no-op once + // config has synced, so the stale path's "return fast, update later" behavior is + // preserved everywhere but the very first launch. + self.deliverWhenConfigReady { + self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(memoryCachedOfferings)) } } } @@ -391,13 +391,24 @@ private extension OfferingsManager { /// `workflows` topic's metadata. The manager's `awaitTopicAndPrefetchBlobsReady()` read no-ops when already synced /// and always calls back, so this never hangs. When no manager is wired (workflows disabled), /// `deliver` runs immediately, leaving offerings unchanged. + /// Invokes `deliver` once the config-endpoint paywall data `getOfferings` depends on is + /// ready, resolved concurrently: + /// - the `workflows` topic is synced (with its prefetch-flagged blobs downloaded), so + /// workflow resolution right after `getOfferings` doesn't race the first config sync; and + /// - the `ui_config` body is resolved, so a paywall render has its styling in hand without + /// a further round trip. + /// + /// Both steps are best-effort: each returns nil on failure rather than throwing, so + /// delivery can never be stranded on either. private func deliverWhenConfigReady(deliver: @escaping () -> Void) { guard let remoteConfigManager = self.remoteConfigManager else { deliver() return } Task { - _ = await remoteConfigManager.awaitTopicAndPrefetchBlobsReady(.workflows) + async let workflowsReady = remoteConfigManager.awaitTopicAndPrefetchBlobsReady(.workflows) + async let uiConfigReady = self.uiConfigProvider?.getUiConfig() + _ = await (workflowsReady, uiConfigReady) deliver() } } diff --git a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift index e034b7e94c..adfc3ce65d 100644 --- a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift +++ b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift @@ -1034,6 +1034,58 @@ extension OfferingsManagerTests { expect(delivered.value).toEventually(beTrue()) } + func testGetOfferingsDoesNotDeliverUntilUiConfigResolved() { + let mockRemoteConfigManager = MockRemoteConfigManager() + // The workflows topic resolves immediately; ui_config's blob reads are held. + mockRemoteConfigManager.shouldStoreBlobDataCompletion = true + let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) + self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) + + let delivered: Atomic = false + manager.offerings(appUserID: MockData.anyAppUserID) { _ in delivered.value = true } + + // The ui_config read was started but is held, so offerings must wait. + expect(mockRemoteConfigManager.invokedBlobDataParameters).toEventuallyNot(beEmpty()) + expect(delivered.value) == false + + mockRemoteConfigManager.completeStoredBlobReads() + expect(delivered.value).toEventually(beTrue()) + } + + func testGetOfferingsDeliversWhenUiConfigResolutionFails() { + // No ui_config blobs stubbed: resolution fails (nil). Readiness is best-effort, so + // delivery must proceed anyway. + let manager = self.makeOfferingsManager(remoteConfigManager: MockRemoteConfigManager()) + self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) + + let result = waitUntilValue { completed in + manager.offerings(appUserID: MockData.anyAppUserID) { completed($0) } + } + + expect(result).to(beSuccess()) + } + + func testGetOfferingsAwaitsWorkflowsAndUiConfigConcurrently() { + let mockRemoteConfigManager = MockRemoteConfigManager() + // Hold BOTH readiness steps: each must have STARTED before either completes, or one + // failing slowly would serialize behind the other. + mockRemoteConfigManager.shouldStoreTopicCompletion = true + mockRemoteConfigManager.shouldStoreBlobDataCompletion = true + let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) + self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) + + let delivered: Atomic = false + manager.offerings(appUserID: MockData.anyAppUserID) { _ in delivered.value = true } + + expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) + expect(mockRemoteConfigManager.invokedBlobDataParameters).toEventuallyNot(beEmpty()) + expect(delivered.value) == false + + mockRemoteConfigManager.completeStoredTopic() + mockRemoteConfigManager.completeStoredBlobReads() + expect(delivered.value).toEventually(beTrue()) + } + func testGetOfferingsDeliversEvenWhenRemoteConfigHasNoWorkflowsTopic() { // A manager with no committed `workflows` topic (e.g. never synced) must still call back, so // offerings delivery can never hang. @@ -1067,9 +1119,8 @@ extension OfferingsManagerTests { expect(delivered.value).toEventually(beTrue()) } - func testGetOfferingsFromStaleMemoryCacheDeliversImmediately() { + func testGetOfferingsFromStaleMemoryCacheGatesDeliveryAndStillRefreshes() { let mockRemoteConfigManager = MockRemoteConfigManager() - // Hold the topic completion so a foreground wait, if present, would block delivery. mockRemoteConfigManager.shouldStoreTopicCompletion = true let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) // Cached but stale, so the background refresh runs. @@ -1080,12 +1131,15 @@ extension OfferingsManagerTests { let delivered: Atomic = false manager.offerings(appUserID: MockData.anyAppUserID) { _ in delivered.value = true } - // Cached offerings are delivered immediately, not blocked on the held topic completion. - expect(delivered.value).toEventually(beTrue()) - - // Drain the background refresh's config-ready wait so its `Task` doesn't leak past this test. + // The background refresh starts regardless of the gate... + expect(self.mockOfferings.invokedGetOfferingsForAppUserID).toEventually(beTrue()) + // ...but even stale cached offerings must not be delivered before config readiness: + // getOfferings returning means the paywall config data is queryable. expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) + expect(delivered.value) == false + mockRemoteConfigManager.completeStoredTopic() + expect(delivered.value).toEventually(beTrue()) } func testGetOfferingsNetworkFetchAwaitsConfigReadyBeforeDelivering() { diff --git a/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift b/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift index dd014eb0fa..f5fe82e8fa 100644 --- a/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift +++ b/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift @@ -743,9 +743,36 @@ final class MockRemoteConfigManager: RemoteConfigManagerType { continuation?.resume(returning: result) } + /// When `true`, `blobData(for:itemKey:)` suspends until `completeStoredBlobReads()` is called, + /// so tests can control the timing of a caller awaiting blob-backed resolution (e.g. + /// `OfferingsManager`'s config-ready gate resolving `ui_config`). + var shouldStoreBlobDataCompletion = false + private typealias StoredBlobRead = ( + topic: RemoteConfigTopic, itemKey: String, continuation: CheckedContinuation + ) + private let _storedBlobReads: Atomic<[StoredBlobRead]> = .init([]) + func blobData(for topic: RemoteConfigTopic, itemKey: String) async -> Data? { - self._invokedBlobDataParameters.modify { $0.append((topic, itemKey)) } - return self.stubbedBlobData[topic]?[itemKey] + guard self.shouldStoreBlobDataCompletion else { + self._invokedBlobDataParameters.modify { $0.append((topic, itemKey)) } + return self.stubbedBlobData[topic]?[itemKey] + } + return await withCheckedContinuation { continuation in + // Store before recording the invocation, so a test polling the invocation list can + // never observe readiness before `completeStoredBlobReads()` has something to resume. + self._storedBlobReads.modify { $0.append((topic, itemKey, continuation)) } + self._invokedBlobDataParameters.modify { $0.append((topic, itemKey)) } + } + } + + /// Resumes every blob read held while `shouldStoreBlobDataCompletion` was `true` with the + /// stubbed data for its key, and stops holding subsequent reads, so sequential read chains + /// (like `mergeItemsBlobData`'s loop) run to completion. + func completeStoredBlobReads() { + self.shouldStoreBlobDataCompletion = false + for read in self._storedBlobReads.getAndSet([]) { + read.continuation.resume(returning: self.stubbedBlobData[read.topic]?[read.itemKey]) + } } func blobData( From b6123c0928857e1aa55347e931173c72b4a3c7f5 Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Thu, 9 Jul 2026 17:40:45 +0200 Subject: [PATCH 2/9] fix(offerings): deliver refreshed offerings when the stale refresh wins the gate Review findings: the gated stale delivery captured the pre-gate snapshot, so a background refresh finishing during the wait still returned stale packages; delivery now re-reads the cache when the gate opens. The mock's topic hook stores every waiter (the stale test has two: gated delivery + background refresh) so completing it is deterministic. Also trims comment verbosity. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 26 +++++------- .../Purchasing/OfferingsManagerTests.swift | 38 +++++++++++++++--- .../Purchases/BasePurchasesTests.swift | 40 +++++++++---------- 3 files changed, 60 insertions(+), 44 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index 2b5fac7628..84005d57dd 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -103,19 +103,18 @@ class OfferingsManager { notFoundProductIds: nil) if cacheStatus == .stale { - // Kick the background refresh before the gated delivery below; the readiness - // gate must not delay it. + // The readiness gate below must not delay this background refresh. self.updateOfferingsCache(appUserID: appUserID, isAppBackgrounded: isAppBackgrounded, fetchPolicy: fetchPolicy, completion: nil) } - // Cached delivery (fresh or stale) goes through the readiness gate: `getOfferings` - // returning means the paywall config data is queryable. The gate is a no-op once - // config has synced, so the stale path's "return fast, update later" behavior is - // preserved everywhere but the very first launch. + // Cached delivery waits for config readiness (a no-op once config has synced), + // re-reading the cache afterwards: if the stale path's refresh finished while + // waiting, the caller gets the fresh offerings it already paid for. self.deliverWhenConfigReady { - self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(memoryCachedOfferings)) + let offerings = self.cachedOfferings ?? memoryCachedOfferings + self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(offerings)) } } } @@ -391,15 +390,10 @@ private extension OfferingsManager { /// `workflows` topic's metadata. The manager's `awaitTopicAndPrefetchBlobsReady()` read no-ops when already synced /// and always calls back, so this never hangs. When no manager is wired (workflows disabled), /// `deliver` runs immediately, leaving offerings unchanged. - /// Invokes `deliver` once the config-endpoint paywall data `getOfferings` depends on is - /// ready, resolved concurrently: - /// - the `workflows` topic is synced (with its prefetch-flagged blobs downloaded), so - /// workflow resolution right after `getOfferings` doesn't race the first config sync; and - /// - the `ui_config` body is resolved, so a paywall render has its styling in hand without - /// a further round trip. - /// - /// Both steps are best-effort: each returns nil on failure rather than throwing, so - /// delivery can never be stranded on either. + /// Invokes `deliver` once the paywall config data `getOfferings` depends on is ready: + /// the `workflows` topic (with its prefetch-flagged blobs) and the `ui_config` body, + /// resolved concurrently. Both are best-effort (nil on failure, never throwing), so + /// delivery can never be stranded. private func deliverWhenConfigReady(deliver: @escaping () -> Void) { guard let remoteConfigManager = self.remoteConfigManager else { deliver() diff --git a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift index adfc3ce65d..f5697fb892 100644 --- a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift +++ b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift @@ -969,7 +969,12 @@ private extension OfferingsManagerTests { static let unexpectedBackendResponseError: BackendError = .unexpectedBackendResponse( .customerInfoNil ) - static let sampleOfferings: Offerings = .init( + static let sampleOfferings: Offerings = MockData.makeSampleOfferings() + + /// A fresh `Offerings` instance per call, for tests that need two distinguishable snapshots. + // swiftlint:disable:next function_body_length + static func makeSampleOfferings() -> Offerings { + return .init( offerings: MockData.anyBackendOfferingsContents.response.offerings .map { offering in Offering( @@ -996,7 +1001,8 @@ private extension OfferingsManagerTests { targeting: nil, contents: MockData.anyBackendOfferingsContents, loadedFromDiskCache: false - ) + ) + } } } @@ -1067,8 +1073,7 @@ extension OfferingsManagerTests { func testGetOfferingsAwaitsWorkflowsAndUiConfigConcurrently() { let mockRemoteConfigManager = MockRemoteConfigManager() - // Hold BOTH readiness steps: each must have STARTED before either completes, or one - // failing slowly would serialize behind the other. + // Hold BOTH readiness steps: each must have started before either completes. mockRemoteConfigManager.shouldStoreTopicCompletion = true mockRemoteConfigManager.shouldStoreBlobDataCompletion = true let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) @@ -1133,8 +1138,7 @@ extension OfferingsManagerTests { // The background refresh starts regardless of the gate... expect(self.mockOfferings.invokedGetOfferingsForAppUserID).toEventually(beTrue()) - // ...but even stale cached offerings must not be delivered before config readiness: - // getOfferings returning means the paywall config data is queryable. + // ...but delivery, even from a stale cache, waits for config readiness. expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) expect(delivered.value) == false @@ -1142,6 +1146,28 @@ extension OfferingsManagerTests { expect(delivered.value).toEventually(beTrue()) } + func testGetOfferingsFromStaleCacheDeliversRefreshedOfferingsWhenRefreshWinsTheGate() { + let mockRemoteConfigManager = MockRemoteConfigManager() + mockRemoteConfigManager.shouldStoreTopicCompletion = true + let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) + self.mockDeviceCache.stubbedOfferings = MockData.makeSampleOfferings() + self.mockDeviceCache.stubbedOfferingCacheStatus = .stale + self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) + + let delivered: Atomic = .init(nil) + manager.offerings(appUserID: MockData.anyAppUserID) { result in delivered.value = result.value } + + // While delivery waits on the gate, the background refresh lands fresh offerings. + expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) + let refreshedOfferings = MockData.makeSampleOfferings() + self.mockDeviceCache.stubbedOfferings = refreshedOfferings + + mockRemoteConfigManager.completeStoredTopic() + // The caller waited through the refresh; it must get the fresh snapshot, not the + // stale one captured before the gate. + expect(delivered.value).toEventually(be(refreshedOfferings)) + } + func testGetOfferingsNetworkFetchAwaitsConfigReadyBeforeDelivering() { let mockRemoteConfigManager = MockRemoteConfigManager() let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) diff --git a/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift b/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift index f5fe82e8fa..a9a25d90b2 100644 --- a/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift +++ b/Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift @@ -714,11 +714,11 @@ final class MockRemoteConfigManager: RemoteConfigManagerType { private let _invokedTopicCount: Atomic = .init(0) var invokedTopicCount: Int { return self._invokedTopicCount.value } - /// When `true`, `topic(_:)` suspends until `completeStoredTopic()` is called, so tests can control - /// the timing of a caller awaiting it (e.g. `OfferingsManager`'s config-ready gate). + /// When `true`, `topic(_:)` suspends until `completeStoredTopic()` resumes every stored + /// waiter (there can be several: e.g. a gated delivery plus a background refresh). var shouldStoreTopicCompletion = false - private let _storedTopicContinuation: Atomic?> = - .init(nil) + private let _storedTopicContinuations: Atomic<[CheckedContinuation]> = + .init([]) func topic(_ topic: RemoteConfigTopic) async -> RemoteConfiguration.ConfigTopic? { guard self.shouldStoreTopicCompletion else { @@ -726,26 +726,24 @@ final class MockRemoteConfigManager: RemoteConfigManagerType { return self.stubbedTopics[topic] } return await withCheckedContinuation { continuation in - // Resume any previously stored continuation so it can't leak if `topic(_:)` is called again - // before the prior call's completion is triggered. - self._storedTopicContinuation.getAndSet(continuation)?.resume(returning: nil) - // Increment only after the continuation is stored, so a test polling `invokedTopicCount` - // can never observe readiness before `completeStoredTopic()` has something to resume. + // Store before incrementing, so a test polling `invokedTopicCount` can never + // observe readiness before `completeStoredTopic()` has something to resume. + self._storedTopicContinuations.modify { $0.append(continuation) } self._invokedTopicCount.modify { $0 += 1 } } } - /// Resumes the continuation captured by a `topic(_:)` call made while `shouldStoreTopicCompletion` - /// was `true`. Requires `shouldStoreTopicCompletion == true`. + /// Resumes every waiter held while `shouldStoreTopicCompletion` was `true`, and stops + /// holding subsequent calls. func completeStoredTopic(with result: RemoteConfiguration.ConfigTopic? = nil) { - let continuation = self._storedTopicContinuation.value - self._storedTopicContinuation.value = nil - continuation?.resume(returning: result) + self.shouldStoreTopicCompletion = false + for continuation in self._storedTopicContinuations.getAndSet([]) { + continuation.resume(returning: result) + } } - /// When `true`, `blobData(for:itemKey:)` suspends until `completeStoredBlobReads()` is called, - /// so tests can control the timing of a caller awaiting blob-backed resolution (e.g. - /// `OfferingsManager`'s config-ready gate resolving `ui_config`). + /// When `true`, `blobData(for:itemKey:)` suspends until `completeStoredBlobReads()` is + /// called, so tests can control the timing of a caller resolving blob-backed data. var shouldStoreBlobDataCompletion = false private typealias StoredBlobRead = ( topic: RemoteConfigTopic, itemKey: String, continuation: CheckedContinuation @@ -758,16 +756,14 @@ final class MockRemoteConfigManager: RemoteConfigManagerType { return self.stubbedBlobData[topic]?[itemKey] } return await withCheckedContinuation { continuation in - // Store before recording the invocation, so a test polling the invocation list can - // never observe readiness before `completeStoredBlobReads()` has something to resume. + // Store before recording, so a poller can't observe readiness with nothing to resume. self._storedBlobReads.modify { $0.append((topic, itemKey, continuation)) } self._invokedBlobDataParameters.modify { $0.append((topic, itemKey)) } } } - /// Resumes every blob read held while `shouldStoreBlobDataCompletion` was `true` with the - /// stubbed data for its key, and stops holding subsequent reads, so sequential read chains - /// (like `mergeItemsBlobData`'s loop) run to completion. + /// Resumes every held blob read with its key's stubbed data and stops holding subsequent + /// reads, so sequential read chains (like `mergeItemsBlobData`'s loop) run to completion. func completeStoredBlobReads() { self.shouldStoreBlobDataCompletion = false for read in self._storedBlobReads.getAndSet([]) { From 0da3fc0d90aa544b6c0402fa2280869796267b56 Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Thu, 9 Jul 2026 17:55:27 +0200 Subject: [PATCH 3/9] fix(offerings): never read the shared offerings cache after the readiness gate Convergence-pass findings: the post-gate cache re-read could deliver an unrelated write (identity or locale change repopulating the single shared slot) to a request that started before it. The stale path's refresh now reports its result through its own completion, same-request by construction, and gated delivery prefers that over the captured snapshot; the shared slot is never read after the gate. Two pinning tests: a fresh-path delivery always returns its captured snapshot, and a stale-path delivery can return the captured snapshot or its own refresh result but never an unrelated slot write. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 17 ++++++--- .../Purchasing/OfferingsManagerTests.swift | 37 +++++++++++++++---- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index 84005d57dd..95f84e7c96 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -102,18 +102,23 @@ class OfferingsManager { requestedProductIds: nil, notFoundProductIds: nil) + // When THIS request's stale refresh finishes before the gate opens, deliver its + // result instead of the captured snapshot. Recorded through the refresh's own + // completion, never by re-reading the shared cache slot after the gate: a + // concurrent identity or locale change repopulating that slot must not leak + // another request's offerings into this one. + let refreshedOfferings: Atomic = .init(nil) if cacheStatus == .stale { // The readiness gate below must not delay this background refresh. self.updateOfferingsCache(appUserID: appUserID, isAppBackgrounded: isAppBackgrounded, - fetchPolicy: fetchPolicy, - completion: nil) + fetchPolicy: fetchPolicy) { result in + refreshedOfferings.value = result.value?.offerings + } } - // Cached delivery waits for config readiness (a no-op once config has synced), - // re-reading the cache afterwards: if the stale path's refresh finished while - // waiting, the caller gets the fresh offerings it already paid for. + // Cached delivery waits for config readiness (a no-op once config has synced). self.deliverWhenConfigReady { - let offerings = self.cachedOfferings ?? memoryCachedOfferings + let offerings = refreshedOfferings.value ?? memoryCachedOfferings self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(offerings)) } } diff --git a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift index f5697fb892..bde3d3a020 100644 --- a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift +++ b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift @@ -972,7 +972,6 @@ private extension OfferingsManagerTests { static let sampleOfferings: Offerings = MockData.makeSampleOfferings() /// A fresh `Offerings` instance per call, for tests that need two distinguishable snapshots. - // swiftlint:disable:next function_body_length static func makeSampleOfferings() -> Offerings { return .init( offerings: MockData.anyBackendOfferingsContents.response.offerings @@ -1146,26 +1145,48 @@ extension OfferingsManagerTests { expect(delivered.value).toEventually(beTrue()) } - func testGetOfferingsFromStaleCacheDeliversRefreshedOfferingsWhenRefreshWinsTheGate() { + func testGatedFreshCacheDeliveryNeverReadsTheSharedCacheSlot() { let mockRemoteConfigManager = MockRemoteConfigManager() mockRemoteConfigManager.shouldStoreTopicCompletion = true let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) + let captured = MockData.makeSampleOfferings() + self.mockDeviceCache.stubbedOfferings = captured + self.mockDeviceCache.stubbedOfferingCacheStatus = .valid + + let delivered: Atomic = .init(nil) + manager.offerings(appUserID: MockData.anyAppUserID) { result in delivered.value = result.value } + + // While delivery waits on the gate, an unrelated write (identity change, another + // request) repopulates the shared cache slot. It must not leak into this request. + expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) self.mockDeviceCache.stubbedOfferings = MockData.makeSampleOfferings() + + mockRemoteConfigManager.completeStoredTopic() + expect(delivered.value).toEventually(beIdenticalTo(captured)) + } + + func testGatedStaleCacheDeliveryNeverDeliversAnUnrelatedCacheWrite() { + let mockRemoteConfigManager = MockRemoteConfigManager() + mockRemoteConfigManager.shouldStoreTopicCompletion = true + let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) + let captured = MockData.makeSampleOfferings() + self.mockDeviceCache.stubbedOfferings = captured self.mockDeviceCache.stubbedOfferingCacheStatus = .stale self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) let delivered: Atomic = .init(nil) manager.offerings(appUserID: MockData.anyAppUserID) { result in delivered.value = result.value } - // While delivery waits on the gate, the background refresh lands fresh offerings. + // While delivery waits on the gate, an unrelated write repopulates the slot. expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) - let refreshedOfferings = MockData.makeSampleOfferings() - self.mockDeviceCache.stubbedOfferings = refreshedOfferings + let unrelated = MockData.makeSampleOfferings() + self.mockDeviceCache.stubbedOfferings = unrelated mockRemoteConfigManager.completeStoredTopic() - // The caller waited through the refresh; it must get the fresh snapshot, not the - // stale one captured before the gate. - expect(delivered.value).toEventually(be(refreshedOfferings)) + // Delivery is the captured stale snapshot or this request's own refresh result + // (whichever the scheduler resolves first); the unrelated slot write, never. + expect(delivered.value).toEventuallyNot(beNil()) + expect(delivered.value).toNot(beIdenticalTo(unrelated)) } func testGetOfferingsNetworkFetchAwaitsConfigReadyBeforeDelivering() { From abbc93b5ef746f419a5c1c3e5c1edef57d15792b Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Fri, 10 Jul 2026 08:57:19 +0200 Subject: [PATCH 4/9] fix(offerings): read the recorded refresh result on the main actor Bugbot follow-up on the stale-path delivery: the refresh completion writes its result on the main actor while the gated delivery read it from the gate task, so a refresh that finished first could still be invisible to the delivery. The read now happens inside the delivery's main-actor hop, where the write also lands, so main-queue ordering makes any earlier-enqueued refresh result visible. Delivery remains best-effort by design: the contract gates on config readiness, not on refresh completion. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 65 ++++++++++++++++------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index 95f84e7c96..05d385ca73 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -102,24 +102,53 @@ class OfferingsManager { requestedProductIds: nil, notFoundProductIds: nil) - // When THIS request's stale refresh finishes before the gate opens, deliver its - // result instead of the captured snapshot. Recorded through the refresh's own - // completion, never by re-reading the shared cache slot after the gate: a - // concurrent identity or locale change repopulating that slot must not leak - // another request's offerings into this one. - let refreshedOfferings: Atomic = .init(nil) - if cacheStatus == .stale { - // The readiness gate below must not delay this background refresh. - self.updateOfferingsCache(appUserID: appUserID, - isAppBackgrounded: isAppBackgrounded, - fetchPolicy: fetchPolicy) { result in - refreshedOfferings.value = result.value?.offerings - } - } - // Cached delivery waits for config readiness (a no-op once config has synced). - self.deliverWhenConfigReady { - let offerings = refreshedOfferings.value ?? memoryCachedOfferings - self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(offerings)) + let refreshedOfferings = self.refreshStaleOfferingsInBackground( + cacheStatus: cacheStatus, + appUserID: appUserID, + isAppBackgrounded: isAppBackgrounded, + fetchPolicy: fetchPolicy + ) + self.deliverCachedOfferingsWhenConfigReady(memoryCachedOfferings, + refreshedOfferings: refreshedOfferings, + completion: completion) + } + } + + /// Kicks the background refresh a stale cache requires (before the readiness gate, which + /// must not delay it) and returns where its result lands. Recorded through the refresh's + /// own completion, never by re-reading the shared cache slot after the gate: a concurrent + /// identity or locale change repopulating that slot must not leak another request's + /// offerings into this one. + private func refreshStaleOfferingsInBackground( + cacheStatus: CacheStatus, + appUserID: String, + isAppBackgrounded: Bool, + fetchPolicy: FetchPolicy + ) -> Atomic { + let refreshedOfferings: Atomic = .init(nil) + guard cacheStatus == .stale else { return refreshedOfferings } + + self.updateOfferingsCache(appUserID: appUserID, + isAppBackgrounded: isAppBackgrounded, + fetchPolicy: fetchPolicy) { result in + refreshedOfferings.value = result.value?.offerings + } + return refreshedOfferings + } + + /// Delivers memory-cached offerings once config is ready (a no-op once config has + /// synced), preferring this request's own refresh result when one landed. The recorded + /// result is read on the main actor, where the refresh's completion also writes it, so a + /// refresh whose completion was enqueued first is always visible to this delivery. + private func deliverCachedOfferingsWhenConfigReady( + _ memoryCachedOfferings: Offerings, + refreshedOfferings: Atomic, + completion: (@MainActor @Sendable (Result) -> Void)? + ) { + self.deliverWhenConfigReady { + guard let completion else { return } + self.operationDispatcher.dispatchOnMainActor { + completion(.success(refreshedOfferings.value ?? memoryCachedOfferings)) } } } From eddaa271b1cab7ae6f29342783658eecb42cc518 Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Fri, 10 Jul 2026 09:30:39 +0200 Subject: [PATCH 5/9] docs(offerings): trim gate helper comments Cut the causal-chain narration and the duplicated main-actor-ordering explanation across the two cached-delivery helpers down to the load-bearing constraint each. No behavior change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index 05d385ca73..619f85e87d 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -28,8 +28,7 @@ class OfferingsManager { private let dateProvider: DateProvider // Nil when remote config is disabled, in which case offerings delivery is unchanged. private let remoteConfigManager: RemoteConfigManagerType? - // Derived from `remoteConfigManager`; resolves the `ui_config` body the delivery gate - // waits on. Nil exactly when the manager is. + // Resolves the `ui_config` body the delivery gate waits on. Nil when the manager is. private let uiConfigProvider: UiConfigProvider? init(deviceCache: DeviceCache, @@ -114,11 +113,9 @@ class OfferingsManager { } } - /// Kicks the background refresh a stale cache requires (before the readiness gate, which - /// must not delay it) and returns where its result lands. Recorded through the refresh's - /// own completion, never by re-reading the shared cache slot after the gate: a concurrent - /// identity or locale change repopulating that slot must not leak another request's - /// offerings into this one. + /// Starts the background refresh a stale cache needs and returns where its result lands. + /// Captured from the refresh's own completion, not the shared cache slot, which a + /// concurrent identity/locale change could repopulate with another request's offerings. private func refreshStaleOfferingsInBackground( cacheStatus: CacheStatus, appUserID: String, @@ -136,10 +133,9 @@ class OfferingsManager { return refreshedOfferings } - /// Delivers memory-cached offerings once config is ready (a no-op once config has - /// synced), preferring this request's own refresh result when one landed. The recorded - /// result is read on the main actor, where the refresh's completion also writes it, so a - /// refresh whose completion was enqueued first is always visible to this delivery. + /// Delivers cached offerings once config is ready, preferring this request's own refresh + /// result. Read on the main actor, where the refresh also writes it, so an earlier refresh + /// is visible. private func deliverCachedOfferingsWhenConfigReady( _ memoryCachedOfferings: Offerings, refreshedOfferings: Atomic, From ccc0a64854581a40870b87498d6625a21e69b30e Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Fri, 10 Jul 2026 09:56:48 +0200 Subject: [PATCH 6/9] refactor(offerings): simplify stale delivery to the captured snapshot Per review: drop the opportunistic 'return the refreshed offerings if the background refresh beats the config gate' behavior. It was never required by the contract (which gates on config readiness, not snapshot freshness) and was the sole reason for the Atomic plumbing, the main-actor read-ordering, and the race findings that followed. Stale delivery now returns the snapshot captured for the request, matching the pre-existing stale-cache model; the background refresh still updates the cache for the next call. UiConfigProvider is stateless, so it's created in the gate instead of stored. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 70 ++++--------------- .../Purchasing/OfferingsManagerTests.swift | 14 ++-- 2 files changed, 21 insertions(+), 63 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index 619f85e87d..535673c0f5 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -28,8 +28,6 @@ class OfferingsManager { private let dateProvider: DateProvider // Nil when remote config is disabled, in which case offerings delivery is unchanged. private let remoteConfigManager: RemoteConfigManagerType? - // Resolves the `ui_config` body the delivery gate waits on. Nil when the manager is. - private let uiConfigProvider: UiConfigProvider? init(deviceCache: DeviceCache, operationDispatcher: OperationDispatcher, @@ -49,7 +47,6 @@ class OfferingsManager { self.diagnosticsTracker = diagnosticsTracker self.dateProvider = dateProvider self.remoteConfigManager = remoteConfigManager - self.uiConfigProvider = remoteConfigManager.map { UiConfigProvider(manager: $0) } } func offerings( @@ -101,50 +98,19 @@ class OfferingsManager { requestedProductIds: nil, notFoundProductIds: nil) - let refreshedOfferings = self.refreshStaleOfferingsInBackground( - cacheStatus: cacheStatus, - appUserID: appUserID, - isAppBackgrounded: isAppBackgrounded, - fetchPolicy: fetchPolicy - ) - self.deliverCachedOfferingsWhenConfigReady(memoryCachedOfferings, - refreshedOfferings: refreshedOfferings, - completion: completion) - } - } - - /// Starts the background refresh a stale cache needs and returns where its result lands. - /// Captured from the refresh's own completion, not the shared cache slot, which a - /// concurrent identity/locale change could repopulate with another request's offerings. - private func refreshStaleOfferingsInBackground( - cacheStatus: CacheStatus, - appUserID: String, - isAppBackgrounded: Bool, - fetchPolicy: FetchPolicy - ) -> Atomic { - let refreshedOfferings: Atomic = .init(nil) - guard cacheStatus == .stale else { return refreshedOfferings } - - self.updateOfferingsCache(appUserID: appUserID, - isAppBackgrounded: isAppBackgrounded, - fetchPolicy: fetchPolicy) { result in - refreshedOfferings.value = result.value?.offerings - } - return refreshedOfferings - } - - /// Delivers cached offerings once config is ready, preferring this request's own refresh - /// result. Read on the main actor, where the refresh also writes it, so an earlier refresh - /// is visible. - private func deliverCachedOfferingsWhenConfigReady( - _ memoryCachedOfferings: Offerings, - refreshedOfferings: Atomic, - completion: (@MainActor @Sendable (Result) -> Void)? - ) { - self.deliverWhenConfigReady { - guard let completion else { return } - self.operationDispatcher.dispatchOnMainActor { - completion(.success(refreshedOfferings.value ?? memoryCachedOfferings)) + if cacheStatus == .stale { + // Refresh in the background; the readiness gate below must not delay it. + self.updateOfferingsCache(appUserID: appUserID, + isAppBackgrounded: isAppBackgrounded, + fetchPolicy: fetchPolicy, + completion: nil) + } + // Cached offerings, stale ones included, wait for config readiness before + // delivery (a no-op once config has synced). A stale snapshot may be returned + // even if the background refresh finished first; that matches the stale-cache + // model, and the refresh still updates the cache for the next call. + self.deliverWhenConfigReady { + self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(memoryCachedOfferings)) } } } @@ -414,16 +380,10 @@ private extension OfferingsManager { } } - /// Runs `deliver` once remote config has synced at least once *and* every workflow flagged for - /// prefetch has finished downloading (or failed), when remote config is wired. Matches Android's - /// guarantee that offerings delivery waits for every prefetched workflow body, not just the - /// `workflows` topic's metadata. The manager's `awaitTopicAndPrefetchBlobsReady()` read no-ops when already synced - /// and always calls back, so this never hangs. When no manager is wired (workflows disabled), - /// `deliver` runs immediately, leaving offerings unchanged. /// Invokes `deliver` once the paywall config data `getOfferings` depends on is ready: /// the `workflows` topic (with its prefetch-flagged blobs) and the `ui_config` body, /// resolved concurrently. Both are best-effort (nil on failure, never throwing), so - /// delivery can never be stranded. + /// delivery can never be stranded; when no manager is wired, `deliver` runs immediately. private func deliverWhenConfigReady(deliver: @escaping () -> Void) { guard let remoteConfigManager = self.remoteConfigManager else { deliver() @@ -431,7 +391,7 @@ private extension OfferingsManager { } Task { async let workflowsReady = remoteConfigManager.awaitTopicAndPrefetchBlobsReady(.workflows) - async let uiConfigReady = self.uiConfigProvider?.getUiConfig() + async let uiConfigReady = UiConfigProvider(manager: remoteConfigManager).getUiConfig() _ = await (workflowsReady, uiConfigReady) deliver() } diff --git a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift index bde3d3a020..b3c1a687c8 100644 --- a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift +++ b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift @@ -1165,7 +1165,7 @@ extension OfferingsManagerTests { expect(delivered.value).toEventually(beIdenticalTo(captured)) } - func testGatedStaleCacheDeliveryNeverDeliversAnUnrelatedCacheWrite() { + func testGatedStaleCacheDeliversTheCapturedSnapshotNotALaterCacheWrite() { let mockRemoteConfigManager = MockRemoteConfigManager() mockRemoteConfigManager.shouldStoreTopicCompletion = true let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) @@ -1177,16 +1177,14 @@ extension OfferingsManagerTests { let delivered: Atomic = .init(nil) manager.offerings(appUserID: MockData.anyAppUserID) { result in delivered.value = result.value } - // While delivery waits on the gate, an unrelated write repopulates the slot. + // While delivery waits on the gate, the background refresh writes a new slot value. expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0)) - let unrelated = MockData.makeSampleOfferings() - self.mockDeviceCache.stubbedOfferings = unrelated + self.mockDeviceCache.stubbedOfferings = MockData.makeSampleOfferings() mockRemoteConfigManager.completeStoredTopic() - // Delivery is the captured stale snapshot or this request's own refresh result - // (whichever the scheduler resolves first); the unrelated slot write, never. - expect(delivered.value).toEventuallyNot(beNil()) - expect(delivered.value).toNot(beIdenticalTo(unrelated)) + // Stale delivery returns the snapshot captured for this request, never a later + // cache write (the refresh's, or another request's). + expect(delivered.value).toEventually(beIdenticalTo(captured)) } func testGetOfferingsNetworkFetchAwaitsConfigReadyBeforeDelivering() { From c81c1e79613377754441b4d978d97c88572b2401 Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Fri, 10 Jul 2026 10:22:16 +0200 Subject: [PATCH 7/9] perf(offerings): skip the readiness gate on background cache refreshes A stale-path background refresh passes a nil completion: it only updates the cache, with nothing to deliver. It still went through deliverWhenConfigReady, which awaits (and decodes) workflows + ui_config for a no-op dispatch. Skip the gate when there is no completion, so background refreshes don't pay the readiness wait or the ui_config decode. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index 535673c0f5..f44b7ca2be 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -323,6 +323,9 @@ private extension OfferingsManager { preferredLocales: preferredLocales, appUserID: appUserID) + // A background refresh (nil completion) only updates the cache; skip the + // readiness gate so it doesn't await (and decode) config for a no-op delivery. + guard let completion else { return } // Delivers offerings only once remote config has synced at least once, so the // `workflows` topic is available for resolving a workflow right after this call. self.deliverWhenConfigReady { From e408849e9b49b64a55e42ed14d58f08bd60e9f17 Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Fri, 10 Jul 2026 10:28:08 +0200 Subject: [PATCH 8/9] fix(offerings): record cached-path diagnostics after the readiness gate The cached getOfferings diagnostic fired before deliverWhenConfigReady, so its recorded latency excluded the readiness wait this PR adds, making a gated cache hit look as fast as an ungated one. Move the tracking into the gated delivery so the metric reflects the true latency. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- Sources/Purchasing/OfferingsManager.swift | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Sources/Purchasing/OfferingsManager.swift b/Sources/Purchasing/OfferingsManager.swift index f44b7ca2be..56c0235adf 100644 --- a/Sources/Purchasing/OfferingsManager.swift +++ b/Sources/Purchasing/OfferingsManager.swift @@ -91,13 +91,6 @@ class OfferingsManager { let cacheStatus = self.deviceCache.offeringsCacheStatus(isAppBackgrounded: isAppBackgrounded) Logger.debug(Strings.offering.vending_offerings_cache_from_memory) - self.trackGetOfferingsResultIfNeeded(trackDiagnostics: trackDiagnostics, - startTime: startTime, - cacheStatus: cacheStatus, - error: nil, - requestedProductIds: nil, - notFoundProductIds: nil) - if cacheStatus == .stale { // Refresh in the background; the readiness gate below must not delay it. self.updateOfferingsCache(appUserID: appUserID, @@ -110,6 +103,14 @@ class OfferingsManager { // even if the background refresh finished first; that matches the stale-cache // model, and the refresh still updates the cache for the next call. self.deliverWhenConfigReady { + // Tracked inside the gate so the recorded latency includes the readiness + // wait this delivery now pays, not just the cache lookup. + self.trackGetOfferingsResultIfNeeded(trackDiagnostics: trackDiagnostics, + startTime: startTime, + cacheStatus: cacheStatus, + error: nil, + requestedProductIds: nil, + notFoundProductIds: nil) self.dispatchCompletionOnMainThreadIfPossible(completion, value: .success(memoryCachedOfferings)) } } From c8d5b097427790045f405248ef7f8142d6e7f976 Mon Sep 17 00:00:00 2001 From: Facundo Menzella Date: Fri, 10 Jul 2026 11:00:54 +0200 Subject: [PATCH 9/9] test(offerings): cover background-refresh gate skip and best-effort delivery Review follow-ups from ajpallares on #7181: a test locking that a background refresh (nil completion) caches without entering the readiness gate; an assertion on the delivered offerings in the ui_config-failure path; and a test pinning that delivery still fires when both readiness branches resolve empty/failed (the non-throwing, always-awaited guarantee). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UXR3hncr2daCqrnXdU5N4H --- .../Purchasing/OfferingsManagerTests.swift | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift index b3c1a687c8..1eef45eec1 100644 --- a/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift +++ b/Tests/UnitTests/Purchasing/OfferingsManagerTests.swift @@ -1059,7 +1059,7 @@ extension OfferingsManagerTests { func testGetOfferingsDeliversWhenUiConfigResolutionFails() { // No ui_config blobs stubbed: resolution fails (nil). Readiness is best-effort, so - // delivery must proceed anyway. + // delivery must proceed anyway with the real offerings. let manager = self.makeOfferingsManager(remoteConfigManager: MockRemoteConfigManager()) self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) @@ -1068,6 +1068,41 @@ extension OfferingsManagerTests { } expect(result).to(beSuccess()) + expect(result?.value?["base"]).toNot(beNil()) + expect(result?.value?["base"]?.monthly?.storeProduct).toNot(beNil()) + } + + func testGetOfferingsDeliversEvenIfTheGateTaskIsCancelled() { + // The gate's two readiness awaits are non-throwing and always awaited before the + // callback fires, so no cancellation can strand it. Model that here by resolving both + // to their empty/failed states (no workflows topic, no ui_config) and asserting the + // completion still runs. + let manager = self.makeOfferingsManager(remoteConfigManager: MockRemoteConfigManager()) + self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) + + let result = waitUntilValue { completed in + manager.offerings(appUserID: MockData.anyAppUserID) { completed($0) } + } + + expect(result).to(beSuccess()) + } + + func testBackgroundCacheRefreshCachesWithoutAwaitingTheConfigGate() { + // A background refresh passes a nil completion: it has nothing to deliver, so it must + // cache the fetched offerings without entering the readiness gate. The held topic + // would block the gate forever; asserting the cache still lands (and the gate's topic + // read never happens) locks the skip. + let mockRemoteConfigManager = MockRemoteConfigManager() + mockRemoteConfigManager.shouldStoreTopicCompletion = true + let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager) + self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents) + + manager.updateOfferingsCache(appUserID: MockData.anyAppUserID, + isAppBackgrounded: false, + completion: nil) + + expect(self.mockDeviceCache.cacheOfferingsCount).toEventually(equal(1)) + expect(mockRemoteConfigManager.invokedTopicCount) == 0 } func testGetOfferingsAwaitsWorkflowsAndUiConfigConcurrently() {