-
Notifications
You must be signed in to change notification settings - Fork 265
feat: [JWT-5] bind each Delta to the user that owns it #1710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: nan/jwt-pr4-identity-api
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /* | ||
| 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 | ||
| @testable import OneSignalOSCore | ||
|
|
||
| /// Covers `OSDelta` archive round trips, including the owning user's external ID. | ||
| final class OSDeltaTests: XCTestCase { | ||
|
|
||
| private func makeDelta(externalId: String?) -> OSDelta { | ||
| OSDelta( | ||
| name: "test_delta", | ||
| identityModelId: "identity-model-a", | ||
| externalId: externalId, | ||
| model: OSModel(changeNotifier: OSEventProducer()), | ||
| property: "language", | ||
| value: "en" | ||
| ) | ||
| } | ||
|
|
||
| private func archiveThenUnarchive(_ delta: OSDelta) throws -> OSDelta { | ||
| let data = try NSKeyedArchiver.archivedData(withRootObject: delta, requiringSecureCoding: false) | ||
| let unarchiver = try NSKeyedUnarchiver(forReadingFrom: data) | ||
| unarchiver.requiresSecureCoding = false | ||
| defer { unarchiver.finishDecoding() } | ||
| return try XCTUnwrap(unarchiver.decodeObject(forKey: NSKeyedArchiveRootObjectKey) as? OSDelta) | ||
| } | ||
|
|
||
| func testExternalIdSurvivesAnArchiveRoundTrip() throws { | ||
| let delta = makeDelta(externalId: "user-a") | ||
|
|
||
| let decoded = try archiveThenUnarchive(delta) | ||
|
|
||
| XCTAssertEqual(decoded.externalId, "user-a") | ||
| XCTAssertEqual(decoded.identityModelId, "identity-model-a") | ||
| XCTAssertEqual(decoded.deltaId, delta.deltaId) | ||
| } | ||
|
|
||
| /// Decode must succeed when externalId is absent, or queued work is dropped on upgrade. | ||
| func testADeltaWithoutAnExternalIdStillDecodes() throws { | ||
| let delta = makeDelta(externalId: nil) | ||
|
|
||
| let decoded = try archiveThenUnarchive(delta) | ||
|
|
||
| XCTAssertNil(decoded.externalId) | ||
| XCTAssertEqual(decoded.identityModelId, "identity-model-a") | ||
| XCTAssertEqual(decoded.property, "language") | ||
| XCTAssertEqual(decoded.value as? String, "en") | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,19 +31,26 @@ import OneSignalOSCore | |
|
|
||
| class OSSubscriptionModelStoreListener: OSModelStoreListener { | ||
| var store: OSModelStore<OSSubscriptionModel> | ||
| let operationRepo: OSOperationRepo | ||
|
|
||
| required init(store: OSModelStore<OSSubscriptionModel>) { | ||
| required init(store: OSModelStore<OSSubscriptionModel>, operationRepo: OSOperationRepo) { | ||
| self.store = store | ||
| self.operationRepo = operationRepo | ||
| } | ||
|
|
||
| func getAddModelDelta(_ model: OSSubscriptionModel) -> OSDelta? { | ||
| guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else { | ||
| OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getAddModelDelta has no user instance") | ||
| // Stale - drop if the model is no longer in this store, the user has switched since | ||
| guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user, | ||
| store.getModel(modelId: model.modelId) != nil | ||
| else { | ||
| OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getAddModelDelta has no user instance or the model is not in the current store") | ||
| return nil | ||
| } | ||
| let identityModel = userInstance.identityModel | ||
| return OSDelta( | ||
| name: OS_ADD_SUBSCRIPTION_DELTA, | ||
| identityModelId: userInstance.identityModel.modelId, | ||
| identityModelId: identityModel.modelId, | ||
| externalId: identityModel.externalId, | ||
| model: model, | ||
| property: model.type.rawValue, // push, email, sms | ||
| value: model.address ?? "" | ||
|
nan-li marked this conversation as resolved.
|
||
|
|
@@ -52,15 +59,18 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener { | |
|
|
||
| /** | ||
| The `property` and `value` is not needed for a remove operation, so just pass in some model data as placeholders. | ||
| Stamps the current user: `onRemoved` runs after the model has left the store, so membership can't be checked. | ||
| */ | ||
| func getRemoveModelDelta(_ model: OSSubscriptionModel) -> OSDelta? { | ||
| guard let userInstance = OneSignalUserManagerImpl.sharedInstance._user else { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The add-path drop (model no longer in this store) is the right fix. Remove/update still copy I’d snapshot the owner from the changed model (or refuse to stamp when it is not the current user’s), the way properties already checks A switch-user test on remove/update, not only add, would catch the hole the add test currently hides. |
||
| OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getRemoveModelDelta has no user instance") | ||
| return nil | ||
| } | ||
| let identityModel = userInstance.identityModel | ||
| return OSDelta( | ||
| name: OS_REMOVE_SUBSCRIPTION_DELTA, | ||
| identityModelId: userInstance.identityModel.modelId, | ||
| identityModelId: identityModel.modelId, | ||
| externalId: identityModel.externalId, | ||
| model: model, | ||
| property: model.type.rawValue, // push, email, sms | ||
| value: model.address ?? "" | ||
|
|
@@ -78,14 +88,16 @@ class OSSubscriptionModelStoreListener: OSModelStoreListener { | |
| OneSignalLog.onesignalLog(.LL_ERROR, message: "OSSubscriptionModelStoreListener.getUpdateModelDelta has no user instance") | ||
| return nil | ||
| } | ||
| if let onesignalId = userInstance.identityModel.onesignalId { | ||
| let identityModel = userInstance.identityModel | ||
| if let onesignalId = identityModel.onesignalId { | ||
| let condition = OSIamFetchReadyCondition.sharedInstance(withId: onesignalId) | ||
| condition.setSubscriptionUpdatePending(value: true) | ||
| } | ||
|
|
||
| return OSDelta( | ||
| name: OS_UPDATE_SUBSCRIPTION_DELTA, | ||
| identityModelId: userInstance.identityModel.modelId, | ||
| identityModelId: identityModel.modelId, | ||
| externalId: identityModel.externalId, | ||
| model: args.model, | ||
| property: args.property, | ||
| value: args.newValue | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.