Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
54 changes: 48 additions & 6 deletions lib/optimizely.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,20 @@ def decide(user_context, key, decide_options = [])
experiment = nil
decision_source = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT']

decision, reasons_received = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes, decide_options)
variation, reasons_received = user_context.find_validated_forced_decision(key, nil)
reasons.push(*reasons_received)

if variation
decision = Optimizely::DecisionService::Decision.new(nil, variation, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'])
else
decision, reasons_received = @decision_service.get_variation_for_feature(config, feature_flag, user_context, decide_options)
reasons.push(*reasons_received)
end

# Send impression event if Decision came from a feature test and decide options doesn't include disableDecisionEvent
if decision.is_a?(Optimizely::DecisionService::Decision)
experiment = decision.experiment
rule_key = experiment['key']
rule_key = experiment ? experiment['key'] : nil
variation = decision['variation']
variation_key = variation['key']
feature_enabled = variation['featureEnabled']
Expand Down Expand Up @@ -291,6 +298,10 @@ def decide_for_keys(user_context, keys, decide_options = [])
decisions
end

def get_flag_variation_by_key(flag_key, variation_key)
project_config.get_variation_from_flag(flag_key, variation_key)
end

# Buckets visitor and sends impression event to Optimizely.
#
# @param experiment_key - Experiment which needs to be activated.
Expand Down Expand Up @@ -458,6 +469,7 @@ def track(event_key, user_id, attributes = nil, event_tags = nil)

# Determine whether a feature is enabled.
# Sends an impression event if the user is bucketed into an experiment using the feature.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String unique key of the feature.
# @param user_id - String ID of the user.
Expand All @@ -468,6 +480,8 @@ def track(event_key, user_id, attributes = nil, event_tags = nil)
# @return [False] if the feature is not found.

def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'is_feature_enabled' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('is_feature_enabled').message)
return false
Expand All @@ -490,7 +504,8 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
return false
end

decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes)
user_context = create_user_context(user_id, attributes)
decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context)

feature_enabled = false
source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT']
Expand Down Expand Up @@ -542,12 +557,15 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
end

# Gets keys of all feature flags which are enabled for the user.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param user_id - ID for user.
# @param attributes - Dict representing user attributes.
# @return [feature flag keys] A List of feature flag keys that are enabled for the user.

def get_enabled_features(user_id, attributes = nil)
@logger.log Logger::WARN, "'get_enabled_features' is deprecated. Use 'decide' methods instead."

enabled_features = []
unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_enabled_features').message)
Expand Down Expand Up @@ -575,6 +593,7 @@ def get_enabled_features(user_id, attributes = nil)
end

# Get the value of the specified variable in the feature flag.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag the variable belongs to
# @param variable_key - String key of variable for which we are getting the value
Expand All @@ -585,6 +604,8 @@ def get_enabled_features(user_id, attributes = nil)
# @return [nil] if the feature flag or variable are not found.

def get_feature_variable(feature_flag_key, variable_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_feature_variable' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable').message)
return nil
Expand All @@ -601,6 +622,7 @@ def get_feature_variable(feature_flag_key, variable_key, user_id, attributes = n
end

# Get the String value of the specified variable in the feature flag.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag the variable belongs to
# @param variable_key - String key of variable for which we are getting the string value
Expand All @@ -611,6 +633,8 @@ def get_feature_variable(feature_flag_key, variable_key, user_id, attributes = n
# @return [nil] if the feature flag or variable are not found.

def get_feature_variable_string(feature_flag_key, variable_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_feature_variable_string' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable_string').message)
return nil
Expand All @@ -627,6 +651,7 @@ def get_feature_variable_string(feature_flag_key, variable_key, user_id, attribu
end

# Get the Json value of the specified variable in the feature flag in a Dict.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag the variable belongs to
# @param variable_key - String key of variable for which we are getting the string value
Expand All @@ -637,6 +662,8 @@ def get_feature_variable_string(feature_flag_key, variable_key, user_id, attribu
# @return [nil] if the feature flag or variable are not found.

def get_feature_variable_json(feature_flag_key, variable_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_feature_variable_json' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable_json').message)
return nil
Expand All @@ -653,6 +680,7 @@ def get_feature_variable_json(feature_flag_key, variable_key, user_id, attribute
end

# Get the Boolean value of the specified variable in the feature flag.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag the variable belongs to
# @param variable_key - String key of variable for which we are getting the string value
Expand All @@ -663,6 +691,8 @@ def get_feature_variable_json(feature_flag_key, variable_key, user_id, attribute
# @return [nil] if the feature flag or variable are not found.

def get_feature_variable_boolean(feature_flag_key, variable_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_feature_variable_boolean' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable_boolean').message)
return nil
Expand All @@ -680,6 +710,7 @@ def get_feature_variable_boolean(feature_flag_key, variable_key, user_id, attrib
end

# Get the Double value of the specified variable in the feature flag.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag the variable belongs to
# @param variable_key - String key of variable for which we are getting the string value
Expand All @@ -690,6 +721,8 @@ def get_feature_variable_boolean(feature_flag_key, variable_key, user_id, attrib
# @return [nil] if the feature flag or variable are not found.

def get_feature_variable_double(feature_flag_key, variable_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_feature_variable_double' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable_double').message)
return nil
Expand All @@ -707,6 +740,7 @@ def get_feature_variable_double(feature_flag_key, variable_key, user_id, attribu
end

# Get values of all the variables in the feature flag and returns them in a Dict
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag
# @param user_id - String user ID
Expand All @@ -716,6 +750,8 @@ def get_feature_variable_double(feature_flag_key, variable_key, user_id, attribu
# @return [nil] if the feature flag is not found.

def get_all_feature_variables(feature_flag_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_all_feature_variables' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_all_feature_variables').message)
return nil
Expand All @@ -739,7 +775,8 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil)
return nil
end

decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes)
user_context = create_user_context(user_id, attributes)
decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context)
variation = decision ? decision['variation'] : nil
feature_enabled = variation ? variation['featureEnabled'] : false
all_variables = {}
Expand Down Expand Up @@ -772,6 +809,7 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil)
end

# Get the Integer value of the specified variable in the feature flag.
# @deprecated Use {#decide} methods of 'OptimizelyUserContext' instead.
#
# @param feature_flag_key - String key of feature flag the variable belongs to
# @param variable_key - String key of variable for which we are getting the string value
Expand All @@ -782,6 +820,8 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil)
# @return [nil] if the feature flag or variable are not found.

def get_feature_variable_integer(feature_flag_key, variable_key, user_id, attributes = nil)
@logger.log Logger::WARN, "'get_feature_variable_integer' is deprecated. Use 'decide' methods instead."

unless is_valid
@logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable_integer').message)
return nil
Expand Down Expand Up @@ -881,7 +921,8 @@ def get_variation_with_config(experiment_key, user_id, attributes, config)

return nil unless user_inputs_valid?(attributes)

variation_id, = @decision_service.get_variation(config, experiment_id, user_id, attributes)
user_context = create_user_context(user_id, attributes)
variation_id, = @decision_service.get_variation(config, experiment_id, user_context)
variation = config.get_variation_from_id(experiment_key, variation_id) unless variation_id.nil?
variation_key = variation['key'] if variation
decision_notification_type = if config.feature_experiment?(experiment_id)
Expand Down Expand Up @@ -947,7 +988,8 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type,
return nil
end

decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes)
user_context = create_user_context(user_id, attributes)
decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context)
Copy link
Contributor

Choose a reason for hiding this comment

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

why there is a comma? decision,

variation = decision ? decision['variation'] : nil
feature_enabled = variation ? variation['featureEnabled'] : false

Expand Down
39 changes: 39 additions & 0 deletions lib/optimizely/config/datafile_project_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class DatafileProjectConfig < ProjectConfig
attr_reader :variation_key_map
attr_reader :variation_id_map_by_experiment_id
attr_reader :variation_key_map_by_experiment_id
attr_reader :flag_variation_map

def initialize(datafile, logger, error_handler)
# ProjectConfig init method to fetch and set project config data
Expand Down Expand Up @@ -123,6 +124,8 @@ def initialize(datafile, logger, error_handler)
@variation_key_map_by_experiment_id = {}
@variation_id_to_variable_usage_map = {}
@variation_id_to_experiment_map = {}
@flag_variation_map = {}

@experiment_id_map.each_value do |exp|
# Excludes experiments from rollouts
variations = exp.fetch('variations')
Expand All @@ -138,6 +141,17 @@ def initialize(datafile, logger, error_handler)
exps = rollout.fetch('experiments')
@rollout_experiment_id_map = @rollout_experiment_id_map.merge(generate_key_map(exps, 'id'))
end

@feature_flags.each do |flag|
variations = []
get_rules_for_flag(flag).each do |rule|
rule['variations'].each do |rule_variation|
variations.push(rule_variation) if variations.select { |variation| variation['id'] == rule_variation['id'] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this filter out redundant variations. Can we add a test case for flag_variation_map?

Copy link
Contributor Author

@ozayr-zaviar ozayr-zaviar Oct 13, 2021

Choose a reason for hiding this comment

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

variations.select can return multiple values but we are using if condition just to check if any value exists.

Copy link
Contributor

Choose a reason for hiding this comment

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

I may be confused but the logic looks opposite to me. We need to collect all variations used in rules for a flag (see related discussion here - optimizely/php-sdk#233 (comment)).
We need push variations only if NOT exist already.
Consider to extract this building flag-variations map as a separate function and add a test case.

end
end
@flag_variation_map[flag['key']] = variations
end

@all_experiments = @experiment_id_map.merge(@rollout_experiment_id_map)
@all_experiments.each do |id, exp|
variations = exp.fetch('variations')
Expand Down Expand Up @@ -165,6 +179,24 @@ def initialize(datafile, logger, error_handler)
end
end

def get_rules_for_flag(feature_flag)
# Retrieves rules for a given feature flag
#
# feature_flag - String key representing the feature_flag
#
# Returns rules in feature flag
rules = feature_flag['experimentIds'].map { |exp_id| @experiment_id_map[exp_id] }
rollout = feature_flag['rolloutId'].empty? ? nil : @rollout_id_map[feature_flag['rolloutId']]

if rollout
rollout_experiments = rollout.fetch('experiments')
rollout_experiments.each do |exp|
rules.push(exp)
end
end
rules
end

def self.create(datafile, logger, error_handler, skip_json_validation)
# Looks up and sets datafile and config based on response body.
#
Expand Down Expand Up @@ -279,6 +311,13 @@ def get_audience_from_id(audience_id)
nil
end

def get_variation_from_flag(flag_key, variation_key)
variations = @flag_variation_map[flag_key]
return variations.select { |variation| variation['key'] == variation_key }.first if variations

nil
end

def get_variation_from_id(experiment_key, variation_id)
# Get variation given experiment key and variation ID
#
Expand Down
Loading