diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index cf3a3629..3cd8d11c 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/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, :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 @@ -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' @@ -122,6 +125,19 @@ def initialize(datafile, logger, error_handler) holdout['layerId'] ||= '' @holdout_id_map[holdout['id']] = holdout + + # Classify holdout as global or local based on includedRules field. + # If includedRules is nil (absent from datafile), holdout is global and applies to all rules. + # If includedRules is an array (even empty), holdout is local and targets specific rule IDs. + included_rules = holdout['includedRules'] + if included_rules.nil? + @global_holdouts << holdout + else + 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| @@ -642,6 +658,17 @@ def get_holdout(holdout_id) nil end + def get_holdouts_for_rule(rule_id) + # Returns local holdouts targeting a specific rule. + # Local holdouts are holdouts where includedRules is a non-nil array containing rule_id. + # + # rule_id - String ID of the rule to look up + # + # Returns Array of holdout hashes (empty array if none found) + + @rule_holdouts_map[rule_id] || [] + end + private def get_everyone_else_variation(feature_flag) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 38d10ab8..9166a689 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -169,11 +169,14 @@ 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) + # Get running holdouts from the holdout_id_map (global and local). + # If any holdouts exist, use get_decision_for_flag which checks global holdouts + # at flag level and local holdouts per-rule. running_holdouts = project_config.holdout_id_map.values if running_holdouts && !running_holdouts.empty? - # Has holdouts - use get_decision_for_flag which checks holdouts first + # Has holdouts - use get_decision_for_flag which checks global holdouts first, + # then experiments/rollouts which check local holdouts per-rule get_decision_for_flag(feature_flag, user_context, project_config, decide_options) else get_variations_for_feature_list(project_config, [feature_flag], user_context, decide_options).first @@ -196,16 +199,17 @@ 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 (those with includedRules == nil). + # Global holdouts apply to all rules and are evaluated at the flag level. + global_holdouts = project_config.global_holdouts - holdouts.each do |holdout| + global_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 - message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'." + message = "The user '#{user_id}' is bucketed into global holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'." @logger.log(Logger::INFO, message) reasons.push(message) return DecisionResult.new(holdout_decision.decision, false, reasons) @@ -446,6 +450,21 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use reasons.push(*forced_reasons) return VariationResult.new(nil, false, reasons, variation['id']) if variation + # Check local holdouts targeting this specific experiment rule. + # Local holdouts are checked after forced decisions but before regular bucketing. + 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 + + message = "The user '#{user.user_id}' is bucketed into local holdout '#{holdout['key']}' for experiment rule '#{rule['key']}' in flag '#{flag_key}'." + @logger.log(Logger::INFO, message) + reasons.push(message) + variation_id = holdout_decision.decision.variation['id'] + return VariationResult.new(nil, false, reasons, variation_id) + end + variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options) variation_result.reasons = reasons + variation_result.reasons variation_result @@ -470,6 +489,20 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index return [variation, skip_to_everyone_else, reasons] if variation + # Check local holdouts targeting this specific delivery rule. + # Local holdouts are checked after forced decisions but before audience/bucketing evaluation. + 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 + + message = "The user '#{user_context.user_id}' is bucketed into local holdout '#{holdout['key']}' for delivery rule '#{rule['key']}' in flag '#{flag_key}'." + @logger.log(Logger::INFO, message) + reasons.push(message) + return [holdout_decision.decision.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) diff --git a/spec/local_holdouts_spec.rb b/spec/local_holdouts_spec.rb new file mode 100644 index 00000000..681d44e6 --- /dev/null +++ b/spec/local_holdouts_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +# +# Copyright 2026, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +require 'spec_helper' +require 'optimizely/config/datafile_project_config' +require 'optimizely/decision_service' +require 'optimizely/error_handler' +require 'optimizely/logger' + +describe 'Local Holdouts' do + let(:error_handler) { Optimizely::NoOpErrorHandler.new } + let(:spy_logger) { spy('logger') } + + describe 'DatafileProjectConfig local holdout classification' do + let(:config_body_json) { OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON } + let(:config) { Optimizely::DatafileProjectConfig.new(config_body_json, spy_logger, error_handler) } + + context 'when holdouts have no includedRules field (old datafile format)' do + it 'treats holdouts as global (backward compatibility)' do + # All holdouts in CONFIG_BODY_WITH_HOLDOUTS have no includedRules = nil = global + running_holdouts = config.holdout_id_map.values + global_holdouts = config.global_holdouts + + # Running holdouts: holdout_1, holdout_boolean_feature, holdout_empty_1, holdout_2 (holdout_3 is Inactive) + expect(global_holdouts.length).to eq(running_holdouts.length) + expect(config.rule_holdouts_map).to be_empty + end + end + + context 'with local holdouts config' do + let(:local_config_json) { OptimizelySpec::CONFIG_BODY_WITH_LOCAL_HOLDOUTS_JSON } + let(:local_config) { Optimizely::DatafileProjectConfig.new(local_config_json, spy_logger, error_handler) } + + it 'classifies holdout with nil includedRules as global' do + global_holdouts = local_config.global_holdouts + expect(global_holdouts.length).to eq(1) + expect(global_holdouts.first['id']).to eq('global_holdout_1') + end + + it 'classifies holdout with non-nil includedRules as local' do + global_holdouts = local_config.global_holdouts + # Only global_holdout_1 should be in global list, not local_holdout_1 + expect(global_holdouts.none? { |h| h['id'] == 'local_holdout_1' }).to be(true) + end + + it 'populates rule_holdouts_map for local holdouts' do + rule_holdouts_map = local_config.rule_holdouts_map + expect(rule_holdouts_map).to have_key('111127') + expect(rule_holdouts_map['111127'].length).to eq(1) + expect(rule_holdouts_map['111127'].first['id']).to eq('local_holdout_1') + end + + it 'get_holdouts_for_rule returns local holdouts for a rule' do + holdouts = local_config.get_holdouts_for_rule('111127') + expect(holdouts.length).to eq(1) + expect(holdouts.first['key']).to eq('local_holdout_exp') + end + + it 'get_holdouts_for_rule returns empty array for unknown rule' do + holdouts = local_config.get_holdouts_for_rule('unknown_rule_id') + expect(holdouts).to be_empty + end + + it 'global_holdouts returns empty array when no global holdouts' do + # Build a config with only a local holdout + only_local = OptimizelySpec::VALID_CONFIG_BODY.merge( + 'holdouts' => [ + { + 'id' => 'local_only', + 'key' => 'local_only_holdout', + 'status' => 'Running', + 'audiences' => [], + 'audienceIds' => [], + 'audienceConditions' => [], + 'includedRules' => ['some_rule'], + 'variations' => [{'id' => 'v1', 'key' => 'off', 'featureEnabled' => false}], + 'trafficAllocation' => [{'entityId' => 'v1', 'endOfRange' => 10_000}] + } + ] + ) + only_local_config = Optimizely::DatafileProjectConfig.new(JSON.dump(only_local), spy_logger, error_handler) + + expect(only_local_config.global_holdouts).to be_empty + expect(only_local_config.rule_holdouts_map['some_rule'].length).to eq(1) + end + end + end + + describe 'is_global? semantics via includedRules' do + it 'treats nil includedRules as global (no field in datafile)' do + holdout = {'id' => 'h1', 'key' => 'global', 'includedRules' => nil} + expect(holdout['includedRules'].nil?).to be(true) + end + + it 'treats non-nil includedRules as local even if empty' do + holdout = {'id' => 'h2', 'key' => 'local_empty', 'includedRules' => []} + expect(holdout['includedRules'].nil?).to be(false) + end + + it 'treats non-nil includedRules with values as local' do + holdout = {'id' => 'h3', 'key' => 'local', 'includedRules' => ['rule1']} + expect(holdout['includedRules'].nil?).to be(false) + end + end +end diff --git a/spec/spec_params.rb b/spec/spec_params.rb index 789c36df..fcaed29d 100644 --- a/spec/spec_params.rb +++ b/spec/spec_params.rb @@ -2041,6 +2041,60 @@ module OptimizelySpec CONFIG_BODY_WITH_HOLDOUTS_JSON = JSON.dump(CONFIG_BODY_WITH_HOLDOUTS).freeze + # Config with both global and local holdouts for testing local holdouts feature + CONFIG_BODY_WITH_LOCAL_HOLDOUTS = VALID_CONFIG_BODY.merge( + { + 'holdouts' => [ + { + 'id' => 'global_holdout_1', + 'key' => 'global_holdout', + 'status' => 'Running', + 'audiences' => [], + 'audienceIds' => [], + 'audienceConditions' => [], + 'variations' => [ + { + 'id' => 'global_var_1', + 'key' => 'holdout_variation', + 'featureEnabled' => true + } + ], + 'trafficAllocation' => [ + { + 'entityId' => 'global_var_1', + 'endOfRange' => 10_000 + } + ] + # No includedRules key => nil => global holdout + }, + { + 'id' => 'local_holdout_1', + 'key' => 'local_holdout_exp', + 'status' => 'Running', + 'audiences' => [], + 'audienceIds' => [], + 'audienceConditions' => [], + 'includedRules' => ['111127'], # targets experiment rule 111127 + 'variations' => [ + { + 'id' => 'local_var_1', + 'key' => 'local_holdout_variation', + 'featureEnabled' => true + } + ], + 'trafficAllocation' => [ + { + 'entityId' => 'local_var_1', + 'endOfRange' => 10_000 + } + ] + } + ] + } + ).freeze + + CONFIG_BODY_WITH_LOCAL_HOLDOUTS_JSON = JSON.dump(CONFIG_BODY_WITH_LOCAL_HOLDOUTS).freeze + def self.deep_clone(obj) obj.dup.tap do |new_obj| case new_obj