diff --git a/packages/remote-feature-flag-controller/CHANGELOG.md b/packages/remote-feature-flag-controller/CHANGELOG.md index f39d1c5a22e..4c8143c9621 100644 --- a/packages/remote-feature-flag-controller/CHANGELOG.md +++ b/packages/remote-feature-flag-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `defaultFeatureFlags` constructor option to `RemoteFeatureFlagController` for client-side defaults as the lowest-precedence layer under processed remote flags and local overrides. Create and persist `processedRemoteFeatureFlag` for flag reconstruction on controller creation. ([#9747](https://github.com/MetaMask/core/pull/9747)) + ## [5.0.0] ### Added diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts index 490b741a828..33cdec16c04 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts @@ -59,6 +59,7 @@ const MOCK_BASE_VERSION = '13.10.0'; * @param options.getMetaMetricsId - Returns metaMetricsId * @param options.clientVersion - The client version string * @param options.prevClientVersion - The previous client version string + * @param options.defaultFeatureFlags - Client-side default feature flags * @returns The controller and the root messenger */ function createController( @@ -69,6 +70,7 @@ function createController( getMetaMetricsId: () => string; clientVersion: string; prevClientVersion: string; + defaultFeatureFlags: FeatureFlags; }> = {}, ): { controller: RemoteFeatureFlagController; messenger: RootMessenger } { const { rootMessenger, controllerMessenger } = buildMessenger(); @@ -83,6 +85,7 @@ function createController( ((): typeof MOCK_METRICS_ID => MOCK_METRICS_ID), clientVersion: options.clientVersion ?? MOCK_BASE_VERSION, prevClientVersion: options.prevClientVersion, + defaultFeatureFlags: options.defaultFeatureFlags, }); return { controller, messenger: rootMessenger }; } @@ -96,6 +99,7 @@ describe('RemoteFeatureFlagController', () => { remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }); }); @@ -107,6 +111,7 @@ describe('RemoteFeatureFlagController', () => { remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }); }); @@ -117,6 +122,7 @@ describe('RemoteFeatureFlagController', () => { cacheTimestamp: 123456789, rawRemoteFeatureFlags: {}, localOverrides: {}, + processedRemoteFeatureFlags: undefined, }; const { controller } = createController({ state: customState }); @@ -175,6 +181,7 @@ describe('RemoteFeatureFlagController', () => { const { controller, messenger } = createController({ state: { remoteFeatureFlags: MOCK_FLAGS, + processedRemoteFeatureFlags: MOCK_FLAGS, }, clientConfigApiService, disabled: true, @@ -195,6 +202,7 @@ describe('RemoteFeatureFlagController', () => { const { controller, messenger } = createController({ state: { remoteFeatureFlags: MOCK_FLAGS, + processedRemoteFeatureFlags: MOCK_FLAGS, cacheTimestamp: Date.now() - 10, }, clientConfigApiService, @@ -425,6 +433,7 @@ describe('RemoteFeatureFlagController', () => { clientConfigApiService, state: { remoteFeatureFlags: MOCK_FLAGS, + processedRemoteFeatureFlags: MOCK_FLAGS, }, }); @@ -1410,6 +1419,7 @@ describe('RemoteFeatureFlagController', () => { remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }); }); @@ -1490,10 +1500,8 @@ describe('RemoteFeatureFlagController', () => { it('removes a specific override', () => { const { controller, messenger } = createController({ state: { - remoteFeatureFlags: { + processedRemoteFeatureFlags: { remoteFlag: 'remoteValue', - flag1: 'value1', - flag2: 'value2', }, localOverrides: { flag1: 'value1', @@ -1519,9 +1527,6 @@ describe('RemoteFeatureFlagController', () => { it('does not affect state when clearing non-existent override', () => { const { controller, messenger } = createController({ state: { - remoteFeatureFlags: { - flag1: 'value1', - }, localOverrides: { flag1: 'value1', }, @@ -1546,10 +1551,8 @@ describe('RemoteFeatureFlagController', () => { it('removes all overrides', () => { const { controller, messenger } = createController({ state: { - remoteFeatureFlags: { + processedRemoteFeatureFlags: { remoteFlag: 'remoteValue', - flag1: 'value1', - flag2: 'value2', }, localOverrides: { flag1: 'value1', @@ -1611,12 +1614,11 @@ describe('RemoteFeatureFlagController', () => { }); }); - it('uses persisted remoteFeatureFlags with overrides on init', () => { + it('uses persisted processedRemoteFeatureFlags with overrides on init', () => { const { controller } = createController({ state: { - remoteFeatureFlags: { + processedRemoteFeatureFlags: { remoteFlag: 'remoteValue', - overrideFlag: 'overrideValue', }, localOverrides: { overrideFlag: 'overrideValue', @@ -1630,10 +1632,10 @@ describe('RemoteFeatureFlagController', () => { }); }); - it('merges legacy persisted localOverrides into remoteFeatureFlags on init', () => { + it('restores processed remote values when removing an override', () => { const { controller, messenger } = createController({ state: { - remoteFeatureFlags: { + processedRemoteFeatureFlags: { remoteFlag: 'remoteValue', overrideFlag: 'remoteOnlyValue', }, @@ -1661,6 +1663,300 @@ describe('RemoteFeatureFlagController', () => { }); }); + describe('defaultFeatureFlags', () => { + it('initializes with defaults when no remote or persisted flags exist', () => { + const { controller } = createController({ + defaultFeatureFlags: { + defaultFlag: 'defaultValue', + anotherDefault: false, + }, + }); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultFlag: 'defaultValue', + anotherDefault: false, + }); + expect(controller.state.processedRemoteFeatureFlags).toBeUndefined(); + }); + + it('applies precedence of override over processed remote over default', () => { + const { controller } = createController({ + state: { + processedRemoteFeatureFlags: { + sharedFlag: 'remoteValue', + remoteOnly: true, + }, + localOverrides: { + sharedFlag: 'overrideValue', + }, + }, + defaultFeatureFlags: { + sharedFlag: 'defaultValue', + defaultOnly: 'fromDefaults', + }, + }); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + sharedFlag: 'overrideValue', + remoteOnly: true, + defaultOnly: 'fromDefaults', + }); + }); + + it('keeps defaults for flags absent from a remote fetch', async () => { + const clientConfigApiService = buildClientConfigApiService({ + remoteFeatureFlags: { remoteFlag: 'fromServer' }, + }); + const { controller, messenger } = createController({ + clientConfigApiService, + defaultFeatureFlags: { + defaultOnly: 'fromDefaults', + remoteFlag: 'defaultRemote', + }, + }); + + await messenger.call( + 'RemoteFeatureFlagController:updateRemoteFeatureFlags', + ); + + expect(controller.state.processedRemoteFeatureFlags).toStrictEqual({ + remoteFlag: 'fromServer', + }); + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'fromDefaults', + remoteFlag: 'fromServer', + }); + expect(controller.state.rawRemoteFeatureFlags).toStrictEqual({ + remoteFlag: 'fromServer', + }); + }); + + it('restores default when removing an override with no remote value', () => { + const { controller, messenger } = createController({ + defaultFeatureFlags: { + defaultFlag: 'defaultValue', + }, + }); + + messenger.call( + 'RemoteFeatureFlagController:setFlagOverride', + 'defaultFlag', + 'overrideValue', + ); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultFlag: 'overrideValue', + }); + + messenger.call( + 'RemoteFeatureFlagController:removeFlagOverride', + 'defaultFlag', + ); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultFlag: 'defaultValue', + }); + expect(controller.state.localOverrides).toStrictEqual({}); + }); + + it('restores defaults when clearing all overrides', () => { + const { controller, messenger } = createController({ + state: { + processedRemoteFeatureFlags: { + remoteFlag: 'remoteValue', + }, + }, + defaultFeatureFlags: { + defaultFlag: 'defaultValue', + }, + }); + + messenger.call( + 'RemoteFeatureFlagController:setFlagOverride', + 'defaultFlag', + 'overrideValue', + ); + messenger.call( + 'RemoteFeatureFlagController:setFlagOverride', + 'remoteFlag', + 'overrideRemote', + ); + + messenger.call('RemoteFeatureFlagController:clearAllFlagOverrides'); + + expect(controller.state.localOverrides).toStrictEqual({}); + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultFlag: 'defaultValue', + remoteFlag: 'remoteValue', + }); + }); + + it('uses persisted processedRemoteFeatureFlags as the remote layer on init', () => { + const { controller } = createController({ + state: { + remoteFeatureFlags: { + // Stale effective blob should not win over processed + defaults + defaultOnly: 'stalePersistedDefault', + remoteFlag: 'staleRemote', + }, + processedRemoteFeatureFlags: { + remoteFlag: 'remoteValue', + }, + cacheTimestamp: 123, + }, + defaultFeatureFlags: { + defaultOnly: 'currentDefault', + }, + }); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'currentDefault', + remoteFlag: 'remoteValue', + }); + expect(controller.state.cacheTimestamp).toBe(123); + }); + + it('forces a refetch when processedRemoteFeatureFlags is missing', async () => { + const clientConfigApiService = buildClientConfigApiService({ + remoteFeatureFlags: { remoteFlag: 'fromServer' }, + }); + const { controller, messenger } = createController({ + clientConfigApiService, + state: { + remoteFeatureFlags: { remoteFlag: true }, + cacheTimestamp: Date.now(), + }, + defaultFeatureFlags: { + defaultOnly: 'fromDefaults', + }, + }); + + expect(controller.state.processedRemoteFeatureFlags).toBeUndefined(); + expect(controller.state.cacheTimestamp).toBe(0); + // Pre-upgrade: keep old effective for first paint, with defaults under. + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'fromDefaults', + remoteFlag: true, + }); + + await messenger.call( + 'RemoteFeatureFlagController:updateRemoteFeatureFlags', + ); + + expect(clientConfigApiService.fetchRemoteFeatureFlags).toHaveBeenCalled(); + expect(controller.state.processedRemoteFeatureFlags).toStrictEqual({ + remoteFlag: 'fromServer', + }); + }); + + it('does not treat an empty processedRemoteFeatureFlags object as missing', () => { + const { controller } = createController({ + state: { + processedRemoteFeatureFlags: {}, + remoteFeatureFlags: { remoteFlag: true }, + cacheTimestamp: 12345, + }, + defaultFeatureFlags: { + defaultOnly: 'fromDefaults', + }, + }); + + expect(controller.state.processedRemoteFeatureFlags).toStrictEqual({}); + expect(controller.state.cacheTimestamp).toBe(12345); + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'fromDefaults', + }); + }); + + it('hydrates processedRemoteFeatureFlags from raw when fetch fails', async () => { + const clientConfigApiService = buildClientConfigApiService({ + error: new Error('API Error'), + }); + const { controller, messenger } = createController({ + clientConfigApiService, + state: { + localOverrides: { + sharedFlag: 'overrideValue', + }, + rawRemoteFeatureFlags: { + sharedFlag: 'remoteValue', + remoteOnly: true, + }, + }, + defaultFeatureFlags: { + sharedFlag: 'defaultValue', + defaultOnly: 'fromDefaults', + }, + }); + + await expect( + messenger.call('RemoteFeatureFlagController:updateRemoteFeatureFlags'), + ).rejects.toThrow('API Error'); + + expect(controller.state.processedRemoteFeatureFlags).toStrictEqual({ + sharedFlag: 'remoteValue', + remoteOnly: true, + }); + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + sharedFlag: 'overrideValue', + remoteOnly: true, + defaultOnly: 'fromDefaults', + }); + // Failed fetch should not refresh the cache timestamp. + expect(controller.state.cacheTimestamp).toBe(0); + }); + + it('does not hydrate from empty raw when fetch fails and processed is missing', async () => { + const clientConfigApiService = buildClientConfigApiService({ + error: new Error('API Error'), + }); + const { controller, messenger } = createController({ + clientConfigApiService, + state: { + remoteFeatureFlags: { remoteFlag: true }, + rawRemoteFeatureFlags: {}, + }, + defaultFeatureFlags: { + defaultOnly: 'fromDefaults', + }, + }); + + await expect( + messenger.call('RemoteFeatureFlagController:updateRemoteFeatureFlags'), + ).rejects.toThrow('API Error'); + + expect(controller.state.processedRemoteFeatureFlags).toBeUndefined(); + // First-paint effective blob is left alone when raw cannot be hydrated. + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'fromDefaults', + remoteFlag: true, + }); + expect(controller.state.cacheTimestamp).toBe(0); + }); + + it('does not modify processedRemoteFeatureFlags when fetch fails after migration', async () => { + const clientConfigApiService = buildClientConfigApiService({ + error: new Error('API Error'), + }); + const { controller, messenger } = createController({ + clientConfigApiService, + state: { + remoteFeatureFlags: MOCK_FLAGS, + processedRemoteFeatureFlags: MOCK_FLAGS, + }, + }); + + await expect( + messenger.call('RemoteFeatureFlagController:updateRemoteFeatureFlags'), + ).rejects.toThrow('API Error'); + + expect(controller.state.processedRemoteFeatureFlags).toStrictEqual( + MOCK_FLAGS, + ); + expect(controller.state.remoteFeatureFlags).toStrictEqual(MOCK_FLAGS); + }); + }); + describe('threshold cache cleanup', () => { it('removes stale threshold cache entries when flags are removed from server', async () => { jest.useRealTimers(); diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts index 6bd6df6d66b..17c40ac98db 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts @@ -31,6 +31,7 @@ export type RemoteFeatureFlagControllerState = { remoteFeatureFlags: FeatureFlags; localOverrides?: FeatureFlags; rawRemoteFeatureFlags?: FeatureFlags; + processedRemoteFeatureFlags?: FeatureFlags; cacheTimestamp: number; thresholdCache?: Record; featureFlagThresholdGroups?: Record; @@ -55,6 +56,12 @@ const remoteFeatureFlagControllerMetadata = { includeInDebugSnapshot: true, usedInUi: false, }, + processedRemoteFeatureFlags: { + includeInStateLogs: true, + persist: true, + includeInDebugSnapshot: true, + usedInUi: false, + }, cacheTimestamp: { includeInStateLogs: true, persist: true, @@ -121,6 +128,7 @@ export function getDefaultRemoteFeatureFlagControllerState(): RemoteFeatureFlagC remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }; } @@ -214,6 +222,8 @@ export class RemoteFeatureFlagController extends BaseController< readonly #clientVersion: SemVerVersion; + readonly #defaultFeatureFlags: FeatureFlags; + #processedRemoteFeatureFlags: FeatureFlags = {}; /** @@ -228,6 +238,7 @@ export class RemoteFeatureFlagController extends BaseController< * @param options.getMetaMetricsId - Returns metaMetricsId. * @param options.clientVersion - The current client version for version-based feature flag filtering. Must be a valid 3-part SemVer version string. * @param options.prevClientVersion - The previous client version for feature flag cache invalidation. + * @param options.defaultFeatureFlags - Client-side default feature flags used as the lowest-precedence layer under processed remote flags and local overrides. Not persisted. */ constructor({ messenger, @@ -238,6 +249,7 @@ export class RemoteFeatureFlagController extends BaseController< getMetaMetricsId, clientVersion, prevClientVersion, + defaultFeatureFlags = {}, }: { messenger: RemoteFeatureFlagControllerMessenger; state?: Partial; @@ -247,6 +259,7 @@ export class RemoteFeatureFlagController extends BaseController< disabled?: boolean; clientVersion: string; prevClientVersion?: string; + defaultFeatureFlags?: FeatureFlags; }) { if (!isValidSemVerVersion(clientVersion)) { throw new Error( @@ -264,6 +277,31 @@ export class RemoteFeatureFlagController extends BaseController< prevClientVersion !== clientVersion; const localOverrides = initialState.localOverrides ?? {}; + const hasPersistedProcessedRemote = + initialState.processedRemoteFeatureFlags !== undefined; + const processedRemoteFeatureFlags = + initialState.processedRemoteFeatureFlags ?? {}; + + // Force a refetch when processed remote has never been persisted so the + // new field is populated (from the API, or from raw on fetch failure). + const shouldForceRefetch = + !hasPersistedProcessedRemote || hasClientVersionChanged; + + // When processed remote is already persisted, derive effective flags from + // defaults + processed + overrides. When missing (pre-upgrade), keep the + // old effective blob for first paint (with defaults under and overrides on + // top) until fetch/raw hydration writes the new field. + const remoteFeatureFlags = hasPersistedProcessedRemote + ? { + ...defaultFeatureFlags, + ...processedRemoteFeatureFlags, + ...localOverrides, + } + : { + ...defaultFeatureFlags, + ...initialState.remoteFeatureFlags, + ...localOverrides, + }; super({ name: controllerName, @@ -271,25 +309,15 @@ export class RemoteFeatureFlagController extends BaseController< messenger, state: { ...initialState, - remoteFeatureFlags: { - ...initialState.remoteFeatureFlags, - ...localOverrides, - }, - cacheTimestamp: hasClientVersionChanged - ? 0 - : initialState.cacheTimestamp, + remoteFeatureFlags, + // Keep processedRemoteFeatureFlags undefined until #updateCache writes + // it, so failed fetches can still detect the migration gap. + cacheTimestamp: shouldForceRefetch ? 0 : initialState.cacheTimestamp, }, }); - this.#processedRemoteFeatureFlags = { - ...initialState.remoteFeatureFlags, - }; - for (const [flagName, overrideValue] of Object.entries(localOverrides)) { - if (this.#processedRemoteFeatureFlags[flagName] === overrideValue) { - delete this.#processedRemoteFeatureFlags[flagName]; - } - } - + this.#defaultFeatureFlags = defaultFeatureFlags; + this.#processedRemoteFeatureFlags = processedRemoteFeatureFlags; this.#fetchInterval = fetchInterval; this.#disabled = disabled; this.#clientConfigApiService = clientConfigApiService; @@ -302,6 +330,25 @@ export class RemoteFeatureFlagController extends BaseController< ); } + /** + * Computes effective feature flags with precedence: + * defaults < processed remote < local overrides. + * + * @param processedRemote - The processed remote feature flags. + * @param localOverrides - Local overrides. Defaults to current state overrides. + * @returns The effective feature flags. + */ + #getEffectiveFeatureFlags( + processedRemote: FeatureFlags, + localOverrides: FeatureFlags = this.state.localOverrides ?? {}, + ): FeatureFlags { + return { + ...this.#defaultFeatureFlags, + ...processedRemote, + ...localOverrides, + }; + } + /** * Checks if the cached feature flags are expired based on the fetch interval. * @@ -315,6 +362,10 @@ export class RemoteFeatureFlagController extends BaseController< * Retrieves the remote feature flags, fetching from the API if necessary. * Uses caching to prevent redundant API calls and handles concurrent fetches. * + * When `processedRemoteFeatureFlags` has never been persisted and the fetch + * fails, falls back to processing persisted `rawRemoteFeatureFlags` so the + * new field can still be populated offline. + * * @returns A promise that resolves to the current set of feature flags. */ async updateRemoteFeatureFlags(): Promise { @@ -322,8 +373,6 @@ export class RemoteFeatureFlagController extends BaseController< return; } - let serverData; - if (this.#inProgressFlagUpdate) { await this.#inProgressFlagUpdate; return; @@ -333,20 +382,48 @@ export class RemoteFeatureFlagController extends BaseController< this.#inProgressFlagUpdate = this.#clientConfigApiService.fetchRemoteFeatureFlags(); - serverData = await this.#inProgressFlagUpdate; + const serverData = await this.#inProgressFlagUpdate; + await this.#updateCache(serverData.remoteFeatureFlags); + } catch (error) { + // Pre-upgrade / missing processed layer: populate from persisted raw + // without treating the failed fetch as a successful cache refresh. + if (this.state.processedRemoteFeatureFlags === undefined) { + const rawRemoteFeatureFlags = this.state.rawRemoteFeatureFlags ?? {}; + if (Object.keys(rawRemoteFeatureFlags).length > 0) { + await this.#updateCache(rawRemoteFeatureFlags, { + persistRaw: false, + refreshCacheTimestamp: false, + }); + } + } + throw error; } finally { this.#inProgressFlagUpdate = undefined; } - - await this.#updateCache(serverData.remoteFeatureFlags); } /** - * Updates the controller's state with new feature flags and resets the cache timestamp. + * Processes remote flags and writes the processed + effective layers to state. * - * @param remoteFeatureFlags - The new feature flags to cache. + * @param remoteFeatureFlags - Raw remote flags to process (API payload or + * persisted `rawRemoteFeatureFlags`). + * @param options - Write options. + * @param options.persistRaw - Whether to update `rawRemoteFeatureFlags`. + * Defaults to `true`. + * @param options.refreshCacheTimestamp - Whether to set `cacheTimestamp` to + * now. Defaults to `true`. Pass `false` when hydrating from persisted raw + * after a failed fetch so a later update can still retry the API. */ - async #updateCache(remoteFeatureFlags: FeatureFlags): Promise { + async #updateCache( + remoteFeatureFlags: FeatureFlags, + { + persistRaw = true, + refreshCacheTimestamp = true, + }: { + persistRaw?: boolean; + refreshCacheTimestamp?: boolean; + } = {}, + ): Promise { const { processedFlags, thresholdCacheUpdates, @@ -356,15 +433,12 @@ export class RemoteFeatureFlagController extends BaseController< const metaMetricsId = this.#getMetaMetricsId(); const currentFlagNames = Object.keys(remoteFeatureFlags); - // Build updated threshold cache const updatedThresholdCache = { ...(this.state.thresholdCache ?? {}) }; - // Apply new thresholds for (const [cacheKey, threshold] of Object.entries(thresholdCacheUpdates)) { updatedThresholdCache[cacheKey] = threshold; } - // Clean up stale entries for (const cacheKey of Object.keys(updatedThresholdCache)) { const [cachedMetaMetricsId, ...cachedFlagNameParts] = cacheKey.split(':'); const cachedFlagName = cachedFlagNameParts.join(':'); @@ -377,23 +451,24 @@ export class RemoteFeatureFlagController extends BaseController< } // Strip metaMetricsIds from processed flags so they never appear in - // remoteFeatureFlags state or #processedRemoteFeatureFlags. Arrays that + // remoteFeatureFlags state or processedRemoteFeatureFlags. Arrays that // were preserved as-is (e.g. when metaMetricsId is missing) would // otherwise leak explicit-targeting IDs into diagnostics. const redactedProcessedFlags = redactMetaMetricsIds(processedFlags); - // Single state update with all changes batched together this.#processedRemoteFeatureFlags = redactedProcessedFlags; this.update(() => { return { ...this.state, - remoteFeatureFlags: { - ...redactedProcessedFlags, - ...this.state.localOverrides, - }, - rawRemoteFeatureFlags: redactMetaMetricsIds(remoteFeatureFlags), - cacheTimestamp: Date.now(), + processedRemoteFeatureFlags: redactedProcessedFlags, + remoteFeatureFlags: this.#getEffectiveFeatureFlags( + redactedProcessedFlags, + ), + ...(persistRaw + ? { rawRemoteFeatureFlags: redactMetaMetricsIds(remoteFeatureFlags) } + : {}), + ...(refreshCacheTimestamp ? { cacheTimestamp: Date.now() } : {}), thresholdCache: updatedThresholdCache, featureFlagThresholdGroups: featureFlagThresholdGroupUpdates, }; @@ -543,10 +618,10 @@ export class RemoteFeatureFlagController extends BaseController< return { ...this.state, localOverrides, - remoteFeatureFlags: { - ...this.state.remoteFeatureFlags, - [flagName]: value, - }, + remoteFeatureFlags: this.#getEffectiveFeatureFlags( + this.#processedRemoteFeatureFlags, + localOverrides, + ), }; }); } @@ -560,20 +635,14 @@ export class RemoteFeatureFlagController extends BaseController< const newLocalOverrides = { ...this.state.localOverrides }; delete newLocalOverrides[flagName]; - const remoteFeatureFlags = { ...this.state.remoteFeatureFlags }; - const processedValue = this.#processedRemoteFeatureFlags[flagName]; - - if (processedValue === undefined) { - delete remoteFeatureFlags[flagName]; - } else { - remoteFeatureFlags[flagName] = processedValue; - } - this.update(() => { return { ...this.state, localOverrides: newLocalOverrides, - remoteFeatureFlags, + remoteFeatureFlags: this.#getEffectiveFeatureFlags( + this.#processedRemoteFeatureFlags, + newLocalOverrides, + ), }; }); } @@ -586,7 +655,10 @@ export class RemoteFeatureFlagController extends BaseController< return { ...this.state, localOverrides: {}, - remoteFeatureFlags: { ...this.#processedRemoteFeatureFlags }, + remoteFeatureFlags: this.#getEffectiveFeatureFlags( + this.#processedRemoteFeatureFlags, + {}, + ), }; }); } diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index ac4e3aa30c3..3ad19724b67 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `instanceOptions.remoteFeatureFlagController.defaultFeatureFlags` to pass client-side default feature flags through to `RemoteFeatureFlagController` ([#9747](https://github.com/MetaMask/core/pull/9747)) + ## [9.0.0] ### Added diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index e370c699aa0..94b3c61a4b6 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -465,6 +465,7 @@ describe('Wallet', () => { remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }); }); diff --git a/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.test.ts b/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.test.ts index 5eece7d554a..e2101ed3c2f 100644 --- a/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.test.ts +++ b/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.test.ts @@ -55,6 +55,7 @@ describe('remoteFeatureFlagController', () => { remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }); }); @@ -212,6 +213,24 @@ describe('remoteFeatureFlagController', () => { ).not.toHaveBeenCalled(); }); + it('forwards defaultFeatureFlags to the controller', () => { + const messenger = + remoteFeatureFlagController.getMessenger(getRootMessenger()); + + const instance = remoteFeatureFlagController.init({ + state: undefined, + messenger, + options: { + clientConfigApiService: getClientConfigApiService(), + defaultFeatureFlags: { defaultFlag: true }, + }, + }); + + expect(instance.state.remoteFeatureFlags).toStrictEqual({ + defaultFlag: true, + }); + }); + it('exposes its state through the root messenger', () => { const rootMessenger = getRootMessenger(); const messenger = remoteFeatureFlagController.getMessenger(rootMessenger); @@ -228,6 +247,7 @@ describe('remoteFeatureFlagController', () => { remoteFeatureFlags: {}, localOverrides: {}, rawRemoteFeatureFlags: {}, + processedRemoteFeatureFlags: undefined, cacheTimestamp: 0, }); }); diff --git a/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.ts b/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.ts index 35e7486ea9e..5d20fde04ea 100644 --- a/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.ts +++ b/packages/wallet/src/initialization/instances/remote-feature-flag-controller/remote-feature-flag-controller.ts @@ -21,6 +21,7 @@ export const remoteFeatureFlagController: InitializationConfiguration< prevClientVersion: options.prevClientVersion, fetchInterval: options.fetchInterval, disabled: options.disabled, + defaultFeatureFlags: options.defaultFeatureFlags, }), getMessenger: (parent) => new Messenger({ diff --git a/packages/wallet/src/initialization/instances/remote-feature-flag-controller/types.ts b/packages/wallet/src/initialization/instances/remote-feature-flag-controller/types.ts index 1477c632cfb..d6210ee0ebb 100644 --- a/packages/wallet/src/initialization/instances/remote-feature-flag-controller/types.ts +++ b/packages/wallet/src/initialization/instances/remote-feature-flag-controller/types.ts @@ -42,4 +42,9 @@ export type RemoteFeatureFlagControllerInstanceOptions = { * `enable`/`disable` actions. */ disabled?: RemoteFeatureFlagControllerOptions['disabled']; + /** + * Client-side default feature flags used as the lowest-precedence layer + * under processed remote flags and local overrides. Not persisted. + */ + defaultFeatureFlags?: RemoteFeatureFlagControllerOptions['defaultFeatureFlags']; };