Skip to content

feat(ForcedDecisions): add forced-decisions APIs to OptimizelyUserContext #287

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

Merged
merged 15 commits into from
Oct 22, 2021
Merged

feat(ForcedDecisions): add forced-decisions APIs to OptimizelyUserContext #287

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0d57a23
added forced variation basic functions
ozayr-zaviar Sep 23, 2021
f5c9354
implementation complete
ozayr-zaviar Sep 30, 2021
83ac8e2
fixes and testcases added
ozayr-zaviar Oct 3, 2021
062773b
methods deprecated
ozayr-zaviar Oct 3, 2021
6a69d5e
deprecated methods testcases corrected
ozayr-zaviar Oct 3, 2021
8d24927
extra lines removed
ozayr-zaviar Oct 3, 2021
a7953de
Merge branch 'master' into uzair/forced-decision
ozayr-zaviar Oct 3, 2021
31241ed
issues resilved and forced decision converted to map
ozayr-zaviar Oct 4, 2021
9a9826e
word corrected in test case description
ozayr-zaviar Oct 5, 2021
a0893b1
deprecation warnings removed
ozayr-zaviar Oct 7, 2021
eb77199
Merge branch 'uzair/forced-decision' of https://github.com/optimizely…
ozayr-zaviar Oct 7, 2021
121993b
All FSC testcases fixed
ozayr-zaviar Oct 11, 2021
14914c7
rubocop offences resolved
ozayr-zaviar Oct 11, 2021
bb221a5
fix for new FSC changes
ozayr-zaviar Oct 11, 2021
c3647ad
add comments addressed
ozayr-zaviar Oct 14, 2021
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions lib/optimizely.rb
View file
Open in desktop
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 @@ -490,7 +501,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 @@ -739,7 +751,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 @@ -881,7 +894,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 +961,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

@msohailhussain msohailhussain Oct 20, 2021

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 Expand Up @@ -1083,8 +1098,12 @@ def send_impression(config, experiment, variation_key, flag_key, rule_key, enabl
experiment_id = experiment['id']
experiment_key = experiment['key']

variation_id = ''
variation_id = config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) if experiment_id != ''
if experiment_id != ''
variation_id = config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key)
else
varaition = get_flag_variation_by_key(flag_key, variation_key)
variation_id = varaition ? varaition['id'] : ''
end

metadata = {
flag_key: flag_key,
Expand Down
39 changes: 39 additions & 0 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 @@ -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

@jaeopt jaeopt Oct 6, 2021

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 •
edited
Loading

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

@jaeopt jaeopt Oct 13, 2021

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