Skip to content

feat(W-23159740): per-user persistent feature flags (iOS)#4086

Open
wmathurin wants to merge 22 commits into
forcedotcom:devfrom
wmathurin:W-21167151-feature-flags-per-user
Open

feat(W-23159740): per-user persistent feature flags (iOS)#4086
wmathurin wants to merge 22 commits into
forcedotcom:devfrom
wmathurin:W-21167151-feature-flags-per-user

Conversation

@wmathurin

@wmathurin wmathurin commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds per-user feature flag storage to SFUserAccount via persistedFeatureFlags (NSSet, serialized with NSKeyedArchiver into UserAccount.plist)
  • SFSDKAppFeatureMarkers gains per-user register/unregister/query methods; +appFeaturesForUser: returns global ∪ per-user union; +loadPersistedFeatures:forUser: hydrates in-memory state without writeback
  • SalesforceSDKManager.userAgentString:qualifier:forUser: resolves per-user flags for UA construction; hydratePerUserFeatureFlags populates in-memory state at SDK init
  • SFUserAccountManager.finalizeAuthCompletion: promotes BW/WD/QR transient global flags to per-user after authentication completes
  • SmartStore (SU) and MobileSync (MS) flags now registered per-user
  • 6 new unit tests in SFSDKAppFeatureMarkersTests

GUS Stories

  • W-23159740 (iOS per-user feature flags)
  • W-21167151 (spike)

Test plan

  • SFSDKAppFeatureMarkersTests — 9 tests, 0 failures
  • SalesforceSDKManagerTests — all existing tests pass
  • Build succeeds: xcodebuild build -scheme SalesforceSDKCore -sdk iphonesimulator
  • Integration tests (require sandbox credentials — run in CI)

@wmathurin
feat(W-23159740): per-user persistent feature flags (iOS)
df198a2 
Implements per-user feature flag storage and hydration so each
user account's active features are accurately reflected in the
User-Agent string across app restarts.

- SFSDKAppFeatureMarkers: +registerAppFeature:forUser:, +unregisterAppFeature:forUser:,
  +appFeaturesForUser: (union of global + per-user), +loadPersistedFeatures:forUser:
- SFUserAccount: persistedFeatureFlags property (NSSet<NSString*>),
  serialized via NSKeyedArchiver into UserAccount.plist
- SalesforceSDKManager: userAgentString:qualifier:forUser: unions global + per-user flags;
  hydratePerUserFeatureFlags called from sharedManager dispatch_once
- SFUserAccountManager: finalizeAuthCompletion promotes BW/WD/QR transient
  global flags to per-user after login completes
- SFSmartStore: SU flag now registered per-user
- SFMobileSyncSyncManager: MS flag now registered per-user
- SFSDKAppFeatureMarkersTests: 6 new tests covering isolation, union,
  nil-user fallback, per-user unregister, hydration, and persistence
This was referenced Jun 25, 2026
Open
Open
@github-actions

github-actions Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
1 Warning
⚠️ Static Analysis found an issue with one or more files you modified. Please fix the issue(s).

Clang Static Analysis Issues

File Type Category Description Line Col
SFUserAccount Nullability Memory error nil assigned to a pointer which is expected to have non-null value 266 22
SFUserAccount Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 450 18
SFUserAccount Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 452 18
SFUserAccountManager Nullability Memory error Null passed to a callee that requires a non-null 2nd parameter 1618 15
SFUserAccountManager Nullability Memory error Null passed to a callee that requires a non-null 2nd parameter 1633 15
SFUserAccountManager Nullability Memory error nil passed to a callee that requires a non-null 2nd parameter 2306 13
SalesforceSDKManager Nil value used as mutex for @synchronized() (no synchronization will occur) Logic error Nil value used as mutex for @synchronized() (no synchronization will occur) 145 5
SalesforceSDKManager Nil value used as mutex for @synchronized() (no synchronization will occur) Logic error Nil value used as mutex for @synchronized() (no synchronization will occur) 157 5
SFSmartStore Nullability Memory error nil passed to a callee that requires a non-null 2nd parameter 583 28
SFSmartStore Nullability Memory error nil passed to a callee that requires a non-null 2nd parameter 596 27
SFSmartStore NSError** null dereference Coding conventions (Apple) Potential null dereference. According to coding standards in 'Creating and Returning NSError Objects' the parameter may be null 1349 16
SFSmartStore Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 1425 28

Generated by 🚫 Danger

@codecov

codecov Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.50943% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.51%. Comparing base (10c1396) to head (a53c8aa).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
...SDKCore/Classes/UserAccount/SFUserAccountManager.m 60.86% 9 Missing ⚠️

❌ Your patch check has failed because the patch coverage (77.88%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev    #4086      +/-   ##
==========================================
+ Coverage   66.71%   68.51%   +1.79%     
==========================================
  Files         246      246              
  Lines       21541    21620      +79     
==========================================
+ Hits        14372    14813     +441     
+ Misses       7169     6807     -362
Components Coverage Δ
Analytics 70.78% <ø> (ø)
Common 71.25% <ø> (+0.46%) ⬆️
Core 62.15% <91.34%> (+2.73%) ⬆️
SmartStore 73.44% <100.00%> (-0.16%) ⬇️
MobileSync 88.88% <100.00%> (+0.32%) ⬆️
Files with missing lines Coverage Δ
...bileSync/Classes/Manager/SFMobileSyncSyncManager.m 76.06% <100.00%> (ø)
...rceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m 100.00% <100.00%> (ø)
...forceSDKCore/Classes/Common/SalesforceSDKManager.m 73.46% <100.00%> (+1.89%) ⬆️
...esforceSDKCore/Classes/UserAccount/SFUserAccount.m 80.76% <100.00%> (+2.47%) ⬆️
libs/SmartStore/SmartStore/Classes/SFSmartStore.m 80.47% <100.00%> (-0.36%) ⬇️
...SDKCore/Classes/UserAccount/SFUserAccountManager.m 55.32% <60.86%> (+10.16%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SmartStore iOS ^26 Test Results177 ran177 ✅
TestResult
No test annotations available

@github-actions

github-actions Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SalesforceSDKCore iOS ^26 Test Results693 ran693 ✅
TestResult
No test annotations available

@github-actions

github-actions Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
TestsPassed ☑️SkippedFailed ❌️
MobileSync iOS ^18 Test Results234 ran213 ✅21 ❌
TestResult
MobileSync iOS ^18 Test Results
BatchSyncUpTargetTests.testSyncUpManyLocallyCreatedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithBadType❌ failure
BatchSyncUpTargetTests.testSyncUpWithCreateAndUpdateFieldList❌ failure
BatchSyncUpTargetTests.testSyncUpWithCreateFieldList❌ failure
BatchSyncUpTargetTests.testSyncUpWithErrors❌ failure
BatchSyncUpTargetTests.testSyncUpWithExternalId❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyCreatedAndDeletedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyCreatedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyCreatedRecordsWithoutOverwrite❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyDeletedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyDeletedRecordsWithoutOverwrite❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyDeletedRemotelyDeletedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyUpdatedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyUpdatedRecordsWithoutOverwrite❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyUpdatedRemotelyDeletedRecords❌ failure
BatchSyncUpTargetTests.testSyncUpWithLocallyUpdatedRemotelyDeletedRecordsWithoutOverwrite❌ failure
BatchSyncUpTargetTests.testSyncUpWithNoLocalUpdates❌ failure
BatchSyncUpTargetTests.testSyncUpWithNoType❌ failure
BatchSyncUpTargetTests.testSyncUpWithUpdateFieldList❌ failure
BriefcaseSyncDownTests.testCleanGhostsOneObjectType()❌ failure
SyncManagerTests.testCleanResyncGhostsForMRUTarget❌ failure

@github-actions

github-actions Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
TestsPassed ☑️SkippedFailed ❌️
MobileSync iOS ^26 Test Results234 ran232 ✅2 ❌
TestResult
MobileSync iOS ^26 Test Results
BatchSyncUpTargetTests.testSyncUpManyLocallyCreatedRecords❌ failure
CollectionSyncUpTargetTests.testSyncUpWithBadType❌ failure

@github-actions

github-actions Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SalesforceSDKCore iOS ^18 Test Results693 ran693 ✅
TestResult
No test annotations available

bbirman
bbirman reviewed Jun 25, 2026
View reviewed changes
Comment thread libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m Outdated
bbirman
bbirman reviewed Jun 25, 2026
View reviewed changes
Comment thread libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m Outdated
@wmathurin
refactor(W-23159740): address bbirman PR review comments
6a497d0 
- Replace custom SFSDKUserKey() with SFKeyForUserAndScope(user, SFUserAccountScopeUser)
  to reuse the existing codebase convention for user-scoped keys
- Capture per-user flag snapshot inside dispatch_sync block before
  assigning to user.persistedFeatureFlags, eliminating the map read
  outside the queue's protection
@github-actions

github-actions Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
TestsPassed ☑️Skipped ⚠️Failed ❌️
AuthFlowTester UI Test Results all84 ran69 ✅2 ⚠️13 ❌
TestResult
AuthFlowTester UI Test Results all
AuthFlowTesterUITests.xctest
LegacyLoginTestsNotHybrid.testCAOpaque_DefaultScopes_WebServerFlow()❌ failure
RefreshTokenMigrationTests.testMigrateCAUserAgentToECAWebServer()❌ failure
RefreshTokenMigrationTests.testMigrateCA_AddMoreScopes()❌ failure
RefreshTokenMigrationTests.testMigrateCAToECA()❌ failure
RefreshTokenMigrationTests.testMigrateOneUserOnly()❌ failure
AdvancedAuthBeaconLoginTests.testBeaconOpaque_AllScopes()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateMultipleUsers_WithRestart()❌ failure
LoginWithRestartTests.testMultiUserRestart()❌ failure
MultiUserLoginTests.testBothDynamic_DifferentApps()❌ failure
MultiUserLoginTests.testFirstStatic_SecondDynamic_DifferentApps()❌ failure
MultiUserLoginTests.testLogoutUserWithDynamicConfig_OtherUserUnaffected()❌ failure
MultiUserLoginTests.testBothStatic_DifferentApps()❌ failure
WelcomeLoginTests.testWelcomeDiscovery_AdvancedAuthLoginHost_DynamicConfig()❌ failure

wmathurin added 2 commits June 25, 2026 13:47
@wmathurin
feat(W-23159740): display user agent in AuthFlowTester; add UA valida…
be4dd4e 
…tion to UI tests

- UserCredentialsView: add "SDK" section with User Agent row and JSON export
- AuthFlowTesterMainPageObject: add getUserAgent() via JSON export
- BaseAuthFlowTester: add validateUserAgent(), loginOtherUser(), isMultiUser param
- MultiUserLoginTests: add testAdvancedAuthUser_HasBWFlag_RegularAuthUser_DoesNot
- SFUserAccountManager: fix BW/WD global flag bleed by clearing global flags after
  promoting to per-user in finalizeAuthCompletion (matches existing QR pattern)
@wmathurin
feat(W-23159740): validate UA in restart tests; verify flag persisten…
c2348eb 
…ce round-trip
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SmartStore iOS ^18 Test Results177 ran177 ✅
TestResult
No test annotations available

wmathurin added 12 commits June 25, 2026 14:05
@wmathurin
test(W-23159740): assert MU gone after second user logs out
002ff46
@wmathurin
refactor(W-23159740): avoid double UA fetch by splitting validateUser…
543ef42 
…Agent into public/private overloads

validateUserAgent(loginHost:...) now fetches the UA once and delegates to a
private overload that takes ua: String. validate() calls the private overload
using credentials already loaded, eliminating a second export-button round-trip.
@wmathurin
test(W-23159740): add unit tests covering per-user feature flag new c…
83948c2 
…ode paths

- SFSDKAppFeatureMarkersTests: appFeaturesForUser:nil equivalence, unregister updates persistedFeatureFlags
- SFUserAccountManagerTests: persistedFeatureFlags NSKeyedArchiver encode/decode roundtrip
- SalesforceSDKManagerTests: userAgentString:forUser: with per-user flag, nil-user fallback, hydratePerUserFeatureFlags
- Expose hydratePerUserFeatureFlags in SalesforceSDKManager+Internal.h for testability
@wmathurin
fix(W-23159740): check global WD flag at auth completion instead of r…
34cd737 
…e-deriving from credentials.domain

By auth completion, handleCustomDomainUpdateWithLoginHint:myDomain: has replaced
credentials.domain with the resolved org domain, which no longer contains '/discovery'.
Re-evaluating isDiscoveryDomain at that point always returns false, so the WD flag
was never promoted to per-user. Use the transient global flag (already set by
SFOAuthCoordinator at auth start) as the source of truth, matching the QR flag pattern.
@wmathurin
test(W-23159740): add auth-completion flag promotion pattern tests fo…
d15e710 
…r BW, WD, QR

Covers the six promotion call patterns from SFUserAccountManager auth-completion
block (lines 2082-2107): BW promoted/cleared on advancedBrowser vs non-advanced
authType, WD promoted/cleared based on global flag presence, QR promoted/cleared
based on global flag presence.
@wmathurin
fix(W-23159740): pass expectAdvancedAuth to validateUserAgent for log…
0f68f2b 
…inForAdmin

LoginForAdminTests use a regular login host but browser-based auth, which
sets the BW flag. validateUserAgent was deciding BW solely from loginHost,
causing LoginForAdminTests to fail. Now callers pass expectAdvancedAuth
(true when loginForAdmin || loginHost == .advancedAuth) and the UA check
uses that instead of re-deriving from the host.
@wmathurin
chore: increase UI test timeout from 6m to 8m in AuthFlowTester.xctes…
79adc98 
…tplan
@wmathurin
test: skip RTRLoginTests JWT hybrid tests until W-22512846
9cec4b9 
Server returns invalid_grant for RTR + JWT tokens in hybrid flow.
Tests will be re-enabled once the server fix lands.
@wmathurin
ci: retrigger — pick up run_all_ui_tests workflow fix from forcedotco…
326ebc3 
…m#2942
@wmathurin
test: restore test bodies in skipped RTR hybrid tests
8b093fd
@wmathurin
Merge remote-tracking branch 'origin/dev' into W-21167151-feature-fla…
afc3689 
…gs-per-user
@wmathurin
Merge branch 'dev' of github.com:forcedotcom/SalesforceMobileSDK-iOS …
0dc6c64 
…into W-21167151-feature-flags-per-user
wmathurin added 6 commits June 27, 2026 11:33
@wmathurin
refactor(W-23159740): fold userAgent into getUserCredentials; validat…
f8dea62 
…e UA before revoke/refresh

- Add userAgent to UserCredentialsData and parse it from the sdk section
  in getUserCredentials(), eliminating the separate getUserAgent() UI round trip
- Remove standalone getUserAgent() from page object and BaseAuthFlowTester
- validateUserAgent() now takes userCredentials instead of calling getUserAgent()
- In validate(), move validateUserAgent before assertRevokeAndRefreshWorks so
  the UA is checked on the just-authenticated state, not after token rotation
@wmathurin
refactor(W-23159740): move validateUserAgent into validateUser for co…
db10120 
…nsistency with Android

validateUser() now validates feature flags inline (matching Android's validateUser()),
so callers never need a separate validateUserAgent step. The expectAdvancedAuth,
usesWelcomeDiscovery, and isMultiUser flags are forwarded through validate(),
restartAndValidateUser, and switchToUserAndValidateUser.
@wmathurin
fix(auth-ui-tests): add loginForAdmin param to restartAndValidateUser…
361ff62 
…/switchToUserAndValidateUser

expectAdvancedAuth was derived from loginHost == .advancedAuth alone, missing the
loginForAdmin case where browser-based auth is used regardless of loginHost.
@wmathurin
test(W-21167151): verify BW and WD feature flags persist across app r…
ec2fd97 
…estart

Adds two tests to LoginWithRestartTests:
- testAdvancedAuth_WithRestart: login via advanced auth (BW flag), restart, assert BW still set
- testWelcomeDiscovery_WithRestart: login via welcome discovery (WD flag), restart, assert WD still set

Also adds usesWelcomeDiscovery param to restartAndValidateUser so the WD flag
can be asserted post-restart.
@wmathurin
fix(auth-ui-tests): remove invalid expectAdvancedAuth arg from restar…
904a4d3 
…tAndValidateUser call

loginHost: .advancedAuth already implies expectAdvancedAuth = true internally.
@wmathurin
docs(W-21167151): update LoginWithRestartTests count and add BW/WD fe…
a53c8aa 
…ature flag tests to overview
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bbirman bbirman bbirman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wmathurin @bbirman