Skip to content

[AI-FSSDK] [FSSDK-12760] add localHoldouts to datafile for backward compatibility #1157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jaeopt wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[AI-FSSDK] [FSSDK-12760] add localHoldouts to datafile for backward compatibility #1157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion lib/core/decision_service/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2882,9 +2882,12 @@ describe('DecisionService', () => {

describe('local holdouts', () => {
// Helper: build a datafile that has a local holdout targeting a specific experiment or delivery rule.
// Per FSSDK-12760, local holdouts live in the top-level `localHoldouts` section
// (separate from `holdouts`, which now only carries global holdouts).
const makeLocalHoldoutDatafile = (targetRuleId: string, ruleIds: string[] = [targetRuleId]) => {
const datafile = getDecisionTestDatafile();
(datafile as any).holdouts = [
(datafile as any).holdouts = [];
(datafile as any).localHoldouts = [
{
id: 'local_holdout_id',
key: 'local_holdout',
Expand Down
234 changes: 178 additions & 56 deletions lib/project_config/project_config.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,12 +416,12 @@ describe('createProjectConfig - holdouts', () => {
});
});

// Level 1 tests for local holdouts (FSSDK-12369): isGlobal, getGlobalHoldouts, getHoldoutsForRule
describe('createProjectConfig - local holdouts (FSSDK-12369)', () => {
const makeLocalHoldoutsDatafile = () => {
// Level 1 tests for local holdouts (FSSDK-12369, FSSDK-12760).
// Section membership (`holdouts` vs `localHoldouts`) is the sole signal for scope.
describe('createProjectConfig - local holdouts (FSSDK-12369, FSSDK-12760)', () => {
const makeHoldoutsDatafile = () => {
const datafile = testDatafile.getTestDecideProjectConfig();
// Holdout with no includedRules → global
// Holdout with includedRules array → local
// Entries in `holdouts` are global by section membership.
(datafile as any).holdouts = [
{
id: 'global_holdout_id',
Expand All @@ -433,8 +433,10 @@ describe('createProjectConfig - local holdouts (FSSDK-12369)', () => {
audienceConditions: [],
variations: [{ id: 'global_var_id', key: 'global_var', variables: [] }],
trafficAllocation: [{ entityId: 'global_var_id', endOfRange: 5000 }],
// no includedRules → isGlobal should be true
},
];
// Entries in `localHoldouts` are local; scoped via includedRules.
(datafile as any).localHoldouts = [
{
id: 'local_holdout_rule_a_id',
key: 'local_holdout_rule_a',
Expand All @@ -447,70 +449,32 @@ describe('createProjectConfig - local holdouts (FSSDK-12369)', () => {
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);
it('should set isGlobal=true for entries in the holdouts section (backward compat with old datafiles)', () => {
const config = projectConfig.createProjectConfig(cloneDeep(makeHoldoutsDatafile()) 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);
it('should set isGlobal=false for entries in the localHoldouts section', () => {
const config = projectConfig.createProjectConfig(cloneDeep(makeHoldoutsDatafile()) 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);
it('getGlobalHoldouts should return only entries from the holdouts section', () => {
const config = projectConfig.createProjectConfig(cloneDeep(makeHoldoutsDatafile()) 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 config = projectConfig.createProjectConfig(cloneDeep(makeHoldoutsDatafile()) as any);
const forRuleA = getHoldoutsForRule(config, 'rule_a');
expect(forRuleA).toHaveLength(1);
expect(forRuleA[0].id).toBe('local_holdout_rule_a_id');
Expand All @@ -521,14 +485,15 @@ describe('createProjectConfig - local holdouts (FSSDK-12369)', () => {
});

it('getHoldoutsForRule should return empty array for an unknown rule ID', () => {
const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any);
const config = projectConfig.createProjectConfig(cloneDeep(makeHoldoutsDatafile()) 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 = [
it('getGlobalHoldouts should return empty array when only the localHoldouts section is populated', () => {
const datafile = cloneDeep(makeHoldoutsDatafile());
(datafile as any).holdouts = [];
(datafile as any).localHoldouts = [
{
id: 'only_local_id',
key: 'only_local',
Expand All @@ -554,7 +519,7 @@ describe('createProjectConfig - local holdouts (FSSDK-12369)', () => {
});

it('a single local holdout targeting multiple rules should appear for each targeted rule', () => {
const config = projectConfig.createProjectConfig(cloneDeep(makeLocalHoldoutsDatafile()) as any);
const config = projectConfig.createProjectConfig(cloneDeep(makeHoldoutsDatafile()) 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
Expand All @@ -563,6 +528,163 @@ describe('createProjectConfig - local holdouts (FSSDK-12369)', () => {
});
});

// Level 1 tests for the FSSDK-12760 backward-compatible localHoldouts section design.
// Older SDKs (Gen 1/Gen 2) ignore the unknown `localHoldouts` top-level key entirely.
// Gen 3 SDKs (this one) treat section membership as the sole signal for scope.
describe('createProjectConfig - localHoldouts section (FSSDK-12760)', () => {
const makeBaseDatafile = () => testDatafile.getTestDecideProjectConfig();

const makeLocal = (id: string, key: string, includedRules: any) => ({
id,
key,
status: 'Running',
includedFlags: [],
excludedFlags: [],
audienceIds: [],
audienceConditions: [],
includedRules,
variations: [{ id: `${id}_var`, key: 'holdout', variables: [] }],
trafficAllocation: [{ entityId: `${id}_var`, endOfRange: 10000 }],
});

const makeGlobal = (id: string, key: string, extra: Record<string, any> = {}) => ({
id,
key,
status: 'Running',
includedFlags: [],
excludedFlags: [],
audienceIds: [],
audienceConditions: [],
variations: [{ id: `${id}_var`, key: 'holdout', variables: [] }],
trafficAllocation: [{ entityId: `${id}_var`, endOfRange: 10000 }],
...extra,
});

it('exposes localHoldouts as a top-level array on the project config', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [];
datafile.localHoldouts = [makeLocal('l1', 'local_h', ['rule_x'])];
const config = projectConfig.createProjectConfig(datafile);
expect(Array.isArray(config.localHoldouts)).toBe(true);
expect(config.localHoldouts).toHaveLength(1);
expect(config.localHoldouts[0].id).toBe('l1');
});

it('defaults localHoldouts to an empty array when the section is absent (backward compat)', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [makeGlobal('g1', 'global_h')];
// No localHoldouts key at all
const config = projectConfig.createProjectConfig(datafile);
expect(config.localHoldouts).toEqual([]);
expect(getGlobalHoldouts(config).map(h => h.id)).toContain('g1');
expect(getHoldoutsForRule(config, 'any_rule')).toEqual([]);
});

it('ignores includedRules on entries in the global holdouts section', () => {
// An includedRules field on a `holdouts` entry must NOT narrow its scope —
// section membership is the sole signal for scope.
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [
makeGlobal('stray', 'stray_global', { includedRules: ['rule_should_be_ignored'] }),
];
datafile.localHoldouts = [];
const config = projectConfig.createProjectConfig(datafile);

const stray = config.holdoutIdMap!['stray'];
// includedRules must be stripped at parse time
expect(stray.includedRules).toBeUndefined();
// Entity is global
expect(stray.isGlobal).toBe(true);
// It appears in global list and NOT in the rule map
expect(getGlobalHoldouts(config).map(h => h.id)).toContain('stray');
expect(getHoldoutsForRule(config, 'rule_should_be_ignored')).toEqual([]);
});

it('does not mutate the caller-provided datafile when stripping includedRules', () => {
// Regression guard: parsing a datafile must not bleed mutations into the
// caller's reference (some hosts re-use the same datafile object).
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [
makeGlobal('stray', 'stray_global', { includedRules: ['rule_x'] }),
];
projectConfig.createProjectConfig(datafile);
// Original datafile still has includedRules on the entry
expect(datafile.holdouts[0].includedRules).toEqual(['rule_x']);
});

it('partitions both sections correctly when both are present', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [makeGlobal('g1', 'g'), makeGlobal('g2', 'g2')];
datafile.localHoldouts = [
makeLocal('l1', 'l1', ['rule_a']),
makeLocal('l2', 'l2', ['rule_b']),
];
const config = projectConfig.createProjectConfig(datafile);

expect(getGlobalHoldouts(config).map(h => h.id).sort()).toEqual(['g1', 'g2']);
expect(getHoldoutsForRule(config, 'rule_a').map(h => h.id)).toEqual(['l1']);
expect(getHoldoutsForRule(config, 'rule_b').map(h => h.id)).toEqual(['l2']);
// ID map covers both sections
expect(config.holdoutIdMap!['g1']).toBeDefined();
expect(config.holdoutIdMap!['l1']).toBeDefined();
});

it('logs an error and excludes localHoldouts entries with no includedRules', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [];
// Invalid: no includedRules at all
const invalid: any = makeLocal('bad', 'invalid_local', undefined);
delete invalid.includedRules;
datafile.localHoldouts = [invalid];

const config = projectConfig.createProjectConfig(datafile, null, logger);

expect(getGlobalHoldouts(config)).toEqual([]);
expect(getHoldoutsForRule(config, 'any_rule')).toEqual([]);
expect(config.holdoutIdMap!['bad']).toBeUndefined();
expect(logger.error).toHaveBeenCalledWith(
expect.stringMatching(/invalid_local.*includedRules/i)
);
});

it('logs an error and excludes localHoldouts entries with null includedRules', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [];
datafile.localHoldouts = [makeLocal('bad_null', 'null_local', null)];

const config = projectConfig.createProjectConfig(datafile, null, logger);

expect(getHoldoutsForRule(config, 'any_rule')).toEqual([]);
expect(config.holdoutIdMap!['bad_null']).toBeUndefined();
expect(logger.error).toHaveBeenCalledWith(
expect.stringMatching(/null_local.*includedRules/i)
);
});

it('logs an error and excludes localHoldouts entries with empty includedRules', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [];
datafile.localHoldouts = [makeLocal('bad_empty', 'empty_local', [])];

const config = projectConfig.createProjectConfig(datafile, null, logger);

expect(getHoldoutsForRule(config, 'any_rule')).toEqual([]);
expect(config.holdoutIdMap!['bad_empty']).toBeUndefined();
expect(logger.error).toHaveBeenCalledWith(
expect.stringMatching(/empty_local.*includedRules/i)
);
});

it('keeps holdout variations in the variationIdMap from both sections', () => {
const datafile = cloneDeep(makeBaseDatafile()) as any;
datafile.holdouts = [makeGlobal('g1', 'g')];
datafile.localHoldouts = [makeLocal('l1', 'l', ['rule_x'])];
const config = projectConfig.createProjectConfig(datafile);
expect(config.variationIdMap['g1_var']).toBeDefined();
expect(config.variationIdMap['l1_var']).toBeDefined();
});
});

describe('getExperimentId', () => {
let testData: Record<string, any>;
let configObj: ProjectConfig;
Expand Down
Loading
Loading