Skip to content

[AI-FSSDK] [FSSDK-12368] Local Holdouts - Cleanup flag base setup and add includedRules and rule-level lookup #394

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
Mat001 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[AI-FSSDK] [FSSDK-12368] Local Holdouts - Cleanup flag base setup and add includedRules and rule-level lookup #394

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
93 changes: 38 additions & 55 deletions lib/optimizely/config/datafile_project_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ 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, :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 @@ -116,9 +116,7 @@ def initialize(datafile, logger, error_handler)
@flag_variation_map = {}
@holdout_id_map = {}
@global_holdouts = []
@included_holdouts = {}
@excluded_holdouts = {}
@flag_holdouts_map = {}
@rule_holdouts_map = {}

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

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

included_flags = holdout['includedFlags'] || []
excluded_flags = holdout['excludedFlags'] || []
# Local Holdouts: included_rules field determines scope
# nil = global holdout (applies to all rules)
# array = local holdout (applies to specific rules)
included_rules = holdout['includedRules']

case [included_flags.empty?, excluded_flags.empty?]
when [true, true]
# No included or excluded flags - this is a global holdout
if included_rules.nil?
# Global holdout - applies to all rules
@global_holdouts << holdout

when [false, true], [false, false]
# Has included flags - add to included_holdouts map
included_flags.each do |flag_id|
@included_holdouts[flag_id] ||= []
@included_holdouts[flag_id] << holdout
end

when [true, false]
# No included flags but has excluded flags - global with exclusions
@global_holdouts << holdout

excluded_flags.each do |flag_id|
@excluded_holdouts[flag_id] ||= []
@excluded_holdouts[flag_id] << holdout
else
# Local holdout - applies to specific rules
included_rules.each do |rule_id|
@rule_holdouts_map[rule_id] ||= []
@rule_holdouts_map[rule_id] << holdout
end
end
end
Expand Down Expand Up @@ -658,44 +647,27 @@ def rollout_experiment?(experiment_id)
@rollout_experiment_id_map.key?(experiment_id)
end

def get_holdouts_for_flag(flag_id)
# Helper method to get holdouts from an applied feature flag
def get_global_holdouts
Copy link
Copy Markdown
Contributor

@esrakartalOpt esrakartalOpt Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns lint issue:

lib/optimizely/config/datafile_project_config.rb:650:9: C: Naming/AccessorMethodName: Do not prefix reader method names with get_.
    def get_global_holdouts
        ^^^^^^^^^^^^^^^^^^^
Suggested change
def get_global_holdouts
def get_global_holdouts # rubocop:disable Naming/AccessorMethodName

Copy link
Copy Markdown
Contributor

@esrakartalOpt esrakartalOpt Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the method name change, there are failed test cases. Even if you replace the method name on the unit tests, it will still fail because the method doesn't have parameter as flag_id .

After fixing the all lint issues, this method will be main problem to unit test failures.

# Helper method to get all global holdouts
#
# flag_id - (REQUIRED) ID of the feature flag
# This parameter is required and should not be null/nil
#
# Returns the holdouts that apply for a specific flag
# Returns array of global holdouts (holdouts with includedRules == nil)

return [] if @holdouts.nil? || @holdouts.empty?

# Check cache first (before validation, so we cache the validation result too)
return @flag_holdouts_map[flag_id] if @flag_holdouts_map.key?(flag_id)

# Validate that the flag exists in the datafile
flag_exists = @feature_flags.any? { |flag| flag['id'] == flag_id }
unless flag_exists
# Cache the empty result for non-existent flags
@flag_holdouts_map[flag_id] = []
return []
end

# Prioritize global holdouts first
excluded = @excluded_holdouts[flag_id] || []

active_holdouts = if excluded.any?
@global_holdouts.reject { |holdout| excluded.include?(holdout) }
else
@global_holdouts.dup
end
@global_holdouts
end

# Append included holdouts
included = @included_holdouts[flag_id] || []
active_holdouts.concat(included)
def get_holdouts_for_rule(rule_id)
# Helper method to get local holdouts for a specific rule
#
# rule_id - (REQUIRED) ID of the rule
#
# Returns array of local holdouts targeting this rule

# Cache the result
@flag_holdouts_map[flag_id] = active_holdouts
return [] if @holdouts.nil? || @holdouts.empty?
return [] if rule_id.nil?

@flag_holdouts_map[flag_id] || []
@rule_holdouts_map[rule_id] || []
end

def get_holdout(holdout_id)
Expand All @@ -712,6 +684,17 @@ def get_holdout(holdout_id)
nil
end

def global_holdout?(holdout)
# Helper method to check if a holdout is global
#
# holdout - Holdout hash
#
# Returns true if holdout has includedRules == nil (global),
# false otherwise (local)

holdout['includedRules'].nil?
end

private

def get_everyone_else_variation(feature_flag)
Expand Down
49 changes: 42 additions & 7 deletions lib/optimizely/decision_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,11 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide
# user_context - Optimizely user context instance
#
# Returns DecisionResult struct.
holdouts = project_config.get_holdouts_for_flag(feature_flag['id'])
# Check if there are global holdouts (flag-level evaluation)
global_holdouts = project_config.get_global_holdouts

if holdouts && !holdouts.empty?
# Has holdouts - use get_decision_for_flag which checks holdouts first
if global_holdouts && !global_holdouts.empty?
# Has global holdouts - use get_decision_for_flag which checks global holdouts first
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
Expand All @@ -195,16 +196,16 @@ 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
holdouts = project_config.get_holdouts_for_flag(feature_flag['id'])
# Check global holdouts (flag-level evaluation)
global_holdouts = project_config.get_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)
Expand Down Expand Up @@ -445,6 +446,23 @@ 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 rule
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

# User is in local holdout - return holdout decision immediately, skip rule evaluation
message = "The user '#{user.user_id}' is bucketed into local holdout '#{holdout['key']}' for rule '#{rule['key']}'."
@logger.log(Logger::INFO, message)
reasons.push(message)

# Return holdout variation as rule variation (with nil variation_id for holdout source)
return VariationResult.new(holdout_decision.decision['holdout']['id'], false, reasons, nil)
end

variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options)
variation_result.reasons = reasons + variation_result.reasons
variation_result
Expand All @@ -469,6 +487,23 @@ 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 delivery rule
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

# User is in local holdout - return holdout variation immediately, skip rule evaluation
message = "The user '#{user_context.user_id}' is bucketed into local holdout '#{holdout['key']}' for delivery rule '#{rule['key']}'."
@logger.log(Logger::INFO, message)
reasons.push(message)

# Return holdout variation and skip_to_everyone_else flag
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)
Expand Down
Loading
Loading