diff --git a/packages/remote-feature-flag-controller/CHANGELOG.md b/packages/remote-feature-flag-controller/CHANGELOG.md index f39d1c5a22e..664b04dc9ae 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 ([#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..e8537484a15 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 }; } @@ -1661,6 +1664,172 @@ 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, + }); + }); + + it('applies precedence of override over remote over default', () => { + const { controller } = createController({ + state: { + remoteFeatureFlags: { + sharedFlag: 'remoteValue', + remoteOnly: true, + }, + rawRemoteFeatureFlags: { + 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.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'fromDefaults', + 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', + ); + messenger.call( + 'RemoteFeatureFlagController:removeFlagOverride', + 'defaultFlag', + ); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultFlag: 'defaultValue', + }); + expect(controller.state.localOverrides).toStrictEqual({}); + }); + + it('strips default-only keys from the processed layer on init', () => { + const { controller, messenger } = createController({ + state: { + // Stale baked-in default from a previous session + remoteFeatureFlags: { + defaultOnly: 'staleDefault', + remoteFlag: 'remoteValue', + }, + rawRemoteFeatureFlags: { + remoteFlag: 'remoteValue', + }, + }, + defaultFeatureFlags: { + defaultOnly: 'currentDefault', + }, + }); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'currentDefault', + remoteFlag: 'remoteValue', + }); + + messenger.call('RemoteFeatureFlagController:clearAllFlagOverrides'); + + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'currentDefault', + remoteFlag: 'remoteValue', + }); + }); + + it('treats undefined rawRemoteFeatureFlags as empty when stripping defaults on init', () => { + const { controller } = createController({ + state: { + remoteFeatureFlags: { + defaultOnly: 'staleDefault', + remoteFlag: 'remoteValue', + }, + rawRemoteFeatureFlags: undefined, + }, + defaultFeatureFlags: { + defaultOnly: 'currentDefault', + }, + }); + + // defaultOnly is stripped from processed (not in raw) and replaced by + // the current default; remoteFlag is kept because it is not a default key. + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'currentDefault', + remoteFlag: 'remoteValue', + }); + }); + + it('treats undefined localOverrides as empty when updating the cache', async () => { + const clientConfigApiService = buildClientConfigApiService({ + remoteFeatureFlags: { remoteFlag: 'fromServer' }, + }); + const { controller, messenger } = createController({ + clientConfigApiService, + state: { + localOverrides: undefined, + }, + defaultFeatureFlags: { + defaultOnly: 'fromDefaults', + }, + }); + + await messenger.call( + 'RemoteFeatureFlagController:updateRemoteFeatureFlags', + ); + + expect(controller.state.localOverrides).toBeUndefined(); + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + defaultOnly: 'fromDefaults', + remoteFlag: 'fromServer', + }); + }); + }); + 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..e4e87356710 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 @@ -214,6 +214,8 @@ export class RemoteFeatureFlagController extends BaseController< readonly #clientVersion: SemVerVersion; + readonly #defaultFeatureFlags: FeatureFlags; + #processedRemoteFeatureFlags: FeatureFlags = {}; /** @@ -228,6 +230,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 +241,7 @@ export class RemoteFeatureFlagController extends BaseController< getMetaMetricsId, clientVersion, prevClientVersion, + defaultFeatureFlags = {}, }: { messenger: RemoteFeatureFlagControllerMessenger; state?: Partial; @@ -247,6 +251,7 @@ export class RemoteFeatureFlagController extends BaseController< disabled?: boolean; clientVersion: string; prevClientVersion?: string; + defaultFeatureFlags?: FeatureFlags; }) { if (!isValidSemVerVersion(clientVersion)) { throw new Error( @@ -264,6 +269,23 @@ export class RemoteFeatureFlagController extends BaseController< prevClientVersion !== clientVersion; const localOverrides = initialState.localOverrides ?? {}; + const rawRemoteFeatureFlags = initialState.rawRemoteFeatureFlags ?? {}; + + // Rebuild the processed remote layer from last session's effective flags by + // stripping local overrides and default-only keys (absent from raw). + const processedRemoteFeatureFlags = { + ...initialState.remoteFeatureFlags, + }; + for (const [flagName, overrideValue] of Object.entries(localOverrides)) { + if (processedRemoteFeatureFlags[flagName] === overrideValue) { + delete processedRemoteFeatureFlags[flagName]; + } + } + for (const flagName of Object.keys(defaultFeatureFlags)) { + if (rawRemoteFeatureFlags[flagName] === undefined) { + delete processedRemoteFeatureFlags[flagName]; + } + } super({ name: controllerName, @@ -272,7 +294,8 @@ export class RemoteFeatureFlagController extends BaseController< state: { ...initialState, remoteFeatureFlags: { - ...initialState.remoteFeatureFlags, + ...defaultFeatureFlags, + ...processedRemoteFeatureFlags, ...localOverrides, }, cacheTimestamp: hasClientVersionChanged @@ -281,15 +304,8 @@ export class RemoteFeatureFlagController extends BaseController< }, }); - 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 +318,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. * @@ -388,10 +423,9 @@ export class RemoteFeatureFlagController extends BaseController< this.update(() => { return { ...this.state, - remoteFeatureFlags: { - ...redactedProcessedFlags, - ...this.state.localOverrides, - }, + remoteFeatureFlags: this.#getEffectiveFeatureFlags( + redactedProcessedFlags, + ), rawRemoteFeatureFlags: redactMetaMetricsIds(remoteFeatureFlags), cacheTimestamp: Date.now(), thresholdCache: updatedThresholdCache, @@ -543,10 +577,10 @@ export class RemoteFeatureFlagController extends BaseController< return { ...this.state, localOverrides, - remoteFeatureFlags: { - ...this.state.remoteFeatureFlags, - [flagName]: value, - }, + remoteFeatureFlags: this.#getEffectiveFeatureFlags( + this.#processedRemoteFeatureFlags, + localOverrides, + ), }; }); } @@ -560,20 +594,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 +614,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 173af1965e0..74addd8e149 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)) + ### Changed - Bump `@metamask/transaction-controller` from `^69.4.0` to `^69.5.0` ([#9780](https://github.com/MetaMask/core/pull/9780)) 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..24765f92e7d 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 @@ -212,6 +212,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); 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']; };