From 7c38c248da0bc2dc43ed5e01b18ee5970d49127e Mon Sep 17 00:00:00 2001 From: Danica Shen Date: Wed, 8 Jan 2025 12:03:05 +0000 Subject: [PATCH] fix(3742): change getMetaMetricsId to only sync func type (#5108) ## Explanation Changes introduced in this PR including: - remove fallback of metaMetricsId and rely on client side always returning a value - refactor getMetaMetricsId to only handle synchronous (extension) function ## References Addressed feedback https://github.com/MetaMask/core/pull/5051#discussion_r1883046468 ### `@metamask/remote-feature-flag-controller` - **CHANGED**: Modify signature of `getMetaMetricsId` to handle only synchronous function - **CHANGED**: Remove fallback of `metaMetricsId` ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- .../remote-feature-flag-controller.test.ts | 30 +++---------------- .../src/remote-feature-flag-controller.ts | 10 +++---- .../src/utils/user-segmentation-utils.test.ts | 11 ------- .../src/utils/user-segmentation-utils.ts | 9 ------ 4 files changed, 8 insertions(+), 52 deletions(-) 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 84e1e224a4..5def1e901a 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 @@ -14,7 +14,6 @@ import type { RemoteFeatureFlagControllerStateChangeEvent, } from './remote-feature-flag-controller'; import type { FeatureFlags } from './remote-feature-flag-controller-types'; -import * as userSegmentationUtils from './utils/user-segmentation-utils'; const MOCK_FLAGS: FeatureFlags = { feature1: true, @@ -42,7 +41,6 @@ const MOCK_FLAGS_WITH_THRESHOLD = { }; const MOCK_METRICS_ID = 'f9e8d7c6-b5a4-4210-9876-543210fedcba'; -const MOCK_METRICS_ID_2 = '987fcdeb-51a2-4c4b-9876-543210fedcba'; /** * Creates a controller instance with default parameters for testing @@ -59,7 +57,7 @@ function createController( state: Partial; clientConfigApiService: AbstractClientConfigApiService; disabled: boolean; - getMetaMetricsId: Promise; + getMetaMetricsId: () => string; }> = {}, ) { return new RemoteFeatureFlagController({ @@ -68,7 +66,7 @@ function createController( clientConfigApiService: options.clientConfigApiService ?? buildClientConfigApiService(), disabled: options.disabled, - getMetaMetricsId: options.getMetaMetricsId, + getMetaMetricsId: options.getMetaMetricsId ?? (() => MOCK_METRICS_ID), }); } @@ -271,7 +269,7 @@ describe('RemoteFeatureFlagController', () => { }); const controller = createController({ clientConfigApiService, - getMetaMetricsId: Promise.resolve(MOCK_METRICS_ID), + getMetaMetricsId: () => MOCK_METRICS_ID, }); await controller.updateRemoteFeatureFlags(); @@ -289,7 +287,7 @@ describe('RemoteFeatureFlagController', () => { }); const controller = createController({ clientConfigApiService, - getMetaMetricsId: Promise.resolve(MOCK_METRICS_ID), + getMetaMetricsId: () => MOCK_METRICS_ID, }); await controller.updateRemoteFeatureFlags(); @@ -297,26 +295,6 @@ describe('RemoteFeatureFlagController', () => { controller.state.remoteFeatureFlags; expect(nonThresholdFlags).toStrictEqual(MOCK_FLAGS); }); - - it('uses a fallback metaMetricsId if none is provided', async () => { - jest - .spyOn(userSegmentationUtils, 'generateFallbackMetaMetricsId') - .mockReturnValue(MOCK_METRICS_ID_2); - const clientConfigApiService = buildClientConfigApiService({ - remoteFeatureFlags: MOCK_FLAGS_WITH_THRESHOLD, - }); - const controller = createController({ - clientConfigApiService, - }); - await controller.updateRemoteFeatureFlags(); - - expect( - controller.state.remoteFeatureFlags.testFlagForThreshold, - ).toStrictEqual({ - name: 'groupA', - value: 'valueA', - }); - }); }); describe('enable and disable', () => { 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 81f78f5afa..9e370998ba 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 @@ -14,7 +14,6 @@ import type { import { generateDeterministicRandomNumber, isFeatureFlagWithScopeValue, - generateFallbackMetaMetricsId, } from './utils/user-segmentation-utils'; // === GENERAL === @@ -103,7 +102,7 @@ export class RemoteFeatureFlagController extends BaseController< #inProgressFlagUpdate?: Promise; - #getMetaMetricsId?: Promise; + #getMetaMetricsId: () => string; /** * Constructs a new RemoteFeatureFlagController instance. @@ -114,7 +113,7 @@ export class RemoteFeatureFlagController extends BaseController< * @param options.clientConfigApiService - The service instance to fetch remote feature flags. * @param options.fetchInterval - The interval in milliseconds before cached flags expire. Defaults to 1 day. * @param options.disabled - Determines if the controller should be disabled initially. Defaults to false. - * @param options.getMetaMetricsId - Promise that resolves to a metaMetricsId. + * @param options.getMetaMetricsId - Returns metaMetricsId. */ constructor({ messenger, @@ -127,7 +126,7 @@ export class RemoteFeatureFlagController extends BaseController< messenger: RemoteFeatureFlagControllerMessenger; state?: Partial; clientConfigApiService: AbstractClientConfigApiService; - getMetaMetricsId?: Promise; + getMetaMetricsId: () => string; fetchInterval?: number; disabled?: boolean; }) { @@ -209,8 +208,7 @@ export class RemoteFeatureFlagController extends BaseController< remoteFeatureFlags: FeatureFlags, ): Promise { const processedRemoteFeatureFlags: FeatureFlags = {}; - const metaMetricsId = - (await this.#getMetaMetricsId) || generateFallbackMetaMetricsId(); + const metaMetricsId = this.#getMetaMetricsId(); const thresholdValue = generateDeterministicRandomNumber(metaMetricsId); for (const [ diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts index be5443cbd6..6ec5933e02 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts @@ -1,9 +1,6 @@ -import { validate as uuidValidate, version as uuidVersion } from 'uuid'; - import { generateDeterministicRandomNumber, isFeatureFlagWithScopeValue, - generateFallbackMetaMetricsId, } from './user-segmentation-utils'; const MOCK_METRICS_IDS = [ @@ -75,12 +72,4 @@ describe('user-segmentation-utils', () => { ).toBe(false); }); }); - - describe('generateFallbackMetaMetricsId', () => { - it('returns a valid uuidv4', () => { - const result = generateFallbackMetaMetricsId(); - expect(uuidValidate(result)).toBe(true); - expect(uuidVersion(result)).toBe(4); - }); - }); }); diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts index 811f09e106..481d0d21c0 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts @@ -1,5 +1,4 @@ import type { Json } from '@metamask/utils'; -import { v4 as uuidV4 } from 'uuid'; import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-types'; @@ -39,11 +38,3 @@ export const isFeatureFlagWithScopeValue = ( 'scope' in featureFlag ); }; - -/** - * Generates UUIDv4 as a fallback metaMetricsId - * @returns A UUIDv4 string - */ -export function generateFallbackMetaMetricsId(): string { - return uuidV4(); -}