Skip to content

Commit

Permalink
fix(3741): refactor generateDeterministicRandomNumber by different cl…
Browse files Browse the repository at this point in the history 
…ients
  • Loading branch information
@DDDDDanica
DDDDDanica committed Dec 25, 2024
1 parent e825794 commit 6e48d4b
Show file tree
Hide file tree
Showing 6 changed files with 186 additions and 64 deletions.
17 changes: 17 additions & 0 deletions ...e-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts
View file
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
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
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
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
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

0 comments on commit 6e48d4b

Please sign in to comment.