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

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)

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
@github-actions

github-actions Bot commented Jun 25, 2026

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

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.33962% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.80%. 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 73.91% 6 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%   70.80%   +4.09%     
==========================================
  Files         246      246              
  Lines       21541    21620      +79     
==========================================
+ Hits        14372    15309     +937     
+ Misses       7169     6311     -858     
Components Coverage Δ
Analytics 70.78% <ø> (ø)
Common 71.44% <ø> (+0.65%) ⬆️
Core 65.68% <94.23%> (+6.26%) ⬆️
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.63% <100.00%> (+2.06%) ⬆️
...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 61.65% <73.91%> (+16.48%) ⬆️

... and 39 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

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

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

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

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

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

Comment thread libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m Outdated
Comment thread libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m Outdated
- 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

Copy link
Copy Markdown
TestsPassed ☑️Skipped ⚠️Failed ❌️
AuthFlowTester UI Test Results all84 ran68 ✅2 ⚠️14 ❌
TestResult
AuthFlowTester UI Test Results all
AuthFlowTesterUITests.xctest
RTRLoginTests.testECAOpaqueRtr_NoHybrid_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateCAToBeacon_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateCAToECA_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateMultipleUsers_WithRestart()❌ failure
MultiUserLoginTests.testFirstDynamic_SecondStatic_DifferentApps()❌ failure
MultiUserLoginTests.testBothStatic_SameApp_SameScopes()❌ failure
MultiUserLoginTests.testDifferentAppTypes_LogoutCaUser_EcaUserUnaffected()❌ failure
MultiUserLoginTests.testRevokeAccessForUserWithDynamicConfig_OtherUserUnaffected()❌ failure
MultiUserLoginTests.testDifferentAppTypes_RevokeAccessForCaUser_EcaUserUnaffected()❌ failure
RefreshTokenMigrationTests.testMigrateOneUserOnly()❌ failure
RefreshTokenMigrationTests.testMigrateCAToBeaconAndBack()❌ failure
RefreshTokenMigrationTests.testMigrateECA_AddMoreScopes()❌ failure
LegacyLoginTestsNotHybrid.testCAOpaque_AllScopes_UserAgentFlow()❌ failure
WelcomeLoginTests.testWelcomeDiscovery_AdvancedAuthLoginHost()❌ failure

…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)
@github-actions

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
…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.
…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
…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.
…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.
…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.
Server returns invalid_grant for RTR + JWT tokens in hybrid flow.
Tests will be re-enabled once the server fix lands.
…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
…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.
…/switchToUserAndValidateUser

expectAdvancedAuth was derived from loginHost == .advancedAuth alone, missing the
loginForAdmin case where browser-based auth is used regardless of loginHost.
…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.
…tAndValidateUser call

loginHost: .advancedAuth already implies expectAdvancedAuth = true internally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants