Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 35 additions & 4 deletions lib/core/decision_service/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
getVariationIdFromExperimentAndVariationKey,
getVariationFromId,
getVariationKeyFromId,
getHoldoutsForRule,
isActive,
ProjectConfig,
} from '../../project_config/project_config';
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
Expand Down
71 changes: 71 additions & 0 deletions lib/project_config/project_config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
29 changes: 29 additions & 0 deletions lib/project_config/project_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) => {

Expand All @@ -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
Expand Down Expand Up @@ -959,6 +987,7 @@ export default {
getSendFlagDecisionsValue,
getAudiencesById,
getAudienceSegments,
getHoldoutsForRule,
eventWithKeyExists,
isFeatureExperiment,
toDatafile,
Expand Down
6 changes: 6 additions & 0 deletions lib/shared_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading