From c3dd2f308a927cfc74e645b31def2e3fe7100cae Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 26 Aug 2026 19:13:58 -0700 Subject: [PATCH 1/2] fix: respect REST API-disabled push subscriptions A push subscription disabled through the REST API (notification_types -31) was re-enabled by the SDK: fetch responses never hydrated the disable onto an existing push subscription, and every Create User and update payload recomputed enabled from device state. Mirror the server's disable code on the subscription model when a response reports it, and echo it back in Create User and update payloads instead of the device-derived values. The mirror clears when the server reports any other state, when the subscription ID resets because the server record is gone, and on an explicit optIn(), whose clear outranks stale in-flight hydration until the server confirms. Both payload builders share one snapshot-based body so concurrent changes cannot tear enabled away from notification_types. --- .../Source/Executors/OSUserExecutor.swift | 61 +++-- .../Source/OSSubscriptionModel.swift | 217 +++++++++++++++--- .../Source/OneSignalUserManagerImpl.swift | 4 +- .../OSRequestUpdateSubscription.swift | 12 +- .../OneSignalUserTests.swift | 175 ++++++++++++++ 5 files changed, 402 insertions(+), 67 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 1b8d0e5b3..da6a6c6c1 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -514,28 +514,7 @@ extension OSUserExecutor { } } - // TODO: Determine how to hydrate the push subscription, which is still faulty. - // Hydrate by token if sub_id exists? - // Problem: a user can have multiple iOS push subscription, and perhaps missing token - // Ideally we only get push subscription for this device in the response, not others - - // Hydrate the push subscription if we don't already have a subscription ID AND token matches the original request - if OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.subscriptionId == nil, - let subscriptionObject = parseSubscriptionObjectResponse(response) - { - for subModel in subscriptionObject { - if subModel["type"] as? String == "iOSPush", - // response may have "" token or no token - areTokensEqual(tokenA: originalPushToken, tokenB: subModel["token"] as? String) - { - OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.hydrate(subModel) - if addNewRecords, let subId = subModel["id"] as? String { - newRecordsState.add(subId) - } - break - } - } - } + hydratePushSubscription(response: response, originalPushToken: originalPushToken, addNewRecords: addNewRecords) // Hydrate onto the user this response is for // If user has changed, don't hydrate, except for push subscription above @@ -576,6 +555,44 @@ extension OSUserExecutor { } } + /// Hydrates the push subscription from a fetch or create response: the whole object before a + /// subscription ID exists, only the server's REST API disable state once one does. + // TODO: Determine how to hydrate the push subscription, which is still faulty. + // Hydrate by token if sub_id exists? + // Problem: a user can have multiple iOS push subscription, and perhaps missing token + // Ideally we only get push subscription for this device in the response, not others + private func hydratePushSubscription(response: [AnyHashable: Any], originalPushToken: String?, addNewRecords: Bool) { + guard let subscriptionObject = parseSubscriptionObjectResponse(response) else { + return + } + // The response's subscription ID is recorded as a new record even when the model is absent. + let pushSubscriptionModel = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel + + // Hydrate the push subscription if we don't already have a subscription ID AND token matches the original request + guard let subscriptionId = pushSubscriptionModel?.subscriptionId else { + for subModel in subscriptionObject { + if subModel["type"] as? String == "iOSPush", + // response may have "" token or no token + areTokensEqual(tokenA: originalPushToken, tokenB: subModel["token"] as? String) + { + pushSubscriptionModel?.hydrate(subModel) + if addNewRecords, let subId = subModel["id"] as? String { + newRecordsState.add(subId) + } + break + } + } + return + } + + // Only the REST API disable state hydrates onto an existing push subscription; the device + // owns the rest. Skipping it lets the next subscription payload re-enable a suppressed device. + for subModel in subscriptionObject where subModel["id"] as? String == subscriptionId { + pushSubscriptionModel?.hydrateRestApiDisabledState(from: subModel) + break + } + } + /** Returns if 2 tokens are equal. This is needed as a nil token is equal to the empty string "". */ diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift index cc9dd2211..1055623b8 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift @@ -114,6 +114,10 @@ class OSSubscriptionModel: OSModel { var deviceModel: String? var appVersion: String? var netType: Int? + var restApiDisabledReason: Int? + // Not persisted; an optIn() clear outranks stale hydration until the server reports + // the subscription in another state. + var restApiDisableClearedByUser = false } /** @@ -182,6 +186,11 @@ class OSSubscriptionModel: OSModel { return } + // The disable code describes a specific server record; the record is gone when the ID resets. + if newValue == nil { + restApiDisabledReason = nil + } + // Cache the subscriptionId to UserDefaults for routine reads, and the OSResilientStorage mirror OneSignalUserDefaults.initShared().saveString(forKey: OSUD_PUSH_SUBSCRIPTION_ID, withValue: newValue) OSResilientStorage.setString(newValue ?? "", forKey: OSResilientStorage.keySubscriptionId) @@ -194,7 +203,12 @@ class OSSubscriptionModel: OSModel { var enabled: Bool { // Does not consider subscription_id in the calculation get { let state = snapshot() - return calculateIsEnabled(address: state.address, reachable: state.reachable, isDisabled: state.isDisabled) + return calculateIsEnabled( + address: state.address, + reachable: state.reachable, + isDisabled: state.isDisabled, + restApiDisabledReason: state.restApiDisabledReason + ) } } @@ -261,6 +275,28 @@ class OSSubscriptionModel: OSModel { } } + /// The notification_types value for a REST API disable, the only server-owned code; other + /// negative codes are device or delivery errors the device recovers by re-asserting its state. + static let restApiDisabledNotificationType = -31 + + /** + The server's REST API disable code, or nil when the server has not disabled this subscription. + Hydrated from responses, never derived from device state, and echoed back in payloads so routine + updates and logins don't re-enable a suppressed subscription. Cleared when a response reports + any other state, when the subscription ID resets, or by `optIn()`. + */ + var restApiDisabledReason: Int? { + get { stateLock.withLock { state.restApiDisabledReason } } + set { + let oldValue = swapValue(\.restApiDisabledReason, to: newValue) + guard newValue != oldValue else { + return + } + // Mirrors server state rather than a local change, so persist without generating a delta. + self.set(property: "restApiDisabledReason", newValue: newValue, preventServerUpdate: true) + } + } + // Properties for push subscription var testType: Int? { get { stateLock.withLock { state.testType } } @@ -371,7 +407,9 @@ class OSSubscriptionModel: OSModel { sdk: ONESIGNAL_VERSION, deviceModel: OSDeviceUtils.getDeviceVariant(), appVersion: Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String, - netType: OSNetworkingUtils.getNetType() as? Int + netType: OSNetworkingUtils.getNetType() as? Int, + restApiDisabledReason: nil, + restApiDisableClearedByUser: false ) super.init(changeNotifier: changeNotifier) @@ -393,6 +431,7 @@ class OSSubscriptionModel: OSModel { coder.encode(state.deviceModel, forKey: "deviceModel") coder.encode(state.appVersion, forKey: "appVersion") coder.encode(state.netType, forKey: "netType") + coder.encode(state.restApiDisabledReason, forKey: "restApiDisabledReason") } required init?(coder: NSCoder) { @@ -415,7 +454,9 @@ class OSSubscriptionModel: OSModel { sdk: coder.decodeObject(forKey: "sdk") as? String ?? ONESIGNAL_VERSION, deviceModel: coder.decodeObject(forKey: "deviceModel") as? String, appVersion: coder.decodeObject(forKey: "appVersion") as? String, - netType: coder.decodeObject(forKey: "netType") as? Int + netType: coder.decodeObject(forKey: "netType") as? Int, + restApiDisabledReason: coder.decodeObject(forKey: "restApiDisabledReason") as? Int, + restApiDisableClearedByUser: false ) super.init(coder: coder) @@ -436,13 +477,11 @@ class OSSubscriptionModel: OSModel { // self.address = property.value as? String case "enabled": if let enabled = property.value as? Bool { - if self.enabled != enabled { // TODO: Is this right? - _isDisabled = !enabled - } + hydrateEnabled(enabled, response: response) } case "notification_types": if let notificationTypes = property.value as? Int { - self.notificationTypes = notificationTypes + hydrateNotificationTypes(notificationTypes) } default: OneSignalLog.onesignalLog(.LL_DEBUG, message: "Unused property on subscription model") @@ -450,6 +489,33 @@ class OSSubscriptionModel: OSModel { } } + /// Applies a hydrated `enabled`. A REST API disable is server-owned, not a user opt-out, + /// so it must not flip `_isDisabled`; the notification_types hydration records it instead. + private func hydrateEnabled(_ enabled: Bool, response: [String: Any]) { + guard !isRestApiDisable(response) else { + return + } + if self.enabled != enabled { // TODO: Is this right? + _isDisabled = !enabled + } + } + + /// Routes a hydrated notification_types: -31 records the server's disable; any other value + /// clears it and becomes the device value. + private func hydrateNotificationTypes(_ value: Int) { + if value == Self.restApiDisabledNotificationType { + recordRestApiDisable(value) + } else { + acceptServerNonDisabledState() + self.notificationTypes = value + } + } + + /// True when the response's notification_types carries a REST API disable code. + private func isRestApiDisable(_ response: [String: Any]) -> Bool { + return response["notification_types"] as? Int == Self.restApiDisabledNotificationType + } + // Using snake_case so we can use this in request bodies public func jsonRepresentation() -> [String: Any] { let state = snapshot() @@ -457,19 +523,22 @@ class OSSubscriptionModel: OSModel { json["id"] = state.subscriptionId json["type"] = state.type.rawValue json["token"] = state.address - json["enabled"] = calculateIsEnabled(address: state.address, reachable: state.reachable, isDisabled: state.isDisabled) + json["enabled"] = calculateIsEnabled( + address: state.address, + reachable: state.reachable, + isDisabled: state.isDisabled, + restApiDisabledReason: state.restApiDisabledReason + ) json["test_type"] = state.testType json["device_os"] = state.deviceOs json["sdk"] = state.sdk json["device_model"] = state.deviceModel json["app_version"] = state.appVersion json["net_type"] = state.netType - // notificationTypes defaults to -1 instead of nil, don't send if it's -1 - if state.notificationTypes != -1 { - json["notification_types"] = state.notificationTypes - } + json["notification_types"] = outboundNotificationTypes(state) return json } + } // Push Subscription related @@ -491,14 +560,87 @@ extension OSSubscriptionModel { // Calculates if push notifications are enabled on the device. // Does not consider the existence of the subscription_id, as we send this in the request to create a push subscription. - func calculateIsEnabled(address: String?, reachable: Bool, isDisabled: Bool) -> Bool { - return address != nil && reachable && !isDisabled + func calculateIsEnabled(address: String?, reachable: Bool, isDisabled: Bool, restApiDisabledReason: Int?) -> Bool { + return address != nil && reachable && !isDisabled && restApiDisabledReason == nil } func updateNotificationTypes() { notificationTypes = Int(OSNotificationsManager.getNotificationTypes(_isDisabled)) } + /// Records the server's disable code unless `optIn()` cleared one and the server has not yet + /// reported the subscription in another state; the user's explicit intent wins that race. + private func recordRestApiDisable(_ code: Int) { + let clearedByUser = stateLock.withLock { state.restApiDisableClearedByUser } + guard !clearedByUser else { + return + } + restApiDisabledReason = code + } + + /// Clears `restApiDisabledReason` and re-arms recording once the server reports a non-disabled state. + private func acceptServerNonDisabledState() { + restApiDisabledReason = nil + stateLock.withLock { state.restApiDisableClearedByUser = false } + } + /// notification_types for outgoing payloads: the recorded disable code (the positive device + /// value would re-enable it), else the device value, nil for the -1 default. + private func outboundNotificationTypes(_ state: State) -> Int? { + if let restApiDisabledReason = state.restApiDisabledReason { + return restApiDisabledReason + } + return state.notificationTypes != -1 ? state.notificationTypes : nil + } + + /// The PATCH body for a subscription update, built from one snapshot so a concurrent hydration + /// or `optIn()` can't tear `enabled` away from `notification_types`. + func updateParams() -> [String: Any] { + let state = snapshot() + var params: [String: Any] = [:] + params["token"] = state.address + params["device_os"] = state.deviceOs + params["sdk"] = state.sdk + params["app_version"] = state.appVersion + params["notification_types"] = outboundNotificationTypes(state) + params["enabled"] = calculateIsEnabled( + address: state.address, + reachable: state.reachable, + isDisabled: state.isDisabled, + restApiDisabledReason: state.restApiDisabledReason + ) + return params + } + + /** + Mirrors the server's REST API disable state from a fetched subscription object: -31 records it, + any other reported value clears it. The device stays the source of truth for the rest of an + existing subscription's state, so nothing else is read. + */ + func hydrateRestApiDisabledState(from serverSubscription: [String: Any]) { + guard type == .push, let serverTypes = serverSubscription["notification_types"] as? Int else { + return + } + if serverTypes == Self.restApiDisabledNotificationType { + recordRestApiDisable(serverTypes) + } else { + acceptServerNonDisabledState() + } + } + + /** + Clears a REST API disable and enqueues an enabled-change delta so the server re-enables the + subscription. Called from `optIn()`, where a deliberate user action overrides the suppression. + */ + func clearRestApiDisable() { + let oldValue = swapValue(\.restApiDisabledReason, to: nil) + guard oldValue != nil else { + return + } + stateLock.withLock { state.restApiDisableClearedByUser = true } + self.set(property: "restApiDisabledReason", newValue: nil as Int?, preventServerUpdate: true) + firePushSubscriptionChanged(.restApiDisabledReason(oldValue)) + } + func updateTestType() { let releaseMode: OSUIApplicationReleaseMode = OneSignalMobileProvision.releaseMode() // Workaround to unsure how to extract the Int value in 1 step... @@ -532,38 +674,47 @@ extension OSSubscriptionModel { case reachable(Bool) case isDisabled(Bool) case address(String?) + case restApiDisabledReason(Int?) } func firePushSubscriptionChanged(_ changedProperty: OSPushPropertyChanged) { - var prevIsOptedIn = true - var prevIsEnabled = true - var prevSubscriptionState = OSPushSubscriptionState(id: "", token: "", optedIn: true) + // The previous state is the current state with only the changed property's old value substituted. + var prevId = subscriptionId + var prevAddress = address + var prevReachable = _reachable + var prevIsDisabled = _isDisabled + var prevRestApiDisabledReason = restApiDisabledReason switch changedProperty { case .subscriptionId(let oldValue): - prevIsEnabled = calculateIsEnabled(address: address, reachable: _reachable, isDisabled: _isDisabled) - prevIsOptedIn = calculateIsOptedIn(reachable: _reachable, isDisabled: _isDisabled) - prevSubscriptionState = OSPushSubscriptionState(id: oldValue, token: address, optedIn: prevIsOptedIn) - + prevId = oldValue case .reachable(let oldValue): - prevIsEnabled = calculateIsEnabled(address: address, reachable: oldValue, isDisabled: _isDisabled) - prevIsOptedIn = calculateIsOptedIn(reachable: oldValue, isDisabled: _isDisabled) - prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, optedIn: prevIsOptedIn) - + prevReachable = oldValue case .isDisabled(let oldValue): - prevIsEnabled = calculateIsEnabled(address: address, reachable: _reachable, isDisabled: oldValue) - prevIsOptedIn = calculateIsOptedIn(reachable: _reachable, isDisabled: oldValue) - prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, optedIn: prevIsOptedIn) - + prevIsDisabled = oldValue case .address(let oldValue): - prevIsEnabled = calculateIsEnabled(address: oldValue, reachable: _reachable, isDisabled: _isDisabled) - prevIsOptedIn = calculateIsOptedIn(reachable: _reachable, isDisabled: _isDisabled) - prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: oldValue, optedIn: prevIsOptedIn) + prevAddress = oldValue + case .restApiDisabledReason(let oldValue): + prevRestApiDisabledReason = oldValue } + let prevIsEnabled = calculateIsEnabled( + address: prevAddress, + reachable: prevReachable, + isDisabled: prevIsDisabled, + restApiDisabledReason: prevRestApiDisabledReason + ) + let prevIsOptedIn = calculateIsOptedIn(reachable: prevReachable, isDisabled: prevIsDisabled) + let prevSubscriptionState = OSPushSubscriptionState(id: prevId, token: prevAddress, optedIn: prevIsOptedIn) + let newIsOptedIn = calculateIsOptedIn(reachable: _reachable, isDisabled: _isDisabled) - let newIsEnabled = calculateIsEnabled(address: address, reachable: _reachable, isDisabled: _isDisabled) + let newIsEnabled = calculateIsEnabled( + address: address, + reachable: _reachable, + isDisabled: _isDisabled, + restApiDisabledReason: restApiDisabledReason + ) if prevIsEnabled != newIsEnabled { self.set(property: "enabled", newValue: newIsEnabled) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index d9850691f..a62b36f29 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -932,7 +932,9 @@ extension OneSignalUserManagerImpl { guard !OneSignalConfig.shouldAwaitAppIdAndLogMissingPrivacyConsent(forMethod: "pushSubscription.optIn") else { return } - pushSubscriptionModelStore.getModel(key: OS_PUSH_SUBSCRIPTION_MODEL_KEY)?._isDisabled = false + let model = pushSubscriptionModelStore.getModel(key: OS_PUSH_SUBSCRIPTION_MODEL_KEY) + model?._isDisabled = false + model?.clearRestApiDisable() OSNotificationsManager.requestPermission(nil, fallbackToSettings: true) } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift index 3f211fad5..bce0be310 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestUpdateSubscription.swift @@ -57,17 +57,7 @@ class OSRequestUpdateSubscription: OneSignalRequest, OSUserRequest { /// 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 - self.parameters = ["subscription": subscriptionParams] + self.parameters = ["subscription": subscriptionModel.updateParams()] } init(subscriptionModel: OSSubscriptionModel) { diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift index e130af24b..4f6647660 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/OneSignalUserTests.swift @@ -324,4 +324,179 @@ final class OneSignalUserTests: XCTestCase { XCTAssertNil(manager._user) XCTAssertNil(manager.currentUser(matching: identityModel.modelId)) } + + // MARK: - REST API disabled push subscriptions + + /// A push model hydrated with the server's REST API disable state (notification_types -31). + private func pushModelWithRestApiDisable() -> OSSubscriptionModel { + let model = OSSubscriptionModel( + type: .push, + address: "test-token", + subscriptionId: "test-sub-id", + reachable: true, + isDisabled: false, + changeNotifier: OSEventProducer() + ) + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": false, "notification_types": -31]) + return model + } + + func testRestApiDisable_overridesOutgoingSubscriptionPayloads() { + let model = pushModelWithRestApiDisable() + XCTAssertEqual(model.restApiDisabledReason, -31) + + let json = model.jsonRepresentation() + XCTAssertEqual(json["enabled"] as? Bool, false) + XCTAssertEqual(json["notification_types"] as? Int, -31) + + let updateRequest = OSRequestUpdateSubscription(subscriptionModel: model) + let params = updateRequest.parameters?["subscription"] as? [String: Any] + XCTAssertEqual(params?["enabled"] as? Bool, false) + XCTAssertEqual(params?["notification_types"] as? Int, -31) + } + + func testRestApiDisable_survivesDeviceStateRefresh() { + let model = pushModelWithRestApiDisable() + + // Device-driven recomputes must not clear server-owned disable state + model.updateNotificationTypes() + model.update() + + XCTAssertEqual(model.restApiDisabledReason, -31) + XCTAssertEqual(model.jsonRepresentation()["enabled"] as? Bool, false) + } + + func testRestApiDisable_survivesArchiving() throws { + let model = pushModelWithRestApiDisable() + + let data = try NSKeyedArchiver.archivedData(withRootObject: model, requiringSecureCoding: false) + let unarchiver = try NSKeyedUnarchiver(forReadingFrom: data) + unarchiver.requiresSecureCoding = false + let restored = try XCTUnwrap(unarchiver.decodeObject(forKey: NSKeyedArchiveRootObjectKey) as? OSSubscriptionModel) + + XCTAssertEqual(restored.restApiDisabledReason, -31) + XCTAssertEqual(restored.jsonRepresentation()["enabled"] as? Bool, false) + } + + func testRestApiDisable_clearedByOptIn() { + let model = pushModelWithRestApiDisable() + + model.clearRestApiDisable() + + XCTAssertNil(model.restApiDisabledReason) + let json = model.jsonRepresentation() + XCTAssertEqual(json["enabled"] as? Bool, true) + XCTAssertNotEqual(json["notification_types"] as? Int, -31) + } + + func testRestApiDisable_mirrorsServerField() { + // Only -31 is ever recorded; any other reported value is not, and clears an existing disable. + let model = OSSubscriptionModel( + type: .push, + address: "test-token", + subscriptionId: "test-sub-id", + reachable: true, + isDisabled: false, + changeNotifier: OSEventProducer() + ) + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": false, "notification_types": -2]) + XCTAssertNil(model.restApiDisabledReason) + + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": false, "notification_types": -31]) + XCTAssertEqual(model.restApiDisabledReason, -31) + + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": false, "notification_types": -2]) + XCTAssertNil(model.restApiDisabledReason) + XCTAssertEqual(model.jsonRepresentation()["enabled"] as? Bool, true) + } + + func testRestApiDisable_clearedWhenSubscriptionIdResets() { + // The disable code describes a specific server record; it must die with the record. + let model = pushModelWithRestApiDisable() + + model.subscriptionId = nil + + XCTAssertNil(model.restApiDisabledReason) + XCTAssertEqual(model.jsonRepresentation()["enabled"] as? Bool, true) + } + + func testRestApiDisable_optInOutranksStaleHydration() { + let model = pushModelWithRestApiDisable() + + // A stale fetch response landing after optIn() cleared the disable must not re-record it + model.clearRestApiDisable() + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": false, "notification_types": -31]) + XCTAssertNil(model.restApiDisabledReason) + + // Once the server reports another state, recording re-arms for a later operator disable + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": true, "notification_types": 1]) + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": false, "notification_types": -31]) + XCTAssertEqual(model.restApiDisabledReason, -31) + } + + func testRestApiDisable_clearedWhenServerReportsEnabled() { + let model = pushModelWithRestApiDisable() + + model.hydrateRestApiDisabledState(from: ["id": "test-sub-id", "enabled": true, "notification_types": 1]) + + XCTAssertNil(model.restApiDisabledReason) + } + + /** + A push subscription disabled through the REST API (server notification_types -31) must stay + disabled across a login to a different external ID. The fetch hydrates the server's disable + state onto the existing subscription, and the login's Create User payload echoes it back. + */ + func testLoginToDifferentUser_afterRestApiDisable_sendsDisabledPushSubscription() throws { + /* Setup */ + let client = MockOneSignalClient() + MockUserRequests.setDefaultCreateAnonUserResponses(with: client) + MockUserRequests.setDefaultIdentifyUserResponses(with: client, externalId: userA_EUID) + MockUserRequests.setDefaultCreateUserResponses(with: client, externalId: userB_EUID) + + // Fetching user A reports the push subscription disabled through the REST API + var disabledResponse = MockUserRequests.testDefaultFullCreateUserResponse( + onesignalId: anonUserOSID, + externalId: userA_EUID, + subscriptionId: testPushSubId + ) + let disabledSub = MockUserRequests.testDefaultPushSubPayload(id: testPushSubId) + .merging(["enabled": false, "notification_types": -31]) { _, new in new } + disabledResponse["subscriptions"] = [disabledSub] + client.setMockResponseForRequest( + request: "", + response: disabledResponse + ) + OneSignalCoreImpl.setSharedClient(client) + + // 1. Start with an anonymous user and log in to user A; the post-identify fetch + // hydrates the REST API disable onto the existing push subscription + OneSignalUserManagerImpl.sharedInstance.start() + OneSignalUserManagerImpl.sharedInstance.login(externalId: userA_EUID, token: nil) + + OneSignalCoreMocks.waitUntil("Fetch did not hydrate the REST API disable") { + OneSignalUserManagerImpl.sharedInstance.user.identityModel.externalId == userA_EUID && + OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModel?.restApiDisabledReason == -31 + } + + /* When */ + + // 2. Log in to user B, which sends a Create User carrying the push subscription + OneSignalUserManagerImpl.sharedInstance.login(externalId: userB_EUID, token: nil) + + func createUserBRequest() -> OSRequestCreateUser? { + return client.executedRequests.compactMap { $0 as? OSRequestCreateUser }.first { + ($0.parameters?["identity"] as? [String: String])?[OS_EXTERNAL_ID] == userB_EUID + } + } + OneSignalCoreMocks.waitUntil("Create User for user B was not sent") { + createUserBRequest() != nil + } + + /* Then */ + + let subscriptions = try XCTUnwrap(createUserBRequest()?.parameters?["subscriptions"] as? [[String: Any]]) + XCTAssertEqual(subscriptions.first?["enabled"] as? Bool, false) + XCTAssertEqual(subscriptions.first?["notification_types"] as? Int, -31) + } } From 0de021e0f4d2a5602bd0b199d17cc076383a720a Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 2 Sep 2026 09:34:16 -0700 Subject: [PATCH 2/2] fix: make REST API disable bookkeeping atomic Recording, clearing, and accepting the server's disable state each read the opt-in guard and wrote the reason under separate lock acquisitions, so a hydrate racing an optIn() could leave the guard armed with the reason re-recorded. Each transition now does its guard and write in one critical section and fires events after release. OSPushSubscription.optedIn documents that it reflects the user's preference and OS permission, not a server-side disable. --- .../Source/OSSubscriptionModel.swift | 36 +++++++++++++++---- .../Source/OneSignalUserManagerImpl.swift | 2 ++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift index 1055623b8..d1ef50361 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift @@ -571,17 +571,33 @@ extension OSSubscriptionModel { /// Records the server's disable code unless `optIn()` cleared one and the server has not yet /// reported the subscription in another state; the user's explicit intent wins that race. private func recordRestApiDisable(_ code: Int) { - let clearedByUser = stateLock.withLock { state.restApiDisableClearedByUser } - guard !clearedByUser else { + let changed: Bool = stateLock.withLock { + guard !state.restApiDisableClearedByUser, state.restApiDisabledReason != code else { + return false + } + state.restApiDisabledReason = code + return true + } + guard changed else { return } - restApiDisabledReason = code + self.set(property: "restApiDisabledReason", newValue: code, preventServerUpdate: true) } /// Clears `restApiDisabledReason` and re-arms recording once the server reports a non-disabled state. private func acceptServerNonDisabledState() { - restApiDisabledReason = nil - stateLock.withLock { state.restApiDisableClearedByUser = false } + let changed: Bool = stateLock.withLock { + state.restApiDisableClearedByUser = false + guard state.restApiDisabledReason != nil else { + return false + } + state.restApiDisabledReason = nil + return true + } + guard changed else { + return + } + self.set(property: "restApiDisabledReason", newValue: nil as Int?, preventServerUpdate: true) } /// notification_types for outgoing payloads: the recorded disable code (the positive device /// value would re-enable it), else the device value, nil for the -1 default. @@ -632,11 +648,17 @@ extension OSSubscriptionModel { subscription. Called from `optIn()`, where a deliberate user action overrides the suppression. */ func clearRestApiDisable() { - let oldValue = swapValue(\.restApiDisabledReason, to: nil) + let oldValue: Int? = stateLock.withLock { + guard let recorded = state.restApiDisabledReason else { + return nil + } + state.restApiDisabledReason = nil + state.restApiDisableClearedByUser = true + return recorded + } guard oldValue != nil else { return } - stateLock.withLock { state.restApiDisableClearedByUser = true } self.set(property: "restApiDisabledReason", newValue: nil as Int?, preventServerUpdate: true) firePushSubscriptionChanged(.restApiDisabledReason(oldValue)) } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift index a62b36f29..4e78b02e1 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift @@ -96,6 +96,8 @@ import OneSignalNotifications @objc public protocol OSPushSubscription { var id: String? { get } var token: String? { get } + /// The user's preference combined with OS permission; a subscription the app owner disabled + /// through the REST API still reports true here. var optedIn: Bool { get } func optIn()