refactor: route identifier reads through OneSignalIdentifiers#1667
refactor: route identifier reads through OneSignalIdentifiers#1667nan-li wants to merge 3 commits into
Conversation
Add OneSignalIdentifiers in OneSignalOSCore with storedAppId and storedSubscriptionId accessors backed by shared UserDefaults. Migrate six call sites; drop the now-dead OSUD_APP_ID write to initStandard.
OSSubscriptionModel.subscriptionId.didSet already persists to UserDefaults on every change, so the static _pushSubscriptionId cache and its setter were duplicating state with drift risk. Route the one caller through OneSignalIdentifiers.subscriptionId directly. Drop the now-unused OSUserExecutor.setPushSubscriptionId call too — the model hydration on the line above already triggers the persistence path.
| isa = PBXFrameworksBuildPhase; | ||
| buildActionMask = 2147483647; | ||
| files = ( | ||
| 3C5C70022FCB935000102E2C /* OneSignalOSCore.framework in Frameworks */, |
There was a problem hiding this comment.
🔴 This PR adds OneSignalOSCore.framework to the link phase of the OneSignalExtension and OneSignalNotifications targets (pbxproj lines 1985, 2070) and the implementations now reference OneSignalIdentifiers from those binaries, but none of the package manifests declare the new dependency. Customers using the standard NSE integration (CocoaPods OneSignalXCFramework/OneSignalExtension or SPM OneSignalExtension product) will not get OneSignalOSCore.framework embedded into their NSE target, so dyld will fail to load the binary when a push fires — silently breaking confirmed delivery and receive-receipts. Fix: add ss.dependency 'OneSignalXCFramework/OneSignalOSCore' to the OneSignalExtension and OneSignalNotifications subspecs in both OneSignalXCFramework.podspec and OneSignal.podspec, and add "OneSignalOSCore" to the OneSignalExtensionWrapper and OneSignalNotificationsWrapper target dependency arrays in Package.swift.
Extended reasoning...
What changed
This PR links OneSignalOSCore.framework into the OneSignalExtension target (pbxproj line 1985, build phase id 3C5C70022FCB935000102E2C) and the OneSignalNotifications target (pbxproj line 2070, build phase id 3C5C6FFD2FCB933100102E2C). The implementations in those frameworks now #import <OneSignalOSCore/OneSignalOSCore-Swift.h> and call OneSignalIdentifiers.subscriptionId / .storedAppId:
OneSignalExtension/OneSignalNotificationServiceExtensionHandler.m(NSE confirmed-delivery path, line 144-145)OneSignalExtension/OneSignalReceiveReceiptsController.m(receive-receipts path, line 42-43)OneSignalNotifications/OSNotificationsManager.m(submitNotificationOpened, line 776)
After this PR, the shipped OneSignalExtension.xcframework and OneSignalNotifications.xcframework binaries will have an LC_LOAD_DYLIB entry for @rpath/OneSignalOSCore.framework/OneSignalOSCore.
What is missing
The package manifests have not been updated to declare the new transitive dependency:
OneSignalXCFramework.podspec (lines 28-39):
s.subspec OneSignalExtension do |ss|
ss.dependency OneSignalXCFramework/OneSignalCore
ss.dependency OneSignalXCFramework/OneSignalOutcomes
# MISSING: ss.dependency OneSignalXCFramework/OneSignalOSCore
end
s.subspec OneSignalNotifications do |ss|
ss.dependency OneSignalXCFramework/OneSignalCore
ss.dependency OneSignalXCFramework/OneSignalOutcomes
ss.dependency OneSignalXCFramework/OneSignalExtension
# MISSING: ss.dependency OneSignalXCFramework/OneSignalOSCore
endOneSignal.podspec (lines 28-39): identical pattern, same omissions.
Package.swift (lines 72-90):
.target(name: "OneSignalNotificationsWrapper",
dependencies: ["OneSignalNotifications", "OneSignalExtension", "OneSignalOutcomes", "OneSignalCore"]), // MISSING "OneSignalOSCore"
.target(name: "OneSignalExtensionWrapper",
dependencies: ["OneSignalExtension", "OneSignalOutcomes", "OneSignalCore"]), // MISSING "OneSignalOSCore"I verified there is no transitive pull-in: the OneSignalOutcomes subspec depends only on OneSignalCore; OneSignalExtension depends only on Core+Outcomes; OneSignalCore has no dependencies. So a consumer wiring only OneSignalXCFramework/OneSignalExtension (or the SPM OneSignalExtension product) into their NSE target will not receive OneSignalOSCore.framework.
Step-by-step proof of failure
- Customer follows the documented NSE integration: their Podfile contains
pod OneSignalXCFramework/OneSignalExtensionunder their NSE target (or.product(name: "OneSignalExtension", package: "OneSignalFramework")for SPM). - CocoaPods/SPM resolves dependencies. Per the manifests above, only
OneSignalCore+OneSignalOutcomes+OneSignalExtensionare embedded into the NSE bundle.OneSignalOSCore.frameworkis not copied or linked. - A push notification with
mutable-content: 1arrives. iOS spins up the NSE process and loadsOneSignalExtension.xcframework. - dyld walks the load commands of
OneSignalExtension. It encountersLC_LOAD_DYLIB @rpath/OneSignalOSCore.framework/OneSignalOSCoreand cannot resolve it (the framework is not in the bundlesFrameworksdirectory). - dyld emits
Library not loaded: @rpath/OneSignalOSCore.framework/OneSignalOSCoreand aborts. The NSE process crashes beforedidReceiveNotificationExtensionRequest:is even invoked. - iOS falls back to displaying the unmodified APNS payload (no media attachments, no confirmed delivery, no receive-receipt). The crash is silent to the end user but observable in Console as a
DYLD, [0x4]termination.
This affects every push the NSE handles — confirmed-delivery and receive-receipts go to zero for the affected install.
Why this is not caught
- The workspace build succeeds because the Xcode workspace embeds all internal frameworks side-by-side.
- The main-app integration (
OneSignal/OneSignalCompletesubspec) already pullsOneSignalOSCorein transitively, so single-target apps are fine. - The break only manifests for the NSE subset integration, which is the documented standard pattern in OneSignals NSE setup guide.
- The PRs own test plan explicitly leaves
[ ] Confirmed delivery (NSE) and receipts flow exercisedunchecked, so this regression has not been validated end-to-end.
Fix
Add the missing dependency declarations:
OneSignalXCFramework.podspec: addss.dependency OneSignalXCFramework/OneSignalOSCoreto both theOneSignalExtensionandOneSignalNotificationssubspecs.OneSignal.podspec: addss.dependency OneSignal/OneSignalOSCoreto both theOneSignalExtensionandOneSignalNotificationssubspecs.Package.swift: add"OneSignalOSCore"to thedependenciesarray of bothOneSignalExtensionWrapperandOneSignalNotificationsWrappertargets.
…gnalIdentifiers.currentAppId OneSignalConfigManager held two distinct things in one ObjC class in OneSignalCore: (1) the in-memory app_id state, and (2) the SDK-readiness predicate `shouldAwaitAppIdAndLogMissingPrivacyConsent`. Split into two Swift classes in OneSignalOSCore so all identifier accessors live in one module: - `OneSignalIdentifiers.currentAppId` — in-memory app_id, replaces `OneSignalConfigManager.getAppId/setAppId`. Sits next to the existing `storedAppId` (persisted) and `subscriptionId` (persisted) accessors. - `OneSignalConfig.shouldAwaitAppIdAndLogMissingPrivacyConsent` — same semantics, now reads `OneSignalIdentifiers.currentAppId` internally. Migrate ~30 call sites across OneSignalCore (umbrella), OneSignalOSCore, OneSignalUser, OneSignalLiveActivities, OneSignalNotifications, OneSignal- Location, OneSignalInAppMessages, OneSignalOutcomes, and the main OneSignal target. Link OneSignalOSCore.framework into OneSignalInAppMessages, One- SignalLocation, and OneSignalOutcomes targets (they didn't previously).
d99f0ad to
eb1a421
Compare
Description
One Line Summary
Consolidate all OneSignal SDK identifier accessors (
app_id,subscription_id) into a single Swift module so future fallback logic (cross-process / locked-state) can be added in one place.Details
Motivation
The SDK currently reads
OSUD_APP_IDandOSUD_PUSH_SUBSCRIPTION_IDfromOneSignalUserDefaultsat ~20 ad-hoc call sites across 8 modules, with three different in-memory caches (OneSignalConfigManager._appId,OSNotificationsManager._pushSubscriptionId,_user.pushSubscriptionModel.subscriptionId). This makes it difficult to add a coordinated fallback (e.g. for the iOS prewarm bug whereUserDefaultsis unreadable before first unlock). Consolidating the accessors intoOneSignalIdentifiers(durable reads) andOneSignalConfig(readiness predicate) sets up a single place for that follow-up.Scope
Three commits:
21ed8132— AddOneSignalIdentifiers.storedAppId/subscriptionIdinOneSignalOSCore. Migrate 6 durable-read sites. LinkOneSignalOSCoreintoOneSignalNotificationsandOneSignalExtensiontargets. Drop now-deadOSUD_APP_IDwrite toinitStandard.07891381— Drop redundantOSNotificationsManager._pushSubscriptionIdstatic cache + setter; the model'sdidSetalready persists. Removes drift risk and one call inOSUserExecutor.eb1a4213— ReplaceOneSignalConfigManagerwithOneSignalConfig+OneSignalIdentifiers.currentAppId(thread-safe viaNSLock). Migrate ~30 call sites across 8 modules. LinkOneSignalOSCoreintoOneSignalInAppMessages,OneSignalLocation,OneSignalOutcomes.Behavior change to note:
OneSignal.m'sprevAppIdreads previously hitinitStandard, now hitinitSharedvia the accessor. Behavior-preserving for any install on SDK 2.13.0+ (Dec 2019, commit7240697d) —handleAppIdChangehas dual-writtenOSUD_APP_IDto both stores ever since.Not changed: any externally-observable behavior. No public API changes.
Testing
Unit testing
Existing tests updated for the renamed
OneSignalConfigManager.setAppId(...)→OneSignalIdentifiers.currentAppId = ....Manual testing
Built and ran the dev app on a physical device with the SDK from source. Verified
OneSignal.User.onesignalIdandOneSignal.User.pushSubscription.idreturn the expected real values. Confirmed via temporary debug logs thatOneSignalIdentifiers.storedAppId/subscriptionIdreturn the persisted values at runtime.Affected code checklist
OneSignalIdentifiers)Checklist
Overview
Testing
Final pass