From 75fc548527388565a4d3be89a29884fc56d0b335 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf <35262377+zashraf1985@users.noreply.github.com> Date: Thu, 7 Jan 2021 08:53:55 -0800 Subject: [PATCH] refactor: Changed the way reasons are being returned from decide APIs (#279) ## Summary Reasons were previously being collected by sending a reasons object down the stack to all the related functions which would mutate it by pushing their own reasons. This is now changed. Every related function now returns the reasons array along with the actual value. ## Test plan - Manually tested thoroughly. - Fixed all the affected unit test. - FSC passing --- lib/optimizely.rb | 13 +- lib/optimizely/audience.rb | 57 ++---- lib/optimizely/bucketer.rb | 41 ++-- lib/optimizely/decision_service.rb | 188 ++++++++++-------- spec/audience_spec.rb | 92 +++++---- spec/bucketing_spec.rb | 61 ++++-- spec/decision_service_spec.rb | 300 ++++++++++++++++++++++------- spec/project_spec.rb | 24 ++- 8 files changed, 498 insertions(+), 278 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 504eeaf7..8e4ec873 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -199,7 +199,8 @@ def decide(user_context, key, decide_options = []) experiment = nil decision_source = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] - decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes, decide_options, reasons) + decision, reasons_received = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes, decide_options) + reasons.push(*reasons_received) # Send impression event if Decision came from a feature test and decide options doesn't include disableDecisionEvent if decision.is_a?(Optimizely::DecisionService::Decision) @@ -405,7 +406,7 @@ def get_forced_variation(experiment_key, user_id) config = project_config forced_variation_key = nil - forced_variation = @decision_service.get_forced_variation(config, experiment_key, user_id) + forced_variation, = @decision_service.get_forced_variation(config, experiment_key, user_id) forced_variation_key = forced_variation['key'] if forced_variation forced_variation_key @@ -489,7 +490,7 @@ 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) + decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes) feature_enabled = false source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] @@ -738,7 +739,7 @@ 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) + decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes) variation = decision ? decision['variation'] : nil feature_enabled = variation ? variation['featureEnabled'] : false all_variables = {} @@ -878,7 +879,7 @@ 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_key, user_id, attributes) + variation_id, = @decision_service.get_variation(config, experiment_key, user_id, attributes) 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']) @@ -944,7 +945,7 @@ 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) + decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes) variation = decision ? decision['variation'] : nil feature_enabled = variation ? variation['featureEnabled'] : false diff --git a/lib/optimizely/audience.rb b/lib/optimizely/audience.rb index d809b773..0a2a6a2e 100644 --- a/lib/optimizely/audience.rb +++ b/lib/optimizely/audience.rb @@ -38,6 +38,7 @@ def user_meets_audience_conditions?(config, experiment, attributes, logger, logg # This defaults to experiment['key']. # # Returns boolean representing if user satisfies audience conditions for the audiences or not. + decide_reasons = [] logging_hash ||= 'EXPERIMENT_AUDIENCE_EVALUATION_LOGS' logging_key ||= experiment['key'] @@ -45,26 +46,15 @@ def user_meets_audience_conditions?(config, experiment, attributes, logger, logg audience_conditions = experiment['audienceConditions'] || experiment['audienceIds'] - logger.log( - Logger::DEBUG, - format( - logs_hash['EVALUATING_AUDIENCES_COMBINED'], - logging_key, - audience_conditions - ) - ) + message = format(logs_hash['EVALUATING_AUDIENCES_COMBINED'], logging_key, audience_conditions) + logger.log(Logger::DEBUG, message) # Return true if there are no audiences if audience_conditions.empty? - logger.log( - Logger::INFO, - format( - logs_hash['AUDIENCE_EVALUATION_RESULT_COMBINED'], - logging_key, - 'TRUE' - ) - ) - return true + message = format(logs_hash['AUDIENCE_EVALUATION_RESULT_COMBINED'], logging_key, 'TRUE') + logger.log(Logger::INFO, message) + decide_reasons.push(message) + return true, decide_reasons end attributes ||= {} @@ -80,39 +70,28 @@ def user_meets_audience_conditions?(config, experiment, attributes, logger, logg return nil unless audience audience_conditions = audience['conditions'] - logger.log( - Logger::DEBUG, - format( - logs_hash['EVALUATING_AUDIENCE'], - audience_id, - audience_conditions - ) - ) + message = format(logs_hash['EVALUATING_AUDIENCE'], audience_id, audience_conditions) + logger.log(Logger::DEBUG, message) + decide_reasons.push(message) audience_conditions = JSON.parse(audience_conditions) if audience_conditions.is_a?(String) result = ConditionTreeEvaluator.evaluate(audience_conditions, evaluate_custom_attr) result_str = result.nil? ? 'UNKNOWN' : result.to_s.upcase - logger.log( - Logger::DEBUG, - format(logs_hash['AUDIENCE_EVALUATION_RESULT'], audience_id, result_str) - ) + message = format(logs_hash['AUDIENCE_EVALUATION_RESULT'], audience_id, result_str) + logger.log(Logger::DEBUG, message) + decide_reasons.push(message) + result end eval_result = ConditionTreeEvaluator.evaluate(audience_conditions, evaluate_audience) - eval_result ||= false - logger.log( - Logger::INFO, - format( - logs_hash['AUDIENCE_EVALUATION_RESULT_COMBINED'], - logging_key, - eval_result.to_s.upcase - ) - ) + message = format(logs_hash['AUDIENCE_EVALUATION_RESULT_COMBINED'], logging_key, eval_result.to_s.upcase) + logger.log(Logger::INFO, message) + decide_reasons.push(message) - eval_result + [eval_result, decide_reasons] end end end diff --git a/lib/optimizely/bucketer.rb b/lib/optimizely/bucketer.rb index 1a4994f6..0400f354 100644 --- a/lib/optimizely/bucketer.rb +++ b/lib/optimizely/bucketer.rb @@ -35,7 +35,7 @@ def initialize(logger) @bucket_seed = HASH_SEED end - def bucket(project_config, experiment, bucketing_id, user_id, decide_reasons = nil) + def bucket(project_config, experiment, bucketing_id, user_id) # Determines ID of variation to be shown for a given experiment key and user ID. # # project_config - Instance of ProjectConfig @@ -44,7 +44,9 @@ def bucket(project_config, experiment, bucketing_id, user_id, decide_reasons = n # user_id - String ID for user. # # Returns variation in which visitor with ID user_id has been placed. Nil if no variation. - return nil if experiment.nil? + return nil, [] if experiment.nil? + + decide_reasons = [] # check if experiment is in a group; if so, check if user is bucketed into specified experiment # this will not affect evaluation of rollout rules. @@ -55,48 +57,52 @@ def bucket(project_config, experiment, bucketing_id, user_id, decide_reasons = n group = project_config.group_id_map.fetch(group_id) if Helpers::Group.random_policy?(group) traffic_allocations = group.fetch('trafficAllocation') - bucketed_experiment_id = find_bucket(bucketing_id, user_id, group_id, traffic_allocations) + bucketed_experiment_id, find_bucket_reasons = find_bucket(bucketing_id, user_id, group_id, traffic_allocations) + decide_reasons.push(*find_bucket_reasons) + # return if the user is not bucketed into any experiment unless bucketed_experiment_id message = "User '#{user_id}' is in no experiment." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end # return if the user is bucketed into a different experiment than the one specified if bucketed_experiment_id != experiment_id message = "User '#{user_id}' is not in experiment '#{experiment_key}' of group #{group_id}." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end # continue bucketing if the user is bucketed into the experiment specified message = "User '#{user_id}' is in experiment '#{experiment_key}' of group #{group_id}." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) + decide_reasons.push(message) end end traffic_allocations = experiment['trafficAllocation'] - variation_id = find_bucket(bucketing_id, user_id, experiment_id, traffic_allocations, decide_reasons) + variation_id, find_bucket_reasons = find_bucket(bucketing_id, user_id, experiment_id, traffic_allocations) + decide_reasons.push(*find_bucket_reasons) + if variation_id && variation_id != '' variation = project_config.get_variation_from_id(experiment_key, variation_id) - return variation + return variation, decide_reasons end # Handle the case when the traffic range is empty due to sticky bucketing if variation_id == '' message = 'Bucketed into an empty traffic range. Returning nil.' @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) + decide_reasons.push(message) end - nil + [nil, decide_reasons] end - def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations, decide_reasons = nil) + def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations) # Helper function to find the matching entity ID for a given bucketing value in a list of traffic allocations. # # bucketing_id - String A customer-assigned value user to generate bucketing key @@ -104,23 +110,24 @@ def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations, decide_re # parent_id - String entity ID to use for bucketing ID # traffic_allocations - Array of traffic allocations # - # Returns entity ID corresponding to the provided bucket value or nil if no match is found. + # Returns and array of two values where first value is the entity ID corresponding to the provided bucket value + # or nil if no match is found. The second value contains the array of reasons stating how the deicision was taken + decide_reasons = [] bucketing_key = format(BUCKETING_ID_TEMPLATE, bucketing_id: bucketing_id, entity_id: parent_id) bucket_value = generate_bucket_value(bucketing_key) message = "Assigned bucket #{bucket_value} to user '#{user_id}' with bucketing ID: '#{bucketing_id}'." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) traffic_allocations.each do |traffic_allocation| current_end_of_range = traffic_allocation['endOfRange'] if bucket_value < current_end_of_range entity_id = traffic_allocation['entityId'] - return entity_id + return entity_id, decide_reasons end end - nil + [nil, decide_reasons] end private diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 77df3635..ecb5c534 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -52,7 +52,7 @@ def initialize(logger, user_profile_service = nil) @forced_variation_map = {} end - def get_variation(project_config, experiment_key, user_id, attributes = nil, decide_options = [], decide_reasons = nil) + def get_variation(project_config, experiment_key, user_id, attributes = nil, decide_options = []) # Determines variation into which user will be bucketed. # # project_config - project_config - Instance of ProjectConfig @@ -63,51 +63,60 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec # Returns variation ID where visitor will be bucketed # (nil if experiment is inactive or user does not meet audience conditions) + decide_reasons = [] # By default, the bucketing ID should be the user ID - bucketing_id = get_bucketing_id(user_id, attributes, decide_reasons) + bucketing_id, bucketing_id_reasons = get_bucketing_id(user_id, attributes) + decide_reasons.push(*bucketing_id_reasons) # Check to make sure experiment is active experiment = project_config.get_experiment_from_key(experiment_key) - return nil if experiment.nil? + return nil, decide_reasons if experiment.nil? experiment_id = experiment['id'] unless project_config.experiment_running?(experiment) message = "Experiment '#{experiment_key}' is not running." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end # Check if a forced variation is set for the user - forced_variation = get_forced_variation(project_config, experiment_key, user_id, decide_reasons) - return forced_variation['id'] if forced_variation + forced_variation, reasons_received = get_forced_variation(project_config, experiment_key, user_id) + decide_reasons.push(*reasons_received) + return forced_variation['id'], decide_reasons if forced_variation # Check if user is in a white-listed variation - whitelisted_variation_id = get_whitelisted_variation_id(project_config, experiment_key, user_id, decide_reasons) - return whitelisted_variation_id if whitelisted_variation_id + whitelisted_variation_id, reasons_received = get_whitelisted_variation_id(project_config, experiment_key, user_id) + decide_reasons.push(*reasons_received) + return whitelisted_variation_id, decide_reasons if whitelisted_variation_id should_ignore_user_profile_service = decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE # Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService unless should_ignore_user_profile_service - user_profile = get_user_profile(user_id, decide_reasons) - saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile, decide_reasons) + user_profile, reasons_received = get_user_profile(user_id) + decide_reasons.push(*reasons_received) + saved_variation_id, reasons_received = get_saved_variation_id(project_config, experiment_id, user_profile) + decide_reasons.push(*reasons_received) if saved_variation_id message = "Returning previously activated variation ID #{saved_variation_id} of experiment '#{experiment_key}' for user '#{user_id}' from user profile." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - return saved_variation_id + decide_reasons.push(message) + return saved_variation_id, decide_reasons end end # Check audience conditions - unless Audience.user_meets_audience_conditions?(project_config, experiment, attributes, @logger) + user_meets_audience_conditions, reasons_received = Audience.user_meets_audience_conditions?(project_config, experiment, attributes, @logger) + decide_reasons.push(*reasons_received) + unless user_meets_audience_conditions message = "User '#{user_id}' does not meet the conditions to be in experiment '#{experiment_key}'." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end # Bucket normally - variation = @bucketer.bucket(project_config, experiment, bucketing_id, user_id, decide_reasons) + variation, bucket_reasons = @bucketer.bucket(project_config, experiment, bucketing_id, user_id) + decide_reasons.push(*bucket_reasons) variation_id = variation ? variation['id'] : nil message = '' @@ -118,14 +127,14 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec message = "User '#{user_id}' is in no variation." end @logger.log(Logger::INFO, message) - decide_reasons&.push(message) + decide_reasons.push(message) # Persist bucketing decision save_user_profile(user_profile, experiment_id, variation_id) unless should_ignore_user_profile_service - variation_id + [variation_id, decide_reasons] end - def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil, decide_options = [], decide_reasons = nil) + def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil, decide_options = []) # Get the variation the user is bucketed into for the given FeatureFlag. # # project_config - project_config - Instance of ProjectConfig @@ -135,16 +144,20 @@ def get_variation_for_feature(project_config, feature_flag, user_id, attributes # # Returns Decision struct (nil if the user is not bucketed into any of the experiments on the feature) + decide_reasons = [] + # check if the feature is being experiment on and whether the user is bucketed into the experiment - decision = get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes, decide_options, decide_reasons) - return decision unless decision.nil? + decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes, decide_options) + decide_reasons.push(*reasons_received) + return decision, decide_reasons unless decision.nil? - decision = get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes, decide_reasons) + decision, reasons_received = get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes) + decide_reasons.push(*reasons_received) - decision + [decision, decide_reasons] end - def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, decide_options = [], decide_reasons = nil) + def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, decide_options = []) # Gets the variation the user is bucketed into for the feature flag's experiment. # # project_config - project_config - Instance of ProjectConfig @@ -154,12 +167,13 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_id, # # Returns Decision struct (nil if the user is not bucketed into any of the experiments on the feature) # or nil if the user is not bucketed into any of the experiments on the feature + decide_reasons = [] feature_flag_key = feature_flag['key'] if feature_flag['experimentIds'].empty? message = "The feature flag '#{feature_flag_key}' is not used in any experiments." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end # Evaluate each experiment and return the first bucketed experiment variation @@ -168,28 +182,29 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_id, unless experiment message = "Feature flag experiment with ID '#{experiment_id}' is not in the datafile." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end experiment_key = experiment['key'] - variation_id = get_variation(project_config, experiment_key, user_id, attributes, decide_options, decide_reasons) + variation_id, reasons_received = get_variation(project_config, experiment_key, user_id, attributes, decide_options) + decide_reasons.push(*reasons_received) next unless variation_id variation = project_config.variation_id_map[experiment_key][variation_id] - return Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST']) + return Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST']), decide_reasons end message = "The user '#{user_id}' is not bucketed into any of the experiments on the feature '#{feature_flag_key}'." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) + decide_reasons.push(message) - nil + [nil, decide_reasons] end - def get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes = nil, decide_reasons = nil) + def get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes = nil) # Determine which variation the user is in for a given rollout. # Returns the variation of the first experiment the user qualifies for. # @@ -199,25 +214,27 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_id, att # attributes - Hash representing user attributes # # Returns the Decision struct or nil if not bucketed into any of the targeting rules - bucketing_id = get_bucketing_id(user_id, attributes, decide_reasons) + decide_reasons = [] + bucketing_id, bucketing_id_reasons = get_bucketing_id(user_id, attributes) + decide_reasons.push(*bucketing_id_reasons) rollout_id = feature_flag['rolloutId'] if rollout_id.nil? || rollout_id.empty? feature_flag_key = feature_flag['key'] message = "Feature flag '#{feature_flag_key}' is not used in a rollout." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end rollout = project_config.get_rollout_from_id(rollout_id) if rollout.nil? message = "Rollout with ID '#{rollout_id}' is not in the datafile '#{feature_flag['key']}'" @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end - return nil if rollout['experiments'].empty? + return nil, decide_reasons if rollout['experiments'].empty? rollout_rules = rollout['experiments'] number_of_rules = rollout_rules.length - 1 @@ -227,22 +244,25 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_id, att rollout_rule = rollout_rules[index] logging_key = index + 1 + user_meets_audience_conditions, reasons_received = Audience.user_meets_audience_conditions?(project_config, rollout_rule, attributes, @logger, 'ROLLOUT_AUDIENCE_EVALUATION_LOGS', logging_key) + decide_reasons.push(*reasons_received) # Check that user meets audience conditions for targeting rule - unless Audience.user_meets_audience_conditions?(project_config, rollout_rule, attributes, @logger, 'ROLLOUT_AUDIENCE_EVALUATION_LOGS', logging_key) + unless user_meets_audience_conditions message = "User '#{user_id}' does not meet the audience conditions for targeting rule '#{logging_key}'." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) + decide_reasons.push(message) # move onto the next targeting rule next end message = "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) + decide_reasons.push(message) # Evaluate if user satisfies the traffic allocation for this rollout rule - variation = @bucketer.bucket(project_config, rollout_rule, bucketing_id, user_id, decide_reasons) - return Decision.new(rollout_rule, variation, DECISION_SOURCES['ROLLOUT']) unless variation.nil? + variation, bucket_reasons = @bucketer.bucket(project_config, rollout_rule, bucketing_id, user_id) + decide_reasons.push(*bucket_reasons) + return Decision.new(rollout_rule, variation, DECISION_SOURCES['ROLLOUT']), decide_reasons unless variation.nil? break end @@ -250,22 +270,26 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_id, att # get last rule which is the everyone else rule everyone_else_experiment = rollout_rules[number_of_rules] logging_key = 'Everyone Else' + + user_meets_audience_conditions, reasons_received = Audience.user_meets_audience_conditions?(project_config, everyone_else_experiment, attributes, @logger, 'ROLLOUT_AUDIENCE_EVALUATION_LOGS', logging_key) + decide_reasons.push(*reasons_received) # Check that user meets audience conditions for last rule - unless Audience.user_meets_audience_conditions?(project_config, everyone_else_experiment, attributes, @logger, 'ROLLOUT_AUDIENCE_EVALUATION_LOGS', logging_key) + unless user_meets_audience_conditions message = "User '#{user_id}' does not meet the audience conditions for targeting rule '#{logging_key}'." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end message = "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) + decide_reasons.push(message) - variation = @bucketer.bucket(project_config, everyone_else_experiment, bucketing_id, user_id, decide_reasons) - return Decision.new(everyone_else_experiment, variation, DECISION_SOURCES['ROLLOUT']) unless variation.nil? + variation, bucket_reasons = @bucketer.bucket(project_config, everyone_else_experiment, bucketing_id, user_id) + decide_reasons.push(*bucket_reasons) + return Decision.new(everyone_else_experiment, variation, DECISION_SOURCES['ROLLOUT']), decide_reasons unless variation.nil? - nil + [nil, decide_reasons] end def set_forced_variation(project_config, experiment_key, user_id, variation_key) @@ -306,7 +330,7 @@ def set_forced_variation(project_config, experiment_key, user_id, variation_key) true end - def get_forced_variation(project_config, experiment_key, user_id, decide_reasons = nil) + def get_forced_variation(project_config, experiment_key, user_id) # Gets the forced variation for the given user and experiment. # # project_config - Instance of ProjectConfig @@ -315,11 +339,11 @@ def get_forced_variation(project_config, experiment_key, user_id, decide_reasons # # Returns Variation The variation which the given user and experiment should be forced into + decide_reasons = [] unless @forced_variation_map.key? user_id message = "User '#{user_id}' is not in the forced variation map." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + return nil, decide_reasons end experiment_to_variation_map = @forced_variation_map[user_id] @@ -327,13 +351,13 @@ def get_forced_variation(project_config, experiment_key, user_id, decide_reasons experiment_id = experiment['id'] if experiment # check for nil and empty string experiment ID # this case is logged in get_experiment_from_key - return nil if experiment_id.nil? || experiment_id.empty? + return nil, decide_reasons if experiment_id.nil? || experiment_id.empty? unless experiment_to_variation_map.key? experiment_id message = "No experiment '#{experiment_key}' mapped to user '#{user_id}' in the forced variation map." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) - return nil + decide_reasons.push(message) + return nil, decide_reasons end variation_id = experiment_to_variation_map[experiment_id] @@ -343,18 +367,18 @@ def get_forced_variation(project_config, experiment_key, user_id, decide_reasons # check if the variation exists in the datafile # this case is logged in get_variation_from_id - return nil if variation_key.empty? + return nil, decide_reasons if variation_key.empty? message = "Variation '#{variation_key}' is mapped to experiment '#{experiment_key}' and user '#{user_id}' in the forced variation map" @logger.log(Logger::DEBUG, message) - decide_reasons&.push(message) + decide_reasons.push(message) - variation + [variation, decide_reasons] end private - def get_whitelisted_variation_id(project_config, experiment_key, user_id, decide_reasons = nil) + def get_whitelisted_variation_id(project_config, experiment_key, user_id) # Determine if a user is whitelisted into a variation for the given experiment and return the ID of that variation # # project_config - project_config - Instance of ProjectConfig @@ -365,29 +389,27 @@ def get_whitelisted_variation_id(project_config, experiment_key, user_id, decide whitelisted_variations = project_config.get_whitelisted_variations(experiment_key) - return nil unless whitelisted_variations + return nil, nil unless whitelisted_variations whitelisted_variation_key = whitelisted_variations[user_id] - return nil unless whitelisted_variation_key + return nil, nil unless whitelisted_variation_key whitelisted_variation_id = project_config.get_variation_id_from_key(experiment_key, whitelisted_variation_key) unless whitelisted_variation_id message = "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}', which is not in the datafile." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - return nil + return nil, message end message = "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}' of experiment '#{experiment_key}'." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - whitelisted_variation_id + [whitelisted_variation_id, message] end - def get_saved_variation_id(project_config, experiment_id, user_profile, decide_reasons = nil) + def get_saved_variation_id(project_config, experiment_id, user_profile) # Retrieve variation ID of stored bucketing decision for a given experiment from a given user profile # # project_config - project_config - Instance of ProjectConfig @@ -395,22 +417,21 @@ def get_saved_variation_id(project_config, experiment_id, user_profile, decide_r # user_profile - Hash user profile # # Returns string variation ID (nil if no decision is found) - return nil unless user_profile[:experiment_bucket_map] + return nil, nil unless user_profile[:experiment_bucket_map] decision = user_profile[:experiment_bucket_map][experiment_id] - return nil unless decision + return nil, nil unless decision variation_id = decision[:variation_id] - return variation_id if project_config.variation_id_exists?(experiment_id, variation_id) + return variation_id, nil if project_config.variation_id_exists?(experiment_id, variation_id) - message = "User '#{user_profile['user_id']}' was previously bucketed into variation ID '#{variation_id}' for experiment '#{experiment_id}', but no matching variation was found. Re-bucketing user." + message = "User '#{user_profile[:user_id]}' was previously bucketed into variation ID '#{variation_id}' for experiment '#{experiment_id}', but no matching variation was found. Re-bucketing user." @logger.log(Logger::INFO, message) - decide_reasons&.push(message) - nil + [nil, message] end - def get_user_profile(user_id, decide_reasons = nil) + def get_user_profile(user_id) # Determine if a user is forced into a variation for the given experiment and return the ID of that variation # # user_id - String ID for the user @@ -422,17 +443,17 @@ def get_user_profile(user_id, decide_reasons = nil) experiment_bucket_map: {} } - return user_profile unless @user_profile_service + return user_profile, nil unless @user_profile_service + message = nil begin user_profile = @user_profile_service.lookup(user_id) || user_profile rescue => e message = "Error while looking up user profile for user ID '#{user_id}': #{e}." @logger.log(Logger::ERROR, message) - decide_reasons&.push(message) end - user_profile + [user_profile, message] end def save_user_profile(user_profile, experiment_id, variation_id) @@ -456,25 +477,24 @@ def save_user_profile(user_profile, experiment_id, variation_id) end end - def get_bucketing_id(user_id, attributes, decide_reasons = nil) + def get_bucketing_id(user_id, attributes) # Gets the Bucketing Id for Bucketing # # user_id - String user ID # attributes - Hash user attributes # Returns String representing bucketing ID if it is a String type in attributes else return user ID - return user_id unless attributes + return user_id, nil unless attributes bucketing_id = attributes[Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID']] if bucketing_id - return bucketing_id if bucketing_id.is_a?(String) + return bucketing_id, nil if bucketing_id.is_a?(String) message = 'Bucketing ID attribute is not a string. Defaulted to user ID.' @logger.log(Logger::WARN, message) - decide_reasons&.push(message) end - user_id + [user_id, message] end end end diff --git a/spec/audience_spec.rb b/spec/audience_spec.rb index dd490e35..1f304a97 100644 --- a/spec/audience_spec.rb +++ b/spec/audience_spec.rb @@ -34,27 +34,25 @@ expect(Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, - spy_logger)).to be true + spy_logger)[0]).to be true # Audience Ids exist but Audience Conditions is Empty experiment = config.experiment_key_map['test_experiment'] experiment['audienceIds'] = ['11154'] experiment['audienceConditions'] = [] - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be true + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be true + expect(reasons).to eq(["Audiences for experiment 'test_experiment' collectively evaluated to TRUE."]) # Audience Ids is Empty and Audience Conditions is nil experiment = config.experiment_key_map['test_experiment'] experiment['audienceIds'] = [] experiment['audienceConditions'] = nil - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be true + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be true + expect(reasons).to eq(["Audiences for experiment 'test_experiment' collectively evaluated to TRUE."]) end it 'should pass conditions when audience conditions exist else audienceIds are passed' do @@ -85,15 +83,23 @@ allow(Optimizely::CustomAttributeConditionEvaluator).to receive(:new).and_call_original # attributes set to empty dict - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - {}, - spy_logger)).to be false + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, {}, spy_logger) + expect(user_meets_audience_conditions).to be false + expect(reasons).to eq([ + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE." + ]) + # attributes set to nil - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - nil, - spy_logger)).to be false + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, nil, spy_logger) + expect(user_meets_audience_conditions).to be false + expect(reasons).to eq([ + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE." + ]) + # asserts nil attributes default to empty dict expect(Optimizely::CustomAttributeConditionEvaluator).to have_received(:new).with({}, spy_logger).twice end @@ -104,10 +110,11 @@ 'test_attribute' => 'test_value_1' } allow(Optimizely::ConditionTreeEvaluator).to receive(:evaluate).and_return(true) - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be true + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be true + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE." + ]) end it 'should return false for user_meets_audience_conditions? when condition tree evaluator returns false or nil' do @@ -118,17 +125,20 @@ # condition tree evaluator returns nil allow(Optimizely::ConditionTreeEvaluator).to receive(:evaluate).and_return(nil) - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be false + + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be false + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE." + ]) # condition tree evaluator returns false allow(Optimizely::ConditionTreeEvaluator).to receive(:evaluate).and_return(false) - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be false + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be false + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE." + ]) end it 'should correctly evaluate audience Ids and call custom attribute evaluator for leaf nodes' do @@ -213,10 +223,12 @@ } experiment['audienceIds'] = %w[11110] - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be false + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be false + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE." + ]) + expect(spy_logger).to have_received(:log).once.with( Logger::DEBUG, "Evaluating audiences for experiment 'test_experiment_with_audience': " + '["11110"].' @@ -236,10 +248,16 @@ experiment['audienceIds'] = %w[11154 11155] experiment['audienceConditions'] = nil - expect(Optimizely::Audience.user_meets_audience_conditions?(config, - experiment, - user_attributes, - spy_logger)).to be false + user_meets_audience_conditions, reasons = Optimizely::Audience.user_meets_audience_conditions?(config, experiment, user_attributes, spy_logger) + expect(user_meets_audience_conditions).to be false + expect(reasons).to eq([ + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Starting to evaluate audience '11155' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"chrome\"}]]].", + "Audience '11155' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE." + ]) + expect(spy_logger).to have_received(:log).once.with( Logger::DEBUG, "Evaluating audiences for experiment 'test_experiment_with_audience': " + '["11154", "11155"].' diff --git a/spec/bucketing_spec.rb b/spec/bucketing_spec.rb index 98a5b711..be3c4e48 100644 --- a/spec/bucketing_spec.rb +++ b/spec/bucketing_spec.rb @@ -40,14 +40,17 @@ def get_bucketing_key(bucketing_id, entity_id = nil) # Variation 1 expected_variation_1 = config.get_variation_from_id('test_experiment', '111128') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to eq(expected_variation_1) + variation_received, = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to eq(expected_variation_1) # Variation 2 expected_variation_2 = config.get_variation_from_id('test_experiment', '111129') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to eq(expected_variation_2) + variation_received, = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to eq(expected_variation_2) # No matching variation - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to be_nil + variation_received, = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to be_nil end it 'should test the output of generate_bucket_value for different inputs' do @@ -66,7 +69,8 @@ def get_bucketing_key(bucketing_id, entity_id = nil) experiment = config.get_experiment_from_key('group1_exp1') expected_variation = config.get_variation_from_id('group1_exp1', '130001') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to eq(expected_variation) + variation_received, = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to eq(expected_variation) expect(spy_logger).to have_received(:log).exactly(3).times expect(spy_logger).to have_received(:log).twice .with(Logger::DEBUG, "Assigned bucket 3000 to user 'test_user' with bucketing ID: 'bucket_id_ignored'.") @@ -78,7 +82,11 @@ def get_bucketing_key(bucketing_id, entity_id = nil) expect(bucketer).to receive(:generate_bucket_value).once.and_return(3000) experiment = config.get_experiment_from_key('group1_exp2') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to be_nil + variation_received, reasons = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to be_nil + expect(reasons).to eq([ + "User 'test_user' is not in experiment 'group1_exp2' of group 101." + ]) expect(spy_logger).to have_received(:log) .with(Logger::DEBUG, "Assigned bucket 3000 to user 'test_user' with bucketing ID: 'bucket_id_ignored'.") expect(spy_logger).to have_received(:log) @@ -86,10 +94,12 @@ def get_bucketing_key(bucketing_id, entity_id = nil) end it 'should return nil when user is not bucketed into any bucket' do - expect(bucketer).to receive(:find_bucket).once.and_return(nil) + expect(bucketer).to receive(:find_bucket).once.and_return([nil, nil]) experiment = config.get_experiment_from_key('group1_exp2') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to be_nil + variation_received, reasons = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to be_nil + expect(reasons).to eq(["User 'test_user' is in no experiment."]) expect(spy_logger).to have_received(:log) .with(Logger::INFO, "User 'test_user' is in no experiment.") end @@ -99,7 +109,9 @@ def get_bucketing_key(bucketing_id, entity_id = nil) experiment = config.get_experiment_from_key('group2_exp1') expected_variation = config.get_variation_from_id('group2_exp1', '144443') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to eq(expected_variation) + variation_received, reasons = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to eq(expected_variation) + expect(reasons).to eq([]) expect(spy_logger).to have_received(:log).once expect(spy_logger).to have_received(:log) .with(Logger::DEBUG, "Assigned bucket 3000 to user 'test_user' with bucketing ID: 'bucket_id_ignored'.") @@ -109,7 +121,9 @@ def get_bucketing_key(bucketing_id, entity_id = nil) expect(bucketer).to receive(:generate_bucket_value).and_return(50_000) experiment = config.get_experiment_from_key('group2_exp1') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to be_nil + variation_received, reasons = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(reasons).to eq([]) + expect(variation_received).to be_nil expect(spy_logger).to have_received(:log).once expect(spy_logger).to have_received(:log) .with(Logger::DEBUG, "Assigned bucket 50000 to user 'test_user' with bucketing ID: 'bucket_id_ignored'.") @@ -138,7 +152,9 @@ def get_bucketing_key(bucketing_id, entity_id = nil) it 'should return nil when user is in an empty traffic allocation range due to sticky bucketing' do expect(bucketer).to receive(:find_bucket).once.and_return('') experiment = config.get_experiment_from_key('test_experiment') - expect(bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user')).to be_nil + variation_received, reasons = bucketer.bucket(config, experiment, 'bucket_id_ignored', 'test_user') + expect(variation_received).to be_nil + expect(reasons).to eq(['Bucketed into an empty traffic range. Returning nil.']) expect(spy_logger).to have_received(:log) .with(Logger::DEBUG, 'Bucketed into an empty traffic range. Returning nil.') end @@ -151,17 +167,23 @@ def get_bucketing_key(bucketing_id, entity_id = nil) # Bucketing with user id as bucketing id - 'test_user111127' produces bucket value < 5000 thus buckets to control expected_variation = config.get_variation_from_id('test_experiment', '111128') - expect(bucketer.bucket(config, experiment, 'test_user', 'test_user')).to be(expected_variation) + variation_received, reasons = bucketer.bucket(config, experiment, 'test_user', 'test_user') + expect(variation_received).to be(expected_variation) + expect(reasons).to eq([]) # Bucketing with bucketing id - 'any_string789111127' produces bucket value btw 5000 to 10,000 # thus buckets to variation expected_variation = config.get_variation_from_id('test_experiment', '111129') - expect(bucketer.bucket(config, experiment, 'any_string789', 'test_user')).to be(expected_variation) + variation_received, reasons = bucketer.bucket(config, experiment, 'any_string789', 'test_user') + expect(variation_received).to be(expected_variation) + expect(reasons).to eq([]) end # Bucketing with invalid experiment key and bucketing ID it 'should return nil with invalid experiment and bucketing ID' do - expect(bucketer.bucket(config, config.get_experiment_from_key('invalid_experiment'), 'some_id', 'test_user')).to be(nil) + variation_received, reasons = bucketer.bucket(config, config.get_experiment_from_key('invalid_experiment'), 'some_id', 'test_user') + expect(variation_received).to be(nil) + expect(reasons).to eq([]) end # Bucketing with grouped experiments and bucketing ID @@ -170,10 +192,17 @@ def get_bucketing_key(bucketing_id, entity_id = nil) experiment = config.get_experiment_from_key('group1_exp1') expected_variation = nil - expect(bucketer.bucket(config, experiment, 'test_user', 'test_user')).to be(expected_variation) - + variation_received, reasons = bucketer.bucket(config, experiment, 'test_user', 'test_user') + expect(variation_received).to be(expected_variation) + expect(reasons).to eq([ + "User 'test_user' is not in experiment 'group1_exp1' of group 101." + ]) expected_variation = config.get_variation_from_id('group1_exp1', '130002') - expect(bucketer.bucket(config, experiment, '123456789', 'test_user')).to be(expected_variation) + variation_received, = bucketer.bucket(config, experiment, '123456789', 'test_user') + expect(variation_received).to be(expected_variation) + expect(reasons).to eq([ + "User 'test_user' is not in experiment 'group1_exp1' of group 101." + ]) end end diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 7cdc65b5..2f149b06 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -42,7 +42,9 @@ it 'should return the correct variation ID for a given user for whom a variation has been forced' do decision_service.set_forced_variation(config, 'test_experiment', 'test_user', 'variation') - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111129') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111129') + expect(reasons).to eq(["Variation 'variation' is mapped to experiment 'test_experiment' and user 'test_user' in the forced variation map"]) # Setting forced variation should short circuit whitelist check, bucketing and audience evaluation expect(decision_service).not_to have_received(:get_whitelisted_variation_id) expect(decision_service.bucketer).not_to have_received(:bucket) @@ -55,7 +57,9 @@ Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 'pid' } decision_service.set_forced_variation(config, 'test_experiment_with_audience', 'test_user', 'control_with_audience') - expect(decision_service.get_variation(config, 'test_experiment_with_audience', 'test_user', user_attributes)).to eq('122228') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment_with_audience', 'test_user', user_attributes) + expect(variation_received).to eq('122228') + expect(reasons).to eq(["Variation 'control_with_audience' is mapped to experiment 'test_experiment_with_audience' and user 'test_user' in the forced variation map"]) # Setting forced variation should short circuit whitelist check, bucketing and audience evaluation expect(decision_service).not_to have_received(:get_whitelisted_variation_id) expect(decision_service.bucketer).not_to have_received(:bucket) @@ -63,7 +67,13 @@ end it 'should return the correct variation ID for a given user ID and key of a running experiment' do - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'test_user' is in variation 'control' of experiment 'test_experiment'.") @@ -73,18 +83,31 @@ it 'should return nil when user ID is not bucketed' do allow(decision_service.bucketer).to receive(:bucket).and_return(nil) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq(nil) + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq(nil) + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in no variation." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'test_user' is in no variation.") end it 'should return correct variation ID if user ID is in whitelisted Variations and variation is valid' do - expect(decision_service.get_variation(config, 'test_experiment', 'forced_user1')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'forced_user1') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "User 'forced_user1' is whitelisted into variation 'control' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user1' is whitelisted into variation 'control' of experiment 'test_experiment'.") - expect(decision_service.get_variation(config, 'test_experiment', 'forced_user2')).to eq('111129') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'forced_user2') + expect(variation_received).to eq('111129') + expect(reasons).to eq([ + "User 'forced_user2' is whitelisted into variation 'variation' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user2' is whitelisted into variation 'variation' of experiment 'test_experiment'.") @@ -99,11 +122,20 @@ 'browser_type' => 'firefox', Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 'pid' } - expect(decision_service.get_variation(config, 'test_experiment', 'forced_user1', user_attributes)).to eq('111128') + + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'forced_user1', user_attributes) + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "User 'forced_user1' is whitelisted into variation 'control' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user1' is whitelisted into variation 'control' of experiment 'test_experiment'.") - expect(decision_service.get_variation(config, 'test_experiment', 'forced_user2', user_attributes)).to eq('111129') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'forced_user2', user_attributes) + expect(variation_received).to eq('111129') + expect(reasons).to eq([ + "User 'forced_user2' is whitelisted into variation 'variation' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user2' is whitelisted into variation 'variation' of experiment 'test_experiment'.") @@ -115,7 +147,11 @@ it 'should return the correct variation ID for a user in a whitelisted variation (even when audience conditions do not match)' do user_attributes = {'browser_type' => 'wrong_browser'} - expect(decision_service.get_variation(config, 'test_experiment_with_audience', 'forced_audience_user', user_attributes)).to eq('122229') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment_with_audience', 'forced_audience_user', user_attributes) + expect(variation_received).to eq('122229') + expect(reasons).to eq([ + "User 'forced_audience_user' is whitelisted into variation 'variation_with_audience' of experiment 'test_experiment_with_audience'." + ]) expect(spy_logger).to have_received(:log) .once.with( Logger::INFO, @@ -129,7 +165,9 @@ end it 'should return nil if the experiment key is invalid' do - expect(decision_service.get_variation(config, 'totally_invalid_experiment', 'test_user', {})).to eq(nil) + variation_received, reasons = decision_service.get_variation(config, 'totally_invalid_experiment', 'test_user', {}) + expect(variation_received).to eq(nil) + expect(reasons).to eq([]) expect(spy_logger).to have_received(:log) .once.with(Logger::ERROR, "Experiment key 'totally_invalid_experiment' is not in datafile.") @@ -137,7 +175,14 @@ it 'should return nil if the user does not meet the audience conditions for a given experiment' do user_attributes = {'browser_type' => 'chrome'} - expect(decision_service.get_variation(config, 'test_experiment_with_audience', 'test_user', user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation(config, 'test_experiment_with_audience', 'test_user', user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq([ + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to FALSE.", + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE.", + "User 'test_user' does not meet the conditions to be in experiment 'test_experiment_with_audience'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'test_user' does not meet the conditions to be in experiment 'test_experiment_with_audience'.") @@ -148,7 +193,9 @@ end it 'should return nil if the given experiment is not running' do - expect(decision_service.get_variation(config, 'test_experiment_not_started', 'test_user')).to eq(nil) + variation_received, reasons = decision_service.get_variation(config, 'test_experiment_not_started', 'test_user') + expect(variation_received).to eq(nil) + expect(reasons).to eq(["Experiment 'test_experiment_not_started' is not running."]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "Experiment 'test_experiment_not_started' is not running.") @@ -161,7 +208,11 @@ end it 'should respect forced variations within mutually exclusive grouped experiments' do - expect(decision_service.get_variation(config, 'group1_exp2', 'forced_group_user1')).to eq('130004') + variation_received, reasons = decision_service.get_variation(config, 'group1_exp2', 'forced_group_user1') + expect(variation_received).to eq('130004') + expect(reasons).to eq([ + "User 'forced_group_user1' is whitelisted into variation 'g1_e2_v2' of experiment 'group1_exp2'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_group_user1' is whitelisted into variation 'g1_e2_v2' of experiment 'group1_exp2'.") @@ -172,7 +223,13 @@ end it 'should bucket normally if user is whitelisted into a forced variation that is not in the datafile' do - expect(decision_service.get_variation(config, 'test_experiment', 'forced_user_with_invalid_variation')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'forced_user_with_invalid_variation') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "User 'forced_user_with_invalid_variation' is whitelisted into variation 'invalid_variation', which is not in the datafile.", + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'forced_user_with_invalid_variation' is in variation 'control' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log) .once.with( Logger::INFO, @@ -181,7 +238,7 @@ # bucketing should have occured experiment = config.get_experiment_from_key('test_experiment') # since we do not pass bucketing id attribute, bucketer will recieve user id as the bucketing id - expect(decision_service.bucketer).to have_received(:bucket).once.with(config, experiment, 'forced_user_with_invalid_variation', 'forced_user_with_invalid_variation', nil) + expect(decision_service.bucketer).to have_received(:bucket).once.with(config, experiment, 'forced_user_with_invalid_variation', 'forced_user_with_invalid_variation') end describe 'when a UserProfile service is provided' do @@ -196,7 +253,12 @@ } expect(spy_user_profile_service).to receive(:lookup).once.and_return(nil) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -221,7 +283,12 @@ } expect(spy_user_profile_service).to receive(:lookup).once.and_return(nil) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user', user_attributes)).to eq('111129') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user', user_attributes) + expect(variation_received).to eq('111129') + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'variation' of experiment 'test_experiment'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -243,7 +310,11 @@ expect(spy_user_profile_service).to receive(:lookup) .with('test_user').once.and_return(saved_user_profile) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111129') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111129') + expect(reasons).to eq([ + "Returning previously activated variation ID 111129 of experiment 'test_experiment' for user 'test_user' from user profile." + ]) expect(spy_logger).to have_received(:log).once .with(Logger::INFO, "Returning previously activated variation ID 111129 of experiment 'test_experiment' for user 'test_user' from user profile.") @@ -268,7 +339,12 @@ expect(spy_user_profile_service).to receive(:lookup) .once.with('test_user').and_return(saved_user_profile) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -301,7 +377,13 @@ expect(spy_user_profile_service).to receive(:lookup) .once.with('test_user').and_return(saved_user_profile) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "User 'test_user' was previously bucketed into variation ID '111111' for experiment '111127', but no matching variation was found. Re-bucketing user.", + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -321,7 +403,13 @@ it 'should bucket normally if the user profile service throws an error during lookup' do expect(spy_user_profile_service).to receive(:lookup).once.with('test_user').and_throw(:LookupError) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "Error while looking up user profile for user ID 'test_user': uncaught throw :LookupError.", + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log).once .with(Logger::ERROR, "Error while looking up user profile for user ID 'test_user': uncaught throw :LookupError.") @@ -332,7 +420,12 @@ it 'should log an error if the user profile service throws an error during save' do expect(spy_user_profile_service).to receive(:save).once.and_throw(:SaveError) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) expect(spy_logger).to have_received(:log).once .with(Logger::ERROR, "Error while saving user profile for user ID 'test_user': uncaught throw :SaveError.") @@ -343,7 +436,12 @@ allow(spy_user_profile_service).to receive(:lookup) .with('test_user').once.and_return(nil) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user', nil, [Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE])).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user', nil, [Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE]) + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) expect(decision_service.bucketer).to have_received(:bucket) expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) @@ -355,7 +453,12 @@ allow(spy_user_profile_service).to receive(:lookup) .with('test_user').once.and_return(nil) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') + variation_received, reasons = decision_service.get_variation(config, 'test_experiment', 'test_user') + expect(variation_received).to eq('111128') + expect(reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment 'test_experiment'." + ]) expect(decision_service.bucketer).to have_received(:bucket) expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) @@ -373,7 +476,9 @@ describe 'when the feature flag\'s experiment ids array is empty' do it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['empty_feature'] - expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["The feature flag 'empty_feature' is not used in any experiments."]) expect(spy_logger).to have_received(:log).once .with(Logger::DEBUG, "The feature flag 'empty_feature' is not used in any experiments.") @@ -385,7 +490,9 @@ feature_flag = config.feature_flag_key_map['boolean_feature'].dup # any string that is not an experiment id in the data file feature_flag['experimentIds'] = ['1333333337'] - expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, user_id, user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["Feature flag experiment with ID '1333333337' is not in the datafile."]) expect(spy_logger).to have_received(:log).once .with(Logger::DEBUG, "Feature flag experiment with ID '1333333337' is not in the datafile.") end @@ -398,13 +505,15 @@ # make sure the user is not bucketed into the feature experiment allow(decision_service).to receive(:get_variation) - .with(config, multivariate_experiment['key'], 'user_1', user_attributes, [], nil) - .and_return(nil) + .with(config, multivariate_experiment['key'], 'user_1', user_attributes, []) + .and_return([nil, nil]) end it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['multi_variate_feature'] - expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes, [], nil)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes, []) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["The user 'user_1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'."]) expect(spy_logger).to have_received(:log).once .with(Logger::INFO, "The user 'user_1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'.") @@ -425,7 +534,9 @@ config.variation_id_map['test_experiment_multivariate']['122231'], Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes)).to eq(expected_decision) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes) + expect(variation_received).to eq(expected_decision) + expect(reasons).to eq([]) end end end @@ -448,7 +559,9 @@ it 'should return the variation the user is bucketed into' do feature_flag = config.feature_flag_key_map['mutex_group_feature'] - expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, user_id, user_attributes)).to eq(expected_decision) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(expected_decision) + expect(reasons).to eq([]) end end @@ -457,16 +570,18 @@ mutex_exp = config.experiment_key_map['group1_exp1'] mutex_exp2 = config.experiment_key_map['group1_exp2'] allow(decision_service).to receive(:get_variation) - .with(config, mutex_exp['key'], user_id, user_attributes, [], nil) - .and_return(nil) + .with(config, mutex_exp['key'], user_id, user_attributes, []) + .and_return([nil, nil]) allow(decision_service).to receive(:get_variation) - .with(config, mutex_exp2['key'], user_id, user_attributes, [], nil) - .and_return(nil) + .with(config, mutex_exp2['key'], user_id, user_attributes, []) + .and_return([nil, nil]) end it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['mutex_group_feature'] - expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, user_id, user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["The user 'user_1' is not bucketed into any of the experiments on the feature 'mutex_group_feature'."]) expect(spy_logger).to have_received(:log).once .with(Logger::INFO, "The user 'user_1' is not bucketed into any of the experiments on the feature 'mutex_group_feature'.") @@ -482,8 +597,9 @@ describe 'when the feature flag is not associated with a rollout' do it 'should log a message and return nil' do feature_flag = config.feature_flag_key_map['boolean_feature'] - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(nil) - + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["Feature flag '#{feature_flag['key']}' is not used in a rollout."]) expect(spy_logger).to have_received(:log).once .with(Logger::DEBUG, "Feature flag '#{feature_flag['key']}' is not used in a rollout.") end @@ -493,7 +609,9 @@ it 'should log a message and return nil' do feature_flag = config.feature_flag_key_map['boolean_feature'].dup feature_flag['rolloutId'] = 'invalid_rollout_id' - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["Rollout with ID 'invalid_rollout_id' is not in the datafile 'boolean_feature'"]) expect(spy_logger).to have_received(:log).once .with(Logger::ERROR, "Rollout with ID 'invalid_rollout_id' is not in the datafile.") @@ -506,7 +624,9 @@ experimentless_rollout['experiments'] = [] allow(config).to receive(:get_rollout_from_id).and_return(experimentless_rollout) feature_flag = config.feature_flag_key_map['boolean_single_variable_feature'] - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq([]) end end @@ -519,9 +639,11 @@ expected_decision = Optimizely::DecisionService::Decision.new(rollout_experiment, variation, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT']) allow(Optimizely::Audience).to receive(:user_meets_audience_conditions?).and_return(true) allow(decision_service.bucketer).to receive(:bucket) - .with(config, rollout_experiment, user_id, user_id, nil) + .with(config, rollout_experiment, user_id, user_id) .and_return(variation) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(expected_decision) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(expected_decision) + expect(reasons).to eq(["User 'user_1' meets the audience conditions for targeting rule '1'."]) end end @@ -534,13 +656,18 @@ allow(Optimizely::Audience).to receive(:user_meets_audience_conditions?).and_return(true) allow(decision_service.bucketer).to receive(:bucket) - .with(config, rollout['experiments'][0], user_id, user_id, nil) + .with(config, rollout['experiments'][0], user_id, user_id) .and_return(nil) allow(decision_service.bucketer).to receive(:bucket) - .with(config, everyone_else_experiment, user_id, user_id, nil) + .with(config, everyone_else_experiment, user_id, user_id) .and_return(nil) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq([ + "User 'user_1' meets the audience conditions for targeting rule '1'.", + "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'." + ]) # make sure we only checked the audience for the first rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -559,13 +686,15 @@ expected_decision = Optimizely::DecisionService::Decision.new(everyone_else_experiment, variation, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT']) allow(Optimizely::Audience).to receive(:user_meets_audience_conditions?).and_return(true) allow(decision_service.bucketer).to receive(:bucket) - .with(config, rollout['experiments'][0], user_id, user_id, nil) + .with(config, rollout['experiments'][0], user_id, user_id) .and_return(nil) allow(decision_service.bucketer).to receive(:bucket) - .with(config, everyone_else_experiment, user_id, user_id, nil) + .with(config, everyone_else_experiment, user_id, user_id) .and_return(variation) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(expected_decision) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(expected_decision) + expect(reasons).to eq(["User 'user_1' meets the audience conditions for targeting rule '1'.", "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'."]) # make sure we only checked the audience for the first rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -590,10 +719,12 @@ .with(config, everyone_else_experiment, user_attributes, spy_logger, 'ROLLOUT_AUDIENCE_EVALUATION_LOGS', 'Everyone Else') .and_return(true) allow(decision_service.bucketer).to receive(:bucket) - .with(config, everyone_else_experiment, user_id, user_id, nil) + .with(config, everyone_else_experiment, user_id, user_id) .and_return(variation) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(expected_decision) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(expected_decision) + expect(reasons).to eq(["User 'user_1' does not meet the audience conditions for targeting rule '1'.", "User 'user_1' does not meet the audience conditions for targeting rule '2'.", "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'."]) # verify we tried to bucket in all targeting rules and the everyone else rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) @@ -621,7 +752,9 @@ expect(decision_service.bucketer).not_to receive(:bucket) .with(config, everyone_else_experiment, user_id, user_id) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(nil) + variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes) + expect(variation_received).to eq(nil) + expect(reasons).to eq(["User 'user_1' does not meet the audience conditions for targeting rule '1'.", "User 'user_1' does not meet the audience conditions for targeting rule '2'.", "User 'user_1' does not meet the audience conditions for targeting rule 'Everyone Else'."]) # verify we tried to bucket in all targeting rules and the everyone else rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -654,9 +787,11 @@ 'experiment' => expected_experiment, 'variation' => expected_variation } - allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return(expected_decision) + allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return([expected_decision, nil]) - expect(decision_service.get_variation_for_feature(config, feature_flag, user_id, user_attributes)).to eq(expected_decision) + decision_received, reasons = decision_service.get_variation_for_feature(config, feature_flag, user_id, user_attributes) + expect(decision_received).to eq(expected_decision) + expect(reasons).to eq([]) end end @@ -671,20 +806,24 @@ variation, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return(nil) - allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return(expected_decision) + allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return([nil, nil]) + allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return([expected_decision, nil]) - expect(decision_service.get_variation_for_feature(config, feature_flag, user_id, user_attributes)).to eq(expected_decision) + decision_received, reasons = decision_service.get_variation_for_feature(config, feature_flag, user_id, user_attributes) + expect(decision_received).to eq(expected_decision) + expect(reasons).to eq([]) end end describe 'and the user is not bucketed into the feature rollout' do it 'should log a message and return nil' do feature_flag = config.feature_flag_key_map['string_single_variable_feature'] - allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return(nil) - allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return(nil) + allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return([nil, nil]) + allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return([nil, nil]) - expect(decision_service.get_variation_for_feature(config, feature_flag, user_id, user_attributes)).to eq(nil) + decision_received, reasons = decision_service.get_variation_for_feature(config, feature_flag, user_id, user_attributes) + expect(decision_received).to eq(nil) + expect(reasons).to eq([]) end end end @@ -695,7 +834,10 @@ 'browser_type' => 'firefox', Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 5 } - expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('test_user') + bucketing_id, reason = decision_service.send(:get_bucketing_id, 'test_user', user_attributes) + expect(bucketing_id).to eq('test_user') + expect(reason).to eq('Bucketing ID attribute is not a string. Defaulted to user ID.') + expect(spy_logger).to have_received(:log).once.with(Logger::WARN, 'Bucketing ID attribute is not a string. Defaulted to user ID.') end @@ -704,7 +846,9 @@ 'browser_type' => 'firefox', Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => nil } - expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('test_user') + bucketing_id, reason = decision_service.send(:get_bucketing_id, 'test_user', user_attributes) + expect(bucketing_id).to eq('test_user') + expect(reason).to eq(nil) expect(spy_logger).not_to have_received(:log) end @@ -713,7 +857,9 @@ 'browser_type' => 'firefox', Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 'i_am_bucketing_id' } - expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('i_am_bucketing_id') + bucketing_id, reason = decision_service.send(:get_bucketing_id, 'test_user', user_attributes) + expect(bucketing_id).to eq('i_am_bucketing_id') + expect(reason).to eq(nil) expect(spy_logger).not_to have_received(:log) end @@ -722,7 +868,9 @@ 'browser_type' => 'firefox', Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => '' } - expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('') + bucketing_id, reason = decision_service.send(:get_bucketing_id, 'test_user', user_attributes) + expect(bucketing_id).to eq('') + expect(reason).to eq(nil) expect(spy_logger).not_to have_received(:log) end end @@ -736,13 +884,17 @@ # User ID is not defined in the forced variation map it 'should log a message and return nil when user is not in forced variation map' do - expect(decision_service.get_forced_variation(config, valid_experiment[:key], user_id)).to eq(nil) + variation_received, reasons = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) + expect(variation_received).to eq(nil) + expect(reasons).to eq([]) expect(spy_logger).to have_received(:log).with(Logger::DEBUG, "User '#{user_id}' is not in the forced variation map.") end # Experiment key does not exist in the datafile it 'should return nil when experiment key is not in datafile' do - expect(decision_service.get_forced_variation(config, invalid_experiment_key, user_id)).to eq(nil) + variation_received, reasons = decision_service.get_forced_variation(config, invalid_experiment_key, user_id) + expect(variation_received).to eq(nil) + expect(reasons).to eq([]) end end @@ -776,40 +928,46 @@ # Call set variation with different variations on one user/experiment to confirm that each set is expected. it 'should set and return expected variations when different variations are set and removed for one user/experiment' do expect(decision_service.set_forced_variation(config, valid_experiment[:key], user_id, valid_variation[:key])).to eq(true) - variation = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) + variation, reasons = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) expect(variation['id']).to eq(valid_variation[:id]) expect(variation['key']).to eq(valid_variation[:key]) + expect(reasons).to eq(["Variation 'control' is mapped to experiment 'test_experiment' and user 'test_user' in the forced variation map"]) expect(decision_service.set_forced_variation(config, valid_experiment[:key], user_id, valid_variation_2[:key])).to eq(true) - variation = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) + variation, reasons = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) expect(variation['id']).to eq(valid_variation_2[:id]) expect(variation['key']).to eq(valid_variation_2[:key]) + expect(reasons).to eq(["Variation 'variation' is mapped to experiment 'test_experiment' and user 'test_user' in the forced variation map"]) end # Set variation on multiple experiments for one user. it 'should set and return expected variations when variation is set for multiple experiments for one user' do expect(decision_service.set_forced_variation(config, valid_experiment[:key], user_id, valid_variation[:key])).to eq(true) - variation = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) + variation, reasons = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) expect(variation['id']).to eq(valid_variation[:id]) expect(variation['key']).to eq(valid_variation[:key]) + expect(reasons).to eq(["Variation 'control' is mapped to experiment 'test_experiment' and user 'test_user' in the forced variation map"]) expect(decision_service.set_forced_variation(config, valid_experiment_2[:key], user_id, valid_variation_for_exp_2[:key])).to eq(true) - variation = decision_service.get_forced_variation(config, valid_experiment_2[:key], user_id) + variation, reasons = decision_service.get_forced_variation(config, valid_experiment_2[:key], user_id) expect(variation['id']).to eq(valid_variation_for_exp_2[:id]) expect(variation['key']).to eq(valid_variation_for_exp_2[:key]) + expect(reasons).to eq(["Variation 'control_with_audience' is mapped to experiment 'test_experiment_with_audience' and user 'test_user' in the forced variation map"]) end # Set variations for multiple users. it 'should set and return expected variations when variations are set for multiple users' do expect(decision_service.set_forced_variation(config, valid_experiment[:key], user_id, valid_variation[:key])).to eq(true) - variation = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) + variation, reasons = decision_service.get_forced_variation(config, valid_experiment[:key], user_id) expect(variation['id']).to eq(valid_variation[:id]) expect(variation['key']).to eq(valid_variation[:key]) + expect(reasons).to eq(["Variation 'control' is mapped to experiment 'test_experiment' and user 'test_user' in the forced variation map"]) expect(decision_service.set_forced_variation(config, valid_experiment[:key], user_id_2, valid_variation[:key])).to eq(true) - variation = decision_service.get_forced_variation(config, valid_experiment[:key], user_id_2) + variation, reasons = decision_service.get_forced_variation(config, valid_experiment[:key], user_id_2) expect(variation['id']).to eq(valid_variation[:id]) expect(variation['key']).to eq(valid_variation[:key]) + expect(reasons).to eq(["Variation 'control' is mapped to experiment 'test_experiment' and user 'test_user_2' in the forced variation map"]) end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index b91685ce..824d0643 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -214,7 +214,7 @@ class InvalidErrorHandler; end params = @expected_activate_params variation_to_return = project_config.get_variation_from_id('test_experiment', '111128') - allow(project_instance.decision_service.bucketer).to receive(:bucket).and_return(variation_to_return) + allow(project_instance.decision_service.bucketer).to receive(:bucket).and_return([variation_to_return, nil]) allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) allow(project_config).to receive(:get_audience_ids_for_experiment) .with('test_experiment') @@ -3903,7 +3903,9 @@ def callback(_args); end variation_key: nil, rule_key: nil, reasons: [ - "User 'user1' is not in the forced variation map.", + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_multivariate' collectively evaluated to FALSE.", "User 'user1' does not meet the conditions to be in experiment 'test_experiment_multivariate'.", "The user 'user1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'.", "Feature flag 'multi_variate_feature' is not used in a rollout." @@ -3917,7 +3919,9 @@ def callback(_args); end flag_key: 'multi_variate_feature', enabled: false, reasons: [ - "User 'user1' is not in the forced variation map.", + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_multivariate' collectively evaluated to FALSE.", "User 'user1' does not meet the conditions to be in experiment 'test_experiment_multivariate'.", "The user 'user1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'.", "Feature flag 'multi_variate_feature' is not used in a rollout." @@ -3974,11 +3978,11 @@ def callback(_args); end user_context = project_instance.create_user_context('user1') expect(project_instance.decision_service).to receive(:get_variation_for_feature) - .with(anything, anything, anything, anything, [], []).once + .with(anything, anything, anything, anything, []).once project_instance.decide(user_context, 'multi_variate_feature') expect(project_instance.decision_service).to receive(:get_variation_for_feature) - .with(anything, anything, anything, anything, [Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT], []).once + .with(anything, anything, anything, anything, [Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT]).once project_instance.decide(user_context, 'multi_variate_feature', [Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT]) expect(project_instance.decision_service).to receive(:get_variation_for_feature) @@ -3989,7 +3993,7 @@ def callback(_args); end Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE, Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS, Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES - ], []).once + ]).once project_instance .decide(user_context, 'multi_variate_feature', [ Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT, @@ -4248,7 +4252,9 @@ def callback(_args); end variation_key: nil, rule_key: nil, reasons: [ - "User 'user1' is not in the forced variation map.", + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_multivariate' collectively evaluated to FALSE.", "User 'user1' does not meet the conditions to be in experiment 'test_experiment_multivariate'.", "The user 'user1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'.", "Feature flag 'multi_variate_feature' is not used in a rollout." @@ -4262,7 +4268,9 @@ def callback(_args); end flag_key: 'multi_variate_feature', enabled: false, reasons: [ - "User 'user1' is not in the forced variation map.", + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to UNKNOWN.", + "Audiences for experiment 'test_experiment_multivariate' collectively evaluated to FALSE.", "User 'user1' does not meet the conditions to be in experiment 'test_experiment_multivariate'.", "The user 'user1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'.", "Feature flag 'multi_variate_feature' is not used in a rollout."