Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(3741): refactor remote-feature-flag controller with generateDeterministicRandomNumber and getMetaMetricsId #5097

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(3741): refactor remote-feature-flag controller with generateDeterministicRandomNumber and getMetaMetricsId #5097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ...e-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,23 @@ describe('ClientConfigApiService', () => {
});
});

describe('getClient', () => {
it('returns configured client type', () => {
const clientConfigApiService = new ClientConfigApiService({
fetch: createMockFetch({}),
config: {
client: ClientType.Extension,
distribution: DistributionType.Main,
environment: EnvironmentType.Production,
},
});

expect(clientConfigApiService.getClient()).toStrictEqual(
ClientType.Extension,
);
});
});

describe('circuit breaker', () => {
it('opens the circuit breaker after consecutive failures', async () => {
const mockFetch = createMockFetch({ error: networkError });
Expand Down
8 changes: 8 additions & 0 deletions ...remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,14 @@ export class ClientConfigApiService {
};
}

/**
* Gets the client type configured for this service.
* @returns The configured ClientType
*/
public getClient(): ClientType {
return this.#client;
}

/**
* Flattens an array of feature flag objects into a single feature flags object.
* @param responseData - Array of objects containing feature flag key-value pairs
Expand Down
29 changes: 17 additions & 12 deletions packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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';
import { ClientType } from './remote-feature-flag-controller-types';

const MOCK_FLAGS: FeatureFlags = {
feature1: true,
Expand Down Expand Up @@ -42,7 +42,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 @@ -51,6 +50,7 @@ const MOCK_METRICS_ID_2 = '987fcdeb-51a2-4c4b-9876-543210fedcba';
* @param options.state - The initial controller state
* @param options.clientConfigApiService - The client config API service instance
* @param options.disabled - Whether the controller should start disabled
* @param options.getMetaMetricsId - The function to get the metaMetricsId
* @returns A configured RemoteFeatureFlagController instance
*/
function createController(
Expand All @@ -59,7 +59,7 @@ function createController(
state: Partial<RemoteFeatureFlagControllerState>;
clientConfigApiService: AbstractClientConfigApiService;
disabled: boolean;
getMetaMetricsId: Promise<string | undefined>;
getMetaMetricsId: () => Promise<string> | string;
}> = {},
) {
return new RemoteFeatureFlagController({
Expand All @@ -68,7 +68,8 @@ function createController(
clientConfigApiService:
options.clientConfigApiService ?? buildClientConfigApiService(),
disabled: options.disabled,
getMetaMetricsId: options.getMetaMetricsId,
getMetaMetricsId:
options.getMetaMetricsId ?? (() => Promise.resolve(MOCK_METRICS_ID)),
});
}

Expand Down Expand Up @@ -219,6 +220,7 @@ describe('RemoteFeatureFlagController', () => {
});

const clientConfigApiService = {
getClient: () => ClientType.Mobile,
fetchRemoteFeatureFlags: fetchSpy,
} as AbstractClientConfigApiService;

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

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

Expand All @@ -298,23 +300,22 @@ describe('RemoteFeatureFlagController', () => {
expect(nonThresholdFlags).toStrictEqual(MOCK_FLAGS);
});

it('uses a fallback metaMetricsId if none is provided', async () => {
jest
.spyOn(userSegmentationUtils, 'generateFallbackMetaMetricsId')
.mockReturnValue(MOCK_METRICS_ID_2);
it('handles synchronous metaMetricsId', async () => {
const clientConfigApiService = buildClientConfigApiService({
remoteFeatureFlags: MOCK_FLAGS_WITH_THRESHOLD,
});
const controller = createController({
clientConfigApiService,
getMetaMetricsId: () => MOCK_METRICS_ID,
});

await controller.updateRemoteFeatureFlags();

expect(
controller.state.remoteFeatureFlags.testFlagForThreshold,
).toStrictEqual({
name: 'groupA',
value: 'valueA',
name: 'groupC',
value: 'valueC',
});
});
});
Expand Down Expand Up @@ -399,18 +400,22 @@ function getControllerMessenger(
* @param options.remoteFeatureFlags - Optional feature flags data to return
* @param options.cacheTimestamp - Optional timestamp to use for the cache
* @param options.error - Optional error to simulate API failure
* @param options.client - The client type to return from getClient
* @returns A mock client config API service
*/
function buildClientConfigApiService({
remoteFeatureFlags,
cacheTimestamp,
error,
client,
}: {
remoteFeatureFlags?: FeatureFlags;
cacheTimestamp?: number;
error?: Error;
client?: ClientType;
} = {}): AbstractClientConfigApiService {
return {
getClient: jest.fn().mockReturnValue(client ?? ClientType.Mobile),
fetchRemoteFeatureFlags: jest.fn().mockImplementation(() => {
if (error) {
return Promise.reject(error);
Expand Down
17 changes: 12 additions & 5 deletions packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts
View file
Open in desktop
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 | Promise<string>;

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

const clientType = this.#clientConfigApiService.getClient();
const thresholdValue = generateDeterministicRandomNumber(
clientType,
metaMetricsId,
);

for (const [
remoteFeatureFlagName,
Expand Down
137 changes: 109 additions & 28 deletions packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import { validate as uuidValidate, version as uuidVersion } from 'uuid';
import { v4 as uuidv4 } from 'uuid';

import { ClientType } from '../remote-feature-flag-controller-types';
import {
generateDeterministicRandomNumber,
isFeatureFlagWithScopeValue,
generateFallbackMetaMetricsId,
} from './user-segmentation-utils';

const MOCK_METRICS_IDS = [
'123e4567-e89b-4456-a456-426614174000',
'987fcdeb-51a2-4c4b-9876-543210fedcba',
'a1b2c3d4-e5f6-4890-abcd-ef1234567890',
'f9e8d7c6-b5a4-4210-9876-543210fedcba',
];
const MOCK_METRICS_IDS = {
MOBILE_VALID: '123e4567-e89b-4456-a456-426614174000',
EXTENSION_VALID:
'0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420',
MOBILE_MIN: '00000000-0000-0000-0000-000000000000',
MOBILE_MAX: 'ffffffff-ffff-ffff-ffff-ffffffffffff',
EXTENSION_MIN: `0x${'0'.repeat(64)}`,
EXTENSION_MAX: `0x${'f'.repeat(64)}`,
};

const MOCK_FEATURE_FLAGS = {
VALID: {
Expand All @@ -31,26 +34,112 @@ const MOCK_FEATURE_FLAGS = {

describe('user-segmentation-utils', () => {
describe('generateDeterministicRandomNumber', () => {
it('generates consistent numbers for the same input', () => {
const result1 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]);
const result2 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]);
describe('Mobile client (uuidv4)', () => {
it('generates consistent results for same uuidv4', () => {
const result1 = generateDeterministicRandomNumber(
ClientType.Mobile,
MOCK_METRICS_IDS.MOBILE_VALID,
);
const result2 = generateDeterministicRandomNumber(
ClientType.Mobile,
MOCK_METRICS_IDS.MOBILE_VALID,
);
expect(result1).toBe(result2);
});

expect(result1).toBe(result2);
});
it('handles minimum uuidv4 value', () => {
const result = generateDeterministicRandomNumber(
ClientType.Mobile,
MOCK_METRICS_IDS.MOBILE_MIN,
);
expect(result).toBeGreaterThanOrEqual(0);
expect(result).toBeLessThanOrEqual(1);
});

it('generates numbers between 0 and 1', () => {
MOCK_METRICS_IDS.forEach((id) => {
const result = generateDeterministicRandomNumber(id);
it('handles maximum uuidv4 value', () => {
const result = generateDeterministicRandomNumber(
ClientType.Mobile,
MOCK_METRICS_IDS.MOBILE_MAX,
);
expect(result).toBeGreaterThanOrEqual(0);
expect(result).toBeLessThanOrEqual(1);
});
});

it('generates different numbers for different inputs', () => {
const result1 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]);
const result2 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[1]);
describe('Extension client (hex string)', () => {
it('generates consistent results for same hex', () => {
const result1 = generateDeterministicRandomNumber(
ClientType.Extension,
MOCK_METRICS_IDS.EXTENSION_VALID,
);
const result2 = generateDeterministicRandomNumber(
ClientType.Extension,
MOCK_METRICS_IDS.EXTENSION_VALID,
);
expect(result1).toBe(result2);
});

it('handles minimum hex value', () => {
const result = generateDeterministicRandomNumber(
ClientType.Extension,
MOCK_METRICS_IDS.EXTENSION_MIN,
);
expect(result).toBe(0);
});

it('handles maximum hex value', () => {
const result = generateDeterministicRandomNumber(
ClientType.Extension,
MOCK_METRICS_IDS.EXTENSION_MAX,
);
expect(result).toBe(1);
});
});

describe('Distribution validation', () => {
it('produces uniform distribution', () => {
const samples = 1000;
const buckets = 10;
const distribution = new Array(buckets).fill(0);

// Generate samples using valid UUIDs
Array.from({ length: samples }).forEach(() => {
const randomUUID = uuidv4();
const value = generateDeterministicRandomNumber(
ClientType.Mobile,
randomUUID,
);
const bucketIndex = Math.floor(value * buckets);
distribution[bucketIndex] += 1;
});

// Check distribution
const expectedPerBucket = samples / buckets;
const tolerance = expectedPerBucket * 0.3; // 30% tolerance

distribution.forEach((count) => {
expect(count).toBeGreaterThanOrEqual(expectedPerBucket - tolerance);
expect(count).toBeLessThanOrEqual(expectedPerBucket + tolerance);
});
});
});

describe('Client type handling', () => {
it('defaults to Extension handling for undefined client type', () => {
const undefinedClient = 'undefined' as unknown as ClientType;
const result = generateDeterministicRandomNumber(
undefinedClient,
MOCK_METRICS_IDS.EXTENSION_VALID,
);

// Should match Extension result
const extensionResult = generateDeterministicRandomNumber(
ClientType.Extension,
MOCK_METRICS_IDS.EXTENSION_VALID,
);

expect(result1).not.toBe(result2);
expect(result).toBe(extensionResult);
});
});
});

Expand All @@ -75,12 +164,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);
});
});
});
Loading
Loading