From fc1c696c0915f0e2f860dcaa5acb32d0ad3db112 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 2 Dec 2024 18:46:35 +0600 Subject: [PATCH 01/18] user profile tracker created --- lib/optimizely/user_profile_tracker.rb | 56 ++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 lib/optimizely/user_profile_tracker.rb diff --git a/lib/optimizely/user_profile_tracker.rb b/lib/optimizely/user_profile_tracker.rb new file mode 100644 index 00000000..a41bb09f --- /dev/null +++ b/lib/optimizely/user_profile_tracker.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Optimizely + class UserProfileTracker + attr_reader :user_profile + + def initialize(user_id, user_profile_service = nil, logger = nil) + @user_id = user_id + @user_profile_service = user_profile_service + @logger = logger || NoOpLogger.new + @profile_updated = false + @user_profile = { + user_id: user_id, + experiment_bucket_map: {} + } + end + + def load_user_profile(reasons = [], error_handler = nil) + return if reasons.nil? + + begin + @user_profile = @user_profile_service.lookup(user_id) || @user_profile + rescue => e + message = "Error while loading user profile in user profile tracker for user ID '#{user_id}': #{e}." + reasons << e.message + @logger.log(Logger::ERROR, message) + error_handler&.handle_error(e) + end + end + + def update_user_profile(experiment_id, variation_id) + user_id = @user_profile[:user_id] + begin + @user_profile[:experiment_bucket_map][experiment_id] = { + variation_id: variation_id + } + @profile_updated = true + @logger.log(Logger::INFO, "Updated variation ID #{variation_id} of experiment ID #{experiment_id} for user '#{user_id}'.") + rescue => e + @logger.log(Logger::ERROR, "Error while updating user profile for user ID '#{user_id}': #{e}.") + end + end + + def save_user_profile(error_handler = nil) + return unless @profile_updated && @user_profile_service + + begin + @user_profile_service.save(@user_profile) + @logger.log(Logger::INFO, "Saved user profile for user '#{@user_profile[:user_id]}'.") + rescue => e + @logger.log(Logger::ERROR, "Failed to save user profile for user '#{@user_profile[:user_id]}': #{e}.") + error_handler&.handle_error(e) + end + end + end +end From 454fd5229ce9fe9c30ec9134d82d72eedd1861b9 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 4 Dec 2024 17:44:34 +0600 Subject: [PATCH 02/18] lib/optimizely.rb -> Added user_profile_tracker require lib/optimizely.rb -> Updated decide_for_keys method lib/optimizely.rb -> Enhanced decision-making logic lib/optimizely.rb -> Integrated UserProfileTracker usage lib/optimizely.rb -> Refined decision reasons handling lib/optimizely/user_profile_tracker.rb -> New user profile tracker class --- lib/optimizely.rb | 105 +++++++++++++++++++++---- lib/optimizely/user_profile_tracker.rb | 2 + 2 files changed, 91 insertions(+), 16 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 1dbd54c4..e40fd09a 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -42,6 +42,7 @@ require_relative 'optimizely/odp/lru_cache' require_relative 'optimizely/odp/odp_manager' require_relative 'optimizely/helpers/sdk_settings' +require_relative 'optimizely/user_profile_tracker' module Optimizely class Project @@ -209,28 +210,23 @@ def decide(user_context, key, decide_options = []) decide_options = @default_decide_options end + decide_options.delete(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) if decide_options.include?(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + + decide_for_keys(user_context, [key], decide_options, true)[key] + end + + def create_optimizely_decision(user_context, flag_key, decision, reasons, decide_options, config) # Create Optimizely Decision Result. user_id = user_context.user_id attributes = user_context.user_attributes variation_key = nil feature_enabled = false rule_key = nil - flag_key = key all_variables = {} decision_event_dispatched = false + feature_flag = config.get_feature_flag_from_key(flag_key) experiment = nil decision_source = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] - context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(key, nil) - variation, reasons_received = @decision_service.validated_forced_decision(config, context, user_context) - 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 @@ -298,7 +294,7 @@ def decide_all(user_context, decide_options = []) decide_for_keys(user_context, keys, decide_options) end - def decide_for_keys(user_context, keys, decide_options = []) + def decide_for_keys(user_context, keys, decide_options = [], ignore_default_options: false) # raising on user context as it is internal and not provided directly by the user. raise if user_context.class != OptimizelyUserContext @@ -308,13 +304,86 @@ def decide_for_keys(user_context, keys, decide_options = []) return {} end - enabled_flags_only = (!decide_options.nil? && (decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY)) || (@default_decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + # merge decide_options and default_decide_options + unless ignore_default_options + if decide_options.is_a?(Array) + decide_options += @default_decide_options + else + @logger.log(Logger::DEBUG, 'Provided decide options is not an array. Using default decide options.') + decide_options = @default_decide_options + end + end + + # enabled_flags_only = (!decide_options.nil? && (decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY)) || (@default_decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY) decisions = {} + valid_keys = [] + decision_reasons_dict = {} + config = project_config + return decisions unless config + + flags_without_forced_decision = [] #: list[entities.FeatureFlag] + flag_decisions = {} #: dict[str, Decision] + keys.each do |key| - decision = decide(user_context, key, decide_options) - decisions[key] = decision unless enabled_flags_only && !decision.enabled + # Retrieve the feature flag from the project's feature flag key map + feature_flag = config.feature_flag_key_map[key] + + # If the feature flag is nil, create a default OptimizelyDecision and move to the next key + if feature_flag.nil? + decisions[key] = OptimizelyDecision.new(nil, false, nil, nil, key, user_context, []) + next + end + valid_keys.push(key) + decision_reasons = [] + decision_reasons_dict[key] = decision_reasons + + config = project_config + context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(key, nil) + variation, reasons_received = @decision_service.validated_forced_decision(config, context, user_context) + decision_reasons_dict[key].push(*reasons_received) + + if variation + decision = Optimizely::DecisionService::Decision.new(nil, variation, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']) + flag_decisions[key] = decision + else + flags_without_forced_decision.push(feature_flag) + # decision, reasons_received = @decision_service.get_variation_for_feature(config, feature_flag, user_context, decide_options) + # reasons.push(*reasons_received) + end + # decision = decide(user_context, key, decide_options) + # decisions[key] = decision unless enabled_flags_only && !decision.enabled + end + + decision_list = @decision_service.get_variations_for_feature_list(config, flags_without_forced_decision, user_context, decide_options) + + flags_without_forced_decision.each_with_index do |flag, i| + decision = decision_list[i][0] + reasons = decision_list[i][1] + flag_key = flag.key + flag_decisions[flag_key] = decision + decision_reasons_dict[flag_key] ||= [] + decision_reasons_dict[flag_key] += reasons end + + valid_keys.each do |key| + flag_decision = flag_decisions[key] + decision_reasons = decision_reasons_dict[key] + optimizely_decision = create_optimizely_decision( + user_context, + key, + flag_decision, + decision_reasons, + decide_options, + config + ) + + enabled_flags_only_missing = !decide_options.include?(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + is_enabled = optimizely_decision.enabled + + decisions[key] = optimizely_decision if enabled_flags_only_missing || is_enabled + end + decisions end @@ -959,7 +1028,11 @@ def get_variation_with_config(experiment_key, user_id, attributes, config) return nil unless user_inputs_valid?(attributes) user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false) + user_profile_tracker = UserProfileTracker.new(user_id, @decision_service, @logger) + user_profile_tracker.load_user_profile + #TODO: Pass user profile tracker to decision service variation_id, = @decision_service.get_variation(config, experiment_id, user_context) + user_profile_tracker.save_user_profile 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) diff --git a/lib/optimizely/user_profile_tracker.rb b/lib/optimizely/user_profile_tracker.rb index a41bb09f..1d623c9e 100644 --- a/lib/optimizely/user_profile_tracker.rb +++ b/lib/optimizely/user_profile_tracker.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative 'logger' + module Optimizely class UserProfileTracker attr_reader :user_profile From 0f5040b8af608bf4f42694ca514c312192ed8d6d Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 6 Dec 2024 13:25:23 +0600 Subject: [PATCH 03/18] Implementation complete. Unit Tests are failing. --- lib/optimizely.rb | 5 +-- lib/optimizely/decision_service.rb | 63 +++++++++++++++++++++--------- spec/decision_service_spec.rb | 3 +- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index e40fd09a..143cb8f1 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -1028,10 +1028,9 @@ def get_variation_with_config(experiment_key, user_id, attributes, config) return nil unless user_inputs_valid?(attributes) user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false) - user_profile_tracker = UserProfileTracker.new(user_id, @decision_service, @logger) + user_profile_tracker = UserProfileTracker.new(user_id, @user_profile_service, @logger) user_profile_tracker.load_user_profile - #TODO: Pass user profile tracker to decision service - variation_id, = @decision_service.get_variation(config, experiment_id, user_context) + variation_id, = @decision_service.get_variation(config, experiment_id, user_context, user_profile_tracker) user_profile_tracker.save_user_profile variation = config.get_variation_from_id(experiment_key, variation_id) unless variation_id.nil? variation_key = variation['key'] if variation diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 3dbbf1d0..4ee26e9a 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -52,17 +52,20 @@ def initialize(logger, user_profile_service = nil) @forced_variation_map = {} end - def get_variation(project_config, experiment_id, user_context, decide_options = []) + def get_variation(project_config, experiment_id, user_context, user_profile_tracker = nil, decide_options = [], reasons = []) # Determines variation into which user will be bucketed. # # project_config - project_config - Instance of ProjectConfig # experiment_id - Experiment for which visitor variation needs to be determined # user_context - Optimizely user context instance + # user_profile_tracker: Tracker for reading and updating user profile of the user. + # reasons: Decision reasons. # # Returns variation ID where visitor will be bucketed # (nil if experiment is inactive or user does not meet audience conditions) - + user_profile_tracker = nil unless user_profile_tracker.is_a?(Optimizely::UserProfileTracker) decide_reasons = [] + decide_reasons.push(*reasons) user_id = user_context.user_id attributes = user_context.user_attributes # By default, the bucketing ID should be the user ID @@ -92,10 +95,8 @@ def get_variation(project_config, experiment_id, user_context, decide_options = 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, 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) + unless should_ignore_user_profile_service && user_profile_tracker + saved_variation_id, reasons_received = get_saved_variation_id(project_config, experiment_id, user_profile_tracker.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." @@ -131,7 +132,7 @@ def get_variation(project_config, experiment_id, user_context, decide_options = decide_reasons.push(message) # Persist bucketing decision - save_user_profile(user_profile, experiment_id, variation_id) unless should_ignore_user_profile_service + user_profile_tracker.update_user_profile(experiment_id, variation_id) unless should_ignore_user_profile_service && user_profile_tracker [variation_id, decide_reasons] end @@ -143,18 +144,44 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide # user_context - Optimizely user context instance # # Returns Decision struct (nil if the user is not bucketed into any of the experiments on the feature) + get_variations_for_feature_list(project_config, [feature_flag], user_context, decide_options).first + end - decide_reasons = [] - - # check if the feature is being experiment on and whether the user is bucketed into the experiment - decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_context, decide_options) - decide_reasons.push(*reasons_received) - return decision, decide_reasons unless decision.nil? - - decision, reasons_received = get_variation_for_feature_rollout(project_config, feature_flag, user_context) - decide_reasons.push(*reasons_received) - - [decision, decide_reasons] + def get_variations_for_feature_list(project_config, feature_flags, user_context, decide_options = []) + # Returns the list of experiment/variation the user is bucketed in for the given list of features. + # + # Args: + # project_config: Instance of ProjectConfig. + # feature_flags: Array of features for which we are determining if it is enabled or not for the given user. + # user_context: User context for user. + # decide_options: Decide options. + # + # Returns: + # Array of Decision struct. + ignore_ups = decide_options&.include?(OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE) || false + user_profile_tracker = nil + unless ignore_ups && @user_profile_service + user_profile_tracker = UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) + user_profile_tracker.load_user_profile + end + decisions = [] + feature_flags.each do |feature_flag| + decide_reasons = [] + # check if the feature is being experiment on and whether the user is bucketed into the experiment + decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_context, decide_options) + if decision + # Push the decision and reasons to decisions if the decision is not nil + decide_reasons.push(*reasons_received) + decisions << [decision, decide_reasons] + else + # Proceed to rollout if the decision is nil + rollout_decision, reasons_received = get_variation_for_feature_rollout(project_config, feature_flag, user_context) + decide_reasons.push(*reasons_received) + decisions << [rollout_decision, decide_reasons] + end + end + user_profile_tracker.save_user_profile unless user_profile_tracker + decisions end def get_variation_for_feature_experiment(project_config, feature_flag, user_context, decide_options = []) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 10f58792..d6124391 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -73,7 +73,8 @@ it 'should return the correct variation ID for a given user ID and key of a running experiment' do user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) expect(variation_received).to eq('111128') expect(reasons).to eq([ From 0e53504662290ee9f279818964449cecb8d89e88 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 6 Dec 2024 22:02:11 +0600 Subject: [PATCH 04/18] lib/optimizely.rb -> Made optional parameter explicit lib/optimizely/decision_service.rb -> Added user profile tracker usage lib/optimizely/decision_service.rb -> Clarified handling of user profiles lib/optimizely/user_profile_tracker.rb -> Fixed user ID reference in error spec/decision_service_spec.rb -> Adjusted tests for user profile tracker --- lib/optimizely.rb | 10 +++---- lib/optimizely/decision_service.rb | 13 +++++----- lib/optimizely/user_profile_tracker.rb | 4 +-- spec/decision_service_spec.rb | 36 +++++++++++++++----------- 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 143cb8f1..ab0362a9 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -245,7 +245,7 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide # Generate all variables map if decide options doesn't include excludeVariables unless decide_options.include? OptimizelyDecideOption::EXCLUDE_VARIABLES feature_flag['variables'].each do |variable| - variable_value = get_feature_variable_for_variation(key, feature_enabled, variation, variable, user_id) + variable_value = get_feature_variable_for_variation(flag_key, feature_enabled, variation, variable, user_id) all_variables[variable['key']] = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) end end @@ -294,7 +294,7 @@ def decide_all(user_context, decide_options = []) decide_for_keys(user_context, keys, decide_options) end - def decide_for_keys(user_context, keys, decide_options = [], ignore_default_options: false) + def decide_for_keys(user_context, keys, decide_options = [], ignore_default_options = false) # rubocop:disable Style/OptionalBooleanParameter # raising on user context as it is internal and not provided directly by the user. raise if user_context.class != OptimizelyUserContext @@ -348,11 +348,7 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti flag_decisions[key] = decision else flags_without_forced_decision.push(feature_flag) - # decision, reasons_received = @decision_service.get_variation_for_feature(config, feature_flag, user_context, decide_options) - # reasons.push(*reasons_received) end - # decision = decide(user_context, key, decide_options) - # decisions[key] = decision unless enabled_flags_only && !decision.enabled end decision_list = @decision_service.get_variations_for_feature_list(config, flags_without_forced_decision, user_context, decide_options) @@ -360,7 +356,7 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti flags_without_forced_decision.each_with_index do |flag, i| decision = decision_list[i][0] reasons = decision_list[i][1] - flag_key = flag.key + flag_key = flag['key'] flag_decisions[flag_key] = decision decision_reasons_dict[flag_key] ||= [] decision_reasons_dict[flag_key] += reasons diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 4ee26e9a..5c4bd108 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -94,6 +94,7 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac 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 + # user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) if user_profile_tracker.nil? # Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService unless should_ignore_user_profile_service && user_profile_tracker saved_variation_id, reasons_received = get_saved_variation_id(project_config, experiment_id, user_profile_tracker.user_profile) @@ -158,7 +159,7 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, # # Returns: # Array of Decision struct. - ignore_ups = decide_options&.include?(OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE) || false + ignore_ups = decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE user_profile_tracker = nil unless ignore_ups && @user_profile_service user_profile_tracker = UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) @@ -168,7 +169,7 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, feature_flags.each do |feature_flag| decide_reasons = [] # check if the feature is being experiment on and whether the user is bucketed into the experiment - decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_context, decide_options) + decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options) if decision # Push the decision and reasons to decisions if the decision is not nil decide_reasons.push(*reasons_received) @@ -184,7 +185,7 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, decisions end - def get_variation_for_feature_experiment(project_config, feature_flag, user_context, decide_options = []) + def get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options = []) # Gets the variation the user is bucketed into for the feature flag's experiment. # # project_config - project_config - Instance of ProjectConfig @@ -214,7 +215,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont end experiment_id = experiment['id'] - variation_id, reasons_received = get_variation_from_experiment_rule(project_config, feature_flag_key, experiment, user_context, decide_options) + variation_id, reasons_received = get_variation_from_experiment_rule(project_config, feature_flag_key, experiment, user_context, user_profile_tracker, decide_options) decide_reasons.push(*reasons_received) next unless variation_id @@ -279,7 +280,7 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context [nil, decide_reasons] end - def get_variation_from_experiment_rule(project_config, flag_key, rule, user, options = []) + def get_variation_from_experiment_rule(project_config, flag_key, rule, user, user_profile_tracker, options = []) # Determine which variation the user is in for a given rollout. # Returns the variation from experiment rules. # @@ -297,7 +298,7 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, opt return [variation['id'], reasons] if variation - variation_id, response_reasons = get_variation(project_config, rule['id'], user, options) + variation_id, response_reasons = get_variation(project_config, rule['id'], user, user_profile_tracker, options) reasons.push(*response_reasons) [variation_id, reasons] diff --git a/lib/optimizely/user_profile_tracker.rb b/lib/optimizely/user_profile_tracker.rb index 1d623c9e..61ae7636 100644 --- a/lib/optimizely/user_profile_tracker.rb +++ b/lib/optimizely/user_profile_tracker.rb @@ -21,9 +21,9 @@ def load_user_profile(reasons = [], error_handler = nil) return if reasons.nil? begin - @user_profile = @user_profile_service.lookup(user_id) || @user_profile + @user_profile = @user_profile_service.lookup(@user_id) || @user_profile rescue => e - message = "Error while loading user profile in user profile tracker for user ID '#{user_id}': #{e}." + message = "Error while loading user profile in user profile tracker for user ID '#{@user_id}': #{e}." reasons << e.message @logger.log(Logger::ERROR, message) error_handler&.handle_error(e) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index d6124391..8927a359 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -91,7 +91,8 @@ it 'should return nil when user ID is not bucketed' do allow(decision_service.bucketer).to receive(:bucket).and_return(nil) user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) expect(variation_received).to eq(nil) expect(reasons).to eq([ "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", @@ -190,7 +191,8 @@ it 'should return nil if the user does not meet the audience conditions for a given experiment' do user_attributes = {'browser_type' => 'chrome'} user_context = project_instance.create_user_context('test_user', user_attributes) - variation_received, reasons = decision_service.get_variation(config, '122227', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation(config, '122227', user_context, user_profile_tracker) 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\"}]]].", @@ -241,7 +243,8 @@ it 'should bucket normally if user is whitelisted into a forced variation that is not in the datafile' do user_context = project_instance.create_user_context('forced_user_with_invalid_variation') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) 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.", @@ -500,11 +503,11 @@ config_body_json = OptimizelySpec::VALID_CONFIG_BODY_JSON project_instance = Optimizely::Project.new(datafile: config_body_json) user_context = project_instance.create_user_context('user_1', {}) - + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) 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'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) expect(variation_received).to eq(nil) expect(reasons).to eq(["The feature flag 'empty_feature' is not used in any experiments."]) @@ -518,7 +521,8 @@ 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'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) 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 @@ -527,19 +531,19 @@ end describe 'when the feature flag is associated with a non-mutex experiment' do + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) describe 'and the user is not bucketed into the feature flag\'s experiments' do before(:each) do multivariate_experiment = config.experiment_key_map['test_experiment_multivariate'] - # make sure the user is not bucketed into the feature experiment allow(decision_service).to receive(:get_variation) - .with(config, multivariate_experiment['id'], user_context, []) + .with(config, multivariate_experiment['id'], user_context, user_profile_tracker, []) .and_return([nil, nil]) end it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['multi_variate_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, []) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker, []) 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'."]) @@ -561,8 +565,8 @@ config.variation_id_map['test_experiment_multivariate']['122231'], Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) expect(variation_received).to eq(expected_decision) expect(reasons).to eq([]) end @@ -587,27 +591,29 @@ it 'should return the variation the user is bucketed into' do feature_flag = config.feature_flag_key_map['mutex_group_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) expect(variation_received).to eq(expected_decision) expect(reasons).to eq([]) end end describe 'and the user is not bucketed into any of the mutex experiments' do + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) before(:each) do 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['id'], user_context, []) + .with(config, mutex_exp['id'], user_context, user_profile_tracker, []) .and_return([nil, nil]) allow(decision_service).to receive(:get_variation) - .with(config, mutex_exp2['id'], user_context, []) + .with(config, mutex_exp2['id'], user_context, user_profile_tracker, []) .and_return([nil, nil]) end it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['mutex_group_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context) + variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) 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'."]) From 061f8bc89fb05b7db3730f023ec7e68638e8295d Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 9 Dec 2024 15:22:31 +0600 Subject: [PATCH 05/18] lib/optimizely/decision_service.rb -> Simplified decision logging lib/optimizely/user_profile_tracker.rb -> Improved user profile lookup handling spec/project_spec.rb -> Updated mocks for decision service calls --- lib/optimizely.rb | 92 ++++++++++++-------------- lib/optimizely/decision_service.rb | 3 +- lib/optimizely/user_profile_tracker.rb | 2 +- spec/project_spec.rb | 31 +++++---- 4 files changed, 65 insertions(+), 63 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index ab0362a9..7c5571b3 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -173,48 +173,6 @@ def create_user_context(user_id, attributes = nil) OptimizelyUserContext.new(self, user_id, attributes) end - def decide(user_context, key, decide_options = []) - # raising on user context as it is internal and not provided directly by the user. - raise if user_context.class != OptimizelyUserContext - - reasons = [] - - # check if SDK is ready - unless is_valid - @logger.log(Logger::ERROR, InvalidProjectConfigError.new('decide').message) - reasons.push(OptimizelyDecisionMessage::SDK_NOT_READY) - return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) - end - - # validate that key is a string - unless key.is_a?(String) - @logger.log(Logger::ERROR, 'Provided key is invalid') - reasons.push(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) - return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) - end - - # validate that key maps to a feature flag - config = project_config - feature_flag = config.get_feature_flag_from_key(key) - unless feature_flag - @logger.log(Logger::ERROR, "No feature flag was found for key '#{key}'.") - reasons.push(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) - return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) - end - - # merge decide_options and default_decide_options - if decide_options.is_a? Array - decide_options += @default_decide_options - else - @logger.log(Logger::DEBUG, 'Provided decide options is not an array. Using default decide options.') - decide_options = @default_decide_options - end - - decide_options.delete(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) if decide_options.include?(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) - - decide_for_keys(user_context, [key], decide_options, true)[key] - end - def create_optimizely_decision(user_context, flag_key, decision, reasons, decide_options, config) # Create Optimizely Decision Result. user_id = user_context.user_id @@ -277,6 +235,47 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide ) end + def decide(user_context, key, decide_options = []) + # raising on user context as it is internal and not provided directly by the user. + raise if user_context.class != OptimizelyUserContext + + reasons = [] + + # check if SDK is ready + unless is_valid + @logger.log(Logger::ERROR, InvalidProjectConfigError.new('decide').message) + reasons.push(OptimizelyDecisionMessage::SDK_NOT_READY) + return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) + end + + # validate that key is a string + unless key.is_a?(String) + @logger.log(Logger::ERROR, 'Provided key is invalid') + reasons.push(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) + return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) + end + + # validate that key maps to a feature flag + config = project_config + feature_flag = config.get_feature_flag_from_key(key) + unless feature_flag + @logger.log(Logger::ERROR, "No feature flag was found for key '#{key}'.") + reasons.push(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) + return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) + end + + # merge decide_options and default_decide_options + if decide_options.is_a? Array + decide_options += @default_decide_options + else + @logger.log(Logger::DEBUG, 'Provided decide options is not an array. Using default decide options.') + decide_options = @default_decide_options + end + + decide_options.delete(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) if decide_options.include?(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + decide_for_keys(user_context, [key], decide_options, true)[key] + end + def decide_all(user_context, decide_options = []) # raising on user context as it is internal and not provided directly by the user. raise if user_context.class != OptimizelyUserContext @@ -322,8 +321,8 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti config = project_config return decisions unless config - flags_without_forced_decision = [] #: list[entities.FeatureFlag] - flag_decisions = {} #: dict[str, Decision] + flags_without_forced_decision = [] + flag_decisions = {} keys.each do |key| # Retrieve the feature flag from the project's feature flag key map @@ -342,7 +341,6 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(key, nil) variation, reasons_received = @decision_service.validated_forced_decision(config, context, user_context) decision_reasons_dict[key].push(*reasons_received) - if variation decision = Optimizely::DecisionService::Decision.new(nil, variation, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']) flag_decisions[key] = decision @@ -350,7 +348,6 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti flags_without_forced_decision.push(feature_flag) end end - decision_list = @decision_service.get_variations_for_feature_list(config, flags_without_forced_decision, user_context, decide_options) flags_without_forced_decision.each_with_index do |flag, i| @@ -359,9 +356,8 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti flag_key = flag['key'] flag_decisions[flag_key] = decision decision_reasons_dict[flag_key] ||= [] - decision_reasons_dict[flag_key] += reasons + decision_reasons_dict[flag_key].push(*reasons) end - valid_keys.each do |key| flag_decision = flag_decisions[key] decision_reasons = decision_reasons_dict[key] diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 5c4bd108..342bd546 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -170,9 +170,8 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, decide_reasons = [] # check if the feature is being experiment on and whether the user is bucketed into the experiment decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options) + decide_reasons.push(*reasons_received) if decision - # Push the decision and reasons to decisions if the decision is not nil - decide_reasons.push(*reasons_received) decisions << [decision, decide_reasons] else # Proceed to rollout if the decision is nil diff --git a/lib/optimizely/user_profile_tracker.rb b/lib/optimizely/user_profile_tracker.rb index 61ae7636..8074a954 100644 --- a/lib/optimizely/user_profile_tracker.rb +++ b/lib/optimizely/user_profile_tracker.rb @@ -21,7 +21,7 @@ def load_user_profile(reasons = [], error_handler = nil) return if reasons.nil? begin - @user_profile = @user_profile_service.lookup(@user_id) || @user_profile + @user_profile = @user_profile_service.lookup(@user_id) if @user_profile_service rescue => e message = "Error while loading user profile in user profile tracker for user ID '#{@user_id}': #{e}." reasons << e.message diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 2c1aeaca..c6dc86fc 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3766,7 +3766,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_list_to_be_returned = [] + decision_list_to_be_returned << [decision_to_return, []] + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_be_returned) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') expect(decision.as_json).to include( @@ -3807,7 +3809,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_list_to_be_returned = [] + decision_list_to_be_returned << [decision_to_return, []] + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_be_returned) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') @@ -3888,7 +3892,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_list_to_return = [[decision_to_return, []]] + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') expect(decision.as_json).to include( @@ -4055,8 +4060,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + decision_list_to_be_returned = [[decision_to_return, []]] allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_be_returned) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature', [Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES]) expect(decision.as_json).to include( @@ -4078,8 +4084,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + decision_list_to_return = [[decision_to_return, []]] allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') expect(decision.as_json).to include( @@ -4096,8 +4103,6 @@ def callback(_args); end describe 'INCLUDE_REASONS' do it 'should include reasons when the option is set' do - expect(project_instance.notification_center).to receive(:send_notifications) - .once.with(Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args) expect(project_instance.notification_center).to receive(:send_notifications) .once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -4119,6 +4124,8 @@ def callback(_args); end ], decision_event_dispatched: true ) + expect(project_instance.notification_center).to receive(:send_notifications) + .once.with(Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args) allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature', [Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS]) @@ -4180,23 +4187,23 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + decision_list_to_return = [[decision_to_return, []]] allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = project_instance.create_user_context('user1') - expect(project_instance.decision_service).to receive(:get_variation_for_feature) + expect(project_instance.decision_service).to receive(:get_variations_for_feature_list) .with(anything, anything, anything, []).once project_instance.decide(user_context, 'multi_variate_feature') - expect(project_instance.decision_service).to receive(:get_variation_for_feature) + expect(project_instance.decision_service).to receive(:get_variations_for_feature_list) .with(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) + expect(project_instance.decision_service).to receive(:get_variations_for_feature_list) .with(anything, anything, anything, [ Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT, Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES, - Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY, Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE, Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS, Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES From 7cd21e7b095d510dadfd64fa68fcab27b6346e6d Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 9 Dec 2024 18:59:12 +0600 Subject: [PATCH 06/18] lib/optimizely/decision_service.rb -> Removed user profile tracker instantiation. lib/optimizely/user_profile_tracker.rb -> Improved error logging message. spec/decision_service_spec.rb -> Refactored user profile tracking in tests. spec/project_spec.rb -> Updated decision service method stubs. spec/user_profile_tracker.rb -> Updated lookup, update and save tests for user_profile_tracker --- lib/optimizely/decision_service.rb | 1 - lib/optimizely/user_profile_tracker.rb | 4 +- spec/decision_service_spec.rb | 134 ++++--------------------- spec/project_spec.rb | 6 +- spec/user_profile_tracker_spec.rb | 102 +++++++++++++++++++ 5 files changed, 130 insertions(+), 117 deletions(-) create mode 100644 spec/user_profile_tracker_spec.rb diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 342bd546..4fa5f189 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -94,7 +94,6 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac 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 - # user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) if user_profile_tracker.nil? # Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService unless should_ignore_user_profile_service && user_profile_tracker saved_variation_id, reasons_received = get_saved_variation_id(project_config, experiment_id, user_profile_tracker.user_profile) diff --git a/lib/optimizely/user_profile_tracker.rb b/lib/optimizely/user_profile_tracker.rb index 8074a954..b9a73577 100644 --- a/lib/optimizely/user_profile_tracker.rb +++ b/lib/optimizely/user_profile_tracker.rb @@ -23,8 +23,8 @@ def load_user_profile(reasons = [], error_handler = nil) begin @user_profile = @user_profile_service.lookup(@user_id) if @user_profile_service rescue => e - message = "Error while loading user profile in user profile tracker for user ID '#{@user_id}': #{e}." - reasons << e.message + message = "Error while looking up user profile for user ID '#{@user_id}': #{e}." + reasons << message @logger.log(Logger::ERROR, message) error_handler&.handle_error(e) end diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 8927a359..af22b18b 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -263,50 +263,14 @@ end describe 'when a UserProfile service is provided' do - it 'should look up the UserProfile, bucket normally, and save the result if no saved profile is found' do - expected_user_profile = { - user_id: 'test_user', - experiment_bucket_map: { - '111127' => { - variation_id: '111128' - } - } - } - expect(spy_user_profile_service).to receive(:lookup).once.and_return(nil) - - user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - 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 '111127'." - ]) - - # bucketing should have occurred - expect(decision_service.bucketer).to have_received(:bucket).once - # bucketing decision should have been saved - expect(spy_user_profile_service).to have_received(:save).once.with(expected_user_profile) - expect(spy_logger).to have_received(:log).once - .with(Logger::INFO, "Saved variation ID 111128 of experiment ID 111127 for user 'test_user'.") - end - - it 'should look up the UserProfile, bucket normally (using Bucketing ID attribute), and save the result if no saved profile is found' do - expected_user_profile = { - user_id: 'test_user', - experiment_bucket_map: { - '111127' => { - variation_id: '111129' - } - } - } + it 'bucket normally (using Bucketing ID attribute)' do user_attributes = { 'browser_type' => 'firefox', Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 'pid' } - expect(spy_user_profile_service).to receive(:lookup).once.and_return(nil) - user_context = project_instance.create_user_context('test_user', user_attributes) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) expect(variation_received).to eq('111129') expect(reasons).to eq([ "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", @@ -315,13 +279,9 @@ # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once - # bucketing decision should have been saved - expect(spy_user_profile_service).to have_received(:save).once.with(expected_user_profile) - expect(spy_logger).to have_received(:log).once - .with(Logger::INFO, "Saved variation ID 111129 of experiment ID 111127 for user 'test_user'.") end - it 'should look up the user profile and skip normal bucketing if a profile with a saved decision is found' do + it 'skip normal bucketing if a profile with a saved decision is found' do saved_user_profile = { user_id: 'test_user', experiment_bucket_map: { @@ -334,7 +294,9 @@ .with('test_user').once.and_return(saved_user_profile) user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) + user_profile_tracker.load_user_profile + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) 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." @@ -350,7 +312,7 @@ expect(spy_user_profile_service).not_to have_received(:save) end - it 'should look up the user profile and bucket normally if a profile without a saved decision is found' do + it 'bucket normally if a profile without a saved decision is found' do saved_user_profile = { user_id: 'test_user', experiment_bucket_map: { @@ -364,7 +326,9 @@ .once.with('test_user').and_return(saved_user_profile) user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) + user_profile_tracker.load_user_profile + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) expect(variation_received).to eq('111128') expect(reasons).to eq([ "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", @@ -373,20 +337,6 @@ # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once - - # user profile should have been updated with bucketing decision - expected_user_profile = { - user_id: 'test_user', - experiment_bucket_map: { - '111127' => { - variation_id: '111128' - }, - '122227' => { - variation_id: '122228' - } - } - } - expect(spy_user_profile_service).to have_received(:save).once.with(expected_user_profile) end it 'should bucket normally if the user profile contains a variation ID not in the datafile' do @@ -403,7 +353,9 @@ .once.with('test_user').and_return(saved_user_profile) user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) + user_profile_tracker.load_user_profile + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) 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.", @@ -413,27 +365,18 @@ # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once - - # user profile should have been updated with bucketing decision - expected_user_profile = { - user_id: 'test_user', - experiment_bucket_map: { - '111127' => { - variation_id: '111128' - } - } - } - expect(spy_user_profile_service).to have_received(:save).with(expected_user_profile) end - it 'should bucket normally if the user profile service throws an error during lookup' do + it 'should bucket normally if the user profile tracker throws an error during lookup' do expect(spy_user_profile_service).to receive(:lookup).once.with('test_user').and_throw(:LookupError) user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) + user_profile_tracker.load_user_profile + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + user_profile_tracker.save_user_profile 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 '111127'." ]) @@ -444,46 +387,15 @@ expect(decision_service.bucketer).to have_received(:bucket).once end - 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) - - user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - 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 '111127'." - ]) - - expect(spy_logger).to have_received(:log).once - .with(Logger::ERROR, "Error while saving user profile for user ID 'test_user': uncaught throw :SaveError.") - end - describe 'IGNORE_USER_PROFILE_SERVICE decide option' do it 'should ignore user profile service if this option is set' do allow(spy_user_profile_service).to receive(:lookup) .with('test_user').once.and_return(nil) user_context = project_instance.create_user_context('test_user', nil) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, [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 '111127'." - ]) - - expect(decision_service.bucketer).to have_received(:bucket) - expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) - expect(spy_user_profile_service).not_to have_received(:lookup) - expect(spy_user_profile_service).not_to have_received(:save) - end - - it 'should not ignore user profile service if this option is not set' do - allow(spy_user_profile_service).to receive(:lookup) - .with('test_user').once.and_return(nil) - - user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) + user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) + user_profile_tracker.load_user_profile + variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker, [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.", @@ -492,8 +404,6 @@ expect(decision_service.bucketer).to have_received(:bucket) expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) - expect(spy_user_profile_service).to have_received(:lookup) - expect(spy_user_profile_service).to have_received(:save) end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index c6dc86fc..7c02f765 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -4407,8 +4407,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + decision_list_to_return = [[decision_to_return, []]] allow(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(custom_project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(custom_project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = custom_project_instance.create_user_context('user1') decision = custom_project_instance.decide(user_context, 'multi_variate_feature') expect(decision.as_json).to include( @@ -4435,8 +4436,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + decision_list_to_return = [[decision_to_return, []]] allow(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(custom_project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(custom_project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = custom_project_instance.create_user_context('user1') decision = custom_project_instance.decide(user_context, 'multi_variate_feature') expect(decision.as_json).to include( diff --git a/spec/user_profile_tracker_spec.rb b/spec/user_profile_tracker_spec.rb new file mode 100644 index 00000000..7e7065db --- /dev/null +++ b/spec/user_profile_tracker_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rspec' + +RSpec.describe Optimizely::UserProfileTracker do + let(:user_id) { 'test_user' } + let(:mock_user_profile_service) { instance_double('UserProfileService') } + let(:mock_logger) { instance_double('Logger') } + let(:user_profile_tracker) { described_class.new(user_id, mock_user_profile_service, mock_logger) } + + describe '#initialize' do + it 'initializes with a user ID and default values' do + tracker = described_class.new(user_id) + expect(tracker.user_profile[:user_id]).to eq(user_id) + expect(tracker.user_profile[:experiment_bucket_map]).to eq({}) + end + + it 'accepts a user profile service and logger' do + expect(user_profile_tracker.instance_variable_get(:@user_profile_service)).to eq(mock_user_profile_service) + expect(user_profile_tracker.instance_variable_get(:@logger)).to eq(mock_logger) + end + end + + describe '#load_user_profile' do + it 'loads the user profile from the service if provided' do + expected_profile = { + user_id: user_id, + experiment_bucket_map: { '111127' => { variation_id: '111128' } } + } + allow(mock_user_profile_service).to receive(:lookup).with(user_id).and_return(expected_profile) + user_profile_tracker.load_user_profile + expect(user_profile_tracker.user_profile).to eq(expected_profile) + end + + it 'handles errors during lookup and logs them' do + allow(mock_user_profile_service).to receive(:lookup).with(user_id).and_raise(StandardError.new('lookup error')) + allow(mock_logger).to receive(:log) + + reasons = [] + user_profile_tracker.load_user_profile(reasons) + + expect(reasons).to include('lookup error') + expect(mock_logger).to have_received(:log).with(Logger::ERROR, "Error while loading user profile in user profile tracker for user ID 'test_user': lookup error.") + end + + it 'does nothing if reasons array is nil' do + expect(mock_user_profile_service).not_to receive(:lookup) + user_profile_tracker.load_user_profile(nil) + end + end + + describe '#update_user_profile' do + let(:experiment_id) { '111127' } + let(:variation_id) { '111128' } + + before do + allow(mock_logger).to receive(:log) + end + + it 'updates the experiment bucket map with the given experiment and variation IDs' do + user_profile_tracker.update_user_profile(experiment_id, variation_id) + + # Verify the experiment and variation were added + expect(user_profile_tracker.user_profile[:experiment_bucket_map][experiment_id][:variation_id]).to eq(variation_id) + # Verify the profile_updated flag was set + expect(user_profile_tracker.instance_variable_get(:@profile_updated)).to eq(true) + # Verify a log message was recorded + expect(mock_logger).to have_received(:log).with(Logger::INFO, "Updated variation ID #{variation_id} of experiment ID #{experiment_id} for user 'test_user'.") + end + end + + describe '#save_user_profile' do + it 'saves the user profile if updates were made and service is available' do + allow(mock_user_profile_service).to receive(:save) + allow(mock_logger).to receive(:log) + + user_profile_tracker.update_user_profile('111127', '111128') + user_profile_tracker.save_user_profile + + expect(mock_user_profile_service).to have_received(:save).with(user_profile_tracker.user_profile) + expect(mock_logger).to have_received(:log).with(Logger::INFO, "Saved user profile for user 'test_user'.") + end + + it 'does not save the user profile if no updates were made' do + allow(mock_user_profile_service).to receive(:save) + + user_profile_tracker.save_user_profile + expect(mock_user_profile_service).not_to have_received(:save) + end + + it 'handles errors during save and logs them' do + allow(mock_user_profile_service).to receive(:save).and_raise(StandardError.new('save error')) + allow(mock_logger).to receive(:log) + + user_profile_tracker.update_user_profile('111127', '111128') + user_profile_tracker.save_user_profile + + expect(mock_logger).to have_received(:log).with(Logger::ERROR, "Failed to save user profile for user 'test_user': save error.") + end + end +end From 437912784c9e6558b2dedc7e73910628810409ba Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 9 Dec 2024 19:03:42 +0600 Subject: [PATCH 07/18] spec/user_profile_tracker_spec.rb -> Updated error messages in tests. --- spec/user_profile_tracker_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/user_profile_tracker_spec.rb b/spec/user_profile_tracker_spec.rb index 7e7065db..713573fc 100644 --- a/spec/user_profile_tracker_spec.rb +++ b/spec/user_profile_tracker_spec.rb @@ -39,9 +39,8 @@ reasons = [] user_profile_tracker.load_user_profile(reasons) - - expect(reasons).to include('lookup error') - expect(mock_logger).to have_received(:log).with(Logger::ERROR, "Error while loading user profile in user profile tracker for user ID 'test_user': lookup error.") + expect(reasons).to include("Error while looking up user profile for user ID 'test_user': lookup error.") + expect(mock_logger).to have_received(:log).with(Logger::ERROR, "Error while looking up user profile for user ID 'test_user': lookup error.") end it 'does nothing if reasons array is nil' do From ce959510f319c9c040f4dbdfdfd059d8e4cebf43 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 9 Dec 2024 19:16:32 +0600 Subject: [PATCH 08/18] spec/user_profile_tracker_spec.rb -> linting fix --- spec/user_profile_tracker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/user_profile_tracker_spec.rb b/spec/user_profile_tracker_spec.rb index 713573fc..85515bb1 100644 --- a/spec/user_profile_tracker_spec.rb +++ b/spec/user_profile_tracker_spec.rb @@ -26,7 +26,7 @@ it 'loads the user profile from the service if provided' do expected_profile = { user_id: user_id, - experiment_bucket_map: { '111127' => { variation_id: '111128' } } + experiment_bucket_map: {'111127' => {variation_id: '111128'}} } allow(mock_user_profile_service).to receive(:lookup).with(user_id).and_return(expected_profile) user_profile_tracker.load_user_profile From 93b67a65d24ca94d294179c7d06959f7693b4217 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 9 Dec 2024 19:19:06 +0600 Subject: [PATCH 09/18] linting fixes --- lib/optimizely/helpers/validator.rb | 4 ++-- lib/optimizely/optimizely_factory.rb | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/optimizely/helpers/validator.rb b/lib/optimizely/helpers/validator.rb index d3baa447..4d975483 100644 --- a/lib/optimizely/helpers/validator.rb +++ b/lib/optimizely/helpers/validator.rb @@ -122,11 +122,11 @@ def inputs_valid?(variables, logger = NoOpLogger.new, level = Logger::ERROR) return false unless variables.respond_to?(:each) && !variables.empty? - is_valid = true + is_valid = true # rubocop:disable Lint/UselessAssignment if variables.include? :user_id # Empty str is a valid user ID. unless variables[:user_id].is_a?(String) - is_valid = false + is_valid = false # rubocop:disable Lint/UselessAssignment logger.log(level, "#{Constants::INPUT_VARIABLES['USER_ID']} is invalid") end variables.delete :user_id diff --git a/lib/optimizely/optimizely_factory.rb b/lib/optimizely/optimizely_factory.rb index 04c7ecdd..717e43d9 100644 --- a/lib/optimizely/optimizely_factory.rb +++ b/lib/optimizely/optimizely_factory.rb @@ -142,7 +142,6 @@ def self.custom_instance( # rubocop:disable Metrics/ParameterLists notification_center = nil, settings = nil ) - error_handler ||= NoOpErrorHandler.new logger ||= NoOpLogger.new notification_center = notification_center.is_a?(Optimizely::NotificationCenter) ? notification_center : NotificationCenter.new(logger, error_handler) From cdd69d00ebc52be3b27afe2c93d62e6146ac3ce3 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 7 Jan 2025 16:43:50 -0800 Subject: [PATCH 10/18] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index be5e0613..21ce08bc 100644 --- a/README.md +++ b/README.md @@ -192,6 +192,7 @@ If you enable event batching, make sure that you call the `close` method, `optim For Further details see the Optimizely [Feature Experimentation documentation](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/welcome) to learn how to set up your first Ruby project and use the SDK. +This is a test line. ## SDK Development From dedd5e1f02a4afc0117416e13004ad39fc3bb63a Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 7 Jan 2025 20:34:32 -0800 Subject: [PATCH 11/18] Update README.md --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 21ce08bc..be5e0613 100644 --- a/README.md +++ b/README.md @@ -192,7 +192,6 @@ If you enable event batching, make sure that you call the `close` method, `optim For Further details see the Optimizely [Feature Experimentation documentation](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/welcome) to learn how to set up your first Ruby project and use the SDK. -This is a test line. ## SDK Development From 8f8d839559dc0b26d8372f3d2fde1dbcd82645eb Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 7 Jan 2025 22:10:46 -0800 Subject: [PATCH 12/18] Trigger checks From c5d84fb9046256b9643b1d70d6a05eeb1254d459 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 8 Jan 2025 12:31:14 -0800 Subject: [PATCH 13/18] Trigger checks From 14d9276bfe6aa4a4a02c3c370cb700390d0da4ef Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 8 Jan 2025 12:43:17 -0800 Subject: [PATCH 14/18] Trigger checks From 543e84525735def0c68416e65a13239bdee519c2 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 8 Jan 2025 18:22:48 -0800 Subject: [PATCH 15/18] Trigger checks From d9e36d2d15f13d02aa0fa356a6257452f5adbb57 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 9 Jan 2025 13:05:26 +0600 Subject: [PATCH 16/18] lib/optimizely/user_profile_tracker.rb -> Added user profile init check. --- lib/optimizely/user_profile_tracker.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/optimizely/user_profile_tracker.rb b/lib/optimizely/user_profile_tracker.rb index b9a73577..082576b0 100644 --- a/lib/optimizely/user_profile_tracker.rb +++ b/lib/optimizely/user_profile_tracker.rb @@ -22,6 +22,12 @@ def load_user_profile(reasons = [], error_handler = nil) begin @user_profile = @user_profile_service.lookup(@user_id) if @user_profile_service + if @user_profile.nil? + @user_profile = { + user_id: @user_id, + experiment_bucket_map: {} + } + end rescue => e message = "Error while looking up user profile for user ID '#{@user_id}': #{e}." reasons << message From 41c0abef18c51206db25d781cefdffabb659d045 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 9 Jan 2025 13:18:35 +0600 Subject: [PATCH 17/18] lib/optimizely/decision_service.rb -> Updated user profile tracker initialization. --- lib/optimizely/decision_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 4fa5f189..89a1cdc0 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -63,7 +63,7 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac # # Returns variation ID where visitor will be bucketed # (nil if experiment is inactive or user does not meet audience conditions) - user_profile_tracker = nil unless user_profile_tracker.is_a?(Optimizely::UserProfileTracker) + user_profile_tracker = UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) unless user_profile_tracker.is_a?(Optimizely::UserProfileTracker) decide_reasons = [] decide_reasons.push(*reasons) user_id = user_context.user_id From 5999ea388593d884614d14dda78de5b4717fc6a2 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 9 Jan 2025 13:25:24 +0600 Subject: [PATCH 18/18] lib/optimizely/decision_service.rb -> Update user profile save method --- lib/optimizely/decision_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 89a1cdc0..3303907d 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -179,7 +179,7 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, decisions << [rollout_decision, decide_reasons] end end - user_profile_tracker.save_user_profile unless user_profile_tracker + user_profile_tracker&.save_user_profile decisions end