diff --git a/packages/remote-feature-flag-controller/CHANGELOG.md b/packages/remote-feature-flag-controller/CHANGELOG.md index 47f685a091..74eacc03c9 100644 --- a/packages/remote-feature-flag-controller/CHANGELOG.md +++ b/packages/remote-feature-flag-controller/CHANGELOG.md @@ -7,8 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `featureFlagThresholdGroups` field to `RemoteFeatureFlagControllerState` to map feature flag names to their selected threshold group names ([#9289](https://github.com/MetaMask/core/pull/9289)) + ### Changed +- **BREAKING:** Threshold feature flags now return the selected `value` directly instead of a `{ name, value }` wrapper. The selected threshold group name is stored separately in `featureFlagThresholdGroups` on controller state when the selected threshold entry includes `name` ([#9289](https://github.com/MetaMask/core/pull/9289)) - Merge `localOverrides` into `remoteFeatureFlags` at the controller level so consumers receive effective flag values directly ([#9259](https://github.com/MetaMask/core/pull/9259)) - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074)) - Bump `@metamask/controller-utils` from `^12.1.0` to `^12.3.0` ([#9058](https://github.com/MetaMask/core/pull/9058), [#9083](https://github.com/MetaMask/core/pull/9083), [#9218](https://github.com/MetaMask/core/pull/9218)) 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 f2009007f9..478a0ffa67 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 @@ -478,83 +478,15 @@ describe('RemoteFeatureFlagController', () => { // With MOCK_METRICS_ID + 'testFlagForThreshold' hashed: // Threshold = 0.380673, which falls in groupB range (0.3 < t <= 0.5) - expect( - controller.state.remoteFeatureFlags.testFlagForThreshold, - ).toStrictEqual({ - name: 'groupB', - value: 'valueB', - }); - }); - - it('preserves selected legacy threshold object value wrappers', async () => { - const thresholdFlagValue = { - enabled: true, - minimumVersion: '13.10.0', - attemptsMax: 5, - }; - const mockFlags = { - thresholdObjectFlag: [ - { - name: 'enabled', - scope: { type: 'threshold', value: 1.0 }, - value: thresholdFlagValue, - }, - ], - }; - const clientConfigApiService = buildClientConfigApiService({ - remoteFeatureFlags: mockFlags, - }); - const { controller, messenger } = createController({ - clientConfigApiService, - getMetaMetricsId: () => MOCK_METRICS_ID, - }); - - await messenger.call( - 'RemoteFeatureFlagController:updateRemoteFeatureFlags', + expect(controller.state.remoteFeatureFlags.testFlagForThreshold).toBe( + 'valueB', ); - - expect( - controller.state.remoteFeatureFlags.thresholdObjectFlag, - ).toStrictEqual({ - name: 'enabled', - value: thresholdFlagValue, + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + testFlagForThreshold: 'groupB', }); }); - it('returns selected threshold version 2 values without wrapper metadata', async () => { - const thresholdFlagValue = { - enabled: true, - minimumVersion: '13.10.0', - attemptsMax: 5, - }; - const mockFlags = { - thresholdObjectFlag: [ - { - thresholdName: 'enabled', - thresholdVersion: ThresholdVersion.DirectValue, - scope: { type: 'threshold', value: 1.0 }, - value: thresholdFlagValue, - }, - ], - }; - const clientConfigApiService = buildClientConfigApiService({ - remoteFeatureFlags: mockFlags, - }); - const { controller, messenger } = createController({ - clientConfigApiService, - getMetaMetricsId: () => MOCK_METRICS_ID, - }); - - await messenger.call( - 'RemoteFeatureFlagController:updateRemoteFeatureFlags', - ); - - expect( - controller.state.remoteFeatureFlags.thresholdObjectFlag, - ).toStrictEqual(thresholdFlagValue); - }); - - it('falls back to legacy threshold wrappers for unrecognized threshold versions', async () => { + it('ignores threshold version field, processing threshold values normally', async () => { const thresholdFlagValue = { enabled: true, }; @@ -582,9 +514,9 @@ describe('RemoteFeatureFlagController', () => { expect( controller.state.remoteFeatureFlags.thresholdObjectFlag, - ).toStrictEqual({ - name: 'enabled', - value: thresholdFlagValue, + ).toStrictEqual(thresholdFlagValue); + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + thresholdObjectFlag: 'enabled', }); }); @@ -649,9 +581,13 @@ describe('RemoteFeatureFlagController', () => { // Assert - User gets different groups because each flag uses unique seed const { featureA, featureB } = controller.state.remoteFeatureFlags; // featureA: hash(MOCK_METRICS_ID + 'featureA') → threshold 0.966682 → groupA2 - expect(featureA).toStrictEqual({ name: 'groupA2', value: 'A2' }); + expect(featureA).toBe('A2'); // featureB: hash(MOCK_METRICS_ID + 'featureB') → threshold 0.398654 → groupB1 - expect(featureB).toStrictEqual({ name: 'groupB1', value: 'B1' }); + expect(featureB).toBe('B1'); + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + featureA: 'groupA2', + featureB: 'groupB1', + }); // Different groups proves independence! }); @@ -711,9 +647,11 @@ describe('RemoteFeatureFlagController', () => { ); // Assert - Invalid item skipped, valid item selected - expect(controller.state.remoteFeatureFlags.mixedArray).toStrictEqual({ - name: 'validGroup', - value: 'selectedValue', + expect(controller.state.remoteFeatureFlags.mixedArray).toBe( + 'selectedValue', + ); + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + mixedArray: 'validGroup', }); }); @@ -761,7 +699,10 @@ describe('RemoteFeatureFlagController', () => { // Assert - Same user always gets same group (deterministic) // testFlag: hash(MOCK_METRICS_ID + 'testFlag') → threshold 0.496587 → control expect(firstResult).toStrictEqual(secondResult); - expect(firstResult).toStrictEqual({ name: 'control', value: false }); + expect(firstResult).toBe(false); + expect(controller1.state.featureFlagThresholdGroups).toStrictEqual({ + testFlag: 'control', + }); }); }); @@ -1073,8 +1014,11 @@ describe('RemoteFeatureFlagController', () => { // With MOCK_METRICS_ID + 'multiVersionABFlag' hashed: // Threshold = 0.094878, which falls in groupA range (t <= 0.3) expect(multiVersionABFlag).toStrictEqual({ - name: 'groupA', - value: { feature: 'A', enabled: true }, + feature: 'A', + enabled: true, + }); + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + multiVersionABFlag: 'groupA', }); expect(regularFlag).toBe(true); }); @@ -1406,6 +1350,75 @@ describe('RemoteFeatureFlagController', () => { jest.useRealTimers(); }); + it('removes stale featureFlagThresholdGroups entries when flags are removed from server', async () => { + jest.useRealTimers(); + const clientConfigApiService = buildClientConfigApiService({ + remoteFeatureFlags: { + flagA: [ + { + name: 'groupA', + thresholdVersion: ThresholdVersion.DirectValue, + scope: { type: 'threshold', value: 1.0 }, + value: true, + }, + ], + flagB: [ + { + name: 'groupB', + thresholdVersion: ThresholdVersion.DirectValue, + scope: { type: 'threshold', value: 1.0 }, + value: false, + }, + ], + }, + }); + const { controller, messenger } = createController({ + clientConfigApiService, + getMetaMetricsId: () => MOCK_METRICS_ID, + }); + + await messenger.call( + 'RemoteFeatureFlagController:updateRemoteFeatureFlags', + ); + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + flagA: 'groupA', + flagB: 'groupB', + }); + + jest + .spyOn(clientConfigApiService, 'fetchRemoteFeatureFlags') + .mockResolvedValue({ + remoteFeatureFlags: { + flagB: [ + { + name: 'groupB', + thresholdVersion: ThresholdVersion.DirectValue, + scope: { type: 'threshold', value: 1.0 }, + value: false, + }, + ], + }, + cacheTimestamp: Date.now(), + }); + + jest.useFakeTimers(); + + try { + jest.advanceTimersByTime(2 * DEFAULT_CACHE_DURATION); + + await messenger.call( + 'RemoteFeatureFlagController:updateRemoteFeatureFlags', + ); + await flushPromises(); + + expect(controller.state.featureFlagThresholdGroups).toStrictEqual({ + flagB: 'groupB', + }); + } finally { + jest.useRealTimers(); + } + }); + it('preserves threshold cache entries for flags still in server response', async () => { // Arrange const mockFlags = { 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 453b4ed9e2..fe6348357f 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 @@ -9,7 +9,6 @@ import type { Json, SemVerVersion } from '@metamask/utils'; import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; import type { RemoteFeatureFlagControllerMethodActions } from './remote-feature-flag-controller-method-action-types'; -import { ThresholdVersion } from './remote-feature-flag-controller-types'; import type { FeatureFlags, ServiceResponse, @@ -34,6 +33,7 @@ export type RemoteFeatureFlagControllerState = { rawRemoteFeatureFlags?: FeatureFlags; cacheTimestamp: number; thresholdCache?: Record; + featureFlagThresholdGroups?: Record; }; const remoteFeatureFlagControllerMetadata = { @@ -67,6 +67,12 @@ const remoteFeatureFlagControllerMetadata = { includeInDebugSnapshot: false, usedInUi: false, }, + featureFlagThresholdGroups: { + includeInStateLogs: true, + persist: true, + includeInDebugSnapshot: true, + usedInUi: false, + }, }; // === MESSENGER === @@ -119,19 +125,6 @@ export function getDefaultRemoteFeatureFlagControllerState(): RemoteFeatureFlagC }; } -function normalizeThresholdValue(featureFlag: FeatureFlagScopeValue): Json { - if (featureFlag.thresholdVersion === ThresholdVersion.DirectValue) { - return featureFlag.value; - } - - // Unknown threshold versions fall back to the legacy wrapper shape for - // backwards compatibility with existing threshold feature flag configs. - return { - name: featureFlag.name, - value: featureFlag.value, - }; -} - /** * The RemoteFeatureFlagController manages the retrieval and caching of remote feature flags. * It fetches feature flags from a remote API, caches them, and provides methods to access @@ -288,8 +281,11 @@ export class RemoteFeatureFlagController extends BaseController< * @param remoteFeatureFlags - The new feature flags to cache. */ async #updateCache(remoteFeatureFlags: FeatureFlags): Promise { - const { processedFlags, thresholdCacheUpdates } = - await this.#processRemoteFeatureFlags(remoteFeatureFlags); + const { + processedFlags, + thresholdCacheUpdates, + featureFlagThresholdGroupUpdates, + } = await this.#processRemoteFeatureFlags(remoteFeatureFlags); const metaMetricsId = this.#getMetaMetricsId(); const currentFlagNames = Object.keys(remoteFeatureFlags); @@ -327,6 +323,7 @@ export class RemoteFeatureFlagController extends BaseController< rawRemoteFeatureFlags: remoteFeatureFlags, cacheTimestamp: Date.now(), thresholdCache: updatedThresholdCache, + featureFlagThresholdGroups: featureFlagThresholdGroupUpdates, }; }); } @@ -348,10 +345,12 @@ export class RemoteFeatureFlagController extends BaseController< async #processRemoteFeatureFlags(remoteFeatureFlags: FeatureFlags): Promise<{ processedFlags: FeatureFlags; thresholdCacheUpdates: Record; + featureFlagThresholdGroupUpdates: Record; }> { const processedFlags: FeatureFlags = {}; const metaMetricsId = this.#getMetaMetricsId(); const thresholdCacheUpdates: Record = {}; + const featureFlagThresholdGroupUpdates: Record = {}; for (const [ remoteFeatureFlagName, @@ -409,14 +408,22 @@ export class RemoteFeatureFlagController extends BaseController< ); if (selectedGroup) { - processedValue = normalizeThresholdValue(selectedGroup); + processedValue = selectedGroup.value; + if (selectedGroup.name) { + featureFlagThresholdGroupUpdates[remoteFeatureFlagName] = + selectedGroup.name; + } } } processedFlags[remoteFeatureFlagName] = processedValue; } - return { processedFlags, thresholdCacheUpdates }; + return { + processedFlags, + thresholdCacheUpdates, + featureFlagThresholdGroupUpdates, + }; } /**