From 8a39daf73e1ead70b562a8e857e1c489c88f47fa Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Fri, 28 Jun 2024 12:10:09 -0500 Subject: [PATCH 1/3] FF-2465 adds "environment" and "allocation.name" to details --- src/assignment-logger.spec.ts | 2 ++ .../eppo-client-assignment-details.spec.ts | 14 ++++++++- src/client/eppo-client.spec.ts | 6 ++++ src/client/eppo-client.ts | 11 ++++--- src/decoding.spec.ts | 4 +++ src/decoding.ts | 1 + src/evaluator.spec.ts | 29 +++++++++++++++++++ src/evaluator.ts | 2 ++ src/flag-evaluation-details-builder.ts | 17 +++++++---- src/interfaces.ts | 4 +++ 10 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/assignment-logger.spec.ts b/src/assignment-logger.spec.ts index 2f4e0a9..7488f69 100644 --- a/src/assignment-logger.spec.ts +++ b/src/assignment-logger.spec.ts @@ -13,6 +13,7 @@ describe('IAssignmentEvent', () => { subjectAttributes: { age: 25, country: 'USA' }, holdoutKey: 'holdout_key_123', details: { + environment: 'Test', variationKey: 'variationKey', variationValue: 'variation_123', flagEvaluationCode: 'MATCH', @@ -22,6 +23,7 @@ describe('IAssignmentEvent', () => { matchedRule: null, matchedAllocation: { key: 'allocation_123', + name: 'Allocation for allocation_123', allocationEvaluationCode: AllocationEvaluationCode.MATCH, orderPosition: 1, }, diff --git a/src/client/eppo-client-assignment-details.spec.ts b/src/client/eppo-client-assignment-details.spec.ts index 2661f7b..0fc68d0 100644 --- a/src/client/eppo-client-assignment-details.spec.ts +++ b/src/client/eppo-client-assignment-details.spec.ts @@ -59,6 +59,7 @@ describe('EppoClient get*AssignmentDetails', () => { ); const expected: IAssignmentDetails = { value: 3, + environment: 'Test', variationKey: 'three', variationValue: 3, flagEvaluationCode: 'MATCH', @@ -77,6 +78,7 @@ describe('EppoClient get*AssignmentDetails', () => { }, matchedAllocation: { key: 'targeted allocation', + name: 'Allocation for targeted allocation', allocationEvaluationCode: AllocationEvaluationCode.MATCH, orderPosition: 0, }, @@ -84,6 +86,7 @@ describe('EppoClient get*AssignmentDetails', () => { unevaluatedAllocations: [ { key: '50/50 split', + name: 'Allocation for 50/50 split', allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, orderPosition: 1, }, @@ -92,7 +95,7 @@ describe('EppoClient get*AssignmentDetails', () => { expect(result).toMatchObject(expected); }); - it.only('should set the details for a matched split', () => { + it('should set the details for a matched split', () => { const client = new EppoClient(storage); client.setIsGracefulFailureMode(false); const subjectAttributes = { email: 'alice@mycompany.com', country: 'Brazil' }; @@ -104,6 +107,7 @@ describe('EppoClient get*AssignmentDetails', () => { ); const expected: IAssignmentDetails = { value: 2, + environment: 'Test', variationKey: 'two', variationValue: 2, flagEvaluationCode: 'MATCH', @@ -114,12 +118,14 @@ describe('EppoClient get*AssignmentDetails', () => { matchedRule: null, matchedAllocation: { key: '50/50 split', + name: 'Allocation for 50/50 split', allocationEvaluationCode: AllocationEvaluationCode.MATCH, orderPosition: 2, }, unmatchedAllocations: [ { key: 'targeted allocation', + name: 'Allocation for targeted allocation', allocationEvaluationCode: AllocationEvaluationCode.FAILING_RULE, orderPosition: 1, }, @@ -141,6 +147,7 @@ describe('EppoClient get*AssignmentDetails', () => { ); const expected: IAssignmentDetails = { value: 'control', + environment: 'Test', flagEvaluationCode: 'MATCH', flagEvaluationDescription: 'Supplied attributes match rules defined in allocation "experiment" and alice belongs to the range of traffic assigned to "control".', @@ -159,17 +166,20 @@ describe('EppoClient get*AssignmentDetails', () => { }, matchedAllocation: { key: 'experiment', + name: 'Allocation for experiment', allocationEvaluationCode: AllocationEvaluationCode.MATCH, orderPosition: 2, }, unmatchedAllocations: [ { key: 'id rule', + name: 'Allocation for id rule', allocationEvaluationCode: AllocationEvaluationCode.FAILING_RULE, orderPosition: 0, }, { key: 'internal users', + name: 'Allocation for internal users', allocationEvaluationCode: AllocationEvaluationCode.FAILING_RULE, orderPosition: 1, }, @@ -177,6 +187,7 @@ describe('EppoClient get*AssignmentDetails', () => { unevaluatedAllocations: [ { key: 'rollout', + name: 'Allocation for rollout', allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, orderPosition: 3, }, @@ -191,6 +202,7 @@ describe('EppoClient get*AssignmentDetails', () => { const result = client.getIntegerAssignmentDetails('asdf', 'alice', {}, 0); expect(result).toMatchObject({ value: 0, + environment: 'Test', flagEvaluationCode: 'FLAG_UNRECOGNIZED_OR_DISABLED', flagEvaluationDescription: 'Unrecognized or disabled flag: asdf', variationKey: null, diff --git a/src/client/eppo-client.spec.ts b/src/client/eppo-client.spec.ts index 0104f57..e5620bb 100644 --- a/src/client/eppo-client.spec.ts +++ b/src/client/eppo-client.spec.ts @@ -65,6 +65,7 @@ describe('EppoClient E2E test', () => { }; const mockFlag: Flag = { + environment: 'Test', key: flagKey, enabled: true, variationType: VariationType.STRING, @@ -72,6 +73,7 @@ describe('EppoClient E2E test', () => { allocations: [ { key: 'allocation-a', + name: 'Allocation for allocation-a', rules: [], splits: [ { @@ -453,6 +455,7 @@ describe('EppoClient E2E test', () => { allocations: [ { key: 'allocation-a-2', + name: 'Allocation for allocation-a-2', rules: [], splits: [ { @@ -473,6 +476,7 @@ describe('EppoClient E2E test', () => { allocations: [ { key: 'allocation-a-3', + name: 'Allocation for allocation-a-3', rules: [], splits: [ { @@ -505,6 +509,7 @@ describe('EppoClient E2E test', () => { allocations: [ { key: 'allocation-a', // note: same key + name: 'Allocation for allocation-a', rules: [], splits: [ { @@ -534,6 +539,7 @@ describe('EppoClient E2E test', () => { allocations: [ { key: 'allocation-b', // note: different key + name: 'Allocation for allocation-b', rules: [], splits: [ { diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index b19c3ea..686f084 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -422,10 +422,12 @@ export default class EppoClient { }; } catch (error) { const eppoValue = this.rethrowIfNotGraceful(error, defaultValue); - const flagEvaluationDetails = new FlagEvaluationDetailsBuilder([], '', '').buildForNoneResult( - 'ASSIGNMENT_ERROR', - `Assignment Error: ${error.message}`, - ); + const flagEvaluationDetails = new FlagEvaluationDetailsBuilder( + '', + [], + '', + '', + ).buildForNoneResult('ASSIGNMENT_ERROR', `Assignment Error: ${error.message}`); return { eppoValue, flagEvaluationDetails, @@ -464,6 +466,7 @@ export default class EppoClient { const { flag, configFetchedAt, configPublishedAt } = this.getFlagDetails(flagKey); const flagEvaluationDetailsBuilder = new FlagEvaluationDetailsBuilder( + flag?.environment ?? '', flag?.allocations ?? [], configFetchedAt, configPublishedAt, diff --git a/src/decoding.spec.ts b/src/decoding.spec.ts index 83bd666..8aeb28a 100644 --- a/src/decoding.spec.ts +++ b/src/decoding.spec.ts @@ -138,6 +138,7 @@ describe('decoding', () => { it('should correctly decode allocation without startAt and endAt', () => { const obfuscatedAllocation = { key: 'ZXhwZXJpbWVudA==', + name: 'QWxsb2NhdGlvbiBmb3IgZXhwZXJpbWVudA==', rules: [], splits: [], // tested in decodeSplit doLog: true, @@ -145,6 +146,7 @@ describe('decoding', () => { const expectedAllocation = { key: 'experiment', + name: 'Allocation for experiment', rules: [], splits: [], doLog: true, @@ -156,6 +158,7 @@ describe('decoding', () => { it('should correctly decode allocation with startAt and endAt', () => { const obfuscatedAllocation = { key: 'ZXhwZXJpbWVudA==', + name: 'QWxsb2NhdGlvbiBmb3IgZXhwZXJpbWVudA==', startAt: 'MjAyMC0wNC0wMVQxODo1ODo1NS44Mjla', endAt: 'MjAyNS0wNy0yOVQwOTowMDoxMy4yMDVa', rules: [], @@ -165,6 +168,7 @@ describe('decoding', () => { const expectedAllocation = { key: 'experiment', + name: 'Allocation for experiment', rules: [], splits: [], doLog: true, diff --git a/src/decoding.ts b/src/decoding.ts index 5247704..cfa9cfb 100644 --- a/src/decoding.ts +++ b/src/decoding.ts @@ -48,6 +48,7 @@ export function decodeAllocation(allocation: ObfuscatedAllocation): Allocation { return { ...allocation, key: decodeBase64(allocation.key), + name: decodeBase64(allocation.name), splits: allocation.splits.map(decodeSplit), startAt: allocation.startAt ? new Date(decodeBase64(allocation.startAt)).toISOString() diff --git a/src/evaluator.spec.ts b/src/evaluator.spec.ts index c8d20cb..d359158 100644 --- a/src/evaluator.spec.ts +++ b/src/evaluator.spec.ts @@ -14,12 +14,14 @@ describe('Evaluator', () => { it('should return none result for disabled flag', () => { const flag: Flag = { key: 'disabled_flag', + environment: 'Test', enabled: false, variationType: VariationType.STRING, variations: { a: VARIATION_A }, allocations: [ { key: 'default', + name: 'Allocation for default', rules: [], splits: [ { @@ -78,6 +80,7 @@ describe('Evaluator', () => { it('should evaluate empty flag to none result', () => { const emptyFlag: Flag = { key: 'empty', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -95,12 +98,14 @@ describe('Evaluator', () => { it('should evaluate simple flag and return control variation', () => { const flag: Flag = { key: 'flag-key', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control-value' } }, allocations: [ { key: 'allocation', + name: 'Allocation for allocation', rules: [], splits: [ { @@ -122,12 +127,14 @@ describe('Evaluator', () => { it('should evaluate flag based on a targeting condition based on id', () => { const flag: Flag = { key: 'flag-key', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control' } }, allocations: [ { key: 'allocation', + name: 'Allocation for allocation', rules: [ { conditions: [ @@ -161,12 +168,14 @@ describe('Evaluator', () => { it('should evaluate flag based on a targeting condition with overwritten id', () => { const flag: Flag = { key: 'flag-key', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control' } }, allocations: [ { key: 'allocation', + name: 'Allocation for allocation', rules: [ { conditions: [ @@ -194,12 +203,14 @@ describe('Evaluator', () => { it('should catch all allocation and return variation A', () => { const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, allocations: [ { key: 'default', + name: 'Allocation for default', rules: [], splits: [ { @@ -224,12 +235,14 @@ describe('Evaluator', () => { it('should match first allocation rule and return variation B', () => { const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, allocations: [ { key: 'first', + name: 'Allocation for first', rules: [ { conditions: [ @@ -248,6 +261,7 @@ describe('Evaluator', () => { }, { key: 'default', + name: 'Allocation for default', rules: [], splits: [ { @@ -278,12 +292,14 @@ describe('Evaluator', () => { it('should not match first allocation rule and return variation A', () => { const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, allocations: [ { key: 'first', + name: 'Allocation for first', rules: [ { conditions: [ @@ -302,6 +318,7 @@ describe('Evaluator', () => { }, { key: 'default', + name: 'Allocation for default', rules: [], splits: [ { @@ -332,12 +349,14 @@ describe('Evaluator', () => { it('should not match first allocation rule and return variation A (obfuscated)', () => { const flag: Flag = { key: 'obfuscated_flag_key', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, allocations: [ { key: 'first', + name: 'Allocation for first', rules: [ { conditions: [ @@ -360,6 +379,7 @@ describe('Evaluator', () => { }, { key: 'default', + name: 'Allocation for default', rules: [], splits: [ { @@ -390,12 +410,14 @@ describe('Evaluator', () => { it('should evaluate sharding and return correct variations', () => { const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B, c: VARIATION_C }, allocations: [ { key: 'first', + name: 'Allocation for first', splits: [ { variationKey: 'a', @@ -418,6 +440,7 @@ describe('Evaluator', () => { }, { key: 'default', + name: 'Allocation for default', splits: [ { variationKey: 'c', @@ -462,12 +485,14 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, allocations: [ { key: 'default', + name: 'Allocation for default', startAt: new Date(now.getFullYear() + 1, 0, 1).toISOString(), endAt: new Date(now.getFullYear() + 1, 1, 1).toISOString(), rules: [], @@ -494,12 +519,14 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, allocations: [ { key: 'default', + name: 'Allocation for default', startAt: new Date(now.getFullYear() - 1, 0, 1).toISOString(), endAt: new Date(now.getFullYear() + 1, 0, 1).toISOString(), rules: [], @@ -526,12 +553,14 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', + environment: 'Test', enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, allocations: [ { key: 'default', + name: 'Allocation for default', startAt: new Date(now.getFullYear() - 2, 0, 1).toISOString(), endAt: new Date(now.getFullYear() - 1, 0, 1).toISOString(), rules: [], diff --git a/src/evaluator.ts b/src/evaluator.ts index a270120..840f4d1 100644 --- a/src/evaluator.ts +++ b/src/evaluator.ts @@ -37,6 +37,7 @@ export class Evaluator { expectedVariationType?: VariationType, ): FlagEvaluation { const flagEvaluationDetailsBuilder = new FlagEvaluationDetailsBuilder( + flag.environment, flag.allocations, configFetchedAt, configPublishedAt, @@ -61,6 +62,7 @@ export class Evaluator { const addUnmatchedAllocation = (code: AllocationEvaluationCode) => { unmatchedAllocations.push({ key: allocation.key, + name: allocation.name, allocationEvaluationCode: code, orderPosition: i + 1, }); diff --git a/src/flag-evaluation-details-builder.ts b/src/flag-evaluation-details-builder.ts index b785dd8..1c07fe6 100644 --- a/src/flag-evaluation-details-builder.ts +++ b/src/flag-evaluation-details-builder.ts @@ -22,11 +22,13 @@ export enum AllocationEvaluationCode { export interface AllocationEvaluation { key: string; + name: string; allocationEvaluationCode: AllocationEvaluationCode; orderPosition: number; } export interface IFlagEvaluationDetails { + environment: string; variationKey: string | null; variationValue: Variation['value'] | null; flagEvaluationCode: FlagEvaluationCode; @@ -48,6 +50,7 @@ export class FlagEvaluationDetailsBuilder { private unevaluatedAllocations: IFlagEvaluationDetails['unevaluatedAllocations']; constructor( + private readonly environment: string, private readonly allocations: Allocation[], private readonly configFetchedAt: string, private readonly configPublishedAt: string, @@ -62,12 +65,12 @@ export class FlagEvaluationDetailsBuilder { this.matchedRule = null; this.unmatchedAllocations = []; this.unevaluatedAllocations = this.allocations.map( - (allocation, i) => - ({ - key: allocation.key, - allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, - orderPosition: i, - } as AllocationEvaluation), + (allocation, i): AllocationEvaluation => ({ + key: allocation.key, + name: allocation.name, + allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, + orderPosition: i, + }), ); return this; }; @@ -102,6 +105,7 @@ export class FlagEvaluationDetailsBuilder { this.matchedRule = matchedRule; this.matchedAllocation = { key: allocation.key, + name: allocation.name, allocationEvaluationCode: AllocationEvaluationCode.MATCH, orderPosition: indexPosition + 1, // orderPosition is 1-indexed to match UI }; @@ -128,6 +132,7 @@ export class FlagEvaluationDetailsBuilder { flagEvaluationCode: FlagEvaluationCode, flagEvaluationDescription: string, ): IFlagEvaluationDetails => ({ + environment: this.environment, flagEvaluationCode, flagEvaluationDescription, variationKey: this.variationKey, diff --git a/src/interfaces.ts b/src/interfaces.ts index 9f68da4..cb6f847 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -31,6 +31,7 @@ export interface Split { export interface Allocation { key: string; + name: string; rules?: Rule[]; startAt?: string; // ISO 8601 endAt?: string; // ISO 8601 @@ -40,6 +41,7 @@ export interface Allocation { export interface Flag { key: string; + environment: string; enabled: boolean; variationType: VariationType; variations: Record; @@ -49,6 +51,7 @@ export interface Flag { export interface ObfuscatedFlag { key: string; + environment: string; enabled: boolean; variationType: VariationType; variations: Record; @@ -63,6 +66,7 @@ export interface ObfuscatedVariation { export interface ObfuscatedAllocation { key: string; + name: string; rules?: Rule[]; startAt?: string; // ISO 8601 endAt?: string; // ISO 8601 From b8f67b8b5d822f2668d3c3593fc9d98340ad4353 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Jul 2024 08:40:02 -0500 Subject: [PATCH 2/3] FF-2526 update to accommodate EnvironmentDto --- src/assignment-logger.spec.ts | 2 +- .../eppo-client-assignment-details.spec.ts | 8 +-- src/client/eppo-client.ts | 2 +- src/evaluator.spec.ts | 52 ++++++++++++++----- src/evaluator.ts | 2 +- src/flag-evaluation-details-builder.ts | 6 +-- src/interfaces.ts | 8 ++- 7 files changed, 55 insertions(+), 25 deletions(-) diff --git a/src/assignment-logger.spec.ts b/src/assignment-logger.spec.ts index 7488f69..a611b51 100644 --- a/src/assignment-logger.spec.ts +++ b/src/assignment-logger.spec.ts @@ -13,7 +13,7 @@ describe('IAssignmentEvent', () => { subjectAttributes: { age: 25, country: 'USA' }, holdoutKey: 'holdout_key_123', details: { - environment: 'Test', + environmentName: 'Test', variationKey: 'variationKey', variationValue: 'variation_123', flagEvaluationCode: 'MATCH', diff --git a/src/client/eppo-client-assignment-details.spec.ts b/src/client/eppo-client-assignment-details.spec.ts index 0fc68d0..5af0e07 100644 --- a/src/client/eppo-client-assignment-details.spec.ts +++ b/src/client/eppo-client-assignment-details.spec.ts @@ -59,7 +59,7 @@ describe('EppoClient get*AssignmentDetails', () => { ); const expected: IAssignmentDetails = { value: 3, - environment: 'Test', + environmentName: 'Test', variationKey: 'three', variationValue: 3, flagEvaluationCode: 'MATCH', @@ -107,7 +107,7 @@ describe('EppoClient get*AssignmentDetails', () => { ); const expected: IAssignmentDetails = { value: 2, - environment: 'Test', + environmentName: 'Test', variationKey: 'two', variationValue: 2, flagEvaluationCode: 'MATCH', @@ -147,7 +147,7 @@ describe('EppoClient get*AssignmentDetails', () => { ); const expected: IAssignmentDetails = { value: 'control', - environment: 'Test', + environmentName: 'Test', flagEvaluationCode: 'MATCH', flagEvaluationDescription: 'Supplied attributes match rules defined in allocation "experiment" and alice belongs to the range of traffic assigned to "control".', @@ -202,7 +202,7 @@ describe('EppoClient get*AssignmentDetails', () => { const result = client.getIntegerAssignmentDetails('asdf', 'alice', {}, 0); expect(result).toMatchObject({ value: 0, - environment: 'Test', + environmentName: 'Test', flagEvaluationCode: 'FLAG_UNRECOGNIZED_OR_DISABLED', flagEvaluationDescription: 'Unrecognized or disabled flag: asdf', variationKey: null, diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 686f084..dae35ce 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -466,7 +466,7 @@ export default class EppoClient { const { flag, configFetchedAt, configPublishedAt } = this.getFlagDetails(flagKey); const flagEvaluationDetailsBuilder = new FlagEvaluationDetailsBuilder( - flag?.environment ?? '', + flag?.environment.name ?? '', flag?.allocations ?? [], configFetchedAt, configPublishedAt, diff --git a/src/evaluator.spec.ts b/src/evaluator.spec.ts index d359158..e8df5bb 100644 --- a/src/evaluator.spec.ts +++ b/src/evaluator.spec.ts @@ -14,7 +14,9 @@ describe('Evaluator', () => { it('should return none result for disabled flag', () => { const flag: Flag = { key: 'disabled_flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: false, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -80,7 +82,9 @@ describe('Evaluator', () => { it('should evaluate empty flag to none result', () => { const emptyFlag: Flag = { key: 'empty', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -98,7 +102,9 @@ describe('Evaluator', () => { it('should evaluate simple flag and return control variation', () => { const flag: Flag = { key: 'flag-key', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control-value' } }, @@ -127,7 +133,9 @@ describe('Evaluator', () => { it('should evaluate flag based on a targeting condition based on id', () => { const flag: Flag = { key: 'flag-key', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control' } }, @@ -168,7 +176,9 @@ describe('Evaluator', () => { it('should evaluate flag based on a targeting condition with overwritten id', () => { const flag: Flag = { key: 'flag-key', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control' } }, @@ -203,7 +213,9 @@ describe('Evaluator', () => { it('should catch all allocation and return variation A', () => { const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -235,7 +247,9 @@ describe('Evaluator', () => { it('should match first allocation rule and return variation B', () => { const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -292,7 +306,9 @@ describe('Evaluator', () => { it('should not match first allocation rule and return variation A', () => { const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -349,7 +365,9 @@ describe('Evaluator', () => { it('should not match first allocation rule and return variation A (obfuscated)', () => { const flag: Flag = { key: 'obfuscated_flag_key', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -410,7 +428,9 @@ describe('Evaluator', () => { it('should evaluate sharding and return correct variations', () => { const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B, c: VARIATION_C }, @@ -485,7 +505,9 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -519,7 +541,9 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -553,7 +577,9 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', - environment: 'Test', + environment: { + name: 'Test', + }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, diff --git a/src/evaluator.ts b/src/evaluator.ts index 840f4d1..816fc50 100644 --- a/src/evaluator.ts +++ b/src/evaluator.ts @@ -37,7 +37,7 @@ export class Evaluator { expectedVariationType?: VariationType, ): FlagEvaluation { const flagEvaluationDetailsBuilder = new FlagEvaluationDetailsBuilder( - flag.environment, + flag.environment.name, flag.allocations, configFetchedAt, configPublishedAt, diff --git a/src/flag-evaluation-details-builder.ts b/src/flag-evaluation-details-builder.ts index 1c07fe6..bf28270 100644 --- a/src/flag-evaluation-details-builder.ts +++ b/src/flag-evaluation-details-builder.ts @@ -28,7 +28,7 @@ export interface AllocationEvaluation { } export interface IFlagEvaluationDetails { - environment: string; + environmentName: string; variationKey: string | null; variationValue: Variation['value'] | null; flagEvaluationCode: FlagEvaluationCode; @@ -50,7 +50,7 @@ export class FlagEvaluationDetailsBuilder { private unevaluatedAllocations: IFlagEvaluationDetails['unevaluatedAllocations']; constructor( - private readonly environment: string, + private readonly environmentName: string, private readonly allocations: Allocation[], private readonly configFetchedAt: string, private readonly configPublishedAt: string, @@ -132,7 +132,7 @@ export class FlagEvaluationDetailsBuilder { flagEvaluationCode: FlagEvaluationCode, flagEvaluationDescription: string, ): IFlagEvaluationDetails => ({ - environment: this.environment, + environmentName: this.environmentName, flagEvaluationCode, flagEvaluationDescription, variationKey: this.variationKey, diff --git a/src/interfaces.ts b/src/interfaces.ts index cb6f847..5b946b6 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -39,9 +39,13 @@ export interface Allocation { doLog: boolean; } +export interface Environment { + name: string; +} + export interface Flag { key: string; - environment: string; + environment: Environment; enabled: boolean; variationType: VariationType; variations: Record; @@ -51,7 +55,7 @@ export interface Flag { export interface ObfuscatedFlag { key: string; - environment: string; + environment: Environment; enabled: boolean; variationType: VariationType; variations: Record; From b57be388c3b4512ef1201a8643ce92d67319f33e Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Jul 2024 11:51:40 -0500 Subject: [PATCH 3/3] FF-2526 FF-2526 fix "environment" and code review changes --- .../eppo-client-assignment-details.spec.ts | 105 ++++++++++++------ src/client/eppo-client.spec.ts | 1 - src/client/eppo-client.ts | 35 +++--- .../configuration-store.ts | 4 + src/configuration-store/hybrid.store.ts | 11 +- src/configuration-store/memory.store.ts | 11 ++ src/evaluator.spec.ts | 104 ++++++----------- src/evaluator.ts | 20 +++- src/flag-configuration-requestor.ts | 1 + src/flag-evaluation-details-builder.ts | 3 +- src/http-client.ts | 3 +- src/interfaces.ts | 7 +- 12 files changed, 177 insertions(+), 128 deletions(-) diff --git a/src/client/eppo-client-assignment-details.spec.ts b/src/client/eppo-client-assignment-details.spec.ts index 5af0e07..8e0d126 100644 --- a/src/client/eppo-client-assignment-details.spec.ts +++ b/src/client/eppo-client-assignment-details.spec.ts @@ -1,8 +1,7 @@ +import * as fs from 'fs'; + import { - readAssignmentTestData, IAssignmentTestCase, - getTestAssignmentDetails, - validateTestAssignmentDetails, MOCK_UFC_RESPONSE_FILE, readMockUFCResponse, } from '../../test/testHelpers'; @@ -80,7 +79,7 @@ describe('EppoClient get*AssignmentDetails', () => { key: 'targeted allocation', name: 'Allocation for targeted allocation', allocationEvaluationCode: AllocationEvaluationCode.MATCH, - orderPosition: 0, + orderPosition: 1, }, unmatchedAllocations: [], unevaluatedAllocations: [ @@ -88,7 +87,7 @@ describe('EppoClient get*AssignmentDetails', () => { key: '50/50 split', name: 'Allocation for 50/50 split', allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, - orderPosition: 1, + orderPosition: 2, }, ], }; @@ -168,20 +167,20 @@ describe('EppoClient get*AssignmentDetails', () => { key: 'experiment', name: 'Allocation for experiment', allocationEvaluationCode: AllocationEvaluationCode.MATCH, - orderPosition: 2, + orderPosition: 3, }, unmatchedAllocations: [ { key: 'id rule', name: 'Allocation for id rule', allocationEvaluationCode: AllocationEvaluationCode.FAILING_RULE, - orderPosition: 0, + orderPosition: 1, }, { key: 'internal users', name: 'Allocation for internal users', allocationEvaluationCode: AllocationEvaluationCode.FAILING_RULE, - orderPosition: 1, + orderPosition: 2, }, ], unevaluatedAllocations: [ @@ -189,7 +188,7 @@ describe('EppoClient get*AssignmentDetails', () => { key: 'rollout', name: 'Allocation for rollout', allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, - orderPosition: 3, + orderPosition: 4, }, ], }; @@ -217,6 +216,23 @@ describe('EppoClient get*AssignmentDetails', () => { }); describe('UFC General Test Cases', () => { + const getTestFilePaths = () => { + const testDir = 'test/data/ufc/tests'; + return fs.readdirSync(testDir).map((testFilename) => `${testDir}/${testFilename}`); + }; + + const parseJSON = (testFilePath: string) => { + try { + const fileContents = fs.readFileSync(testFilePath, 'utf-8'); + const parsedJSON = JSON.parse(fileContents); + return parsedJSON as IAssignmentTestCase; + } catch (err) { + console.error(`failed to parse JSON in ${testFilePath}`); + console.error(err); + process.exit(1); + } + }; + beforeAll(async () => { global.fetch = jest.fn(() => { return Promise.resolve({ @@ -232,32 +248,57 @@ describe('EppoClient get*AssignmentDetails', () => { afterAll(() => { jest.restoreAllMocks(); }); - it.each(readAssignmentTestData())( - 'test variation assignment details', - async ({ flag, variationType, defaultValue, subjects }: IAssignmentTestCase) => { - const client = new EppoClient(storage); - client.setIsGracefulFailureMode(false); - const typeAssignmentDetailsFunctions = { - [VariationType.BOOLEAN]: client.getBooleanAssignmentDetails.bind(client), - [VariationType.NUMERIC]: client.getNumericAssignmentDetails.bind(client), - [VariationType.INTEGER]: client.getIntegerAssignmentDetails.bind(client), - [VariationType.STRING]: client.getStringAssignmentDetails.bind(client), - [VariationType.JSON]: client.getJSONAssignmentDetails.bind(client), - }; + describe.each(getTestFilePaths())('for file: %s', (testFilePath: string) => { + const testCase = parseJSON(testFilePath); + describe.each(testCase.subjects.map(({ subjectKey }) => subjectKey))( + 'with subjectKey %s', + (subjectKey) => { + const { flag, variationType, defaultValue, subjects } = testCase; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const subject = subjects.find((subject) => subject.subjectKey === subjectKey)!; - const assignmentFn = typeAssignmentDetailsFunctions[variationType]; - if (!assignmentFn) { - throw new Error(`Unknown variation type: ${variationType}`); - } + const client = new EppoClient(storage); + client.setIsGracefulFailureMode(false); - const assignments = getTestAssignmentDetails( - { flag, variationType, defaultValue, subjects }, - assignmentFn, - ); + const focusOn = { + testFilePath: '', // focus on test file paths (don't forget to set back to empty string!) + subjectKey: '', // focus on subject (don't forget to set back to empty string!) + }; - validateTestAssignmentDetails(assignments, flag); - }, - ); + const shouldRunTestForFilePath = + !focusOn.testFilePath || focusOn.testFilePath === testFilePath; + + const shouldRunTestForSubject = !focusOn.subjectKey || focusOn.subjectKey === subjectKey; + + if (shouldRunTestForFilePath && shouldRunTestForSubject) { + it('handles variation assignment details', async () => { + const typeAssignmentDetailsFunctions = { + [VariationType.BOOLEAN]: client.getBooleanAssignmentDetails.bind(client), + [VariationType.NUMERIC]: client.getNumericAssignmentDetails.bind(client), + [VariationType.INTEGER]: client.getIntegerAssignmentDetails.bind(client), + [VariationType.STRING]: client.getStringAssignmentDetails.bind(client), + [VariationType.JSON]: client.getJSONAssignmentDetails.bind(client), + }; + const assignmentFn = typeAssignmentDetailsFunctions[variationType]; + if (!assignmentFn) { + throw new Error(`Unknown variation type: ${variationType}`); + } + const assignmentDetails = assignmentFn( + flag, + subject.subjectKey, + subject.subjectAttributes, + defaultValue, + ); + expect(assignmentDetails).toMatchObject({ + ...subject.assignmentDetails, + configFetchedAt: expect.any(String), + configPublishedAt: expect.any(String), + }); + }); + } + }, + ); + }); }); }); diff --git a/src/client/eppo-client.spec.ts b/src/client/eppo-client.spec.ts index e5620bb..5a6670a 100644 --- a/src/client/eppo-client.spec.ts +++ b/src/client/eppo-client.spec.ts @@ -65,7 +65,6 @@ describe('EppoClient E2E test', () => { }; const mockFlag: Flag = { - environment: 'Test', key: flagKey, enabled: true, variationType: VariationType.STRING, diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index dae35ce..4a989b9 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -23,7 +23,7 @@ import { FlagEvaluationDetailsBuilder, } from '../flag-evaluation-details-builder'; import FetchHttpClient from '../http-client'; -import { Flag, ObfuscatedFlag, Variation, VariationType } from '../interfaces'; +import { ConfigDetails, Flag, ObfuscatedFlag, Variation, VariationType } from '../interfaces'; import { getMD5Hash } from '../obfuscation'; import initPoller, { IPoller } from '../poller'; import { AttributeType, ValueType } from '../types'; @@ -464,12 +464,13 @@ export default class EppoClient { validateNotBlank(subjectKey, 'Invalid argument: subjectKey cannot be blank'); validateNotBlank(flagKey, 'Invalid argument: flagKey cannot be blank'); - const { flag, configFetchedAt, configPublishedAt } = this.getFlagDetails(flagKey); + const flag = this.getFlag(flagKey); + const configDetails = this.getConfigDetails(); const flagEvaluationDetailsBuilder = new FlagEvaluationDetailsBuilder( - flag?.environment.name ?? '', + configDetails.configEnvironment.name, flag?.allocations ?? [], - configFetchedAt, - configPublishedAt, + configDetails.configFetchedAt, + configDetails.configPublishedAt, ); if (flag === null) { @@ -500,11 +501,10 @@ export default class EppoClient { const result = this.evaluator.evaluateFlag( flag, + configDetails, subjectKey, subjectAttributes, this.isObfuscated, - configFetchedAt, - configPublishedAt, expectedVariationType, ); if (this.isObfuscated) { @@ -533,21 +533,20 @@ export default class EppoClient { return result; } - private getFlagDetails(flagKey: string): { - flag: Flag | null; - configFetchedAt: string; - configPublishedAt: string; - } { - const flag = this.isObfuscated - ? this.getObfuscatedFlag(flagKey) - : this.configurationStore.get(flagKey); + private getConfigDetails(): ConfigDetails { return { - flag, - configFetchedAt: this.configurationStore.getConfigFetchedAt(), - configPublishedAt: this.configurationStore.getConfigPublishedAt(), + configFetchedAt: this.configurationStore.getConfigFetchedAt() ?? '', + configPublishedAt: this.configurationStore.getConfigPublishedAt() ?? '', + configEnvironment: this.configurationStore.getEnvironment() ?? { name: '' }, }; } + private getFlag(flagKey: string): Flag | null { + return this.isObfuscated + ? this.getObfuscatedFlag(flagKey) + : this.configurationStore.get(flagKey); + } + private getObfuscatedFlag(flagKey: string): Flag | null { const flag: ObfuscatedFlag | null = this.configurationStore.get( getMD5Hash(flagKey), diff --git a/src/configuration-store/configuration-store.ts b/src/configuration-store/configuration-store.ts index 0471a4c..fc87301 100644 --- a/src/configuration-store/configuration-store.ts +++ b/src/configuration-store/configuration-store.ts @@ -1,3 +1,5 @@ +import { Environment } from '../interfaces'; + /** * ConfigurationStore interface * @@ -27,6 +29,8 @@ export interface IConfigurationStore { isInitialized(): boolean; isExpired(): Promise; setEntries(entries: Record): Promise; + setEnvironment(environment: Environment): void; + getEnvironment(): Environment; getConfigFetchedAt(): string; setConfigFetchedAt(configFetchedAt: string): void; getConfigPublishedAt(): string; diff --git a/src/configuration-store/hybrid.store.ts b/src/configuration-store/hybrid.store.ts index 3317818..bc2d1e2 100644 --- a/src/configuration-store/hybrid.store.ts +++ b/src/configuration-store/hybrid.store.ts @@ -1,4 +1,5 @@ import { logger, loggerPrefix } from '../application-logger'; +import { Environment } from '../interfaces'; import { IAsyncStore, IConfigurationStore, ISyncStore } from './configuration-store'; @@ -7,7 +8,7 @@ export class HybridConfigurationStore implements IConfigurationStore { protected readonly servingStore: ISyncStore, protected readonly persistentStore: IAsyncStore | null, ) {} - + private environment: Environment; private configFetchedAt: string; private configPublishedAt: string; @@ -65,6 +66,14 @@ export class HybridConfigurationStore implements IConfigurationStore { return true; } + setEnvironment(environment: Environment): void { + this.environment = environment; + } + + getEnvironment(): Environment { + return this.environment; + } + public getConfigFetchedAt(): string { return this.configFetchedAt; } diff --git a/src/configuration-store/memory.store.ts b/src/configuration-store/memory.store.ts index fa988fb..497bf6e 100644 --- a/src/configuration-store/memory.store.ts +++ b/src/configuration-store/memory.store.ts @@ -1,3 +1,5 @@ +import { Environment } from '../interfaces'; + import { IConfigurationStore, ISyncStore } from './configuration-store'; export class MemoryStore implements ISyncStore { @@ -29,6 +31,7 @@ export class MemoryOnlyConfigurationStore implements IConfigurationStore { private initialized = false; private configFetchedAt: string; private configPublishedAt: string; + private environment: Environment; init(): Promise { this.initialized = true; @@ -57,6 +60,14 @@ export class MemoryOnlyConfigurationStore implements IConfigurationStore { return true; } + public getEnvironment(): Environment { + return this.environment; + } + + public setEnvironment(environment: Environment): void { + this.environment = environment; + } + public getConfigFetchedAt(): string { return this.configFetchedAt; } diff --git a/src/evaluator.spec.ts b/src/evaluator.spec.ts index e8df5bb..98e4e3d 100644 --- a/src/evaluator.spec.ts +++ b/src/evaluator.spec.ts @@ -1,5 +1,5 @@ import { Evaluator, hashKey, isInShardRange, matchesRules } from './evaluator'; -import { Flag, Variation, Shard, VariationType } from './interfaces'; +import { Flag, Variation, Shard, VariationType, ConfigDetails } from './interfaces'; import { getMD5Hash } from './obfuscation'; import { ObfuscatedOperatorType, OperatorType, Rule } from './rules'; import { DeterministicSharder } from './sharders'; @@ -11,12 +11,21 @@ describe('Evaluator', () => { const evaluator = new Evaluator(); + let configDetails: ConfigDetails; + + beforeEach(() => { + configDetails = { + configEnvironment: { + name: 'Test', + }, + configFetchedAt: new Date().toISOString(), + configPublishedAt: new Date().toISOString(), + }; + }); + it('should return none result for disabled flag', () => { const flag: Flag = { key: 'disabled_flag', - environment: { - name: 'Test', - }, enabled: false, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -38,7 +47,7 @@ describe('Evaluator', () => { totalShards: 10, }; - const result = evaluator.evaluateFlag(flag, 'subject_key', {}, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'subject_key', {}, false); expect(result.flagKey).toEqual('disabled_flag'); expect(result.allocationKey).toBeNull(); expect(result.variation).toBeNull(); @@ -82,9 +91,6 @@ describe('Evaluator', () => { it('should evaluate empty flag to none result', () => { const emptyFlag: Flag = { key: 'empty', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -92,7 +98,7 @@ describe('Evaluator', () => { totalShards: 10, }; - const result = evaluator.evaluateFlag(emptyFlag, 'subject_key', {}, false, '', ''); + const result = evaluator.evaluateFlag(emptyFlag, configDetails, 'subject_key', {}, false); expect(result.flagKey).toEqual('empty'); expect(result.allocationKey).toBeNull(); expect(result.variation).toBeNull(); @@ -102,9 +108,6 @@ describe('Evaluator', () => { it('should evaluate simple flag and return control variation', () => { const flag: Flag = { key: 'flag-key', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control-value' } }, @@ -126,16 +129,13 @@ describe('Evaluator', () => { totalShards: 10000, }; - const result = evaluator.evaluateFlag(flag, 'user-1', {}, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'user-1', {}, false); expect(result.variation).toEqual({ key: 'control', value: 'control-value' }); }); it('should evaluate flag based on a targeting condition based on id', () => { const flag: Flag = { key: 'flag-key', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control' } }, @@ -163,22 +163,19 @@ describe('Evaluator', () => { totalShards: 10000, }; - let result = evaluator.evaluateFlag(flag, 'alice', {}, false, '', ''); + let result = evaluator.evaluateFlag(flag, configDetails, 'alice', {}, false); expect(result.variation).toEqual({ key: 'control', value: 'control' }); - result = evaluator.evaluateFlag(flag, 'bob', {}, false, '', ''); + result = evaluator.evaluateFlag(flag, configDetails, 'bob', {}, false); expect(result.variation).toEqual({ key: 'control', value: 'control' }); - result = evaluator.evaluateFlag(flag, 'charlie', {}, false, '', ''); + result = evaluator.evaluateFlag(flag, configDetails, 'charlie', {}, false); expect(result.variation).toBeNull(); }); it('should evaluate flag based on a targeting condition with overwritten id', () => { const flag: Flag = { key: 'flag-key', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { control: { key: 'control', value: 'control' } }, @@ -206,16 +203,13 @@ describe('Evaluator', () => { totalShards: 10000, }; - const result = evaluator.evaluateFlag(flag, 'alice', { id: 'charlie' }, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'alice', { id: 'charlie' }, false); expect(result.variation).toBeNull(); }); it('should catch all allocation and return variation A', () => { const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -237,7 +231,7 @@ describe('Evaluator', () => { totalShards: 10, }; - const result = evaluator.evaluateFlag(flag, 'subject_key', {}, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'subject_key', {}, false); expect(result.flagKey).toEqual('flag'); expect(result.allocationKey).toEqual('default'); expect(result.variation).toEqual(VARIATION_A); @@ -247,9 +241,6 @@ describe('Evaluator', () => { it('should match first allocation rule and return variation B', () => { const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -292,11 +283,10 @@ describe('Evaluator', () => { const result = evaluator.evaluateFlag( flag, + configDetails, 'subject_key', { email: 'eppo@example.com' }, false, - '', - '', ); expect(result.flagKey).toEqual('flag'); expect(result.allocationKey).toEqual('first'); @@ -306,9 +296,6 @@ describe('Evaluator', () => { it('should not match first allocation rule and return variation A', () => { const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -351,11 +338,10 @@ describe('Evaluator', () => { const result = evaluator.evaluateFlag( flag, + configDetails, 'subject_key', { email: 'eppo@test.com' }, false, - '', - '', ); expect(result.flagKey).toEqual('flag'); expect(result.allocationKey).toEqual('default'); @@ -365,9 +351,6 @@ describe('Evaluator', () => { it('should not match first allocation rule and return variation A (obfuscated)', () => { const flag: Flag = { key: 'obfuscated_flag_key', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B }, @@ -414,11 +397,10 @@ describe('Evaluator', () => { const result = evaluator.evaluateFlag( flag, + configDetails, 'subject_key', { email: 'eppo@test.com' }, false, - '', - '', ); expect(result.flagKey).toEqual('obfuscated_flag_key'); expect(result.allocationKey).toEqual('default'); @@ -428,9 +410,6 @@ describe('Evaluator', () => { it('should evaluate sharding and return correct variations', () => { const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A, b: VARIATION_B, c: VARIATION_C }, @@ -487,27 +466,24 @@ describe('Evaluator', () => { }), ); - expect(deterministicEvaluator.evaluateFlag(flag, 'alice', {}, false, '', '').variation).toEqual( - VARIATION_A, - ); - expect(deterministicEvaluator.evaluateFlag(flag, 'bob', {}, false, '', '').variation).toEqual( - VARIATION_B, - ); expect( - deterministicEvaluator.evaluateFlag(flag, 'charlie', {}, false, '', '').variation, + deterministicEvaluator.evaluateFlag(flag, configDetails, 'alice', {}, false).variation, + ).toEqual(VARIATION_A); + expect( + deterministicEvaluator.evaluateFlag(flag, configDetails, 'bob', {}, false).variation, + ).toEqual(VARIATION_B); + expect( + deterministicEvaluator.evaluateFlag(flag, configDetails, 'charlie', {}, false).variation, + ).toEqual(VARIATION_C); + expect( + deterministicEvaluator.evaluateFlag(flag, configDetails, 'dave', {}, false).variation, ).toEqual(VARIATION_C); - expect(deterministicEvaluator.evaluateFlag(flag, 'dave', {}, false, '', '').variation).toEqual( - VARIATION_C, - ); }); it('should not match on allocation before startAt has passed', () => { const now = new Date(); const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -531,7 +507,7 @@ describe('Evaluator', () => { totalShards: 10, }; - const result = evaluator.evaluateFlag(flag, 'subject_key', {}, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'subject_key', {}, false); expect(result.flagKey).toEqual('flag'); expect(result.allocationKey).toBeNull(); expect(result.variation).toBeNull(); @@ -541,9 +517,6 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -567,7 +540,7 @@ describe('Evaluator', () => { totalShards: 10, }; - const result = evaluator.evaluateFlag(flag, 'subject_key', {}, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'subject_key', {}, false); expect(result.flagKey).toEqual('flag'); expect(result.allocationKey).toEqual('default'); expect(result.variation).toEqual(VARIATION_A); @@ -577,9 +550,6 @@ describe('Evaluator', () => { const now = new Date(); const flag: Flag = { key: 'flag', - environment: { - name: 'Test', - }, enabled: true, variationType: VariationType.STRING, variations: { a: VARIATION_A }, @@ -603,7 +573,7 @@ describe('Evaluator', () => { totalShards: 10, }; - const result = evaluator.evaluateFlag(flag, 'subject_key', {}, false, '', ''); + const result = evaluator.evaluateFlag(flag, configDetails, 'subject_key', {}, false); expect(result.flagKey).toEqual('flag'); expect(result.allocationKey).toBeNull(); expect(result.variation).toBeNull(); diff --git a/src/evaluator.ts b/src/evaluator.ts index 816fc50..2d46ce1 100644 --- a/src/evaluator.ts +++ b/src/evaluator.ts @@ -4,7 +4,16 @@ import { IFlagEvaluationDetails, FlagEvaluationDetailsBuilder, } from './flag-evaluation-details-builder'; -import { Flag, Shard, Range, Variation, Allocation, Split, VariationType } from './interfaces'; +import { + Flag, + Shard, + Range, + Variation, + Allocation, + Split, + VariationType, + ConfigDetails, +} from './interfaces'; import { Rule, matchesRule } from './rules'; import { MD5Sharder, Sharder } from './sharders'; import { SubjectAttributes } from './types'; @@ -29,18 +38,17 @@ export class Evaluator { evaluateFlag( flag: Flag, + configDetails: ConfigDetails, subjectKey: string, subjectAttributes: SubjectAttributes, obfuscated: boolean, - configFetchedAt: string, - configPublishedAt: string, expectedVariationType?: VariationType, ): FlagEvaluation { const flagEvaluationDetailsBuilder = new FlagEvaluationDetailsBuilder( - flag.environment.name, + configDetails.configEnvironment.name, flag.allocations, - configFetchedAt, - configPublishedAt, + configDetails.configFetchedAt, + configDetails.configPublishedAt, ); if (!flag.enabled) { diff --git a/src/flag-configuration-requestor.ts b/src/flag-configuration-requestor.ts index fef2a74..ace2659 100644 --- a/src/flag-configuration-requestor.ts +++ b/src/flag-configuration-requestor.ts @@ -16,6 +16,7 @@ export default class FlagConfigurationRequestor { } const didUpdateServingStore = await this.configurationStore.setEntries(responseData.flags); if (didUpdateServingStore) { + this.configurationStore.setEnvironment(responseData.environment); this.configurationStore.setConfigFetchedAt(new Date().toISOString()); this.configurationStore.setConfigPublishedAt(responseData.createdAt); } diff --git a/src/flag-evaluation-details-builder.ts b/src/flag-evaluation-details-builder.ts index bf28270..e315513 100644 --- a/src/flag-evaluation-details-builder.ts +++ b/src/flag-evaluation-details-builder.ts @@ -69,7 +69,7 @@ export class FlagEvaluationDetailsBuilder { key: allocation.key, name: allocation.name, allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, - orderPosition: i, + orderPosition: i + 1, }), ); return this; @@ -116,6 +116,7 @@ export class FlagEvaluationDetailsBuilder { (allocation, i) => ({ key: allocation.key, + name: allocation.name, allocationEvaluationCode: AllocationEvaluationCode.UNEVALUATED, orderPosition: unevaluatedStartOrderPosition + i, } as AllocationEvaluation), diff --git a/src/http-client.ts b/src/http-client.ts index 1088e2b..cf43626 100644 --- a/src/http-client.ts +++ b/src/http-client.ts @@ -1,5 +1,5 @@ import ApiEndpoints from './api-endpoints'; -import { Flag } from './interfaces'; +import { Environment, Flag } from './interfaces'; export interface IQueryParams { apiKey: string; @@ -18,6 +18,7 @@ export class HttpRequestError extends Error { export interface IUniversalFlagConfig { createdAt: string; // ISO formatted string + environment: Environment; flags: Record; } diff --git a/src/interfaces.ts b/src/interfaces.ts index 5b946b6..9440e54 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -43,9 +43,14 @@ export interface Environment { name: string; } +export interface ConfigDetails { + configFetchedAt: string; + configPublishedAt: string; + configEnvironment: Environment; +} + export interface Flag { key: string; - environment: Environment; enabled: boolean; variationType: VariationType; variations: Record;