diff --git a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj index b69de6d9b..718b61175 100644 --- a/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj +++ b/iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj @@ -199,6 +199,7 @@ 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 */; }; + E85C1C500D87196C5E20144F /* ExecutorReadYourWriteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C95B2AD05FFF9FC12C9D5878 /* ExecutorReadYourWriteTests.swift */; }; 3C5181A1B2C3D4E5F6A7B802 /* SubscriptionCreateResponseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3C5181A1B2C3D4E5F6A7B801 /* SubscriptionCreateResponseTests.swift */; }; 3CA93BC7300B0100000724B3 /* SubscriptionModelConcurrencyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CA93BC6300B0100000724B3 /* SubscriptionModelConcurrencyTests.swift */; }; 3CAA4BB72F0BAFBA00A16682 /* TriggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CAA4BB62F0BAFBA00A16682 /* TriggerTests.swift */; }; @@ -1481,6 +1482,7 @@ 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 = ""; }; + C95B2AD05FFF9FC12C9D5878 /* ExecutorReadYourWriteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExecutorReadYourWriteTests.swift; sourceTree = ""; }; 3C5181A1B2C3D4E5F6A7B801 /* SubscriptionCreateResponseTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionCreateResponseTests.swift; sourceTree = ""; }; 3CA93BC6300B0100000724B3 /* SubscriptionModelConcurrencyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionModelConcurrencyTests.swift; sourceTree = ""; }; 3CAA4BB62F0BAFBA00A16682 /* TriggerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TriggerTests.swift; sourceTree = ""; }; @@ -2552,6 +2554,7 @@ children = ( 3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */, 3CA93BC3300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift */, + C95B2AD05FFF9FC12C9D5878 /* ExecutorReadYourWriteTests.swift */, 3C5181A1B2C3D4E5F6A7B801 /* SubscriptionCreateResponseTests.swift */, 3CB331692F281692000E1801 /* OSCustomEventsExecutorTests.swift */, FE740F9E6B87D215510B5982 /* ExecutorAnonymousPurgeTests.swift */, @@ -4714,6 +4717,7 @@ 3CC890352C5BF9A7002CB4CC /* UserConcurrencyTests.swift in Sources */, 3CB3316A2F281692000E1801 /* OSCustomEventsExecutorTests.swift in Sources */, 3CA93BC4300AEFFA000724B3 /* SubscriptionUpdateRaceTests.swift in Sources */, + E85C1C500D87196C5E20144F /* ExecutorReadYourWriteTests.swift in Sources */, 3C5181A1B2C3D4E5F6A7B802 /* SubscriptionCreateResponseTests.swift in Sources */, 3CDE664C2BFC2A56006DA114 /* OneSignalUserObjcTests.m in Sources */, D465D9B81F58B242ADF14874 /* OSIdentityModelRepoTests.swift in Sources */, diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Consistency/OSConsistencyManager.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Consistency/OSConsistencyManager.swift index 50de9c001..6e071ad5f 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Consistency/OSConsistencyManager.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/Consistency/OSConsistencyManager.swift @@ -33,10 +33,10 @@ import OneSignalCore @objc public static let shared = OSConsistencyManager() // Serial, and the only place `indexedTokens` and `indexedConditions` may be touched. - // Non-private so test helpers can synchronize with it. + // Neither is private, so test helpers can read them through it. let queue = DispatchQueue(label: "com.consistencyManager.queue") - private var indexedTokens: [String: [NSNumber: OSReadYourWriteData]] = [:] - // Waiters, indexed by the id passed to getRywTokenFromAwaitableCondition. Non-private for tests. + var indexedTokens: [String: [NSNumber: OSReadYourWriteData]] = [:] + // Waiters, indexed by the id passed to getRywTokenFromAwaitableCondition. var indexedConditions: [String: [(OSCondition, DispatchSemaphore)]] = [:] /** @@ -58,7 +58,12 @@ import OneSignalCore } } - // Function to set the token in a thread-safe manner + /** + Records the outcome of a write for `id` under the caller's key, and releases any waiter it satisfies. + A response with no `ryw_token` is filed as a blank entry: the write still completed, and a condition + that only needs it to have completed is met by the entry being there. A fetch that registers later + finds it on file, instead of waiting the full timeout for a token that is never coming. + */ public func setRywTokenAndDelay(id: String, key: any OSConsistencyKeyEnum, value: OSReadYourWriteData) { queue.sync { let nsKey = NSNumber(value: key.rawValue) @@ -73,21 +78,27 @@ import OneSignalCore /// Blocks the caller until the condition is met or `waitTimeout` elapses, then returns the newest /// token the condition accepts, which is nil when it was released without one. @objc public func getRywTokenFromAwaitableCondition(_ condition: OSCondition, forId id: String) -> OSReadYourWriteData? { + // The timeout in force when the waiter registers is the one it waits. + let timeout = OSConsistencyManager.waitTimeout let semaphore = DispatchSemaphore(value: 0) queue.sync { self.indexedConditions[id, default: []].append((condition, semaphore)) self.checkConditionsAndComplete(forId: id) } - if semaphore.wait(timeout: .now() + OSConsistencyManager.waitTimeout) == .timedOut { + if semaphore.wait(timeout: .now() + timeout) == .timedOut { OneSignalLog.onesignalLog(.LL_WARN, message: "OSConsistencyManager timed out waiting on \(condition.conditionId) for id: \(id)") queue.sync { // Skip if a met-path release already removed this waiter. guard self.indexedConditions[id]?.contains(where: { $0.1 === semaphore }) == true else { return } + // Deregister first, so the re-check below cannot release this waiter a second time. + self.indexedConditions[id]?.removeAll { $0.1 === semaphore } // Clear so later fetches for this id are not held to a subscription token that never arrives. condition.onConditionSatisfied?() - self.indexedConditions[id]?.removeAll { $0.1 === semaphore } + // The lowered bar can be all a sibling waiter on this id was missing, so measure the rest + // against it now instead of leaving them to sit out their own timeouts. + self.checkConditionsAndComplete(forId: id) } } return queue.sync { @@ -95,24 +106,6 @@ import OneSignalCore } } - /** - Releases waiters on `conditionId` registered under `id` (e.g. onesignalId). Used when that user's - response carried no `ryw_token`, so those waiters have nothing left to wait for. - */ - @objc(resolveConditionsWithConditionId:forId:) - public func resolveConditions(conditionId: String, forId id: String) { - queue.sync { - guard let waiters = self.indexedConditions[id] else { - return - } - for (condition, semaphore) in waiters where condition.conditionId == conditionId { - OneSignalLog.onesignalLog(.LL_INFO, message: "Condition \(conditionId) resolved for id: \(id)") - self.release(condition, semaphore) - } - self.indexedConditions[id] = waiters.filter { $0.0.conditionId != conditionId } - } - } - // Private method to check conditions for a specific id (unique ID like onesignalId) private func checkConditionsAndComplete(forId id: String) { guard let waiters = indexedConditions[id] else { return } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/ConsistencyManagerTestHelpers.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/ConsistencyManagerTestHelpers.swift index 8649a1a07..23a3037ab 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/ConsistencyManagerTestHelpers.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/ConsistencyManagerTestHelpers.swift @@ -25,7 +25,7 @@ THE SOFTWARE. */ -import OneSignalOSCore +@testable import OneSignalOSCore public class ConsistencyManagerTestHelpers { /// Clears both halves of the read-your-write state: the manager's tokens and waiters, and the @@ -44,3 +44,10 @@ public class ConsistencyManagerTestHelpers { OSConsistencyManager.shared.setRywTokenAndDelay(id: id, key: key, value: rywData) } } + +extension OSConsistencyManager { + /// The tokens on file right now, read through the manager's own queue. + public func snapshotTokens() -> [String: [NSNumber: OSReadYourWriteData]] { + return queue.sync { indexedTokens } + } +} diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSConsistencyManagerTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSConsistencyManagerTests.swift index e422a0e65..ba23f19c0 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSConsistencyManagerTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSConsistencyManagerTests.swift @@ -281,42 +281,51 @@ class OSConsistencyManagerTests: XCTestCase { XCTAssertEqual(rywData?.rywToken, "456") } - // MARK: - Releasing waiters + // MARK: - Writes that came back without a token - /// A response with no `ryw_token` releases waiters for that user only. - func testResolvingByConditionIdReleasesWaitersForThatId() { + private let blank = OSReadYourWriteData(rywToken: nil, rywDelay: nil) + + /// A write that completed with no `ryw_token` is filed like any other, so a waiter that registers + /// afterwards has nothing left to wait for. + func testAWaiterRegisteredAfterATokenlessWriteIsReleasedAtOnce() { + let id = "onesignal-id" + let key = NSNumber(value: OSIamFetchOffsetKey.userUpdate.rawValue) + consistencyManager.setRywTokenAndDelay(id: id, key: OSIamFetchOffsetKey.userUpdate, value: blank) + + let condition = TestMetCondition(expectedTokens: [id: [key: blank]]) let returned = expectation(description: "waiter returned") DispatchQueue.global().async { - _ = self.consistencyManager.getRywTokenFromAwaitableCondition(TestUnmetCondition(), forId: "onesignal-id") + _ = self.consistencyManager.getRywTokenFromAwaitableCondition(condition, forId: id) returned.fulfill() } - OneSignalCoreMocks.waitUntil("waiter registered") { self.consistencyManager.waiterCount == 1 } - - consistencyManager.resolveConditions(conditionId: TestUnmetCondition.CONDITIONID, forId: "onesignal-id") + // The timeout is the full 30 seconds here, so only a release brings the waiter back in time. wait(for: [returned], timeout: 2.0) XCTAssertEqual(consistencyManager.waiterCount, 0) } - /// Resolving user B leaves user A's waiter registered. - func testResolvingOneIdLeavesAnotherIdsWaiterWaiting() { + /// The entry is filed under its id, so user B's tokenless write does not release user A. + func testATokenlessWriteForOneIdLeavesAnotherIdsWaiterWaiting() { OSConsistencyManager.waitTimeout = .milliseconds(200) + let key = NSNumber(value: OSIamFetchOffsetKey.userUpdate.rawValue) let userA = "onesignal-id-a" let userB = "onesignal-id-b" + let conditionA = TestMetCondition(expectedTokens: [userA: [key: blank]]) + let conditionB = TestMetCondition(expectedTokens: [userB: [key: blank]]) let aReturned = expectation(description: "user A waiter returned") let bReturned = expectation(description: "user B waiter returned") DispatchQueue.global().async { - _ = self.consistencyManager.getRywTokenFromAwaitableCondition(TestUnmetCondition(), forId: userA) + _ = self.consistencyManager.getRywTokenFromAwaitableCondition(conditionA, forId: userA) aReturned.fulfill() } DispatchQueue.global().async { - _ = self.consistencyManager.getRywTokenFromAwaitableCondition(TestUnmetCondition(), forId: userB) + _ = self.consistencyManager.getRywTokenFromAwaitableCondition(conditionB, forId: userB) bReturned.fulfill() } OneSignalCoreMocks.waitUntil("both waiters registered") { self.consistencyManager.waiterCount == 2 } - consistencyManager.resolveConditions(conditionId: TestUnmetCondition.CONDITIONID, forId: userB) + consistencyManager.setRywTokenAndDelay(id: userB, key: OSIamFetchOffsetKey.userUpdate, value: blank) wait(for: [bReturned], timeout: 2.0) XCTAssertEqual(consistencyManager.waiterCount, 1, "user A's waiter must still be registered") @@ -324,16 +333,20 @@ class OSConsistencyManagerTests: XCTestCase { wait(for: [aReturned], timeout: 2.0) } - func testResolvingADifferentConditionLeavesTheWaiterWaiting() { + /// The entry is filed under the writer's key, so a waiter that needs a different write keeps waiting. + func testATokenlessWriteUnderAnotherKeyLeavesTheWaiterWaiting() { OSConsistencyManager.waitTimeout = .milliseconds(200) + let id = "onesignal-id" + let subscriptionKey = NSNumber(value: OSIamFetchOffsetKey.subscriptionUpdate.rawValue) + let condition = TestMetCondition(expectedTokens: [id: [subscriptionKey: blank]]) let returned = expectation(description: "waiter returned") DispatchQueue.global().async { - _ = self.consistencyManager.getRywTokenFromAwaitableCondition(TestUnmetCondition(), forId: "onesignal-id") + _ = self.consistencyManager.getRywTokenFromAwaitableCondition(condition, forId: id) returned.fulfill() } OneSignalCoreMocks.waitUntil("waiter registered") { self.consistencyManager.waiterCount == 1 } - consistencyManager.resolveConditions(conditionId: "SomeOtherCondition", forId: "onesignal-id") + consistencyManager.setRywTokenAndDelay(id: id, key: OSIamFetchOffsetKey.userUpdate, value: blank) XCTAssertEqual(consistencyManager.waiterCount, 1) // Drain the timeout so the thread is not left blocked after the test. diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSIamFetchReadyConditionTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSIamFetchReadyConditionTests.swift index f1c122a23..72cbe63e8 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSIamFetchReadyConditionTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSIamFetchReadyConditionTests.swift @@ -101,11 +101,12 @@ final class OSIamFetchReadyConditionTests: XCTestCase { func testTheManagerLowersTheBarWhenItReleasesTheWaiter() { let manager = OSConsistencyManager.shared - OSIamFetchReadyCondition.sharedInstance(withId: userA).setSubscriptionUpdatePending(value: true) + let condition = OSIamFetchReadyCondition.sharedInstance(withId: userA) + condition.setSubscriptionUpdatePending(value: true) let firstReturned = expectation(description: "first fetch released") DispatchQueue.global().async { - _ = manager.getRywTokenFromAwaitableCondition(OSIamFetchReadyCondition.sharedInstance(withId: self.userA), forId: self.userA) + _ = manager.getRywTokenFromAwaitableCondition(condition, forId: self.userA) firstReturned.fulfill() } OneSignalCoreMocks.waitUntil("first fetch waiting") { manager.waiterCount == 1 } @@ -115,33 +116,61 @@ final class OSIamFetchReadyConditionTests: XCTestCase { manager.setRywTokenAndDelay(id: userA, key: OSIamFetchOffsetKey.subscriptionUpdate, value: token("200")) wait(for: [firstReturned], timeout: 2.0) + XCTAssertTrue(condition.isMet(indexedTokens: [userA: userUpdateToken()]), "the release has to lower the bar") - // The next fetch has no subscription update behind it, so the user token alone releases it. + // Forget the tokens, so the subscription token still on file cannot be what releases the next fetch. + manager.reset() let secondReturned = expectation(description: "second fetch released") DispatchQueue.global().async { - _ = manager.getRywTokenFromAwaitableCondition(OSIamFetchReadyCondition.sharedInstance(withId: self.userA), forId: self.userA) + _ = manager.getRywTokenFromAwaitableCondition(condition, forId: self.userA) secondReturned.fulfill() } + OneSignalCoreMocks.waitUntil("second fetch waiting") { manager.waiterCount == 1 } + + // The next fetch has no subscription update behind it, so the user token alone releases it. + manager.setRywTokenAndDelay(id: userA, key: OSIamFetchOffsetKey.userUpdate, value: token("300")) wait(for: [secondReturned], timeout: 2.0) } - /// The `ryw_token`-missing fallback the executors call for that user. - func testResolvingTheConditionReleasesTheFetch() { + /// A write that came back with no `ryw_token` is filed as a blank entry, and the condition only asks + /// whether the entry is there, so the fetch goes out without a token rather than waiting for one. + func testAWriteWithNoTokenReleasesTheFetch() { let manager = OSConsistencyManager.shared + let condition = OSIamFetchReadyCondition.sharedInstance(withId: userA) let returned = expectation(description: "fetch released") + var rywData: OSReadYourWriteData? DispatchQueue.global().async { - _ = manager.getRywTokenFromAwaitableCondition(OSIamFetchReadyCondition.sharedInstance(withId: self.userA), forId: self.userA) + rywData = manager.getRywTokenFromAwaitableCondition(condition, forId: self.userA) returned.fulfill() } OneSignalCoreMocks.waitUntil("fetch waiting") { manager.waiterCount == 1 } - manager.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: userA) + manager.setRywTokenAndDelay(id: userA, key: OSIamFetchOffsetKey.userUpdate, value: blank()) wait(for: [returned], timeout: 2.0) + XCTAssertNil(rywData?.rywToken) } - /// Resolving user B leaves user A's subscription bar raised. - func testResolvingOneUserDoesNotLowerAnotherUsersSubscriptionBar() { + /// The entry stays on file, so a fetch that registers after the tokenless write is released at once + /// instead of waiting the full timeout for a token that is never coming. + func testAFetchThatRegistersAfterATokenlessWriteDoesNotWait() { + let manager = OSConsistencyManager.shared + let condition = OSIamFetchReadyCondition.sharedInstance(withId: userA) + manager.setRywTokenAndDelay(id: userA, key: OSIamFetchOffsetKey.userUpdate, value: blank()) + + let returned = expectation(description: "fetch released") + DispatchQueue.global().async { + _ = manager.getRywTokenFromAwaitableCondition(condition, forId: self.userA) + returned.fulfill() + } + + // The timeout is the full 30 seconds here, so only a release brings the fetch back in time. + wait(for: [returned], timeout: 2.0) + XCTAssertEqual(manager.waiterCount, 0) + } + + /// User B's tokenless write leaves user A's subscription bar raised. + func testATokenlessWriteForOneUserDoesNotLowerAnotherUsersSubscriptionBar() { let manager = OSConsistencyManager.shared let conditionA = OSIamFetchReadyCondition.sharedInstance(withId: userA) let conditionB = OSIamFetchReadyCondition.sharedInstance(withId: userB) @@ -155,13 +184,14 @@ final class OSIamFetchReadyConditionTests: XCTestCase { } OneSignalCoreMocks.waitUntil("user B waiting") { manager.waiterCount == 1 } - manager.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: userB) + // A completed Create User is enough on its own, bar or no bar. + manager.setRywTokenAndDelay(id: userB, key: OSIamFetchOffsetKey.userCreate, value: blank()) wait(for: [bReturned], timeout: 2.0) XCTAssertFalse(conditionA.isMet(indexedTokens: [userA: userUpdateToken()]), "user A's subscription bar must still be up") XCTAssertTrue(conditionB.isMet(indexedTokens: [userB: userUpdateToken()]), - "user B's bar comes down with its own resolve") + "user B's bar comes down with its own release") } /// After a timeout, a later fetch for the same id is released by the user token alone. @@ -177,7 +207,10 @@ final class OSIamFetchReadyConditionTests: XCTestCase { firstReturned.fulfill() } wait(for: [firstReturned], timeout: 2.0) + XCTAssertTrue(condition.isMet(indexedTokens: [userA: userUpdateToken()]), "the timeout has to lower the bar") + // Back to the full timeout, so the second fetch can only return by being released. + OSConsistencyManager.waitTimeout = .seconds(30) manager.setRywTokenAndDelay(id: userA, key: OSIamFetchOffsetKey.userUpdate, value: token("100")) let secondReturned = expectation(description: "second fetch released by user token alone") @@ -188,12 +221,52 @@ final class OSIamFetchReadyConditionTests: XCTestCase { wait(for: [secondReturned], timeout: 2.0) } + /// The bar is shared by every fetch waiting on the id, so lowering it on one timeout can be all a sibling + /// was missing. That sibling goes out with the user token it already had, not after a timeout of its own. + func testATimeoutReleasesASiblingThatAlreadyHasItsUserToken() { + let manager = OSConsistencyManager.shared + let condition = OSIamFetchReadyCondition.sharedInstance(withId: userA) + condition.setSubscriptionUpdatePending(value: true) + + OSConsistencyManager.waitTimeout = .seconds(1) + let firstReturned = expectation(description: "first fetch timed out") + DispatchQueue.global().async { + _ = manager.getRywTokenFromAwaitableCondition(condition, forId: self.userA) + firstReturned.fulfill() + } + OneSignalCoreMocks.waitUntil("first fetch waiting") { manager.waiterCount == 1 } + + // Left to itself, the sibling would sit out the full timeout. + OSConsistencyManager.waitTimeout = .seconds(30) + let siblingReturned = expectation(description: "sibling fetch released") + var siblingData: OSReadYourWriteData? + DispatchQueue.global().async { + siblingData = manager.getRywTokenFromAwaitableCondition(condition, forId: self.userA) + siblingReturned.fulfill() + } + OneSignalCoreMocks.waitUntil("sibling fetch waiting") { manager.waiterCount == 2 } + + // A user token alone satisfies neither while the bar is up. + manager.setRywTokenAndDelay(id: userA, key: OSIamFetchOffsetKey.userUpdate, value: token("100")) + XCTAssertEqual(manager.waiterCount, 2) + + wait(for: [firstReturned], timeout: 2.0) + wait(for: [siblingReturned], timeout: 2.0) + XCTAssertEqual(siblingData?.rywToken, "100", "the sibling proceeds with the token it already had") + XCTAssertEqual(manager.waiterCount, 0) + } + // MARK: - Helpers private func token(_ value: String) -> OSReadYourWriteData { return OSReadYourWriteData(rywToken: value, rywDelay: 0) } + /// What a write that came back with no `ryw_token` files. + private func blank() -> OSReadYourWriteData { + return OSReadYourWriteData(rywToken: nil, rywDelay: nil) + } + private func userCreateToken() -> [NSNumber: OSReadYourWriteData] { return [NSNumber(value: OSIamFetchOffsetKey.userCreate.rawValue): token("create")] } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift index c91eb50fe..4dbb9e25c 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift @@ -292,19 +292,13 @@ class OSPropertyOperationExecutor: OSOperationExecutor { } if let onesignalId = request.identityModel.onesignalId { - if let rywToken = response?["ryw_token"] as? String - { - let rywDelay = response?["ryw_delay"] as? NSNumber - - OSConsistencyManager.shared.setRywTokenAndDelay( - id: onesignalId, - key: OSIamFetchOffsetKey.userUpdate, - value: OSReadYourWriteData(rywToken: rywToken, rywDelay: rywDelay) - ) - } else { - // handle a potential regression where ryw_token is no longer returned by API - OSConsistencyManager.shared.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: onesignalId) - } + // Filed even when the response has no ryw_token, so a fetch that registers after this write + // is not held for a token that never comes. + let rywData = OSReadYourWriteData( + rywToken: response?["ryw_token"] as? String, + rywDelay: response?["ryw_delay"] as? NSNumber + ) + OSConsistencyManager.shared.setRywTokenAndDelay(id: onesignalId, key: OSIamFetchOffsetKey.userUpdate, value: rywData) } } onFailure: { error in OneSignalLog.onesignalLog(.LL_ERROR, message: "OSPropertyOperationExecutor update properties request failed with error: \(error.debugDescription)") diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift index 81745cbc4..78fb01c8d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSSubscriptionOperationExecutor.swift @@ -358,19 +358,13 @@ extension OSSubscriptionOperationExecutor { OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_ADD_REQUEST_QUEUE_KEY, withValue: self.addRequestQueue) if let onesignalId = request.identityModel.onesignalId { - // ryw_token and ryw_delay are top-level fields, siblings of "subscription". - if let rywToken = response?["ryw_token"] as? String - { - let rywDelay = response?["ryw_delay"] as? NSNumber - OSConsistencyManager.shared.setRywTokenAndDelay( - id: onesignalId, - key: OSIamFetchOffsetKey.subscriptionUpdate, - value: OSReadYourWriteData(rywToken: rywToken, rywDelay: rywDelay) - ) - } else { - // handle a potential regression where ryw_token is no longer returned by API - OSConsistencyManager.shared.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: onesignalId) - } + // Filed even when the response has no ryw_token, so a fetch that registers after this write + // is not held for a token that never comes. The token sits beside `subscription`, not inside it. + let rywData = OSReadYourWriteData( + rywToken: response?["ryw_token"] as? String, + rywDelay: response?["ryw_delay"] as? NSNumber + ) + OSConsistencyManager.shared.setRywTokenAndDelay(id: onesignalId, key: OSIamFetchOffsetKey.subscriptionUpdate, value: rywData) } // A 2xx with no subscription object is the server's no-op for a subscription that already exists on this user. @@ -489,18 +483,13 @@ extension OSSubscriptionOperationExecutor { OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_SUBSCRIPTION_EXECUTOR_UPDATE_REQUEST_QUEUE_KEY, withValue: self.updateRequestQueue) if let onesignalId = request.identityModel?.onesignalId { - if let rywToken = response?["ryw_token"] as? String - { - let rywDelay = response?["ryw_delay"] as? NSNumber - OSConsistencyManager.shared.setRywTokenAndDelay( - id: onesignalId, - key: OSIamFetchOffsetKey.subscriptionUpdate, - value: OSReadYourWriteData(rywToken: rywToken, rywDelay: rywDelay) - ) - } else { - // handle a potential regression where ryw_token is no longer returned by API - OSConsistencyManager.shared.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: onesignalId) - } + // Filed even when the response has no ryw_token, so a fetch that registers after this write + // is not held for a token that never comes. + let rywData = OSReadYourWriteData( + rywToken: response?["ryw_token"] as? String, + rywDelay: response?["ryw_delay"] as? NSNumber + ) + OSConsistencyManager.shared.setRywTokenAndDelay(id: onesignalId, key: OSIamFetchOffsetKey.subscriptionUpdate, value: rywData) } self.executeNextPendingUpdateSubscription(for: modelId, inBackground: inBackground) diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift index 9688b8809..4e939717d 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift @@ -362,18 +362,13 @@ extension OSUserExecutor { } if let onesignalId = request.identityModel.onesignalId { - if let rywToken = response["ryw_token"] as? String - { - let rywDelay = response["ryw_delay"] as? NSNumber - OSConsistencyManager.shared.setRywTokenAndDelay( - id: onesignalId, - key: OSIamFetchOffsetKey.userCreate, - value: OSReadYourWriteData(rywToken: rywToken, rywDelay: rywDelay) - ) - } else { - // handle a potential regression where ryw_token is no longer returned by API - OSConsistencyManager.shared.resolveConditions(conditionId: OSIamFetchReadyCondition.CONDITIONID, forId: onesignalId) - } + // Filed even when the response has no ryw_token, so a fetch that registers after this write + // is not held for a token that never comes. + let rywData = OSReadYourWriteData( + rywToken: response["ryw_token"] as? String, + rywDelay: response["ryw_delay"] as? NSNumber + ) + OSConsistencyManager.shared.setRywTokenAndDelay(id: onesignalId, key: OSIamFetchOffsetKey.userCreate, value: rywData) } } OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = false diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorReadYourWriteTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorReadYourWriteTests.swift new file mode 100644 index 000000000..de5b34644 --- /dev/null +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/ExecutorReadYourWriteTests.swift @@ -0,0 +1,183 @@ +/* + 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 OneSignalOSCore +import OneSignalCoreMocks +import OneSignalOSCoreMocks +import OneSignalUserMocks +@testable import OneSignalUser + +/** + What each executor records for the in-app message fetch, with and without a `ryw_token` in the response. + + The fetch waits on `OSIamFetchReadyCondition`, which only asks whether the writes it cares about have + completed. A response with no token still completes the write, so the executor files an entry with no + token under its own key. A fetch that registers afterwards finds the entry and goes out, instead of + waiting the full timeout for a token that is never coming. + */ +final class ExecutorReadYourWriteTests: XCTestCase { + private var client = MockOneSignalClient() + private var newRecordsState = MockNewRecordsState() + private var user = OSIdentityModel(aliases: nil, changeNotifier: OSEventProducer()) + + override func setUpWithError() throws { + OneSignalCoreMocks.clearUserDefaults() + OneSignalUserMocks.reset() + ConsistencyManagerTestHelpers.reset() + OneSignalIdentifiers.currentAppId = "test-app-id" + + client = MockOneSignalClient() + client.fireSuccessForAllRequests = true + OneSignalCoreImpl.setSharedClient(client) + newRecordsState = MockNewRecordsState() + user = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: userA_OSID, OS_EXTERNAL_ID: userA_EUID], changeNotifier: OSEventProducer()) + OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(user) + } + + override func tearDownWithError() throws { + ConsistencyManagerTestHelpers.reset() + OneSignalCoreMocks.clearUserDefaults() + } + + // MARK: - Tests + + func testACreateUserResponseWithNoTokenStillReadiesTheFetch() { + MockUserRequests.setDefaultCreateUserResponses(with: client, externalId: userA_EUID) + let executor = OSUserExecutor( + newRecordsState: newRecordsState, + identityVerificationService: OneSignalUserManagerImpl.sharedInstance.identityVerificationService, + auth: auth + ) + let identity = OSIdentityModel(aliases: [OS_EXTERNAL_ID: userA_EUID], changeNotifier: OSEventProducer()) + + executor.createUser(aliasLabel: OS_EXTERNAL_ID, aliasId: userA_EUID, identityModel: identity) + + OneSignalCoreMocks.waitUntil("A tokenless Create User left the fetch waiting") { self.fetchIsReady(for: userA_OSID) } + } + + func testAnUpdatePropertiesResponseWithNoTokenStillReadiesTheFetch() { + let executor = OSPropertyOperationExecutor(newRecordsState: newRecordsState, auth: auth) + executor.enqueueDelta(OSDelta( + name: OS_UPDATE_PROPERTIES_DELTA, + identityModelId: user.modelId, + externalId: user.externalId, + model: OSModel(changeNotifier: OSEventProducer()), + property: "language", + value: "en" + )) + + executor.processDeltaQueue(inBackground: false) + + OneSignalCoreMocks.waitUntil("A tokenless Update Properties left the fetch waiting") { self.fetchIsReady(for: userA_OSID) } + } + + func testACreateSubscriptionResponseWithNoTokenStillReadiesAFetchHeldForIt() { + holdTheFetchForASubscriptionUpdate() + let email = "a@example.com" + MockUserRequests.setAddEmailResponse(with: client, email: email) + let executor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState, auth: auth) + executor.enqueueDelta(subscriptionDelta(OS_ADD_SUBSCRIPTION_DELTA, model: subscription(type: .email, address: email, id: nil))) + + executor.processDeltaQueue(inBackground: false) + + OneSignalCoreMocks.waitUntil("A tokenless Create Subscription left the fetch waiting") { self.fetchIsReady(for: userA_OSID) } + } + + /// The create response carries `ryw_token` beside `subscription`, and it has to be read from there. + func testACreateSubscriptionResponseWithATokenFilesItForTheFetch() { + let email = "a@example.com" + client.setMockResponseForRequest( + request: "", + response: [ + "subscription": ["id": "\(email)_id", "type": "Email", "token": email], + "ryw_token": "0900", + "ryw_delay": 250 + ] + ) + let executor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState, auth: auth) + executor.enqueueDelta(subscriptionDelta(OS_ADD_SUBSCRIPTION_DELTA, model: subscription(type: .email, address: email, id: nil))) + + executor.processDeltaQueue(inBackground: false) + + OneSignalCoreMocks.waitUntil("The Create Subscription token was not filed") { + self.filedEntry(for: userA_OSID, key: .subscriptionUpdate)?.rywToken == "0900" + } + XCTAssertEqual(filedEntry(for: userA_OSID, key: .subscriptionUpdate)?.rywDelay?.intValue, 250) + } + + func testAnUpdateSubscriptionResponseWithNoTokenStillReadiesAFetchHeldForIt() { + holdTheFetchForASubscriptionUpdate() + let executor = OSSubscriptionOperationExecutor(newRecordsState: newRecordsState, auth: auth) + let push = subscription(type: .push, address: "push-token", id: "push-sub-id") + executor.enqueueDelta(subscriptionDelta(OS_UPDATE_SUBSCRIPTION_DELTA, model: push)) + + executor.processDeltaQueue(inBackground: false) + + OneSignalCoreMocks.waitUntil("A tokenless Update Subscription left the fetch waiting") { self.fetchIsReady(for: userA_OSID) } + } + + // MARK: - Helpers + + private var auth: OSRequestAuthorizing { + return OneSignalUserManagerImpl.sharedInstance.requestAuth + } + + /// Whether a fetch for the user would go out now rather than wait. + private func fetchIsReady(for onesignalId: String) -> Bool { + return OSIamFetchReadyCondition.sharedInstance(withId: onesignalId) + .isMet(indexedTokens: OSConsistencyManager.shared.snapshotTokens()) + } + + /// What is on file for the user under one key, or nil when nothing has been filed there. + private func filedEntry(for onesignalId: String, key: OSIamFetchOffsetKey) -> OSReadYourWriteData? { + return OSConsistencyManager.shared.snapshotTokens()[onesignalId]?[NSNumber(value: key.rawValue)] + } + + /// A subscription update in flight holds the fetch until that write completes, even with a user token on file. + private func holdTheFetchForASubscriptionUpdate() { + OSIamFetchReadyCondition.sharedInstance(withId: userA_OSID).setSubscriptionUpdatePending(value: true) + ConsistencyManagerTestHelpers.setDefaultRywToken(id: userA_OSID) + XCTAssertFalse(fetchIsReady(for: userA_OSID), "the fetch has to be held before the write completes") + } + + private func subscription(type: OSSubscriptionType, address: String, id: String?) -> OSSubscriptionModel { + return OSSubscriptionModel( + type: type, + address: address, + subscriptionId: id, + reachable: true, + isDisabled: false, + changeNotifier: OSEventProducer() + ) + } + + private func subscriptionDelta(_ name: String, model: OSSubscriptionModel) -> OSDelta { + return OSDelta(name: name, identityModelId: user.modelId, externalId: user.externalId, model: model, property: "optedIn", value: true) + } +} diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionCreateResponseTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionCreateResponseTests.swift index cf6edcd7a..a1b9851a3 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionCreateResponseTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionCreateResponseTests.swift @@ -52,8 +52,8 @@ final class SubscriptionCreateResponseTests: XCTestCase { /** The server answers a create for a subscription that already exists on the user with a 2xx and no - subscription object. The request is finished, not retried, the model is left as it was, and a - fetch waiting on this user's read-your-write token is released because none is coming. + subscription object. The request is finished, not retried, the model is left as it was, and the + write is filed with no token, so a fetch held for it is released instead of waiting the timeout out. */ func testResponseWithoutSubscription_completesWithoutHydratingAndReleasesWaiters() throws { let client = MockOneSignalClient() @@ -64,10 +64,10 @@ final class SubscriptionCreateResponseTests: XCTestCase { let user = OneSignalUserMocks.setUserManagerInternalUser(onesignalId: onesignalId) let model = makeEmailSubscriptionModel() - // Only a resolve can release this waiter, so it stays blocked if the response is dropped instead of handled. + // Only the executor filing this write can release the waiter, so it stays blocked if the response is dropped instead of handled. let released = expectation(description: "IAM fetch waiter was not released") DispatchQueue.global().async { - _ = OSConsistencyManager.shared.getRywTokenFromAwaitableCondition(UnmetIamFetchCondition(), forId: self.onesignalId) + _ = OSConsistencyManager.shared.getRywTokenFromAwaitableCondition(SubscriptionWriteFiledCondition(id: self.onesignalId), forId: self.onesignalId) released.fulfill() } OneSignalCoreMocks.waitUntil("Waiter was not registered") { self.waiterCount(forId: self.onesignalId) == 1 } @@ -83,6 +83,9 @@ final class SubscriptionCreateResponseTests: XCTestCase { XCTAssertTrue(client.hasExecutedRequestOfType(OSRequestCreateSubscription.self, expectedCount: 1)) XCTAssertNil(model.subscriptionId) wait(for: [released], timeout: 3.0) + let filed = filedSubscriptionEntry(forId: onesignalId) + XCTAssertNotNil(filed, "the tokenless write was not filed for the fetch") + XCTAssertNil(filed?.rywToken) } /** @@ -154,6 +157,12 @@ final class SubscriptionCreateResponseTests: XCTestCase { } } + private func filedSubscriptionEntry(forId id: String) -> OSReadYourWriteData? { + OSConsistencyManager.shared.queue.sync { + OSConsistencyManager.shared.indexedTokens[id]?[NSNumber(value: OSIamFetchOffsetKey.subscriptionUpdate.rawValue)] + } + } + private func waitForAddRequestQueueToDrain() { OneSignalCoreMocks.waitUntil("Create subscription request was not removed from the cache") { let requests = OneSignalUserDefaults.initShared().getSavedCodeableData( @@ -184,15 +193,21 @@ private final class SubscriptionUpdateTokenCondition: NSObject, OSCondition { } } -/// Never met on its own and carries the IAM fetch condition id, so only the executor's resolve releases it. -private final class UnmetIamFetchCondition: NSObject, OSCondition { - var conditionId: String { OSIamFetchReadyCondition.CONDITIONID } +/// Met once a subscription write is on file for the id, with or without a token, so only the executor filing that write releases it. +private final class SubscriptionWriteFiledCondition: NSObject, OSCondition { + private let id: String + + init(id: String) { + self.id = id + } + + var conditionId: String { "SubscriptionWriteFiledCondition" } func isMet(indexedTokens: [String: [NSNumber: OSReadYourWriteData]]) -> Bool { - false + indexedTokens[id]?[NSNumber(value: OSIamFetchOffsetKey.subscriptionUpdate.rawValue)] != nil } func getNewestToken(indexedTokens: [String: [NSNumber: OSReadYourWriteData]]) -> OSReadYourWriteData? { - nil + indexedTokens[id]?[NSNumber(value: OSIamFetchOffsetKey.subscriptionUpdate.rawValue)] } }