fix: [SDK-4874] single-flight UpdateSubscription to stop in-flight PATCH races

Gate concurrent updates per subscription model, drain pending after completion, and clear sentToClient on retryable failure so the gate does not stick.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: nan-li

iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift

@@ -36,9 +36,15 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
     public var lastHTTPRequest: OneSignalRequest?
     public var networkRequestCount = 0
     public var executedRequests: [OneSignalRequest] = []
+    /// Requests that have entered `execute` (including those still held / delayed).
+    public private(set) var startedRequests: [OneSignalRequest] = []
     public var executeInstantaneously = false
     /// Set to true to make it unnecessary to setup mock responses for every request possible
     public var fireSuccessForAllRequests = false
+    /// When true, `execute` records the request but does not complete until `releaseHeldResponses()`.
+    public var holdResponses = false
+
+    private var heldExecutions: [(request: OneSignalRequest, onSuccess: OSResultSuccessBlock, onFailure: OSClientFailureBlock)] = []
 
     var remoteParamsResponse: [String: Any]?
     var shouldUseProvisionalAuthorization = false // new in iOS 12 (aka Direct to History)
@@ -84,6 +90,9 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
         lastHTTPRequest = nil
         networkRequestCount = 0
         executedRequests.removeAll()
+        startedRequests.removeAll()
+        heldExecutions.removeAll()
+        holdResponses = false
         executeInstantaneously = true
         remoteParamsResponse = nil
         shouldUseProvisionalAuthorization = false
@@ -93,6 +102,17 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
     public func execute(_ request: OneSignalRequest, onSuccess successBlock: @escaping OSResultSuccessBlock, onFailure failureBlock: @escaping OSClientFailureBlock) {
         print("🧪 MockOneSignalClient execute called")
 
+        lock.withLock {
+            startedRequests.append(request)
+        }
+
+        if holdResponses {
+            lock.withLock {
+                heldExecutions.append((request, successBlock, failureBlock))
+            }
+            return
+        }
+
         if executeInstantaneously {
             finishExecutingRequest(request, onSuccess: successBlock, onFailure: failureBlock)
         } else {
@@ -102,6 +122,18 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
         }
     }
 
+    /// Completes every request currently held by `holdResponses`.
+    public func releaseHeldResponses() {
+        let held: [(request: OneSignalRequest, onSuccess: OSResultSuccessBlock, onFailure: OSClientFailureBlock)] = lock.withLock {
+            let copy = heldExecutions
+            heldExecutions.removeAll()
+            return copy
+        }
+        for item in held {
+            finishExecutingRequest(item.request, onSuccess: item.onSuccess, onFailure: item.onFailure)
+        }
+    }
+
     /// Helper method to stringify the name of a request for identification and comparison
     private func stringify(_ request: OneSignalRequest) -> String {
         var stringified = request.description

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift

@@ -397,6 +397,12 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
         guard !request.sentToClient else {
             return
         }
+        // Single-flight: don't send a second UpdateSubscription for the same model while one is in flight.
+        // A later coalesced request stays queued and is drained when the in-flight one finishes.
+        let modelId = request.subscriptionModel.modelId
+        guard !updateRequestQueue.contains(where: { $0 !== request && $0.sentToClient && $0.subscriptionModel.modelId == modelId }) else {
+            return
+        }
         guard request.prepareForExecution(newRecordsState: newRecordsState) else {
             return
         }
@@ -413,6 +419,7 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
             self.dispatchQueue.async {
                 self.updateRequestQueue.removeAll(where: { $0 == request})
                 OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
+                self.executeNextPendingUpdateSubscription(for: modelId, inBackground: inBackground)
                 if inBackground {
                     OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
                 }
@@ -440,11 +447,27 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
                     // Fail, no retry, remove from cache and queue
                     self.updateRequestQueue.removeAll(where: { $0 == request})
                     OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue)
+                    self.executeNextPendingUpdateSubscription(for: modelId, inBackground: inBackground)
+                } else {
+                    // Make the request eligible for the next flush
+                    request.sentToClient = false
                 }
                 if inBackground {
                     OSBackgroundTaskManager.endBackgroundTask(backgroundTaskIdentifier)
                 }
             }
         }
     }
+
+}
+
+extension OSSubscriptionOperationExecutor {
+    /// Sends the oldest unsent UpdateSubscription for `modelId`, if any. Caller must be on `dispatchQueue`.
+    private func executeNextPendingUpdateSubscription(for modelId: String, inBackground: Bool) {
+        let pending = updateRequestQueue.filter { !$0.sentToClient && $0.subscriptionModel.modelId == modelId }
+        guard let next = pending.min(by: { $0.timestamp < $1.timestamp }) else {
+            return
+        }
+        executeUpdateSubscriptionRequest(next, inBackground: inBackground)
+    }
 }

iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionUpdateRaceTests.swift

@@ -133,6 +133,119 @@ final class SubscriptionUpdateRaceTests: XCTestCase {
         XCTAssertEqual(updateRequests.count, 1, "Unsent updates for the same subscription should be coalesced")
     }
 
+    /**
+     An UpdateSubscription already on the wire must not race a follow-up PATCH.
+     The follow-up stays queued until the in-flight request completes, then sends live state.
+     */
+    func testInFlightUpdateBlocksFollowUpUntilCompleteThenSendsLiveState() throws {
+        let client = MockOneSignalClient()
+        client.holdResponses = true
+        client.fireSuccessForAllRequests = true
+        OneSignalCoreImpl.setSharedClient(client)
+
+        let executor = OSSubscriptionOperationExecutor(newRecordsState: OSNewRecordsState())
+        let model = makePushSubscriptionModel(notificationTypes: promptedNeverAnswered, subscriptionId: subscriptionId)
+        let identityModelId = UUID().uuidString
+
+        executor.enqueueDelta(OSDelta(
+            name: OS_UPDATE_SUBSCRIPTION_DELTA,
+            identityModelId: identityModelId,
+            model: model,
+            property: "notificationTypes",
+            value: promptedNeverAnswered
+        ))
+        executor.processDeltaQueue(inBackground: false)
+        OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.2)
+
+        XCTAssertEqual(client.startedRequests.count, 1, "First UpdateSubscription should be in flight")
+        let firstPayload = try XCTUnwrap(
+            (client.startedRequests[0] as? OSRequestUpdateSubscription)?.parameters?["subscription"] as? [String: Any]
+        )
+        XCTAssertEqual(firstPayload["notification_types"] as? Int, promptedNeverAnswered)
+        XCTAssertEqual(firstPayload["enabled"] as? Bool, false)
+
+        // Permission granted while first PATCH is still in flight.
+        model.notificationTypes = subscribedNotificationTypes
+        XCTAssertTrue(model.enabled)
+
+        executor.enqueueDelta(OSDelta(
+            name: OS_UPDATE_SUBSCRIPTION_DELTA,
+            identityModelId: identityModelId,
+            model: model,
+            property: "notificationTypes",
+            value: subscribedNotificationTypes
+        ))
+        executor.processDeltaQueue(inBackground: false)
+        OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.2)
+
+        XCTAssertEqual(client.startedRequests.count, 1, "Follow-up must wait for in-flight UpdateSubscription")
+
+        client.holdResponses = false
+        client.releaseHeldResponses()
+        OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
+
+        XCTAssertEqual(client.startedRequests.count, 2, "Pending follow-up should send after in-flight completes")
+        let secondPayload = try XCTUnwrap(
+            (client.startedRequests[1] as? OSRequestUpdateSubscription)?.parameters?["subscription"] as? [String: Any]
+        )
+        XCTAssertEqual(secondPayload["notification_types"] as? Int, subscribedNotificationTypes)
+        XCTAssertEqual(secondPayload["enabled"] as? Bool, true)
+    }
+
+    /**
+     A retryable failure (e.g. 500/timeout) must not leave the single-flight gate locked.
+     The failed request becomes resendable and later updates for the model still go out.
+     */
+    func testRetryableFailureDoesNotBlockSubsequentUpdates() throws {
+        let client = MockOneSignalClient()
+        client.fireSuccessForAllRequests = true
+        OneSignalCoreImpl.setSharedClient(client)
+
+        let executor = OSSubscriptionOperationExecutor(newRecordsState: OSNewRecordsState())
+        let model = makePushSubscriptionModel(notificationTypes: promptedNeverAnswered, subscriptionId: subscriptionId)
+        let identityModelId = UUID().uuidString
+
+        // Fail the first update with a retryable error (mock responses are keyed by request description).
+        let requestKey = "OSRequestUpdateSubscription with model: \(model.modelId)"
+        client.setMockFailureResponseForRequest(
+            request: requestKey,
+            error: OneSignalClientError(code: 500, message: "retryable", responseHeaders: nil, response: nil, underlyingError: nil)
+        )
+
+        executor.enqueueDelta(OSDelta(
+            name: OS_UPDATE_SUBSCRIPTION_DELTA,
+            identityModelId: identityModelId,
+            model: model,
+            property: "notificationTypes",
+            value: promptedNeverAnswered
+        ))
+        executor.processDeltaQueue(inBackground: false)
+        OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
+
+        XCTAssertEqual(client.executedRequests.count, 1, "First update should have been attempted and failed retryably")
+
+        // Server recovers; user accepts permission.
+        client.setMockResponseForRequest(request: requestKey, response: [:])
+        model.notificationTypes = subscribedNotificationTypes
+        XCTAssertTrue(model.enabled)
+
+        executor.enqueueDelta(OSDelta(
+            name: OS_UPDATE_SUBSCRIPTION_DELTA,
+            identityModelId: identityModelId,
+            model: model,
+            property: "notificationTypes",
+            value: subscribedNotificationTypes
+        ))
+        executor.processDeltaQueue(inBackground: false)
+        OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
+
+        let updateRequests = client.executedRequests.compactMap { $0 as? OSRequestUpdateSubscription }
+        XCTAssertEqual(updateRequests.count, 2, "Follow-up update must still send after a retryable failure")
+        let lastPayload = try XCTUnwrap(updateRequests.last?.parameters?["subscription"] as? [String: Any])
+        XCTAssertEqual(lastPayload["notification_types"] as? Int, subscribedNotificationTypes)
+        XCTAssertEqual(lastPayload["enabled"] as? Bool, true)
+    }
+
     // MARK: - Helpers
 
     private func makePushSubscriptionModel(notificationTypes: Int, subscriptionId: String?) -> OSSubscriptionModel {