Conversation
`OSUserJwtConfig.hydrate` wrote the requirement to UserDefaults while holding its lock, and that write flushes to disk. Every reader of `requirement`, on the repo queue, on main for in-app messages, and on the executor queues, waited on that flush. Update the value under the lock and write after it is released, as the log and the handler already were. Two hydrates racing with different values could now leave disk holding the older value while memory holds the newer. `hydrate` has one guarded call site per session, and the next session's params fetch heals a stale cache.
…m the cache `OSUserJwtConfig.refreshIfUnknown` moved the requirement from unknown to the cached value without firing the hydrated handler, while `hydrate` fires it for the same transition. Not a live bug today: its only caller runs in `OneSignalUserManagerImpl.start()` before any handler is registered, and a late registrant is called immediately when the requirement is already known. It was a trap for any future caller, and the User executor's doc comment cited it as the reason for re-reading the requirement on every send, which now stands on its own. Two tests in OSUserJwtConfigTests: the handler fires with the adopted value, and stays quiet while the cache is still empty. The first fails without the fix.
…n ask nobody hears `OneSignalUserManagerImpl.userJwtInvalidatedObserver` was a lazy optional built with no lock. The ask path reaches it from the executor queues while `addUserJwtInvalidatedListener` runs on the app's thread, so two first touches could each build an observer and the app's listener could land in the one that was discarded. Build it in `init`, and let the JWT repo's notify closure capture it directly instead of reaching through `sharedInstance`. The observable holds listeners weakly, and the ask is the only way the SDK gets a token, so an ask nobody hears now logs at WARN naming the external ID. A cold-start ask that beats the app's registration is the normal path and is replayed when the listener is added, which the message says. The public listener methods, in the JWT extension, OneSignalFramework.h, and OneSignalSwiftInterface.swift, now say the listener is held weakly. Tests in the new UserJwtAskTests, since UserJwtLifecycleTests is at SwiftLint's type body limit: an unheard ask warns with the external ID, and a heard one is delivered without a warning. The old closure discarded the result, so there is no seam to run these against.
…r a logout `OSUserJwtRepo` asks the app once per external ID per session, and only a stored token cleared that. `logout` leaves the entry in place, so a user who was asked, never answered, logged out, and logged back in without a token was never asked again: `park` got false from `askForToken` and logged nothing, and every user-scoped call for that user stayed held for the rest of the process. The same held for login(A), login(B), login(A). Clear the ask wherever `login` builds a new Identity Model for the user, in `createNewUser` and `identifyUser`, so each login is a fresh chance to be asked. Clearing the outgoing user on logout instead would re-ask for A right after a switch to B whenever A's held Create User is still queued, which the app did not invite. Tests: `clearAsk` in OSUserJwtRepoTests, and in UserJwtAskTests a login with no token, a logout, and the same login again, asserting two asks. The second fails without the fix.
18 tasks
…at still said once per session `clearAsk` made three comments stale: the `askForToken` and `pendingTokenAsks` docs in OSUserJwtRepo, and the `park` doc in OSRequestAuth. Each now says a stored token or a login as that user clears the ask.
…kTests teardown Same gate as UserJwtLifecycleTests: the mock answers late, and a Request still in flight at teardown would land in the next test and hydrate the shared models and JWT repo out from under it.
nan-li
marked this pull request as ready for review
September 18, 2026 01:24
…ble token `storeJwt` clears the ask itself when it stores, so the explicit clear before it was redundant on that path and left a window where a flush between the two lines asked for the token the caller had just supplied. The clear now runs only when the login has no token or the repo refused it, so a login with an empty token still asks again.
…ide log Log listeners hear every line in the process, on whichever thread logged it, so the exact count and the empty check could both trip on a warning from another test class, such as the executor's blocked-request retry. The capture now filters by level and text and appends under a lock.
…er was registered An ask that fires before the app registers its listener is the cold-start order, and the replay on registration delivers it, so it now logs at debug instead of printing a warning at the default console level on every such launch. The warning stays for an ask after a listener was registered, which means the app let its listener go. The listeners and the flag live together in OSUserJwtInvalidatedListeners, which replaces the static and the bare observer. Tests: an ask before any registration leaves no warning and one debug line; an ask after the registered listener was released warns. The first fails with the warning forced on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
One Line Summary
Four follow-ups from the Identity Verification review, covering how the requirement is cached and how the app is asked for a token. Tracked on SDK-5137.
Details
Motivation
Review feedback on the Identity Verification stack was collected on SDK-5137 instead of amending the stacked branches. This PR takes the items around the cached requirement and the token request to the app. #1740 covers the request queue items.
Scope
Each fix is its own commit.
Follow-ups from review, each its own commit:
Not changed: the public API surface, request signing, in-app messaging.
Testing
Unit testing
One or more tests per fix except the first, next to the code they cover. For the second and fourth fixes, the new test was run against the previous commit and failed there, then passed with the fix. The first moves a disk write out of a lock, which no test can observe deterministically, so it has none. The third has no old behavior a test can observe, so its tests cover the new behavior only. The token request tests live in a new file,
UserJwtAskTests, because the existing lifecycle test class is at SwiftLint's size limit. The follow-ups add three tests there: a login with an empty token asks again, an unheard request before any listener is registered logs no warning, and one after the registered listener was released does. The second fails with the warning forced on.Manual testing
Not run on a device. The full unit test plan was run locally on an iPhone 17 Pro simulator.
Affected code checklist
Checklist
Overview
Testing
Final pass
🤖 Generated with Claude Code