diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj index 01d0071f8..1a57c17ae 100644 --- a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj +++ b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj @@ -163,6 +163,7 @@ 3CA6CE0A28E4F19B00CA0585 /* OSUserRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA6CE0928E4F19B00CA0585 /* OSUserRequest.swift */; }; 3CA8B8822BEC2FCB0010ADA1 /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3C7A39D42B7C18EE0082665E /* XCTest.framework */; }; 3CA8B8832BEC2FCB0010ADA1 /* XCTest.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 3C7A39D42B7C18EE0082665E /* XCTest.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; + 3CA93BC4300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */; }; 3CAA4BB72F0BAFBA00A16682 /* TriggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CAA4BB62F0BAFBA00A16682 /* TriggerTests.swift */; }; 3CB331682F281679000E1801 /* CustomEventsIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CB331672F281679000E1801 /* CustomEventsIntegrationTests.swift */; }; 3CB3316A2F281692000E1801 /* OSCustomEventsExecutorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */; }; @@ -1403,6 +1404,7 @@ 3C9AD6D02B228B9200BC1540 /* OSRequestRemoveAlias.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRequestRemoveAlias.swift; sourceTree = ""; }; 3C9AD6D22B228BB000BC1540 /* OSRequestUpdateProperties.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRequestUpdateProperties.swift; sourceTree = ""; }; 3CA6CE0928E4F19B00CA0585 /* OSUserRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSUserRequest.swift; sourceTree = ""; }; + 3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionUpdateRaceTests.swift; sourceTree = ""; }; 3CAA4BB62F0BAFBA00A16682 /* TriggerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TriggerTests.swift; sourceTree = ""; }; 3CB331672F281679000E1801 /* CustomEventsIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomEventsIntegrationTests.swift; sourceTree = ""; }; 3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSCustomEventsExecutorTests.swift; sourceTree = ""; }; @@ -2379,6 +2381,7 @@ isa = PBXGroup; children = ( 3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */, + 3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */, 3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */, ); path = Executors; @@ -4471,6 +4474,7 @@ 3CC063EE2B6D7FE8002BB07F /* OneSignalUserTests.swift in Sources */, 3CC890352C5BF9A7002CB4CC /* UserConcurrencyTests.swift in Sources */, 3CB3316A2F281692000E1801 /* OSCustomEventsExecutorTests.swift in Sources */, + 3CA93BC4300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift in Sources */, 3CDE664C2BFC2A56006DA114 /* OneSignalUserObjcTests.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift index 18e66de80..d2253c9d4 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift @@ -211,10 +211,12 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor { self.removeRequestQueue.append(request) case OS_UPDATE_SUBSCRIPTION_DELTA: - let request = OSRequestUpdateSubscription( - subscriptionObject: [delta.property: delta.value], - subscriptionModel: subModel - ) + // Keep at most one unsent UpdateSubscription per subscription (last wins). + let modelId = subModel.modelId + self.updateRequestQueue.removeAll { request in + !request.sentToClient && request.subscriptionModel.modelId == modelId + } + let request = OSRequestUpdateSubscription(subscriptionModel: subModel) self.updateRequestQueue.append(request) default: diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift index 9edba5375..3f211fad5 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift @@ -47,38 +47,34 @@ class OSRequestUpdateSubscription: OneSignalRequest, OSUserRequest { let appId = OneSignalIdentifiers.currentAppId { self.path = "apps/\(appId)/subscriptions/\(subscriptionId)" + // Refresh so a stale snapshot queued earlier can't overwrite newer local state. + refreshParametersFromLiveModel() return true } else { return false } } - // TODO: just need the sub model and send it - // But the model may be outdated or not sync with the subscriptionObject - init(subscriptionObject: [String: Any], subscriptionModel: OSSubscriptionModel) { - self.subscriptionModel = subscriptionModel - self.stringDescription = "OSRequestUpdateSubscription with subscriptionObject: \(subscriptionObject)" - super.init() - - // Rename "address" key as "token", if it exists - var subscriptionParams = subscriptionObject - subscriptionParams.removeValue(forKey: "address") - subscriptionParams.removeValue(forKey: "notificationTypes") + /// Rebuild the PATCH body from the current subscription model. + func refreshParametersFromLiveModel() { + var subscriptionParams: [String: Any] = [:] subscriptionParams["token"] = subscriptionModel.address subscriptionParams["device_os"] = subscriptionModel.deviceOs subscriptionParams["sdk"] = subscriptionModel.sdk subscriptionParams["app_version"] = subscriptionModel.appVersion - // notificationTypes defaults to -1 instead of nil, don't send if it's -1 if subscriptionModel.notificationTypes != -1 { subscriptionParams["notification_types"] = subscriptionModel.notificationTypes } - subscriptionParams["enabled"] = subscriptionModel.enabled - // TODO: The above is not quite right. If we hydrate, we will over-write any pending updates - // May use subscriptionObject, but enabled and notification_types should be sent together... - self.parameters = ["subscription": subscriptionParams] + } + + init(subscriptionModel: OSSubscriptionModel) { + self.subscriptionModel = subscriptionModel + self.stringDescription = "OSRequestUpdateSubscription with model: \(subscriptionModel.modelId)" + super.init() + refreshParametersFromLiveModel() self.method = PATCH } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionUpdateRaceTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionUpdateRaceTests.swift new file mode 100644 index 000000000..88c7a014c --- /dev/null +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionUpdateRaceTests.swift @@ -0,0 +1,150 @@ +/* + Modified MIT License + + Copyright 2026 OneSignal + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + 1. The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + 2. All copies of substantial portions of the Software may only be used in connection + with services provided by OneSignal. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + */ + +import XCTest +import OneSignalCore +import OneSignalCoreMocks +import OneSignalUserMocks +@testable import OneSignalOSCore +@testable import OneSignalUser + +/** + Regression coverage for stale UpdateSubscription races after permission grant. + */ +final class SubscriptionUpdateRaceTests: XCTestCase { + + private let subscriptionId = "test-subscription-id" + private let pushToken = "test-push-token" + /// Typical iOS alert+badge+sound permission bitmask after Accept. + private let subscribedNotificationTypes = 31 + private let promptedNeverAnswered = Int(ERROR_PUSH_PROMPT_NEVER_ANSWERED) + + override func setUpWithError() throws { + OneSignalCoreMocks.clearUserDefaults() + OneSignalUserMocks.reset() + OneSignalIdentifiers.currentAppId = "test-app-id" + OneSignalLog.setLogLevel(.LL_VERBOSE) + } + + override func tearDownWithError() throws { } + + /** + Parameters are built from the live model at init and refreshed again in prepareForExecution before send + */ + func testPrepareForExecutionRefreshesEnabledAndNotificationTypesFromLiveModel() throws { + let model = makePushSubscriptionModel(notificationTypes: promptedNeverAnswered, subscriptionId: subscriptionId) + XCTAssertFalse(model.enabled) + + let request = OSRequestUpdateSubscription(subscriptionModel: model) + let atInit = try XCTUnwrap(request.parameters?["subscription"] as? [String: Any]) + XCTAssertEqual(atInit["notification_types"] as? Int, promptedNeverAnswered) + XCTAssertEqual(atInit["enabled"] as? Bool, false) + + // Permission granted — live model is now subscribed. + model.notificationTypes = subscribedNotificationTypes + XCTAssertTrue(model.enabled) + + XCTAssertTrue(request.prepareForExecution(newRecordsState: OSNewRecordsState())) + + let refreshed = try XCTUnwrap(request.parameters?["subscription"] as? [String: Any]) + XCTAssertEqual(refreshed["notification_types"] as? Int, subscribedNotificationTypes) + XCTAssertEqual(refreshed["enabled"] as? Bool, true) + } + + /** + Regression: a pre-permission UpdateSubscription left pending must not send `-19` + after permission is granted. Coalesce + live refresh should yield only subscribed payload(s). + */ + func testPendingPrePermissionUpdateSendsLiveSubscribedPayloadAfterGrant() throws { + let client = MockOneSignalClient() + client.executeInstantaneously = false + client.fireSuccessForAllRequests = true + OneSignalCoreImpl.setSharedClient(client) + + let executor = OSSubscriptionOperationExecutor(newRecordsState: OSNewRecordsState()) + // Without a subscriptionId, prepareForExecution keeps the update pending. + let model = makePushSubscriptionModel(notificationTypes: promptedNeverAnswered, subscriptionId: nil) + 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) + + XCTAssertTrue(client.executedRequests.isEmpty, "Update should still be pending without subscriptionId") + + // User accepts — live model subscribed — then hydrate subscription id and flush again. + model.notificationTypes = subscribedNotificationTypes + model.subscriptionId = subscriptionId + 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 } + XCTAssertFalse(updateRequests.isEmpty, "Expected at least one UpdateSubscription after id hydration") + + let payloads = updateRequests.compactMap { $0.parameters?["subscription"] as? [String: Any] } + let sentMinus19 = payloads.contains { + ($0["notification_types"] as? Int) == promptedNeverAnswered && ($0["enabled"] as? Bool) == false + } + let allSubscribed = payloads.allSatisfy { + ($0["notification_types"] as? Int) == subscribedNotificationTypes && ($0["enabled"] as? Bool) == true + } + + XCTAssertFalse(sentMinus19, "Must not send stale enabled:false / notification_types:-19 after grant") + XCTAssertTrue(allSubscribed, "All executed UpdateSubscription payloads should reflect the live subscribed model") + XCTAssertEqual(updateRequests.count, 1, "Unsent updates for the same subscription should be coalesced") + } + + // MARK: - Helpers + + private func makePushSubscriptionModel(notificationTypes: Int, subscriptionId: String?) -> OSSubscriptionModel { + let model = OSSubscriptionModel( + type: .push, + address: pushToken, + subscriptionId: subscriptionId, + reachable: notificationTypes > 0, + isDisabled: false, + changeNotifier: OSEventProducer() + ) + model.notificationTypes = notificationTypes + return model + } +}