diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 2334a386b..77e942e0a 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -19,7 +19,7 @@ import { USER_HAS_NO_FORCED_VARIATION } from 'log_message'; import { beforeEach, describe, expect, it, MockInstance, vi } from 'vitest'; -import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.'; +import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService, TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT } from '.'; import OptimizelyUserContext from '../../optimizely_user_context'; import { createProjectConfig, ProjectConfig } from '../../project_config/project_config'; import { BucketerParams, Experiment, ExperimentBucketMap, Holdout, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types'; @@ -2196,6 +2196,170 @@ describe('DecisionService', () => { decisionSource: DECISION_SOURCES.HOLDOUT, }); }); + + describe('excludeTargetedDeliveries', () => { + const getExcludeTDDatafile = (excludeTargetedDeliveries?: boolean) => { + const datafile = getHoldoutTestDatafile(); + datafile.holdouts = datafile.holdouts.map((holdout: any) => { + if (holdout.id === 'holdout_running_id') { + return { + ...holdout, + ...(excludeTargetedDeliveries !== undefined ? { excludeTargetedDeliveries } : {}), + }; + } + return holdout; + }); + return datafile; + }; + + it('should return holdout variation immediately when excludeTargetedDeliveries is false', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(false))); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should return holdout variation immediately when excludeTargetedDeliveries is undefined', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(undefined))); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should return delivery variation with holdout info when excludeTargetedDeliveries is true and delivery matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'delivery_1') { + return { result: '5004', reasons: [] }; + } + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['delivery_1'], + variation: config.variationIdMap['5004'], + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }, + }); + + const reasonStrings = variation.reasons.map((r: any) => typeof r === 'string' ? r : r[0]); + expect(reasonStrings).toContain(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT); + }); + + it('should return null decision with holdout info when excludeTargetedDeliveries is true but no delivery matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }, + }); + }); + + it('should still block experiment rules when excludeTargetedDeliveries is true', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + if (param.experimentKey === 'exp_1') { + return { result: '5001', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }, + }); + }); + }); }); }); @@ -3101,6 +3265,34 @@ describe('DecisionService', () => { expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST); expect(value[0].result.variation?.key).toBe('variation_1'); }); + + it('local holdout ignores excludeTargetedDeliveries and applies normally', async () => { + const datafile = makeLocalHoldoutDatafile('2001'); + (datafile as any).localHoldouts[0].excludeTargetedDeliveries = true; + const config = createProjectConfig(JSON.stringify(datafile)); + const { decisionService } = getDecisionService(); + + 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', + attributes: { age: 15 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Local holdout applies normally even with excludeTargetedDeliveries set + 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'); + }); }); }); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 2064ec2a2..65ff3e8a6 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -119,12 +119,17 @@ export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for h export const USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT = 'User %s does not meet conditions for holdout %s.'; export const USER_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in variation %s of holdout %s.'; export const USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in no holdout variation.'; +export const TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT = "Holdout '%s' has excludeTargetedDeliveries enabled, continuing to rollout evaluation."; export interface DecisionObj { experiment: Experiment | Holdout | null; variation: Variation | null; decisionSource: DecisionSource; cmabUuid?: string; + holdout?: { + experiment: Holdout; + variation: Variation; + }; } interface DecisionServiceOptions { @@ -969,11 +974,23 @@ export class DecisionService { // getGlobalHoldouts() returns holdouts with includedRules == null/undefined. const globalHoldouts = getGlobalHoldouts(configObj); + let activeHoldoutDecision: DecisionObj | null = null; + let activeHoldout: Holdout | null = null; + for (const holdout of globalHoldouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); if (holdoutDecision.result.variation) { + if (holdout.excludeTargetedDeliveries) { + const userId = user.getUserId(); + this.logger?.info(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, holdout.key); + decideReasons.push([TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, holdout.key]); + activeHoldoutDecision = holdoutDecision.result; + activeHoldout = holdout; + break; + } + return Value.of(op, { result: holdoutDecision.result, reasons: decideReasons, @@ -981,33 +998,72 @@ export class DecisionService { } } - return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { - if (experimentDecision.error || experimentDecision.result.variation !== null) { + if (!activeHoldoutDecision) { + return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { + if (experimentDecision.error || experimentDecision.result.variation !== null) { + return Value.of(op, { + ...experimentDecision, + reasons: [...decideReasons, ...experimentDecision.reasons], + }); + } + + decideReasons.push(...experimentDecision.reasons); + + const rolloutDecision = this.getVariationForRollout(configObj, feature, user); + decideReasons.push(...rolloutDecision.reasons); + const rolloutDecisionResult = rolloutDecision.result; + const userId = user.getUserId(); + + if (rolloutDecisionResult.variation) { + this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); + } else { + this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); + } + return Value.of(op, { - ...experimentDecision, - reasons: [...decideReasons, ...experimentDecision.reasons], + result: rolloutDecisionResult, + reasons: decideReasons, }); - } + }); + } - decideReasons.push(...experimentDecision.reasons); - - const rolloutDecision = this.getVariationForRollout(configObj, feature, user); - decideReasons.push(...rolloutDecision.reasons); - const rolloutDecisionResult = rolloutDecision.result; - const userId = user.getUserId(); - - if (rolloutDecisionResult.variation) { - this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); - decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); - } else { - this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); - decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); - } - + // User is in holdout with excludeTargetedDeliveries — skip experiments, evaluate delivery rules + const rolloutDecision = this.getVariationForRollout(configObj, feature, user); + decideReasons.push(...rolloutDecision.reasons); + const rolloutDecisionResult = rolloutDecision.result; + const userId = user.getUserId(); + + if (rolloutDecisionResult.variation) { + this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); return Value.of(op, { - result: rolloutDecisionResult, + result: { + ...rolloutDecisionResult, + holdout: { + experiment: activeHoldout!, + variation: activeHoldoutDecision!.variation!, + }, + }, reasons: decideReasons, }); + } + + this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); + + return Value.of(op, { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: activeHoldout!, + variation: activeHoldoutDecision!.variation!, + }, + }, + reasons: decideReasons, }); } diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index d891e283a..07f31dcc1 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -872,6 +872,66 @@ describe('Optimizely', () => { }), }); }); + + it('should dispatch separate holdout impression when decisionObj.holdout is populated', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); + const notificationSpyLocal = vi.fn(); + optimizely.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + + const holdoutExperiment = projectConfig.holdouts[0]; + const holdoutVariation = projectConfig.holdouts[0].variations[0]; + const rolloutExperiment = projectConfig.experimentKeyMap['delivery_1'] || Object.values(projectConfig.experimentIdMap)[0]; + const rolloutVariation = rolloutExperiment?.variations?.[0] || { id: 'test_var_id', key: 'test_var_key', variables: [] }; + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: rolloutVariation, + experiment: rolloutExperiment, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: holdoutExperiment, + variation: holdoutVariation, + }, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + await optimizely.decideAsync(user, 'flag_1', []); + + // The holdout impression is dispatched even when the main rollout impression is not + // (sendFlagDecisions not set). Verify at least one impression is a holdout event. + expect(processSpy).toHaveBeenCalled(); + + const holdoutEvent = processSpy.mock.calls.find( + (call: any) => (call[0] as ImpressionEvent).ruleType === 'holdout' + ); + expect(holdoutEvent).toBeDefined(); + const holdoutImpression = holdoutEvent![0] as ImpressionEvent; + expect(holdoutImpression.ruleKey).toBe('holdout_test_key'); + expect(holdoutImpression.ruleType).toBe('holdout'); + expect(holdoutImpression.enabled).toBe(false); + + expect(notificationSpyLocal).toHaveBeenCalledWith( + expect.objectContaining({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: true, + }), + }) + ); + }); }); it('should flush eventProcessor and odpManager on flushImmediately()', async () => { diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 251c08b0e..04442e1aa 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -1542,6 +1542,16 @@ export default class Optimizely extends BaseService implements Client { decisionEventDispatched = true; } + if (decisionObj.holdout && !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT]) { + const holdoutDecisionObj: DecisionObj = { + experiment: decisionObj.holdout.experiment, + variation: decisionObj.holdout.variation, + decisionSource: DECISION_SOURCES.HOLDOUT, + }; + this.sendImpressionEvent(holdoutDecisionObj, key, userId, false, attributes); + decisionEventDispatched = true; + } + const shouldIncludeReasons = options[OptimizelyDecideOption.INCLUDE_REASONS]; let reportedReasons: string[] = []; diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 6640622ab..4cc772019 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -190,6 +190,7 @@ export interface Holdout extends ExperimentCore { * membership. */ isGlobal: boolean; + excludeTargetedDeliveries?: boolean; } export function isHoldout(obj: Experiment | Holdout): obj is Holdout {