From e8f280e2ccd1e96c5bf42ee635a075e09321b4e7 Mon Sep 17 00:00:00 2001 From: muzahidul-opti Date: Tue, 3 Feb 2026 23:19:39 +0600 Subject: [PATCH 1/2] [FSSDK-12265] Add experiments field and mapping to Holdout data model - Added experiments field to holdout hash with default empty array - Added experiment_holdouts_map to build experiment-to-holdout mappings - Added get_holdouts_for_experiment(experiment_id) method - Added local_holdout?(holdout) helper method - Added comprehensive unit tests covering: * Experiments field parsing and defaults * Experiment-to-holdout mapping functionality * local_holdout? helper method * Backward compatibility with legacy datafiles * Edge cases and multiple holdout scenarios - Ensured backward compatibility (experiments field defaults to empty array) - All 1042 tests pass successfully --- .../config/datafile_project_config.rb | 35 ++- spec/config/datafile_project_config_spec.rb | 294 ++++++++++++++++++ 2 files changed, 328 insertions(+), 1 deletion(-) diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index cfb67f24..841a87bc 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/lib/optimizely/config/datafile_project_config.rb @@ -34,7 +34,8 @@ class DatafileProjectConfig < ProjectConfig :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, - :global_holdouts, :included_holdouts, :excluded_holdouts, :flag_holdouts_map + :global_holdouts, :included_holdouts, :excluded_holdouts, :flag_holdouts_map, + :experiment_holdouts_map # Boolean - denotes if Optimizely should remove the last block of visitors' IP address before storing event data attr_reader :anonymize_ip @@ -119,6 +120,7 @@ def initialize(datafile, logger, error_handler) @included_holdouts = {} @excluded_holdouts = {} @flag_holdouts_map = {} + @experiment_holdouts_map = {} @holdouts.each do |holdout| next unless holdout['status'] == 'Running' @@ -126,6 +128,9 @@ def initialize(datafile, logger, error_handler) # Ensure holdout has layerId field (holdouts don't have campaigns) holdout['layerId'] ||= '' + # Ensure experiments field defaults to empty array + holdout['experiments'] ||= [] + @holdout_id_map[holdout['id']] = holdout included_flags = holdout['includedFlags'] || [] @@ -152,6 +157,13 @@ def initialize(datafile, logger, error_handler) @excluded_holdouts[flag_id] << holdout end end + + # Build experiment-to-holdout mapping + experiments = holdout['experiments'] || [] + experiments.each do |experiment_id| + @experiment_holdouts_map[experiment_id] ||= [] + @experiment_holdouts_map[experiment_id] << holdout + end end @experiment_id_map.each_value do |exp| @@ -688,6 +700,27 @@ def get_holdout(holdout_id) nil end + def get_holdouts_for_experiment(experiment_id) + # Returns the holdouts applicable to the given experiment ID + # + # experiment_id - String experiment ID + # + # Returns Array of holdouts targeting this experiment + + @experiment_holdouts_map[experiment_id] || [] + end + + def local_holdout?(holdout) + # Determines if a holdout is local (targets specific experiments) + # + # holdout - Holdout hash + # + # Returns true if holdout has experiments array and is not empty + + experiments = holdout['experiments'] || [] + !experiments.empty? + end + private def generate_feature_variation_map(feature_flags) diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index ea47fe6f..8dbd3954 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -1801,4 +1801,298 @@ end end end + + describe 'holdout experiments field and mapping' do + let(:config_with_experiment_holdouts) do + config_body_with_experiments = config_body.dup + config_body_with_experiments['holdouts'] = [ + { + 'id' => 'holdout_exp_1', + 'key' => 'local_holdout_1', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => ['exp_123', 'exp_456'], + 'variations' => [], + 'trafficAllocation' => [] + }, + { + 'id' => 'holdout_exp_2', + 'key' => 'local_holdout_2', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => ['exp_789'], + 'variations' => [], + 'trafficAllocation' => [] + }, + { + 'id' => 'holdout_global', + 'key' => 'global_holdout', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_with_experiments), + logger, + error_handler + ) + end + + describe '#get_holdouts_for_experiment' do + it 'should return holdouts targeting a specific experiment' do + holdouts = config_with_experiment_holdouts.get_holdouts_for_experiment('exp_123') + expect(holdouts.length).to eq(1) + expect(holdouts.first['id']).to eq('holdout_exp_1') + expect(holdouts.first['key']).to eq('local_holdout_1') + end + + it 'should return multiple holdouts if experiment is targeted by multiple holdouts' do + # Add another holdout targeting exp_123 + config_body_multi = config_body.dup + config_body_multi['holdouts'] = [ + { + 'id' => 'holdout_1', + 'key' => 'local_holdout_1', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => ['exp_shared'], + 'variations' => [], + 'trafficAllocation' => [] + }, + { + 'id' => 'holdout_2', + 'key' => 'local_holdout_2', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => ['exp_shared'], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + config = Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_multi), + logger, + error_handler + ) + + holdouts = config.get_holdouts_for_experiment('exp_shared') + expect(holdouts.length).to eq(2) + expect(holdouts.map { |h| h['id'] }).to contain_exactly('holdout_1', 'holdout_2') + end + + it 'should return empty array for experiment not targeted by any holdout' do + holdouts = config_with_experiment_holdouts.get_holdouts_for_experiment('exp_not_exists') + expect(holdouts).to eq([]) + end + + it 'should not return global holdouts (without experiments field)' do + holdouts = config_with_experiment_holdouts.get_holdouts_for_experiment('exp_random') + expect(holdouts).to eq([]) + + # Verify global holdout exists but is not returned + global_holdout = config_with_experiment_holdouts.get_holdout('holdout_global') + expect(global_holdout).not_to be_nil + expect(global_holdout['experiments']).to eq([]) + end + + it 'should handle holdout targeting multiple experiments' do + holdouts_exp1 = config_with_experiment_holdouts.get_holdouts_for_experiment('exp_123') + holdouts_exp2 = config_with_experiment_holdouts.get_holdouts_for_experiment('exp_456') + + expect(holdouts_exp1.length).to eq(1) + expect(holdouts_exp2.length).to eq(1) + expect(holdouts_exp1.first['id']).to eq('holdout_exp_1') + expect(holdouts_exp2.first['id']).to eq('holdout_exp_1') + expect(holdouts_exp1.first['id']).to eq(holdouts_exp2.first['id']) + end + end + + describe '#local_holdout?' do + it 'should return true for holdouts with experiments array' do + holdout = config_with_experiment_holdouts.get_holdout('holdout_exp_1') + expect(config_with_experiment_holdouts.local_holdout?(holdout)).to be true + end + + it 'should return false for holdouts without experiments' do + holdout = config_with_experiment_holdouts.get_holdout('holdout_global') + expect(config_with_experiment_holdouts.local_holdout?(holdout)).to be false + end + + it 'should return false for holdouts with empty experiments array' do + config_body_empty = config_body.dup + config_body_empty['holdouts'] = [ + { + 'id' => 'holdout_empty', + 'key' => 'empty_holdout', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => [], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + config = Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_empty), + logger, + error_handler + ) + + holdout = config.get_holdout('holdout_empty') + expect(config.local_holdout?(holdout)).to be false + end + end + + describe 'experiments field parsing and defaults' do + it 'should parse experiments field from datafile' do + holdout = config_with_experiment_holdouts.get_holdout('holdout_exp_1') + expect(holdout['experiments']).to eq(['exp_123', 'exp_456']) + end + + it 'should default experiments to empty array if not provided' do + config_body_no_experiments = config_body.dup + config_body_no_experiments['holdouts'] = [ + { + 'id' => 'holdout_no_exp', + 'key' => 'holdout_without_experiments', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + config = Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_no_experiments), + logger, + error_handler + ) + + holdout = config.get_holdout('holdout_no_exp') + expect(holdout['experiments']).to eq([]) + end + + it 'should build experiment_holdouts_map correctly' do + expect(config_with_experiment_holdouts.experiment_holdouts_map).to be_a(Hash) + expect(config_with_experiment_holdouts.experiment_holdouts_map.keys).to include('exp_123', 'exp_456', 'exp_789') + expect(config_with_experiment_holdouts.experiment_holdouts_map['exp_123'].first['id']).to eq('holdout_exp_1') + end + + it 'should not include non-running holdouts in experiment mapping' do + config_body_inactive = config_body.dup + config_body_inactive['holdouts'] = [ + { + 'id' => 'holdout_inactive', + 'key' => 'inactive_holdout', + 'status' => 'Paused', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => ['exp_inactive'], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + config = Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_inactive), + logger, + error_handler + ) + + holdouts = config.get_holdouts_for_experiment('exp_inactive') + expect(holdouts).to eq([]) + end + end + + describe 'backward compatibility' do + it 'should work with datafiles without experiments field' do + config_body_legacy = config_body.dup + config_body_legacy['holdouts'] = [ + { + 'id' => 'holdout_legacy', + 'key' => 'legacy_holdout', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + config = Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_legacy), + logger, + error_handler + ) + + holdout = config.get_holdout('holdout_legacy') + expect(holdout).not_to be_nil + expect(holdout['experiments']).to eq([]) + expect(config.local_holdout?(holdout)).to be false + end + + it 'should handle mix of holdouts with and without experiments field' do + config_body_mixed = config_body.dup + config_body_mixed['holdouts'] = [ + { + 'id' => 'holdout_with_exp', + 'key' => 'holdout_1', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'experiments' => ['exp_1'], + 'variations' => [], + 'trafficAllocation' => [] + }, + { + 'id' => 'holdout_without_exp', + 'key' => 'holdout_2', + 'status' => 'Running', + 'audiences' => [], + 'includedFlags' => [], + 'excludedFlags' => [], + 'variations' => [], + 'trafficAllocation' => [] + } + ] + + config = Optimizely::DatafileProjectConfig.new( + JSON.dump(config_body_mixed), + logger, + error_handler + ) + + holdout1 = config.get_holdout('holdout_with_exp') + holdout2 = config.get_holdout('holdout_without_exp') + + expect(config.local_holdout?(holdout1)).to be true + expect(config.local_holdout?(holdout2)).to be false + + holdouts = config.get_holdouts_for_experiment('exp_1') + expect(holdouts.length).to eq(1) + expect(holdouts.first['id']).to eq('holdout_with_exp') + end + end + end end From 28802bc916aeb2ccaf5099fe602049d9b58e1c29 Mon Sep 17 00:00:00 2001 From: muzahidul-opti Date: Wed, 4 Feb 2026 18:53:37 +0600 Subject: [PATCH 2/2] [FSSDK-12265] Fix RuboCop style violations Use %w[] syntax for word arrays instead of string arrays Co-Authored-By: Claude Sonnet 4.5 --- spec/config/datafile_project_config_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index 8dbd3954..6331225e 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -1813,7 +1813,7 @@ 'audiences' => [], 'includedFlags' => [], 'excludedFlags' => [], - 'experiments' => ['exp_123', 'exp_456'], + 'experiments' => %w[exp_123 exp_456], 'variations' => [], 'trafficAllocation' => [] }, @@ -1962,7 +1962,7 @@ describe 'experiments field parsing and defaults' do it 'should parse experiments field from datafile' do holdout = config_with_experiment_holdouts.get_holdout('holdout_exp_1') - expect(holdout['experiments']).to eq(['exp_123', 'exp_456']) + expect(holdout['experiments']).to eq(%w[exp_123 exp_456]) end it 'should default experiments to empty array if not provided' do