diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 4b26f3804..c6b356b4d 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -29,6 +29,7 @@ import { getVariationIdFromExperimentAndVariationKey, getVariationFromId, getVariationKeyFromId, + getHoldoutsForRule, isActive, ProjectConfig, } from '../../project_config/project_config'; @@ -943,11 +944,11 @@ export class DecisionService { }); } - // all global holouts should be evaluated for all flags - // global holdouts are available in configObj.holdouts - const { holdouts } = configObj; + // Check global holdouts first (holdouts where includedRules == null). + // Global holdouts apply to all rules and are evaluated at the flag level. + const globalHoldouts = configObj.globalHoldouts || []; - for (const holdout of holdouts) { + for (const holdout of globalHoldouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); @@ -1562,6 +1563,21 @@ export class DecisionService { reasons: decideReasons, }); } + + // Check local holdouts targeting this specific experiment rule. + // Local holdouts are checked after forced decisions but before regular bucketing. + const localHoldouts = getHoldoutsForRule(configObj, rule.id); + for (const holdout of localHoldouts) { + const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); + decideReasons.push(...holdoutDecision.reasons); + if (holdoutDecision.result.variation) { + return Value.of(op, { + result: { variationKey: holdoutDecision.result.variation.key }, + reasons: decideReasons, + }); + } + } + const decisionVariationValue = this.resolveVariation(op, configObj, rule, user, decideOptions, userProfileTracker); return decisionVariationValue.then((variationResult) => { @@ -1608,6 +1624,21 @@ export class DecisionService { }; } + // Check local holdouts targeting this specific delivery rule. + // Local holdouts are checked after forced decisions but before audience/bucketing evaluation. + const localHoldouts = getHoldoutsForRule(configObj, rule.id); + for (const holdout of localHoldouts) { + const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); + decideReasons.push(...holdoutDecision.reasons); + if (holdoutDecision.result.variation) { + return { + result: holdoutDecision.result.variation, + reasons: decideReasons, + skipToEveryoneElse, + }; + } + } + const userId = user.getUserId(); const attributes = user.getAttributes(); const bucketingId = this.getBucketingId(userId, attributes); diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 13c086539..f4a789594 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -414,6 +414,77 @@ describe('createProjectConfig - holdouts', () => { expect(configObj.holdouts[0].includedFlags).toEqual([]); expect(configObj.holdouts[0].excludedFlags).toEqual([]); }); + + it('should populate globalHoldouts with holdouts where includedRules is null or undefined', function() { + const datafile = getHoldoutDatafile(); + // holdout_1, holdout_2, holdout_3 all have no includedRules (undefined = global) + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + expect(configObj.globalHoldouts).toHaveLength(3); + expect(configObj.ruleHoldoutsMap).toEqual({}); + }); + + it('should classify holdout as local when includedRules is an array', function() { + const datafile = getHoldoutDatafile(); + datafile.holdouts[0].includedRules = ['rule_id_1', 'rule_id_2']; + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + // holdout_1 is now local (has includedRules array) + expect(configObj.globalHoldouts).toHaveLength(2); // holdout_2, holdout_3 + expect(configObj.ruleHoldoutsMap['rule_id_1']).toHaveLength(1); + expect(configObj.ruleHoldoutsMap['rule_id_2']).toHaveLength(1); + }); + + it('should classify holdout as local when includedRules is empty array', function() { + const datafile = getHoldoutDatafile(); + datafile.holdouts[0].includedRules = []; // empty array = local, targets no rules + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + // holdout_1 is local but has no rules to target + expect(configObj.globalHoldouts).toHaveLength(2); // holdout_2, holdout_3 + expect(configObj.ruleHoldoutsMap).toEqual({}); + }); + + it('should support multiple holdouts targeting the same rule', function() { + const datafile = getHoldoutDatafile(); + datafile.holdouts[0].includedRules = ['rule_shared']; + datafile.holdouts[1].includedRules = ['rule_shared']; + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + expect(configObj.ruleHoldoutsMap['rule_shared']).toHaveLength(2); + expect(configObj.globalHoldouts).toHaveLength(1); // only holdout_3 + }); + + it('getHoldoutsForRule returns local holdouts for a rule', function() { + const datafile = getHoldoutDatafile(); + datafile.holdouts[0].includedRules = ['rule_a']; + datafile.holdouts[1].includedRules = ['rule_a', 'rule_b']; + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + const ruleAHoldouts = projectConfig.getHoldoutsForRule(configObj, 'rule_a'); + expect(ruleAHoldouts).toHaveLength(2); + + const ruleBHoldouts = projectConfig.getHoldoutsForRule(configObj, 'rule_b'); + expect(ruleBHoldouts).toHaveLength(1); + + const ruleCHoldouts = projectConfig.getHoldoutsForRule(configObj, 'rule_c'); + expect(ruleCHoldouts).toHaveLength(0); + }); + + it('backward compatibility: old datafiles without includedRules treat holdouts as global', function() { + const datafile = getHoldoutDatafile(); + // Simulating old datafile: holdouts have no includedRules field at all + datafile.holdouts.forEach((h: any) => delete h.includedRules); + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + expect(configObj.globalHoldouts).toHaveLength(3); + expect(configObj.ruleHoldoutsMap).toEqual({}); + }); }); describe('getExperimentId', () => { diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index 9f966549b..fbf8afa59 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -113,6 +113,8 @@ export interface ProjectConfig { odpIntegrationConfig: OdpIntegrationConfig; holdouts: Holdout[]; holdoutIdMap?: { [id: string]: Holdout }; + globalHoldouts: Holdout[]; + ruleHoldoutsMap: { [ruleId: string]: Holdout[] }; } const EXPERIMENT_RUNNING_STATUS = 'Running'; @@ -386,6 +388,8 @@ const getEveryoneElseVariation = function( const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { projectConfig.holdouts = projectConfig.holdouts || []; projectConfig.holdoutIdMap = keyBy(projectConfig.holdouts, 'id'); + projectConfig.globalHoldouts = []; + projectConfig.ruleHoldoutsMap = {}; projectConfig.holdouts.forEach((holdout) => { @@ -396,9 +400,33 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { holdout.excludedFlags = []; holdout.variationKeyMap = keyBy(holdout.variations, 'key'); assignBy(holdout.variations, 'id', projectConfig.variationIdMap); + + // Classify holdout as global or local based on includedRules field. + // If includedRules is null or undefined, the holdout is global (applies to all rules). + // If includedRules is an array (even empty), the holdout is local (targets specific rules). + if (holdout.includedRules == null) { + projectConfig.globalHoldouts.push(holdout); + } else { + holdout.includedRules.forEach((ruleId) => { + if (!projectConfig.ruleHoldoutsMap[ruleId]) { + projectConfig.ruleHoldoutsMap[ruleId] = []; + } + projectConfig.ruleHoldoutsMap[ruleId].push(holdout); + }); + } }); } +/** + * Returns holdouts that target a specific rule (local holdouts). + * @param {ProjectConfig} projectConfig - The project config + * @param {string} ruleId - The rule ID to look up + * @returns {Holdout[]} - Array of holdouts targeting this rule + */ +export const getHoldoutsForRule = (projectConfig: ProjectConfig, ruleId: string): Holdout[] => { + return projectConfig.ruleHoldoutsMap[ruleId] || []; +} + /** * Extract all audience segments used in this audience's conditions * @param {Audience} audience Object representing the audience being parsed @@ -959,6 +987,7 @@ export default { getSendFlagDecisionsValue, getAudiencesById, getAudienceSegments, + getHoldoutsForRule, eventWithKeyExists, isFeatureExperiment, toDatafile, diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 5fc10498b..b6fa96ada 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -176,6 +176,12 @@ export interface Holdout extends ExperimentCore { status: HoldoutStatus; includedFlags: string[]; excludedFlags: string[]; + /** + * List of rule IDs this holdout targets (local holdouts only). + * If null or undefined, the holdout is global and applies to all rules. + * If an array (even empty), the holdout is local and targets specific rule IDs. + */ + includedRules?: string[] | null; } export function isHoldout(obj: Experiment | Holdout): obj is Holdout {