other(offerings): gate getOfferings delivery on ui_config and stale-cache paths

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(
@@ -98,25 +102,25 @@ 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<Offerings?> = .init(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))
+                // The readiness gate below must not delay this background refresh.
                 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))
+                                          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))
+            }
         }
     }
 
@@ -391,13 +395,19 @@ 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 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()
             return
         }
         Task {
-            _ = await remoteConfigManager.awaitTopicAndPrefetchBlobsReady(.workflows)
+            async let workflowsReady = remoteConfigManager.awaitTopicAndPrefetchBlobsReady(.workflows)
+            async let uiConfigReady = self.uiConfigProvider?.getUiConfig()
+            _ = await (workflowsReady, uiConfigReady)
             deliver()
         }
     }

Tests/UnitTests/Purchasing/OfferingsManagerTests.swift

@@ -969,7 +969,11 @@ 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.
+        static func makeSampleOfferings() -> Offerings {
+            return .init(
             offerings: MockData.anyBackendOfferingsContents.response.offerings
                 .map { offering in
                     Offering(
@@ -996,7 +1000,8 @@ private extension OfferingsManagerTests {
             targeting: nil,
             contents: MockData.anyBackendOfferingsContents,
             loadedFromDiskCache: false
-        )
+            )
+        }
     }
 
 }
@@ -1034,6 +1039,57 @@ 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<Bool> = 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.
+        mockRemoteConfigManager.shouldStoreTopicCompletion = true
+        mockRemoteConfigManager.shouldStoreBlobDataCompletion = true
+        let manager = self.makeOfferingsManager(remoteConfigManager: mockRemoteConfigManager)
+        self.mockOfferings.stubbedGetOfferingsCompletionResult = .success(MockData.anyBackendOfferingsContents)
+
+        let delivered: Atomic<Bool> = 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 +1123,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 +1135,58 @@ extension OfferingsManagerTests {
         let delivered: Atomic<Bool> = false
         manager.offerings(appUserID: MockData.anyAppUserID) { _ in delivered.value = true }
 
-        // Cached offerings are delivered immediately, not blocked on the held topic completion.
+        // The background refresh starts regardless of the gate...
+        expect(self.mockOfferings.invokedGetOfferingsForAppUserID).toEventually(beTrue())
+        // ...but delivery, even from a stale cache, waits for config readiness.
+        expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0))
+        expect(delivered.value) == false
+
+        mockRemoteConfigManager.completeStoredTopic()
         expect(delivered.value).toEventually(beTrue())
+    }
+
+    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<Offerings?> = .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))
+    }
 
-        // Drain the background refresh's config-ready wait so its `Task` doesn't leak past this test.
+    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<Offerings?> = .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.
         expect(mockRemoteConfigManager.invokedTopicCount).toEventually(beGreaterThan(0))
+        let unrelated = MockData.makeSampleOfferings()
+        self.mockDeviceCache.stubbedOfferings = unrelated
+
         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))
     }
 
     func testGetOfferingsNetworkFetchAwaitsConfigReadyBeforeDelivering() {

Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift

@@ -714,38 +714,61 @@ final class MockRemoteConfigManager: RemoteConfigManagerType {
 
     private let _invokedTopicCount: Atomic<Int> = .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<CheckedContinuation<RemoteConfiguration.ConfigTopic?, Never>?> =
-        .init(nil)
+    private let _storedTopicContinuations: Atomic<[CheckedContinuation<RemoteConfiguration.ConfigTopic?, Never>]> =
+        .init([])
 
     func topic(_ topic: RemoteConfigTopic) async -> RemoteConfiguration.ConfigTopic? {
         guard self.shouldStoreTopicCompletion else {
             self._invokedTopicCount.modify { $0 += 1 }
             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 resolving blob-backed data.
+    var shouldStoreBlobDataCompletion = false
+    private typealias StoredBlobRead = (
+        topic: RemoteConfigTopic, itemKey: String, continuation: CheckedContinuation<Data?, Never>
+    )
+    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, 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 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([]) {
+            read.continuation.resume(returning: self.stubbedBlobData[read.topic]?[read.itemKey])
+        }
     }
 
     func blobData<T: Decodable>(