diff --git a/src/client/eppo-client.spec.ts b/src/client/eppo-client.spec.ts index 98b54075..208f877e 100644 --- a/src/client/eppo-client.spec.ts +++ b/src/client/eppo-client.spec.ts @@ -16,7 +16,6 @@ import { import { IAssignmentLogger } from '../assignment-logger'; import { IConfigurationStore } from '../configuration-store'; import { MAX_EVENT_QUEUE_SIZE, POLL_INTERVAL_MS, POLL_JITTER_PCT } from '../constants'; -import { Evaluator } from '../evaluator'; import FlagConfigurationRequestor from '../flag-configuration-requestor'; import HttpClient from '../http-client'; import { Flag, VariationType } from '../interfaces'; @@ -301,7 +300,6 @@ describe('EppoClient E2E test', () => { mock.setup(); mock.get(flagEndpoint, (_req, res) => { const ufc = readMockUFCResponse(OBFUSCATED_MOCK_UFC_RESPONSE_FILE); - console.log(ufc); return res.status(200).body(JSON.stringify(ufc)); }); await init(storage); diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index f8f634ad..394fd757 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -23,7 +23,7 @@ import HttpClient from '../http-client'; import { Flag, VariationType } from '../interfaces'; import { getMD5Hash } from '../obfuscation'; import initPoller, { IPoller } from '../poller'; -import { AttributeType } from '../types'; +import { AttributeType, ValueType } from '../types'; import { validateNotBlank } from '../validation'; import { LIB_VERSION } from '../version'; @@ -364,6 +364,13 @@ export default class EppoClient implements IEppoClient { result.flagKey = flagKey; } + if ( + result?.variation && + !this.checkValueTypeMatch(expectedVariationType, result.variation.value) + ) { + return noneResult(flagKey, subjectKey, subjectAttributes); + } + try { if (result?.doLog) { this.logAssignment(result); @@ -386,6 +393,28 @@ export default class EppoClient implements IEppoClient { return expectedType === undefined || actualType === expectedType; } + private checkValueTypeMatch(expectedType: VariationType | undefined, value: ValueType): boolean { + if (expectedType == undefined) { + return true; + } + + switch (expectedType) { + case VariationType.STRING: + return typeof value === 'string'; + case VariationType.BOOLEAN: + return typeof value === 'boolean'; + case VariationType.INTEGER: + return typeof value === 'number' && Number.isInteger(value); + case VariationType.NUMERIC: + return typeof value === 'number'; + case VariationType.JSON: + // note: converting to object downstream + return typeof value === 'string'; + default: + return false; + } + } + public getFlagKeys() { /** * Returns a list of all flag keys that have been initialized. diff --git a/src/evaluator.spec.ts b/src/evaluator.spec.ts index c6c6eafd..88ab9732 100644 --- a/src/evaluator.spec.ts +++ b/src/evaluator.spec.ts @@ -1,7 +1,7 @@ -import { Evaluator, hashKey, isInShardRange } from './evaluator'; +import { Evaluator, hashKey, isInShardRange, matchesRules } from './evaluator'; import { Flag, Variation, Shard, VariationType } from './interfaces'; import { getMD5Hash } from './obfuscation'; -import { ObfuscatedOperatorType, OperatorType } from './rules'; +import { ObfuscatedOperatorType, OperatorType, Rule } from './rules'; import { DeterministicSharder } from './sharders'; describe('Evaluator', () => { @@ -380,7 +380,6 @@ describe('Evaluator', () => { allocations: [ { key: 'first', - rules: [], splits: [ { variationKey: 'a', @@ -403,7 +402,6 @@ describe('Evaluator', () => { }, { key: 'default', - rules: [], splits: [ { variationKey: 'c', @@ -454,8 +452,8 @@ describe('Evaluator', () => { allocations: [ { key: 'default', - startAt: new Date(now.getFullYear() + 1, 0, 1), - endAt: new Date(now.getFullYear() + 1, 1, 1), + startAt: new Date(now.getFullYear() + 1, 0, 1).toISOString(), + endAt: new Date(now.getFullYear() + 1, 1, 1).toISOString(), rules: [], splits: [ { @@ -486,8 +484,8 @@ describe('Evaluator', () => { allocations: [ { key: 'default', - startAt: new Date(now.getFullYear() - 1, 0, 1), - endAt: new Date(now.getFullYear() + 1, 0, 1), + startAt: new Date(now.getFullYear() - 1, 0, 1).toISOString(), + endAt: new Date(now.getFullYear() + 1, 0, 1).toISOString(), rules: [], splits: [ { @@ -518,8 +516,8 @@ describe('Evaluator', () => { allocations: [ { key: 'default', - startAt: new Date(now.getFullYear() - 2, 0, 1), - endAt: new Date(now.getFullYear() - 1, 0, 1), + startAt: new Date(now.getFullYear() - 2, 0, 1).toISOString(), + endAt: new Date(now.getFullYear() - 1, 0, 1).toISOString(), rules: [], splits: [ { @@ -554,3 +552,101 @@ describe('Evaluator', () => { expect(isInShardRange(1, { start: 1, end: 1 })).toBeFalsy(); }); }); + +describe('matchesRules', () => { + describe('matchesRules function', () => { + it('should return true when there are no rules', () => { + const rules: Rule[] = []; + const subjectAttributes = { id: 'test-subject' }; + const obfuscated = false; + expect(matchesRules(rules, subjectAttributes, obfuscated)).toBeTruthy(); + }); + + it('should return true when a rule matches', () => { + const rules: Rule[] = [ + { + conditions: [ + { + attribute: 'age', + operator: OperatorType.GTE, + value: 18, + }, + ], + }, + ]; + const subjectAttributes = { id: 'test-subject', age: 20 }; + const obfuscated = false; + expect(matchesRules(rules, subjectAttributes, obfuscated)).toBeTruthy(); + }); + + it('should return true when one of two rules matches', () => { + const rules: Rule[] = [ + { + conditions: [ + { + attribute: 'age', + operator: OperatorType.GTE, + value: 18, + }, + ], + }, + { + conditions: [ + { + attribute: 'age', + operator: OperatorType.LTE, + value: 10, + }, + ], + }, + ]; + const subjectAttributes = { id: 'test-subject', age: 10 }; + const obfuscated = false; + expect(matchesRules(rules, subjectAttributes, obfuscated)).toBeTruthy(); + }); + + it('should return true when null or rule is passed', () => { + const rules: Rule[] = [ + { + conditions: [ + { + attribute: 'age', + operator: OperatorType.IS_NULL, + value: true, + }, + ], + }, + { + conditions: [ + { + attribute: 'age', + operator: OperatorType.GTE, + value: 20, + }, + ], + }, + ]; + const obfuscated = false; + expect(matchesRules(rules, { id: 'test-subject', age: 20 }, obfuscated)).toBeTruthy(); + expect(matchesRules(rules, { id: 'test-subject', age: 10 }, obfuscated)).toBeFalsy(); + expect(matchesRules(rules, { id: 'test-subject', country: 'UK' }, obfuscated)).toBeTruthy(); + }); + + it('should return false when no rules match', () => { + const rules: Rule[] = [ + { + conditions: [ + { + attribute: 'age', + operator: OperatorType.GTE, + value: 18, + }, + ], + }, + ]; + const subjectAttributes = { id: 'test-subject', age: 16 }; + const obfuscated = false; + expect(matchesRules(rules, subjectAttributes, obfuscated)).toBeFalsy(); + }); + }); +}); diff --git a/src/evaluator.ts b/src/evaluator.ts index c952fef8..4fe9e818 100644 --- a/src/evaluator.ts +++ b/src/evaluator.ts @@ -1,11 +1,12 @@ import { Flag, Shard, Range, Variation } from './interfaces'; -import { matchesRule } from './rules'; +import { Rule, matchesRule } from './rules'; import { MD5Sharder, Sharder } from './sharders'; +import { SubjectAttributes } from './types'; export interface FlagEvaluation { flagKey: string; subjectKey: string; - subjectAttributes: Record; + subjectAttributes: SubjectAttributes; allocationKey: string | null; variation: Variation | null; extraLogging: Record; @@ -22,7 +23,7 @@ export class Evaluator { evaluateFlag( flag: Flag, subjectKey: string, - subjectAttributes: Record, + subjectAttributes: SubjectAttributes, obfuscated: boolean, ): FlagEvaluation { if (!flag.enabled) { @@ -31,14 +32,11 @@ export class Evaluator { const now = new Date(); for (const allocation of flag.allocations) { - if (allocation.startAt && now < allocation.startAt) continue; - if (allocation.endAt && now > allocation.endAt) continue; + if (allocation.startAt && now < new Date(allocation.startAt)) continue; + if (allocation.endAt && now > new Date(allocation.endAt)) continue; if ( - !allocation.rules.length || - allocation.rules.some((rule) => - matchesRule(rule, { id: subjectKey, ...subjectAttributes }, obfuscated), - ) + matchesRules(allocation?.rules ?? [], { id: subjectKey, ...subjectAttributes }, obfuscated) ) { for (const split of allocation.splits) { if ( @@ -78,7 +76,7 @@ export function hashKey(salt: string, subjectKey: string): string { export function noneResult( flagKey: string, subjectKey: string, - subjectAttributes: Record, + subjectAttributes: SubjectAttributes, ): FlagEvaluation { return { flagKey, @@ -90,3 +88,11 @@ export function noneResult( doLog: false, }; } + +export function matchesRules( + rules: Rule[], + subjectAttributes: SubjectAttributes, + obfuscated: boolean, +): boolean { + return !rules.length || rules.some((rule) => matchesRule(rule, subjectAttributes, obfuscated)); +} diff --git a/src/interfaces.ts b/src/interfaces.ts index d949bd72..e0f69187 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -31,9 +31,9 @@ export interface Split { export interface Allocation { key: string; - rules: Rule[]; - startAt?: Date; - endAt?: Date; + rules?: Rule[]; + startAt?: string; // ISO 8601 + endAt?: string; // ISO 8601 splits: Split[]; doLog: boolean; } diff --git a/src/rules.spec.ts b/src/rules.spec.ts index a2563e99..167e7273 100644 --- a/src/rules.spec.ts +++ b/src/rules.spec.ts @@ -52,6 +52,26 @@ describe('rules', () => { ], }; + const ruleWithNullCondition: Rule = { + conditions: [ + { + operator: OperatorType.IS_NULL, + attribute: 'country', + value: true, + }, + ], + }; + + const ruleWithNotNullCondition: Rule = { + conditions: [ + { + operator: OperatorType.IS_NULL, + attribute: 'country', + value: false, + }, + ], + }; + const semverRule: Rule = { conditions: [ { @@ -136,6 +156,30 @@ describe('rules', () => { expect(matchesRule(ruleWithNotOneOfCondition, { country: null }, false)).toBe(false); }); + it('should return true for a rule with IS_NULL condition when subject attribute is null', () => { + expect(matchesRule(ruleWithNullCondition, { country: null }, false)).toBe(true); + }); + + it('should return false for a rule with IS_NULL condition when subject attribute is not null', () => { + expect(matchesRule(ruleWithNullCondition, { country: 'UK' }, false)).toBe(false); + }); + + it('should return true for a rule with IS_NULL condition when subject attribute is missing', () => { + expect(matchesRule(ruleWithNullCondition, { age: 10 }, false)).toBe(true); + }); + + it('should return false for a rule with NOT IS_NULL condition when subject attribute is null', () => { + expect(matchesRule(ruleWithNotNullCondition, { country: null }, false)).toBe(false); + }); + + it('should return true for a rule with NOT IS_NULL condition when subject attribute is not null', () => { + expect(matchesRule(ruleWithNotNullCondition, { country: 'UK' }, false)).toBe(true); + }); + + it('should return false for a rule with NOT IS_NULL condition when subject attribute is missing', () => { + expect(matchesRule(ruleWithNotNullCondition, { age: 10 }, false)).toBe(false); + }); + it('should return true for a semver rule that matches the subject attributes', () => { expect(matchesRule(semverRule, subjectAttributes, false)).toBe(true); }); @@ -194,6 +238,26 @@ describe('rules', () => { ], }; + const obfuscatedRuleWithNullCondition: Rule = { + conditions: [ + { + operator: ObfuscatedOperatorType.IS_NULL, + attribute: 'e909c2d7067ea37437cf97fe11d91bd0', + value: 'b326b5062b2f0e69046810717534cb09', + }, + ], + }; + + const obfuscatedRuleWithNotNullCondition: Rule = { + conditions: [ + { + operator: ObfuscatedOperatorType.IS_NULL, + attribute: 'e909c2d7067ea37437cf97fe11d91bd0', + value: '68934a3e9455fa72420237eb05902327', + }, + ], + }; + const obfuscatedRuleWithGTECondition: Rule = { conditions: [ { @@ -278,6 +342,32 @@ describe('rules', () => { ); }); + it('should return true for a rule with IS_NULL condition when subject attribute is null', () => { + expect(matchesRule(obfuscatedRuleWithNullCondition, { country: null }, true)).toBe(true); + }); + + it('should return false for a rule with IS_NULL condition when subject attribute is not null', () => { + expect(matchesRule(obfuscatedRuleWithNullCondition, { country: 'UK' }, true)).toBe(false); + }); + + it('should return true for a rule with IS_NULL condition when subject attribute is missing', () => { + expect(matchesRule(obfuscatedRuleWithNullCondition, { age: 10 }, true)).toBe(true); + }); + + it('should return false for a rule with NOT IS_NULL condition when subject attribute is null', () => { + expect(matchesRule(obfuscatedRuleWithNotNullCondition, { country: null }, false)).toBe( + false, + ); + }); + + it('should return true for a rule with NOT IS_NULL condition when subject attribute is not null', () => { + expect(matchesRule(obfuscatedRuleWithNotNullCondition, { country: 'UK' }, true)).toBe(true); + }); + + it('should return false for a rule with NOT IS_NULL condition when subject attribute is missing', () => { + expect(matchesRule(obfuscatedRuleWithNotNullCondition, { age: 10 }, true)).toBe(false); + }); + it('should return true for an obfuscated rule with GTE condition that matches the subject attributes', () => { expect(matchesRule(obfuscatedRuleWithGTECondition, { age: 18 }, true)).toBe(true); }); diff --git a/src/rules.ts b/src/rules.ts index 14077d43..968c4c06 100644 --- a/src/rules.ts +++ b/src/rules.ts @@ -19,6 +19,7 @@ export enum OperatorType { LT = 'LT', ONE_OF = 'ONE_OF', NOT_ONE_OF = 'NOT_ONE_OF', + IS_NULL = 'IS_NULL', } export enum ObfuscatedOperatorType { @@ -30,6 +31,7 @@ export enum ObfuscatedOperatorType { LT = 'c562607189d77eb9dfb707464c1e7b0b', ONE_OF = '27457ce369f2a74203396a35ef537c0b', NOT_ONE_OF = '602f5ee0b6e84fe29f43ab48b9e1addf', + IS_NULL = 'dbd9c38e0339e6c34bd48cafc59be388', } enum OperatorValueType { @@ -91,13 +93,28 @@ type ObfuscatedNumericCondition = { type NumericCondition = StandardNumericCondition | ObfuscatedNumericCondition; +type StandardNullCondition = { + operator: OperatorType.IS_NULL; + attribute: string; + value: boolean; +}; + +type ObfuscatedNullCondition = { + operator: ObfuscatedOperatorType.IS_NULL; + attribute: string; + value: string; +}; + +type NullCondition = StandardNullCondition | ObfuscatedNullCondition; + export type Condition = | MatchesCondition | NotMatchesCondition | OneOfCondition | NotOneOfCondition | SemVerCondition - | NumericCondition; + | NumericCondition + | NullCondition; export interface Rule { conditions: Condition[]; @@ -139,6 +156,13 @@ function evaluateCondition(subjectAttributes: Record, condition: Co const conditionValueType = targetingRuleConditionValuesTypesFromValues(condition.value); + if (condition.operator === OperatorType.IS_NULL) { + if (condition.value) { + return value === null || value === undefined; + } + return value !== null && value !== undefined; + } + if (value != null) { switch (condition.operator) { case OperatorType.GTE: @@ -185,6 +209,13 @@ function evaluateObfuscatedCondition( const value = hashedSubjectAttributes[condition.attribute]; const conditionValueType = targetingRuleConditionValuesTypesFromValues(value); + if (condition.operator === ObfuscatedOperatorType.IS_NULL) { + if (condition.value === getMD5Hash('true')) { + return value === null || value === undefined; + } + return value !== null && value !== undefined; + } + if (value != null) { switch (condition.operator) { case ObfuscatedOperatorType.GTE: diff --git a/test/writeObfuscatedMockUFC.ts b/test/writeObfuscatedMockUFC.ts index fc7cd102..7845f9d5 100644 --- a/test/writeObfuscatedMockUFC.ts +++ b/test/writeObfuscatedMockUFC.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import { Flag } from '../src/interfaces'; import { encodeBase64, getMD5Hash } from '../src/obfuscation'; -import { Rule } from '../src/rules'; +import { Condition, Rule } from '../src/rules'; import { MOCK_UFC_RESPONSE_FILE, @@ -11,6 +11,18 @@ import { TEST_DATA_DIR, } from './testHelpers'; +function encodeRuleValue(condition: Condition) { + switch (condition.operator) { + case 'ONE_OF': + case 'NOT_ONE_OF': + return condition.value.map((value) => getMD5Hash(value.toLowerCase())); + case 'IS_NULL': + return getMD5Hash(`${condition.value}`); + default: + return encodeBase64(`${condition.value}`); + } +} + function obfuscateRule(rule: Rule) { return { ...rule, @@ -18,10 +30,7 @@ function obfuscateRule(rule: Rule) { ...condition, attribute: getMD5Hash(condition.attribute), operator: getMD5Hash(condition.operator), - value: - ['ONE_OF', 'NOT_ONE_OF'].includes(condition.operator) && typeof condition.value === 'object' - ? condition.value.map((value) => getMD5Hash(value.toLowerCase())) - : encodeBase64(`${condition.value}`), + value: encodeRuleValue(condition), })), }; } @@ -32,7 +41,7 @@ function obfuscateFlag(flag: Flag) { key: getMD5Hash(flag.key), allocations: flag.allocations.map((allocation) => ({ ...allocation, - rules: allocation.rules.map(obfuscateRule), + rules: allocation.rules?.map(obfuscateRule), })), }; }