Skip to content
Open
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: 38 additions & 1 deletion lib/optimizely/config/datafile_project_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ class DatafileProjectConfig < ProjectConfig
:group_id_map, :rollout_id_map, :rollout_experiment_id_map, :variation_id_map,
:variation_id_to_variable_usage_map, :variation_key_map, :variation_id_map_by_experiment_id,
:variation_key_map_by_experiment_id, :flag_variation_map, :integration_key_map, :integrations,
:public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map
:public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map,
:global_holdouts, :rule_holdouts_map
# Boolean - denotes if Optimizely should remove the last block of visitors' IP address before storing event data
attr_reader :anonymize_ip

Expand Down Expand Up @@ -114,6 +115,8 @@ def initialize(datafile, logger, error_handler)
@variation_id_to_experiment_map = {}
@flag_variation_map = {}
@holdout_id_map = {}
@global_holdouts = []
@rule_holdouts_map = {}

@holdouts.each do |holdout|
next unless holdout['status'] == 'Running'
Expand All @@ -122,6 +125,19 @@ def initialize(datafile, logger, error_handler)
holdout['layerId'] ||= ''

@holdout_id_map[holdout['id']] = holdout

# Build global vs local holdout mappings
# A holdout is global when includedRules is nil/absent (applies to all rules)
# A holdout is local when includedRules is a non-nil array (applies only to specified rules)
if holdout_global?(holdout)
@global_holdouts << holdout
else
included_rules = holdout['includedRules'] || []
included_rules.each do |rule_id|
@rule_holdouts_map[rule_id] ||= []
@rule_holdouts_map[rule_id] << holdout
end
end
end

@experiment_id_map.each_value do |exp|
Expand Down Expand Up @@ -642,6 +658,27 @@ def get_holdout(holdout_id)
nil
end

def get_holdouts_for_rule(rule_id)
# Returns running local holdouts that target a specific rule ID.
# Local holdouts apply only to the rules listed in their includedRules array.
#
# rule_id - String ID of the experiment/delivery rule
#
# Returns Array of holdout hashes targeting the rule (empty array if none)
@rule_holdouts_map[rule_id] || []
end

def holdout_global?(holdout)
# Determines whether a holdout is global (applies to all rules) or local (applies to specific rules).
# A holdout is global when includedRules is nil or absent from the datafile.
# A holdout with an empty array [] is a local holdout with no matching rules (NOT global).
#
# holdout - Holdout hash from the datafile
#
# Returns true if the holdout is global, false if local
!holdout.key?('includedRules') || holdout['includedRules'].nil?
end

private

def get_everyone_else_variation(feature_flag)
Expand Down
34 changes: 31 additions & 3 deletions lib/optimizely/decision_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide
# user_context - Optimizely user context instance
#
# Returns DecisionResult struct.
# Get running holdouts from the holdout_id_map (all holdouts are global now)
# Check for any running holdouts (global or local)
running_holdouts = project_config.holdout_id_map.values

if running_holdouts && !running_holdouts.empty?
Expand All @@ -196,8 +196,8 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
reasons = decide_reasons ? decide_reasons.dup : []
user_id = user_context.user_id

# Check holdouts (all holdouts are global now - apply to all flags)
holdouts = project_config.holdout_id_map.values
# Check global holdouts first (flag level) — these apply to all rules across all flags
holdouts = project_config.global_holdouts

holdouts.each do |holdout|
holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
Expand Down Expand Up @@ -441,11 +441,25 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use
# Returns variation_id and reasons
reasons = []

# Step 1: Forced decision check — existing logic, evaluated per rule
context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(flag_key, rule['key'])
variation, forced_reasons = validated_forced_decision(project_config, context, user)
reasons.push(*forced_reasons)
return VariationResult.new(nil, false, reasons, variation['id']) if variation

# Step 2: Local holdout check — check holdouts targeting this specific rule (FSSDK-12369)
# Local holdouts are evaluated per-rule, after forced decisions, before audience/traffic checks.
local_holdouts = project_config.get_holdouts_for_rule(rule['id'])
local_holdouts.each do |holdout|
holdout_decision = get_variation_for_holdout(holdout, user, project_config)
reasons.push(*holdout_decision.reasons)
next unless holdout_decision.decision

holdout_variation = holdout_decision.decision.variation
return VariationResult.new(nil, false, reasons, holdout_variation['id'])
end

# Step 3: Regular rule evaluation — existing logic
variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options)
variation_result.reasons = reasons + variation_result.reasons
variation_result
Expand All @@ -464,12 +478,26 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index
reasons = []
skip_to_everyone_else = false
rule = rules[rule_index]

# Step 1: Forced decision check — existing logic, evaluated per rule
context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(flag_key, rule['key'])
variation, forced_reasons = validated_forced_decision(project_config, context, user_context)
reasons.push(*forced_reasons)

return [variation, skip_to_everyone_else, reasons] if variation

# Step 2: Local holdout check — check holdouts targeting this specific delivery rule (FSSDK-12369)
# Local holdouts are evaluated per-rule, after forced decisions, before audience/traffic checks.
local_holdouts = project_config.get_holdouts_for_rule(rule['id'])
local_holdouts.each do |holdout|
holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
reasons.push(*holdout_decision.reasons)
next unless holdout_decision.decision

holdout_variation = holdout_decision.decision.variation
return [holdout_variation, skip_to_everyone_else, reasons]
end

user_id = user_context.user_id
attributes = user_context.user_attributes
bucketing_id, bucketing_id_reasons = get_bucketing_id(user_id, attributes)
Expand Down
3 changes: 3 additions & 0 deletions lib/optimizely/helpers/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,9 @@ module Constants
},
'status' => {
'type' => 'string'
},
'includedRules' => {
'type' => %w[array null]
}
}
}
Expand Down
159 changes: 159 additions & 0 deletions spec/config/datafile_project_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1935,4 +1935,163 @@ def build_datafile(experiments: [], rollouts: [], feature_flags: [])
expect(experiment['trafficAllocation'].length).to eq(1)
end
end

# Level 1 — Local Holdouts config/parsing tests (FSSDK-12369)
describe 'local holdouts data model and config parsing' do
let(:config_with_local_holdouts) do
Optimizely::DatafileProjectConfig.new(
OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON,
logger,
error_handler
)
end

describe '#holdout_global?' do
it 'returns true for holdout with no includedRules key (old datafile format)' do
global_holdout = config_with_local_holdouts.get_holdout('holdout_1')
expect(global_holdout).not_to be_nil
expect(global_holdout.key?('includedRules')).to be false
expect(config_with_local_holdouts.holdout_global?(global_holdout)).to be true
end

it 'returns true for holdout with explicit nil includedRules' do
holdout = {'id' => 'test', 'key' => 'test', 'includedRules' => nil}
expect(config_with_local_holdouts.holdout_global?(holdout)).to be true
end

it 'returns false for holdout with non-nil includedRules array (local holdout)' do
local_holdout = config_with_local_holdouts.get_holdout('holdout_local_1')
expect(local_holdout).not_to be_nil
expect(config_with_local_holdouts.holdout_global?(local_holdout)).to be false
end

it 'returns false for holdout with empty includedRules array (local holdout with no matching rules)' do
local_holdout_empty = config_with_local_holdouts.get_holdout('holdout_local_empty_rules')
expect(local_holdout_empty).not_to be_nil
expect(local_holdout_empty['includedRules']).to eq([])
# Empty array is local holdout (not global) — DIFFERENT from nil
expect(config_with_local_holdouts.holdout_global?(local_holdout_empty)).to be false
end
end

describe '#get_global_holdouts' do
it 'returns only holdouts without includedRules (global holdouts)' do
global_holdouts = config_with_local_holdouts.get_global_holdouts

expect(global_holdouts).not_to be_nil
expect(global_holdouts).to be_an(Array)

# All returned holdouts must be global
global_holdouts.each do |holdout|
expect(config_with_local_holdouts.holdout_global?(holdout)).to be true
end
end

it 'does not include local holdouts (those with includedRules array)' do
global_holdouts = config_with_local_holdouts.get_global_holdouts
local_holdout = config_with_local_holdouts.get_holdout('holdout_local_1')

expect(global_holdouts).not_to include(local_holdout)
end

it 'does not include holdouts with empty includedRules array' do
global_holdouts = config_with_local_holdouts.get_global_holdouts
empty_local_holdout = config_with_local_holdouts.get_holdout('holdout_local_empty_rules')

# Empty [] is local, not global — must not appear in global_holdouts
expect(global_holdouts).not_to include(empty_local_holdout)
end

it 'returns empty array when no global holdouts are present' do
config_no_global = Optimizely::DatafileProjectConfig.new(
JSON.dump(
OptimizelySpec::VALID_CONFIG_BODY.merge(
'holdouts' => [
{
'id' => 'only_local',
'key' => 'only_local_holdout',
'status' => 'Running',
'audiences' => [],
'includedRules' => ['some_rule_id'],
'variations' => [{'id' => 'v1', 'key' => 'holdout', 'featureEnabled' => false}],
'trafficAllocation' => [{'entityId' => 'v1', 'endOfRange' => 10_000}]
}
]
)
),
logger,
error_handler
)
expect(config_no_global.get_global_holdouts).to eq([])
end
end

describe '#get_holdouts_for_rule' do
it 'returns local holdouts targeting the specified rule ID' do
rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('122227')
local_holdout = config_with_local_holdouts.get_holdout('holdout_local_1')

expect(rule_holdouts).to include(local_holdout)
end

it 'returns empty array for a rule ID with no targeting holdouts' do
rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('unknown_rule_id_xyz')
expect(rule_holdouts).to eq([])
end

it 'does not return global holdouts (those without includedRules)' do
rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('122227')
global_holdout = config_with_local_holdouts.get_holdout('holdout_1')

# Global holdout must not appear in rule-specific list
expect(rule_holdouts).not_to include(global_holdout)
end

it 'does not return holdouts targeting other rules' do
# holdout_local_2 targets rule 122238, not 122227
holdouts_for_rule_a = config_with_local_holdouts.get_holdouts_for_rule('122227')
local_holdout_2 = config_with_local_holdouts.get_holdout('holdout_local_2')

expect(holdouts_for_rule_a).not_to include(local_holdout_2)
end

it 'returns empty array for holdout with empty includedRules — does not match any rule' do
# holdout_local_empty_rules has includedRules: [] — should not appear for any rule
rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('122227')
empty_rules_holdout = config_with_local_holdouts.get_holdout('holdout_local_empty_rules')

expect(rule_holdouts).not_to include(empty_rules_holdout)
end
end

describe 'backward compatibility — old datafiles without includedRules' do
it 'parses holdouts without includedRules field as global (backward compatible)' do
# Use the config with only global holdouts (no includedRules key)
config_global_only = Optimizely::DatafileProjectConfig.new(
OptimizelySpec::CONFIG_BODY_WITH_GLOBAL_HOLDOUTS_ONLY_JSON,
logger,
error_handler
)

holdout = config_global_only.get_holdout('global_holdout_only_1')
expect(holdout).not_to be_nil
expect(holdout.key?('includedRules')).to be false

# Must be classified as global
expect(config_global_only.holdout_global?(holdout)).to be true
expect(config_global_only.get_global_holdouts).to include(holdout)
expect(config_global_only.get_holdouts_for_rule('any_rule')).to eq([])
end

it 'does not raise errors when processing old datafiles' do
expect do
Optimizely::DatafileProjectConfig.new(
OptimizelySpec::CONFIG_BODY_WITH_GLOBAL_HOLDOUTS_ONLY_JSON,
logger,
error_handler
)
end.not_to raise_error
end
end
end
end
Loading
Loading