Skip to content

Commit

Permalink
fix(3742): change getMetaMetricsId to only sync func type (#5108)
Browse files Browse the repository at this point in the history
## 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

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

## References
Addressed feedback
#5051 (comment)
<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->
<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@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
  • Loading branch information
DDDDDanica authored Jan 8, 2025
1 parent a572cdd commit 7c38c24
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -59,7 +57,7 @@ function createController(
state: Partial<RemoteFeatureFlagControllerState>;
clientConfigApiService: AbstractClientConfigApiService;
disabled: boolean;
getMetaMetricsId: Promise<string | undefined>;
getMetaMetricsId: () => string;
}> = {},
) {
return new RemoteFeatureFlagController({
Expand All @@ -68,7 +66,7 @@ function createController(
clientConfigApiService:
options.clientConfigApiService ?? buildClientConfigApiService(),
disabled: options.disabled,
getMetaMetricsId: options.getMetaMetricsId,
getMetaMetricsId: options.getMetaMetricsId ?? (() => MOCK_METRICS_ID),
});
}

Expand Down Expand Up @@ -271,7 +269,7 @@ describe('RemoteFeatureFlagController', () => {
});
const controller = createController({
clientConfigApiService,
getMetaMetricsId: Promise.resolve(MOCK_METRICS_ID),
getMetaMetricsId: () => MOCK_METRICS_ID,
});
await controller.updateRemoteFeatureFlags();

Expand All @@ -289,34 +287,14 @@ describe('RemoteFeatureFlagController', () => {
});
const controller = createController({
clientConfigApiService,
getMetaMetricsId: Promise.resolve(MOCK_METRICS_ID),
getMetaMetricsId: () => MOCK_METRICS_ID,
});
await controller.updateRemoteFeatureFlags();

const { testFlagForThreshold, ...nonThresholdFlags } =
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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import type {
import {
generateDeterministicRandomNumber,
isFeatureFlagWithScopeValue,
generateFallbackMetaMetricsId,
} from './utils/user-segmentation-utils';

// === GENERAL ===
Expand Down Expand Up @@ -103,7 +102,7 @@ export class RemoteFeatureFlagController extends BaseController<

#inProgressFlagUpdate?: Promise<ServiceResponse>;

#getMetaMetricsId?: Promise<string | undefined>;
#getMetaMetricsId: () => string;

/**
* Constructs a new RemoteFeatureFlagController instance.
Expand All @@ -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,
Expand All @@ -127,7 +126,7 @@ export class RemoteFeatureFlagController extends BaseController<
messenger: RemoteFeatureFlagControllerMessenger;
state?: Partial<RemoteFeatureFlagControllerState>;
clientConfigApiService: AbstractClientConfigApiService;
getMetaMetricsId?: Promise<string | undefined>;
getMetaMetricsId: () => string;
fetchInterval?: number;
disabled?: boolean;
}) {
Expand Down Expand Up @@ -209,8 +208,7 @@ export class RemoteFeatureFlagController extends BaseController<
remoteFeatureFlags: FeatureFlags,
): Promise<FeatureFlags> {
const processedRemoteFeatureFlags: FeatureFlags = {};
const metaMetricsId =
(await this.#getMetaMetricsId) || generateFallbackMetaMetricsId();
const metaMetricsId = this.#getMetaMetricsId();
const thresholdValue = generateDeterministicRandomNumber(metaMetricsId);

for (const [
Expand Down
Original file line number Diff line number Diff line change
@@ -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 = [
Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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();
}

0 comments on commit 7c38c24

Please sign in to comment.