[FSSDK-11575][FSSDK-11576] Ruby: Parsing holdout to datafile and update project config (#373)

* [FSSDK-11575][FSSDK-11576] Ruby: Parsing holdout to datafile and update project config * Fix lint issues * Fix lint issues * Fix lint * Fix lint * Move holdout before the private * Fix tests * Fix lint * Fix test and add debug * Fix lint * Fix lint * Fix lint whitespace * Fix failed tests * Update the schema * Implement comment

Author: esrakartalOpt

lib/optimizely/config/datafile_project_config.rb

@@ -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
+ :public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map,
+ :global_holdouts, :included_holdouts, :excluded_holdouts, :flag_holdouts_map
  # Boolean - denotes if Optimizely should remove the last block of visitors' IP address before storing event data
  attr_reader :anonymize_ip
 
@@ -70,6 +71,7 @@ def initialize(datafile, logger, error_handler)
  @send_flag_decisions = config.fetch('sendFlagDecisions', false)
  @integrations = config.fetch('integrations', [])
  @region = config.fetch('region', 'US')
+ @holdouts = config.fetch('holdouts', [])
 
  # Default to US region if not specified
  @region = 'US' if @region.nil? || @region.empty?
@@ -112,6 +114,34 @@ def initialize(datafile, logger, error_handler)
  @variation_id_to_variable_usage_map = {}
  @variation_id_to_experiment_map = {}
  @flag_variation_map = {}
+ @holdout_id_map = {}
+ @global_holdouts = {}
+ @included_holdouts = {}
+ @excluded_holdouts = {}
+ @flag_holdouts_map = {}
+
+ @holdouts.each do |holdout|
+ next unless holdout['status'] == 'Running'
+
+ @holdout_id_map[holdout['id']] = holdout
+
+ if holdout['includedFlags'].nil? || holdout['includedFlags'].empty?
+ @global_holdouts[holdout['id']] = holdout
+
+ excluded_flags = holdout['excludedFlags']
+ if excluded_flags && !excluded_flags.empty?
+ excluded_flags.each do |flag_id|
+ @excluded_holdouts[flag_id] ||= []
+ @excluded_holdouts[flag_id] << holdout
+ end
+ end
+ else
+ holdout['includedFlags'].each do |flag_id|
+ @included_holdouts[flag_id] ||= []
+ @included_holdouts[flag_id] << holdout
+ end
+ end
+ end
 
  @experiment_id_map.each_value do |exp|
  # Excludes experiments from rollouts
@@ -568,6 +598,61 @@ def rollout_experiment?(experiment_id)
  @rollout_experiment_id_map.key?(experiment_id)
  end
 
+ def get_holdouts_for_flag(flag_key)
+ # Helper method to get holdouts from an applied feature flag
+ #
+ # flag_key - Key of the feature flag
+ #
+ # Returns the holdouts that apply for a specific flag
+
+ feature_flag = @feature_flag_key_map[flag_key]
+ return [] unless feature_flag
+
+ flag_id = feature_flag['id']
+
+ # Check catch first
+ return @flag_holdouts_map[flag_id] if @flag_holdouts_map.key?(flag_id)
+
+ holdouts = []
+
+ # Add global holdouts that don't exclude this flag
+ @global_holdouts.each_value do |holdout|
+ is_excluded = false
+ excluded_flags = holdout['excludedFlags']
+ if excluded_flags && !excluded_flags.empty?
+ excluded_flags.each do |excluded_flag_id|
+ if excluded_flag_id == flag_id
+ is_excluded = true
+ break
+ end
+ end
+ end
+ holdouts << holdout unless is_excluded
+ end
+
+ # Add holdouts that specifically include this flag
+ holdouts.concat(@included_holdouts[flag_id]) if @included_holdouts.key?(flag_id)
+
+ # Cache the result
+ @flag_holdouts_map[flag_id] = holdouts
+
+ holdouts
+ end
+
+ def get_holdout(holdout_id)
+ # Helper method to get holdout from holdout ID
+ #
+ # holdout_id - ID of the holdout
+ #
+ # Returns the holdout
+
+ holdout = @holdout_id_map[holdout_id]
+ return holdout if holdout
+
+ @logger.log Logger::ERROR, "Holdout with ID '#{holdout_id}' not found."
+ nil
+ end
+
  private
 
  def generate_feature_variation_map(feature_flags)

lib/optimizely/config_manager/http_project_config_manager.rb

@@ -326,7 +326,7 @@ def get_datafile_url(sdk_key, url, url_template)
  unless url
  url_template ||= @access_token.nil? ? Helpers::Constants::CONFIG_MANAGER['DATAFILE_URL_TEMPLATE'] : Helpers::Constants::CONFIG_MANAGER['AUTHENTICATED_DATAFILE_URL_TEMPLATE']
  begin
- return (url_template % sdk_key)
+ return url_template % sdk_key
  rescue
  error_msg = "Invalid url_template #{url_template} provided."
  @logger.log(Logger::ERROR, error_msg)

lib/optimizely/helpers/constants.rb

@@ -204,6 +204,9 @@ module Constants
  },
  'cmab' => {
  'type' => 'object'
+ },
+ 'holdouts' => {
+ 'type' => 'array'
  }
  },
  'required' => %w[
@@ -318,6 +321,31 @@ module Constants
  'type' => 'integer'
  }
  }
+ },
+ 'holdouts' => {
+ 'type' => 'array',
+ 'items' => {
+ 'type' => 'object',
+ 'properties' => {
+ 'id' => {
+ 'type' => 'string'
+ },
+ 'key' => {
+ 'type' => 'string'
+ },
+ 'status' => {
+ 'type' => 'string'
+ },
+ 'includedFlags' => {
+ 'type' => 'array',
+ 'items' => {'type' => 'string'}
+ },
+ 'excludedFlags' => {
+ 'type' => 'array',
+ 'items' => {'type' => 'string'}
+ }
+ }
+ }
  }
  },
  'required' => %w[

spec/config/datafile_project_config_spec.rb

@@ -1234,4 +1234,193 @@
  expect(config.send(:generate_feature_variation_map, config.feature_flags)).to eq(expected_feature_variation_map)
  end
  end
+
+ describe '#get_holdouts_for_flag' do
+ let(:config_with_holdouts) do
+ Optimizely::DatafileProjectConfig.new(
+ OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON,
+ logger,
+ error_handler
+ )
+ end
+
+ it 'should return empty array for non-existent flag' do
+ holdouts = config_with_holdouts.get_holdouts_for_flag('non_existent_flag')
+ expect(holdouts).to eq([])
+ end
+
+ it 'should return global holdouts that do not exclude the flag' do
+ holdouts = config_with_holdouts.get_holdouts_for_flag('multi_variate_feature')
+ expect(holdouts.length).to eq(2)
+
+ global_holdout = holdouts.find { |h| h['key'] == 'global_holdout' }
+ expect(global_holdout).not_to be_nil
+ expect(global_holdout['id']).to eq('holdout_1')
+
+ specific_holdout = holdouts.find { |h| h['key'] == 'specific_holdout' }
+ expect(specific_holdout).not_to be_nil
+ expect(specific_holdout['id']).to eq('holdout_2')
+ end
+
+ it 'should not return global holdouts that exclude the flag' do
+ holdouts = config_with_holdouts.get_holdouts_for_flag('boolean_single_variable_feature')
+ expect(holdouts.length).to eq(0)
+
+ global_holdout = holdouts.find { |h| h['key'] == 'global_holdout' }
+ expect(global_holdout).to be_nil
+ end
+
+ it 'should cache results for subsequent calls' do
+ holdouts1 = config_with_holdouts.get_holdouts_for_flag('multi_variate_feature')
+ holdouts2 = config_with_holdouts.get_holdouts_for_flag('multi_variate_feature')
+ expect(holdouts1).to equal(holdouts2)
+ expect(holdouts1.length).to eq(2)
+ end
+
+ it 'should return only global holdouts for flags not specifically targeted' do
+ holdouts = config_with_holdouts.get_holdouts_for_flag('string_single_variable_feature')
+
+ # Should only include global holdout (not excluded and no specific targeting)
+ expect(holdouts.length).to eq(1)
+ expect(holdouts.first['key']).to eq('global_holdout')
+ end
+ end
+
+ describe '#get_holdout' do
+ let(:config_with_holdouts) do
+ Optimizely::DatafileProjectConfig.new(
+ OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON,
+ logger,
+ error_handler
+ )
+ end
+
+ it 'should return holdout when valid ID is provided' do
+ holdout = config_with_holdouts.get_holdout('holdout_1')
+ expect(holdout).not_to be_nil
+ expect(holdout['id']).to eq('holdout_1')
+ expect(holdout['key']).to eq('global_holdout')
+ expect(holdout['status']).to eq('Running')
+ end
+
+ it 'should return holdout regardless of status when valid ID is provided' do
+ holdout = config_with_holdouts.get_holdout('holdout_2')
+ expect(holdout).not_to be_nil
+ expect(holdout['id']).to eq('holdout_2')
+ expect(holdout['key']).to eq('specific_holdout')
+ expect(holdout['status']).to eq('Running')
+ end
+
+ it 'should return nil for non-existent holdout ID' do
+ holdout = config_with_holdouts.get_holdout('non_existent_holdout')
+ expect(holdout).to be_nil
+ end
+ end
+
+ describe '#get_holdout with logging' do
+ let(:spy_logger) { spy('logger') }
+ let(:config_with_holdouts) do
+ config_body_with_holdouts = config_body.dup
+ config_body_with_holdouts['holdouts'] = [
+ {
+ 'id' => 'holdout_1',
+ 'key' => 'test_holdout',
+ 'status' => 'Running',
+ 'includedFlags' => [],
+ 'excludedFlags' => []
+ }
+ ]
+ config_json = JSON.dump(config_body_with_holdouts)
+ Optimizely::DatafileProjectConfig.new(config_json, spy_logger, error_handler)
+ end
+
+ it 'should log error when holdout is not found' do
+ result = config_with_holdouts.get_holdout('invalid_holdout_id')
+
+ expect(result).to be_nil
+ expect(spy_logger).to have_received(:log).with(
+ Logger::ERROR,
+ "Holdout with ID 'invalid_holdout_id' not found."
+ )
+ end
+
+ it 'should not log when holdout is found' do
+ result = config_with_holdouts.get_holdout('holdout_1')
+
+ expect(result).not_to be_nil
+ expect(spy_logger).not_to have_received(:log).with(
+ Logger::ERROR,
+ anything
+ )
+ end
+ end
+
+ describe 'holdout initialization' do
+ let(:config_with_complex_holdouts) do
+ config_body_with_holdouts = config_body.dup
+
+ # Use the correct feature flag IDs from the debug output
+ boolean_feature_id = '155554'
+ multi_variate_feature_id = '155559'
+ empty_feature_id = '594032'
+ string_feature_id = '594060'
+
+ config_body_with_holdouts['holdouts'] = [
+ {
+ 'id' => 'global_holdout',
+ 'key' => 'global',
+ 'status' => 'Running',
+ 'includedFlags' => [],
+ 'excludedFlags' => [boolean_feature_id, string_feature_id]
+ },
+ {
+ 'id' => 'specific_holdout',
+ 'key' => 'specific',
+ 'status' => 'Running',
+ 'includedFlags' => [multi_variate_feature_id, empty_feature_id],
+ 'excludedFlags' => []
+ },
+ {
+ 'id' => 'inactive_holdout',
+ 'key' => 'inactive',
+ 'status' => 'Inactive',
+ 'includedFlags' => [boolean_feature_id],
+ 'excludedFlags' => []
+ }
+ ]
+ config_json = JSON.dump(config_body_with_holdouts)
+ Optimizely::DatafileProjectConfig.new(config_json, logger, error_handler)
+ end
+
+ it 'should properly categorize holdouts during initialization' do
+ expect(config_with_complex_holdouts.holdout_id_map.keys).to contain_exactly('global_holdout', 'specific_holdout')
+ expect(config_with_complex_holdouts.global_holdouts.keys).to contain_exactly('global_holdout')
+
+ # Use the correct feature flag IDs
+ boolean_feature_id = '155554'
+ multi_variate_feature_id = '155559'
+ empty_feature_id = '594032'
+ string_feature_id = '594060'
+
+ expect(config_with_complex_holdouts.included_holdouts[multi_variate_feature_id]).not_to be_nil
+ expect(config_with_complex_holdouts.included_holdouts[multi_variate_feature_id]).not_to be_empty
+ expect(config_with_complex_holdouts.included_holdouts[empty_feature_id]).not_to be_nil
+ expect(config_with_complex_holdouts.included_holdouts[empty_feature_id]).not_to be_empty
+ expect(config_with_complex_holdouts.included_holdouts[boolean_feature_id]).to be_nil
+
+ expect(config_with_complex_holdouts.excluded_holdouts[boolean_feature_id]).not_to be_nil
+ expect(config_with_complex_holdouts.excluded_holdouts[boolean_feature_id]).not_to be_empty
+ expect(config_with_complex_holdouts.excluded_holdouts[string_feature_id]).not_to be_nil
+ expect(config_with_complex_holdouts.excluded_holdouts[string_feature_id]).not_to be_empty
+ end
+
+ it 'should only process running holdouts during initialization' do
+ expect(config_with_complex_holdouts.holdout_id_map['inactive_holdout']).to be_nil
+ expect(config_with_complex_holdouts.global_holdouts['inactive_holdout']).to be_nil
+
+ boolean_feature_id = '155554'
+ included_for_boolean = config_with_complex_holdouts.included_holdouts[boolean_feature_id]
+ expect(included_for_boolean).to be_nil
+ end
+ end
 end

spec/spec_params.rb

@@ -1939,6 +1939,36 @@ module OptimizelySpec
 
  CONFIG_DICT_WITH_INTEGRATIONS_JSON = JSON.dump(CONFIG_DICT_WITH_INTEGRATIONS)
 
+ CONFIG_BODY_WITH_HOLDOUTS = VALID_CONFIG_BODY.merge(
+ {
+ 'holdouts' => [
+ {
+ 'id' => 'holdout_1',
+ 'key' => 'global_holdout',
+ 'status' => 'Running',
+ 'includedFlags' => [],
+ 'excludedFlags' => ['155554']
+ },
+ {
+ 'id' => 'holdout_2',
+ 'key' => 'specific_holdout',
+ 'status' => 'Running',
+ 'includedFlags' => ['155559'],
+ 'excludedFlags' => []
+ },
+ {
+ 'id' => 'holdout_3',
+ 'key' => 'inactive_holdout',
+ 'status' => 'Inactive',
+ 'includedFlags' => ['155554'],
+ 'excludedFlags' => []
+ }
+ ]
+ }
+ ).freeze
+
+ CONFIG_BODY_WITH_HOLDOUTS_JSON = JSON.dump(CONFIG_BODY_WITH_HOLDOUTS).freeze
+
  def self.deep_clone(obj)
  obj.dup.tap do |new_obj|
  case new_obj