diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 866bb8166..90aa5016e 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -2879,4 +2879,225 @@ describe('DecisionService', () => { expect(variation).toBe(null); }); }); + + describe('local holdouts', () => { + // Helper: build a datafile that has a local holdout targeting a specific experiment or delivery rule. + const makeLocalHoldoutDatafile = (targetRuleId: string, ruleIds: string[] = [targetRuleId]) => { + const datafile = getDecisionTestDatafile(); + (datafile as any).holdouts = [ + { + id: 'local_holdout_id', + key: 'local_holdout', + status: 'Running', + includedFlags: [], + excludedFlags: [], + includedRules: ruleIds, + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'local_holdout_variation_id', + key: 'local_holdout_variation', + variables: [] + } + ], + trafficAllocation: [ + { entityId: 'local_holdout_variation_id', endOfRange: 10000 } + ] + } + ]; + return datafile; + }; + + beforeEach(() => { + mockBucket.mockReset(); + }); + + it('global holdout branch: global holdout is evaluated before per-rule logic', async () => { + const datafile = getDecisionTestDatafile(); + (datafile as any).holdouts = [ + { + id: 'global_holdout_id', + key: 'global_holdout', + status: 'Running', + includedFlags: [], + excludedFlags: [], + // No includedRules → global holdout + audienceIds: [], + audienceConditions: [], + variations: [ + { id: 'global_holdout_var_id', key: 'global_holdout_var', variables: [] } + ], + trafficAllocation: [{ entityId: 'global_holdout_var_id', endOfRange: 10000 }] + } + ]; + const config = createProjectConfig(datafile); + const { decisionService } = getDecisionService(); + + // bucket returns the global holdout variation for the holdout, nothing for experiments + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'global_holdout_id') { + return { result: 'global_holdout_var_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1' }); + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Decision should be from the global holdout, not from any experiment + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT); + expect(value[0].result.experiment?.id).toBe('global_holdout_id'); + }); + + it('local holdout hit branch: user bucketed into local holdout for experiment rule returns holdout variation; audience and traffic not evaluated for that rule', async () => { + // exp_1 has id '2001' + const config = createProjectConfig(makeLocalHoldoutDatafile('2001')); + const { decisionService } = getDecisionService(); + + // bucket returns holdout variation when evaluating the local holdout + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1' }); + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Should return holdout decision for the local holdout + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT); + expect(value[0].result.experiment?.id).toBe('local_holdout_id'); + expect(value[0].result.variation?.id).toBe('local_holdout_variation_id'); + }); + + it('local holdout miss branch: user not bucketed into local holdout falls through to regular rule evaluation', async () => { + // exp_1 has id '2001' and audience 4001 (age <= 22) + const config = createProjectConfig(makeLocalHoldoutDatafile('2001')); + const { decisionService } = getDecisionService(); + + // bucket returns null for the local holdout, then succeeds for the experiment + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: null, reasons: [] }; + } + if (params.experimentId === '2001') { + return { result: '5001', reasons: [] }; // variation_1 in exp_1 + } + return { result: null, reasons: [] }; + }); + + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'user1', + attributes: { age: 15 }, // satisfies 4001 audience (age <= 22) + }); + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Should fall through to experiment evaluation (not holdout) + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST); + expect(value[0].result.variation?.id).toBe('5001'); + }); + + it('rule specificity: local holdout targeting experiment rule X does not affect experiment rule Y', async () => { + // exp_1 = '2001', exp_2 = '2002'. Local holdout targets only '2002' (exp_2). + // Audience for exp_1: 4001 (age <= 22). User satisfies exp_1 audience but not exp_2. + const config = createProjectConfig(makeLocalHoldoutDatafile('2002')); + const { decisionService } = getDecisionService(); + + // bucket returns holdout variation only for the local holdout when evaluating for '2002', + // and returns experiment variation for '2001' + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + if (params.experimentId === '2001') { + return { result: '5001', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + // User satisfies exp_1 audience (age <= 22) + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'user1', + attributes: { age: 15 }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // exp_1 is evaluated first; local holdout targets '2002' not '2001', so exp_1 is evaluated normally + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST); + expect(value[0].result.experiment?.id).toBe('2001'); + }); + + it('local holdout applies to delivery rules (rollouts) as well as experiment rules', async () => { + // delivery_1 has id '3001' + const config = createProjectConfig(makeLocalHoldoutDatafile('3001')); + const { decisionService } = getDecisionService(); + + // bucket returns null for all experiments and the local holdout variation for delivery rule + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + // No audience attributes → experiments won't match, falls through to rollout + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'user1', + attributes: { age: 15 }, // satisfies 4001 used by delivery_1 + }); + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Should be a holdout decision from the local holdout targeting the delivery rule + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT); + expect(value[0].result.experiment?.id).toBe('local_holdout_id'); + }); + + it('forced decision beats a 100% traffic local holdout: forced decision takes precedence over local holdout', async () => { + // exp_1 has id '2001', key 'exp_1', variation key 'variation_1' (id '5001') + // Local holdout targets '2001' with 100% traffic allocation. + // User also has a forced decision set for exp_1. + // Expected: forced decision wins; decisionSource is FEATURE_TEST, not HOLDOUT. + const config = createProjectConfig(makeLocalHoldoutDatafile('2001')); + const { decisionService } = getDecisionService(); + + // bucket should NOT be called for local_holdout_id because forced decision short-circuits first + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + // returning holdout variation here to prove the test fails if ordering is wrong + return { result: 'local_holdout_variation_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'user1', + attributes: { age: 15 }, + }); + + // Set forced decision for exp_1 → variation_1 + user.setForcedDecision( + { flagKey: 'flag_1', ruleKey: 'exp_1' }, + { variationKey: 'variation_1' } + ); + + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Forced decision must win — source must be FEATURE_TEST, not HOLDOUT + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST); + expect(value[0].result.variation?.key).toBe('variation_1'); + }); + }); }); + diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 4b26f3804..7d2ebf093 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -29,6 +29,8 @@ import { getVariationIdFromExperimentAndVariationKey, getVariationFromId, getVariationKeyFromId, + getGlobalHoldouts, + getHoldoutsForRule, isActive, ProjectConfig, } from '../../project_config/project_config'; @@ -72,10 +74,12 @@ import { } from 'log_message'; import { OptimizelyError } from '../../error/optimizly_error'; import { CmabService } from './cmab/cmab_service'; -import { Maybe, OpType, OpValue } from '../../utils/type'; +import { Maybe, OpType } from '../../utils/type'; import { Value } from '../../utils/promise/operation_value'; import { Platform } from '../../platform_support'; +import { localHoldout } from '../../feature_toggle'; + export const EXPERIMENT_NOT_RUNNING = 'Experiment %s is not running.'; export const RETURNING_STORED_VARIATION = 'Returning previously activated variation "%s" of experiment "%s" for user "%s" from user profile.'; @@ -133,28 +137,48 @@ interface DecisionServiceOptions { cmabService: CmabService; } -interface DeliveryRuleResponse extends DecisionResponse { - skipToEveryoneElse: K; -} -interface UserProfileTracker { - userProfile: ExperimentBucketMap | null; - isProfileUpdated: boolean; -} +export type DecisionReason = [string, ...any[]]; -type VarationKeyWithCmabParams = { - variationKey?: string; +export type VariationIdWithCmabParams = { + variationId? : string; cmabUuid?: string; }; -export type DecisionReason = [string, ...any[]]; -export type VariationResult = DecisionResponse; -export type DecisionResult = DecisionResponse; -type VariationIdWithCmabParams = { - variationId? : string; + +export type VariationKeyWithCmabParams = { + variationKey? : string; cmabUuid?: string; }; + +export type DecisionResult = DecisionResponse; + + +type LocalHoldoutResult = DecisionResponse & { + holdoutDecision: true +} + +type ExperimentEvaluationResult = DecisionResponse & { + holdoutDecision?: false; +} + +export type ExperimentRuleResponse = ExperimentEvaluationResult | LocalHoldoutResult; + +type DeliveryEvaluationResult = DecisionResponse & { + holdoutDecision?: false; + skipToEveryoneElse: boolean; +} + +type DeliveryRuleResponse = DeliveryEvaluationResult | LocalHoldoutResult; + +type VariationKeyResult = DecisionResponse + export type DecideOptionsMap = Partial>; +interface UserProfileTracker { + userProfile: ExperimentBucketMap | null; + isProfileUpdated: boolean; +} + export const CMAB_DUMMY_ENTITY_ID= '$' export const LOGGER_NAME = 'DecisionService'; @@ -212,7 +236,7 @@ export class DecisionService { user: OptimizelyUserContext, decideOptions: DecideOptionsMap, userProfileTracker?: UserProfileTracker, - ): Value { + ): Value { const userId = user.getUserId(); const experimentKey = experiment.key; @@ -300,7 +324,7 @@ export class DecisionService { this.getDecisionForCmabExperiment(op, configObj, experiment, user, decideOptions) : this.getDecisionFromBucketer(op, configObj, experiment, user); - return decisionVariationValue.then((variationResult): Value => { + return decisionVariationValue.then((variationResult): Value => { decideReasons.push(...variationResult.reasons); if (variationResult.error) { return Value.of(op, { @@ -943,11 +967,11 @@ export class DecisionService { }); } - // all global holouts should be evaluated for all flags - // global holdouts are available in configObj.holdouts - const { holdouts } = configObj; + // Global holdouts are evaluated at flag level, before any per-rule logic. + // getGlobalHoldouts() returns holdouts with includedRules == null/undefined. + const globalHoldouts = getGlobalHoldouts(configObj); - for (const holdout of holdouts) { + for (const holdout of globalHoldouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); @@ -1073,14 +1097,22 @@ export class DecisionService { op, configObj, feature, fromIndex + 1, user, decideReasons, decideOptions, userProfileTracker); } - const decisionVariationValue = this.getVariationFromExperimentRule( + const experimentResponse = this.getVariationFromExperimentRule( op, configObj, feature.key, experiment, user, decideOptions, userProfileTracker, ); - return decisionVariationValue.then((decisionVariation) => { - decideReasons.push(...decisionVariation.reasons); + return experimentResponse.then((experimentResponse) => { + decideReasons.push(...experimentResponse.reasons); - if (decisionVariation.error) { + // If a local holdout was hit, return the holdout decision directly (preserves holdout as experiment). + if (experimentResponse.holdoutDecision) { + return Value.of(op, { + result: experimentResponse.result, + reasons: decideReasons, + }); + } + + if (experimentResponse.error) { return Value.of(op, { error: true, result: { @@ -1092,12 +1124,12 @@ export class DecisionService { }); } - if(!decisionVariation.result.variationKey) { + if(!experimentResponse.result.variationKey) { return this.traverseFeatureExperimentList( op, configObj, feature, fromIndex + 1, user, decideReasons, decideOptions, userProfileTracker); } - - const variationKey = decisionVariation.result.variationKey; + + const variationKey = experimentResponse.result.variationKey; let variation: Variation | null = experiment.variationKeyMap[variationKey]; if (!variation) { variation = getFlagVariationByKey(configObj, feature.key, variationKey); @@ -1105,7 +1137,7 @@ export class DecisionService { return Value.of(op, { result: { - cmabUuid: decisionVariation.result.cmabUuid, + cmabUuid: experimentResponse.result.cmabUuid, experiment, variation, decisionSource: DECISION_SOURCES.FEATURE_TEST, @@ -1121,11 +1153,10 @@ export class DecisionService { user: OptimizelyUserContext, ): DecisionResponse { const decideReasons: DecisionReason[] = []; - let decisionObj: DecisionObj; if (!feature.rolloutId) { this.logger?.debug(NO_ROLLOUT_EXISTS, feature.key); decideReasons.push([NO_ROLLOUT_EXISTS, feature.key]); - decisionObj = { + const decisionObj = { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, @@ -1145,7 +1176,7 @@ export class DecisionService { feature.key, ); decideReasons.push([INVALID_ROLLOUT_ID, feature.rolloutId, feature.key]); - decisionObj = { + const decisionObj = { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, @@ -1163,7 +1194,7 @@ export class DecisionService { feature.rolloutId, ); decideReasons.push([ROLLOUT_HAS_NO_EXPERIMENTS, feature.rolloutId]); - decisionObj = { + const decisionObj = { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, @@ -1173,19 +1204,34 @@ export class DecisionService { reasons: decideReasons, }; } - let decisionVariation; - let skipToEveryoneElse; - let variation; - let rolloutRule; + let index = 0; while (index < rolloutRules.length) { - decisionVariation = this.getVariationFromDeliveryRule(configObj, feature.key, rolloutRules, index, user); - decideReasons.push(...decisionVariation.reasons); - variation = decisionVariation.result; - skipToEveryoneElse = decisionVariation.skipToEveryoneElse; + const deliveryRuleResponse = this.getVariationFromDeliveryRule(configObj, feature.key, rolloutRules, index, user); + + decideReasons.push(...deliveryRuleResponse.reasons); + + // If a local holdout was hit for this delivery rule, use its decision object directly. + if (deliveryRuleResponse.holdoutDecision) { + return { + reasons: decideReasons, + result: deliveryRuleResponse.result, + } + } + + + const variation = deliveryRuleResponse.result; + const skipToEveryoneElse = deliveryRuleResponse.skipToEveryoneElse; + if (variation) { - rolloutRule = configObj.experimentIdMap[rolloutRules[index].id]; - decisionObj = { + // if (decisionVariation.localHoldoutDecision) { + // return { + // result: decisionVariation.localHoldoutDecision, + // reasons: decideReasons, + // }; + // } + const rolloutRule = configObj.experimentIdMap[rolloutRules[index].id]; + const decisionObj = { experiment: rolloutRule, variation: variation, decisionSource: DECISION_SOURCES.ROLLOUT @@ -1199,7 +1245,7 @@ export class DecisionService { index = skipToEveryoneElse ? (rolloutRules.length - 1) : (index + 1); } - decisionObj = { + const decisionObj = { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, @@ -1548,7 +1594,7 @@ export class DecisionService { user: OptimizelyUserContext, decideOptions: DecideOptionsMap, userProfileTracker?: UserProfileTracker, - ): Value { + ): Value { const decideReasons: DecisionReason[] = []; // check forced decision first @@ -1562,26 +1608,35 @@ export class DecisionService { reasons: decideReasons, }); } + + if (localHoldout()) { + // Check local holdouts targeting this specific experiment rule. + // Inserted immediately after the forced-decision block, before regular rule evaluation. + const localHoldoutsForExperiment = getHoldoutsForRule(configObj, rule.id); + for (const holdout of localHoldoutsForExperiment) { + const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); + decideReasons.push(...holdoutDecision.reasons); + if (holdoutDecision.result.variation) { + // Signal the caller to use the holdout decision directly, preserving the holdout as experiment. + return Value.of(op, { + result: holdoutDecision.result, + reasons: decideReasons, + holdoutDecision: true, + }); + } + } + } + const decisionVariationValue = this.resolveVariation(op, configObj, rule, user, decideOptions, userProfileTracker); return decisionVariationValue.then((variationResult) => { decideReasons.push(...variationResult.reasons); - return Value.of(op, { + return Value.of(op, { error: variationResult.error, result: variationResult.result, reasons: decideReasons, }); }); - - // return response; - - // decideReasons.push(...decisionVariation.reasons); - // const variationKey = decisionVariation.result; - - // return { - // result: variationKey, - // reasons: decideReasons, - // }; } private getVariationFromDeliveryRule( @@ -1590,7 +1645,7 @@ export class DecisionService { rules: Experiment[], ruleIndex: number, user: OptimizelyUserContext - ): DeliveryRuleResponse { + ): DeliveryRuleResponse { const decideReasons: DecisionReason[] = []; let skipToEveryoneElse = false; @@ -1608,6 +1663,23 @@ export class DecisionService { }; } + if (localHoldout()) { + // Check local holdouts targeting this specific delivery rule (FSSDK-12369). + // Inserted immediately after the forced-decision block, before audience and traffic allocation checks. + const localHoldoutsForDelivery = getHoldoutsForRule(configObj, rule.id); + for (const holdout of localHoldoutsForDelivery) { + const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); + decideReasons.push(...holdoutDecision.reasons); + if (holdoutDecision.result.variation) { + return { + result: holdoutDecision.result, + reasons: decideReasons, + holdoutDecision: true, + }; + } + } + } + const userId = user.getUserId(); const attributes = user.getAttributes(); const bucketingId = this.getBucketingId(userId, attributes); diff --git a/lib/feature_toggle.ts b/lib/feature_toggle.ts index c567c46a3..85fba2a88 100644 --- a/lib/feature_toggle.ts +++ b/lib/feature_toggle.ts @@ -39,4 +39,7 @@ import { Platform } from './platform_support'; export type IfActive boolean, Y, N = unknown> = ReturnType extends true ? Y : N; + +export const localHoldout = () => true as const; + export const __platforms: Platform[] = ['__universal__']; diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 13c086539..7eee6a096 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -30,7 +30,7 @@ import testDatafile from '../tests/test_data'; import configValidator from '../utils/config_validator'; import { FEATURE_VARIABLE_TYPES } from '../utils/enums'; import { keyBy, sprintf } from '../utils/fns'; -import projectConfig, { ProjectConfig } from './project_config'; +import projectConfig, { ProjectConfig, getGlobalHoldouts, getHoldoutsForRule } from './project_config'; const cloneDeep = (obj: any) => JSON.parse(JSON.stringify(obj)); const logger = getMockLogger(); @@ -416,6 +416,153 @@ describe('createProjectConfig - holdouts', () => { }); }); +// Level 1 tests for local holdouts (FSSDK-12369): isGlobal, getGlobalHoldouts, getHoldoutsForRule +describe('createProjectConfig - local holdouts (FSSDK-12369)', () => { + const makeLocalHoldoutsDatafile = () => { + const datafile = testDatafile.getTestDecideProjectConfig(); + // Holdout with no includedRules → global + // Holdout with includedRules array → local + (datafile as any).holdouts = [ + { + id: 'global_holdout_id', + key: 'global_holdout', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [{ id: 'global_var_id', key: 'global_var', variables: [] }], + trafficAllocation: [{ entityId: 'global_var_id', endOfRange: 5000 }], + // no includedRules → isGlobal should be true + }, + { + id: 'local_holdout_rule_a_id', + key: 'local_holdout_rule_a', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + includedRules: ['rule_a', 'rule_b'], + variations: [{ id: 'local_var_id', key: 'local_var', variables: [] }], + trafficAllocation: [{ entityId: 'local_var_id', endOfRange: 5000 }], + }, + { + id: 'empty_local_holdout_id', + key: 'empty_local_holdout', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + includedRules: [], // empty array → local holdout (NOT global) + variations: [{ id: 'empty_var_id', key: 'empty_var', variables: [] }], + trafficAllocation: [{ entityId: 'empty_var_id', endOfRange: 5000 }], + }, + { + id: 'null_included_rules_id', + key: 'null_included_rules', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + includedRules: null, // explicit null → global holdout + variations: [{ id: 'null_var_id', key: 'null_var', variables: [] }], + trafficAllocation: [{ entityId: 'null_var_id', endOfRange: 5000 }], + }, + ]; + return datafile; + }; + + it('should set isGlobal=true for holdout with no includedRules field (backward compat with old datafiles)', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const holdout = config.holdoutIdMap!['global_holdout_id']; + expect(holdout.isGlobal).toBe(true); + }); + + it('should set isGlobal=false for holdout with includedRules array', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const holdout = config.holdoutIdMap!['local_holdout_rule_a_id']; + expect(holdout.isGlobal).toBe(false); + }); + + it('should set isGlobal=false for holdout with empty includedRules array (empty array is still local)', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const holdout = config.holdoutIdMap!['empty_local_holdout_id']; + expect(holdout.isGlobal).toBe(false); + }); + + it('should set isGlobal=true for holdout with explicit null includedRules', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const holdout = config.holdoutIdMap!['null_included_rules_id']; + expect(holdout.isGlobal).toBe(true); + }); + + it('getGlobalHoldouts should return only holdouts with isGlobal=true', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const globals = getGlobalHoldouts(config); + const globalIds = globals.map(h => h.id); + expect(globalIds).toContain('global_holdout_id'); + expect(globalIds).toContain('null_included_rules_id'); + expect(globalIds).not.toContain('local_holdout_rule_a_id'); + expect(globalIds).not.toContain('empty_local_holdout_id'); + }); + + it('getHoldoutsForRule should return local holdouts targeting the given rule ID', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const forRuleA = getHoldoutsForRule(config, 'rule_a'); + expect(forRuleA).toHaveLength(1); + expect(forRuleA[0].id).toBe('local_holdout_rule_a_id'); + + const forRuleB = getHoldoutsForRule(config, 'rule_b'); + expect(forRuleB).toHaveLength(1); + expect(forRuleB[0].id).toBe('local_holdout_rule_a_id'); + }); + + it('getHoldoutsForRule should return empty array for an unknown rule ID', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const forUnknown = getHoldoutsForRule(config, 'nonexistent_rule'); + expect(forUnknown).toHaveLength(0); + }); + + it('getGlobalHoldouts should return empty array when all holdouts are local', () => { + const datafile = cloneDeep(makeLocalHoldoutsDatafile()); + (datafile as any).holdouts = [ + { + id: 'only_local_id', + key: 'only_local', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + includedRules: ['some_rule'], + variations: [{ id: 'only_local_var_id', key: 'only_local_var', variables: [] }], + trafficAllocation: [{ entityId: 'only_local_var_id', endOfRange: 5000 }], + }, + ]; + const config = projectConfig.createProjectConfig(datafile as any); + expect(getGlobalHoldouts(config)).toHaveLength(0); + }); + + it('should handle datafile with no holdouts gracefully', () => { + const datafile = testDatafile.getTestProjectConfig(); + const config = projectConfig.createProjectConfig(datafile as any); + expect(getGlobalHoldouts(config)).toHaveLength(0); + expect(getHoldoutsForRule(config, 'any_rule')).toHaveLength(0); + }); + + it('a single local holdout targeting multiple rules should appear for each targeted rule', () => { + const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any); + const forRuleA = getHoldoutsForRule(config, 'rule_a'); + const forRuleB = getHoldoutsForRule(config, 'rule_b'); + // Both rule_a and rule_b point to the same holdout + expect(forRuleA[0].id).toBe('local_holdout_rule_a_id'); + expect(forRuleB[0].id).toBe('local_holdout_rule_a_id'); + }); +}); + describe('getExperimentId', () => { let testData: Record; let configObj: ProjectConfig; diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index 9f966549b..9401e8a66 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -113,6 +113,18 @@ export interface ProjectConfig { odpIntegrationConfig: OdpIntegrationConfig; holdouts: Holdout[]; holdoutIdMap?: { [id: string]: Holdout }; + holdoutConfig?: HoldoutConfig; +} + +/** + * Holds pre-computed holdout lookup structures built during config parsing. + * Stored as plain data (no methods) to be serializable and equality-safe. + */ +export interface HoldoutConfig { + /** All holdouts whose includedRules is null or undefined (global holdouts). */ + global: Holdout[]; + /** Maps a rule ID to the local holdouts that target it. */ + ruleHoldoutsMap: { [ruleId: string]: Holdout[] }; } const EXPERIMENT_RUNNING_STATUS = 'Running'; @@ -387,6 +399,9 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { projectConfig.holdouts = projectConfig.holdouts || []; projectConfig.holdoutIdMap = keyBy(projectConfig.holdouts, 'id'); + const global: Holdout[] = []; + const ruleHoldoutsMap: { [ruleId: string]: Holdout[] } = {}; + projectConfig.holdouts.forEach((holdout) => { // Original design of holdouts made use of the includeFlags and excludeFlags fields to identify local holdouts. @@ -396,9 +411,45 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { holdout.excludedFlags = []; holdout.variationKeyMap = keyBy(holdout.variations, 'key'); assignBy(holdout.variations, 'id', projectConfig.variationIdMap); + + // Compute isGlobal: null/undefined includedRules means global holdout. + // An empty array ([]) means local holdout targeting no rules — still NOT global. + holdout.isGlobal = holdout.includedRules === null || holdout.includedRules === undefined; + + if (holdout.isGlobal) { + global.push(holdout); + } else { + // Local holdout: register under each rule ID it targets. + for (const ruleId of holdout.includedRules!) { + if (!ruleHoldoutsMap[ruleId]) { + ruleHoldoutsMap[ruleId] = []; + } + ruleHoldoutsMap[ruleId].push(holdout); + } + } }); + + projectConfig.holdoutConfig = { + global, + ruleHoldoutsMap, + }; } +/** + * Returns all global holdouts from the holdout config. + * Global holdouts have includedRules === null or undefined. + */ +export const getGlobalHoldouts = (projectConfig: ProjectConfig): Holdout[] => { + return projectConfig.holdoutConfig?.global ?? []; +}; + +/** + * Returns local holdouts targeting the given rule ID, or an empty array. + */ +export const getHoldoutsForRule = (projectConfig: ProjectConfig, ruleId: string): Holdout[] => { + return projectConfig.holdoutConfig?.ruleHoldoutsMap[ruleId] ?? []; +}; + /** * Extract all audience segments used in this audience's conditions * @param {Audience} audience Object representing the audience being parsed diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 5fc10498b..f82aa0bce 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -176,6 +176,19 @@ export interface Holdout extends ExperimentCore { status: HoldoutStatus; includedFlags: string[]; excludedFlags: string[]; + /** + * When null or undefined, this is a global holdout (applies to all rules across all flags). + * When an array of rule ID strings, this is a local holdout (applies only to those rules). + * An empty array means a local holdout with no matching rules (still local, not global). + * This field may be absent in old datafiles — treated as null (global). + */ + includedRules?: string[] | null; + /** + * True if this is a global holdout (includedRules is null or undefined). + * False if this is a local holdout targeting specific rules. + * Computed and set during config parsing in parseHoldoutsConfig. + */ + isGlobal: boolean; } export function isHoldout(obj: Experiment | Holdout): obj is Holdout {