Skip to content

Commit 7300e55

Browse files
nan-licursoragent
andauthored
fix: [SDK-4874] prevent stale UpdateSubscription leaving users Never Subscribed (#1687)
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 862910a commit 7300e55

4 files changed

Lines changed: 172 additions & 20 deletions

File tree

iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@
163163
3CA6CE0A28E4F19B00CA0585 /* OSUserRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA6CE0928E4F19B00CA0585 /* OSUserRequest.swift */; };
164164
3CA8B8822BEC2FCB0010ADA1 /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3C7A39D42B7C18EE0082665E /* XCTest.framework */; };
165165
3CA8B8832BEC2FCB0010ADA1 /* XCTest.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 3C7A39D42B7C18EE0082665E /* XCTest.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
166+
3CA93BC4300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */; };
166167
3CAA4BB72F0BAFBA00A16682 /* TriggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CAA4BB62F0BAFBA00A16682 /* TriggerTests.swift */; };
167168
3CB331682F281679000E1801 /* CustomEventsIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CB331672F281679000E1801 /* CustomEventsIntegrationTests.swift */; };
168169
3CB3316A2F281692000E1801 /* OSCustomEventsExecutorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */; };
@@ -1403,6 +1404,7 @@
14031404
3C9AD6D02B228B9200BC1540 /* OSRequestRemoveAlias.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRequestRemoveAlias.swift; sourceTree = "<group>"; };
14041405
3C9AD6D22B228BB000BC1540 /* OSRequestUpdateProperties.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSRequestUpdateProperties.swift; sourceTree = "<group>"; };
14051406
3CA6CE0928E4F19B00CA0585 /* OSUserRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSUserRequest.swift; sourceTree = "<group>"; };
1407+
3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionUpdateRaceTests.swift; sourceTree = "<group>"; };
14061408
3CAA4BB62F0BAFBA00A16682 /* TriggerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TriggerTests.swift; sourceTree = "<group>"; };
14071409
3CB331672F281679000E1801 /* CustomEventsIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomEventsIntegrationTests.swift; sourceTree = "<group>"; };
14081410
3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSCustomEventsExecutorTests.swift; sourceTree = "<group>"; };
@@ -2379,6 +2381,7 @@
23792381
isa = PBXGroup;
23802382
children = (
23812383
3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */,
2384+
3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */,
23822385
3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */,
23832386
);
23842387
path = Executors;
@@ -4471,6 +4474,7 @@
44714474
3CC063EE2B6D7FE8002BB07F /* OneSignalUserTests.swift in Sources */,
44724475
3CC890352C5BF9A7002CB4CC /* UserConcurrencyTests.swift in Sources */,
44734476
3CB3316A2F281692000E1801 /* OSCustomEventsExecutorTests.swift in Sources */,
4477+
3CA93BC4300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift in Sources */,
44744478
3CDE664C2BFC2A56006DA114 /* OneSignalUserObjcTests.m in Sources */,
44754479
);
44764480
runOnlyForDeploymentPostprocessing = 0;

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,12 @@ class OSSubscriptionOperationExecutor: OSOperationExecutor {
211211
self.removeRequestQueue.append(request)
212212

213213
case OS_UPDATE_SUBSCRIPTION_DELTA:
214-
let request = OSRequestUpdateSubscription(
215-
subscriptionObject: [delta.property: delta.value],
216-
subscriptionModel: subModel
217-
)
214+
// Keep at most one unsent UpdateSubscription per subscription (last wins).
215+
let modelId = subModel.modelId
216+
self.updateRequestQueue.removeAll { request in
217+
!request.sentToClient && request.subscriptionModel.modelId == modelId
218+
}
219+
let request = OSRequestUpdateSubscription(subscriptionModel: subModel)
218220
self.updateRequestQueue.append(request)
219221

220222
default:

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,38 +47,34 @@ class OSRequestUpdateSubscription: OneSignalRequest, OSUserRequest {
4747
let appId = OneSignalIdentifiers.currentAppId
4848
{
4949
self.path = "apps/\(appId)/subscriptions/\(subscriptionId)"
50+
// Refresh so a stale snapshot queued earlier can't overwrite newer local state.
51+
refreshParametersFromLiveModel()
5052
return true
5153
} else {
5254
return false
5355
}
5456
}
5557

56-
// TODO: just need the sub model and send it
57-
// But the model may be outdated or not sync with the subscriptionObject
58-
init(subscriptionObject: [String: Any], subscriptionModel: OSSubscriptionModel) {
59-
self.subscriptionModel = subscriptionModel
60-
self.stringDescription = "OSRequestUpdateSubscription with subscriptionObject: \(subscriptionObject)"
61-
super.init()
62-
63-
// Rename "address" key as "token", if it exists
64-
var subscriptionParams = subscriptionObject
65-
subscriptionParams.removeValue(forKey: "address")
66-
subscriptionParams.removeValue(forKey: "notificationTypes")
58+
/// Rebuild the PATCH body from the current subscription model.
59+
func refreshParametersFromLiveModel() {
60+
var subscriptionParams: [String: Any] = [:]
6761
subscriptionParams["token"] = subscriptionModel.address
6862
subscriptionParams["device_os"] = subscriptionModel.deviceOs
6963
subscriptionParams["sdk"] = subscriptionModel.sdk
7064
subscriptionParams["app_version"] = subscriptionModel.appVersion
71-
7265
// notificationTypes defaults to -1 instead of nil, don't send if it's -1
7366
if subscriptionModel.notificationTypes != -1 {
7467
subscriptionParams["notification_types"] = subscriptionModel.notificationTypes
7568
}
76-
7769
subscriptionParams["enabled"] = subscriptionModel.enabled
78-
// TODO: The above is not quite right. If we hydrate, we will over-write any pending updates
79-
// May use subscriptionObject, but enabled and notification_types should be sent together...
80-
8170
self.parameters = ["subscription": subscriptionParams]
71+
}
72+
73+
init(subscriptionModel: OSSubscriptionModel) {
74+
self.subscriptionModel = subscriptionModel
75+
self.stringDescription = "OSRequestUpdateSubscription with model: \(subscriptionModel.modelId)"
76+
super.init()
77+
refreshParametersFromLiveModel()
8278
self.method = PATCH
8379
}
8480

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
Modified MIT License
3+
4+
Copyright 2026 OneSignal
5+
6+
Permission is hereby granted, free of charge, to any person obtaining a copy
7+
of this software and associated documentation files (the "Software"), to deal
8+
in the Software without restriction, including without limitation the rights
9+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
copies of the Software, and to permit persons to whom the Software is
11+
furnished to do so, subject to the following conditions:
12+
13+
1. The above copyright notice and this permission notice shall be included in
14+
all copies or substantial portions of the Software.
15+
16+
2. All copies of substantial portions of the Software may only be used in connection
17+
with services provided by OneSignal.
18+
19+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
20+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
21+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
22+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
23+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
24+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
25+
THE SOFTWARE.
26+
*/
27+
28+
import XCTest
29+
import OneSignalCore
30+
import OneSignalCoreMocks
31+
import OneSignalUserMocks
32+
@testable import OneSignalOSCore
33+
@testable import OneSignalUser
34+
35+
/**
36+
Regression coverage for stale UpdateSubscription races after permission grant.
37+
*/
38+
final class SubscriptionUpdateRaceTests: XCTestCase {
39+
40+
private let subscriptionId = "test-subscription-id"
41+
private let pushToken = "test-push-token"
42+
/// Typical iOS alert+badge+sound permission bitmask after Accept.
43+
private let subscribedNotificationTypes = 31
44+
private let promptedNeverAnswered = Int(ERROR_PUSH_PROMPT_NEVER_ANSWERED)
45+
46+
override func setUpWithError() throws {
47+
OneSignalCoreMocks.clearUserDefaults()
48+
OneSignalUserMocks.reset()
49+
OneSignalIdentifiers.currentAppId = "test-app-id"
50+
OneSignalLog.setLogLevel(.LL_VERBOSE)
51+
}
52+
53+
override func tearDownWithError() throws { }
54+
55+
/**
56+
Parameters are built from the live model at init and refreshed again in prepareForExecution before send
57+
*/
58+
func testPrepareForExecutionRefreshesEnabledAndNotificationTypesFromLiveModel() throws {
59+
let model = makePushSubscriptionModel(notificationTypes: promptedNeverAnswered, subscriptionId: subscriptionId)
60+
XCTAssertFalse(model.enabled)
61+
62+
let request = OSRequestUpdateSubscription(subscriptionModel: model)
63+
let atInit = try XCTUnwrap(request.parameters?["subscription"] as? [String: Any])
64+
XCTAssertEqual(atInit["notification_types"] as? Int, promptedNeverAnswered)
65+
XCTAssertEqual(atInit["enabled"] as? Bool, false)
66+
67+
// Permission granted — live model is now subscribed.
68+
model.notificationTypes = subscribedNotificationTypes
69+
XCTAssertTrue(model.enabled)
70+
71+
XCTAssertTrue(request.prepareForExecution(newRecordsState: OSNewRecordsState()))
72+
73+
let refreshed = try XCTUnwrap(request.parameters?["subscription"] as? [String: Any])
74+
XCTAssertEqual(refreshed["notification_types"] as? Int, subscribedNotificationTypes)
75+
XCTAssertEqual(refreshed["enabled"] as? Bool, true)
76+
}
77+
78+
/**
79+
Regression: a pre-permission UpdateSubscription left pending must not send `-19`
80+
after permission is granted. Coalesce + live refresh should yield only subscribed payload(s).
81+
*/
82+
func testPendingPrePermissionUpdateSendsLiveSubscribedPayloadAfterGrant() throws {
83+
let client = MockOneSignalClient()
84+
client.executeInstantaneously = false
85+
client.fireSuccessForAllRequests = true
86+
OneSignalCoreImpl.setSharedClient(client)
87+
88+
let executor = OSSubscriptionOperationExecutor(newRecordsState: OSNewRecordsState())
89+
// Without a subscriptionId, prepareForExecution keeps the update pending.
90+
let model = makePushSubscriptionModel(notificationTypes: promptedNeverAnswered, subscriptionId: nil)
91+
let identityModelId = UUID().uuidString
92+
93+
executor.enqueueDelta(OSDelta(
94+
name: OS_UPDATE_SUBSCRIPTION_DELTA,
95+
identityModelId: identityModelId,
96+
model: model,
97+
property: "notificationTypes",
98+
value: promptedNeverAnswered
99+
))
100+
executor.processDeltaQueue(inBackground: false)
101+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.2)
102+
103+
XCTAssertTrue(client.executedRequests.isEmpty, "Update should still be pending without subscriptionId")
104+
105+
// User accepts — live model subscribed — then hydrate subscription id and flush again.
106+
model.notificationTypes = subscribedNotificationTypes
107+
model.subscriptionId = subscriptionId
108+
XCTAssertTrue(model.enabled)
109+
110+
executor.enqueueDelta(OSDelta(
111+
name: OS_UPDATE_SUBSCRIPTION_DELTA,
112+
identityModelId: identityModelId,
113+
model: model,
114+
property: "notificationTypes",
115+
value: subscribedNotificationTypes
116+
))
117+
executor.processDeltaQueue(inBackground: false)
118+
OneSignalCoreMocks.waitForBackgroundThreads(seconds: 0.5)
119+
120+
let updateRequests = client.executedRequests.compactMap { $0 as? OSRequestUpdateSubscription }
121+
XCTAssertFalse(updateRequests.isEmpty, "Expected at least one UpdateSubscription after id hydration")
122+
123+
let payloads = updateRequests.compactMap { $0.parameters?["subscription"] as? [String: Any] }
124+
let sentMinus19 = payloads.contains {
125+
($0["notification_types"] as? Int) == promptedNeverAnswered && ($0["enabled"] as? Bool) == false
126+
}
127+
let allSubscribed = payloads.allSatisfy {
128+
($0["notification_types"] as? Int) == subscribedNotificationTypes && ($0["enabled"] as? Bool) == true
129+
}
130+
131+
XCTAssertFalse(sentMinus19, "Must not send stale enabled:false / notification_types:-19 after grant")
132+
XCTAssertTrue(allSubscribed, "All executed UpdateSubscription payloads should reflect the live subscribed model")
133+
XCTAssertEqual(updateRequests.count, 1, "Unsent updates for the same subscription should be coalesced")
134+
}
135+
136+
// MARK: - Helpers
137+
138+
private func makePushSubscriptionModel(notificationTypes: Int, subscriptionId: String?) -> OSSubscriptionModel {
139+
let model = OSSubscriptionModel(
140+
type: .push,
141+
address: pushToken,
142+
subscriptionId: subscriptionId,
143+
reachable: notificationTypes > 0,
144+
isDisabled: false,
145+
changeNotifier: OSEventProducer()
146+
)
147+
model.notificationTypes = notificationTypes
148+
return model
149+
}
150+
}

0 commit comments

Comments
 (0)