From 4f8765b1f9e5f04ebf692fc64a7fbeccd3b3b0d7 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 21 Oct 2020 19:32:52 +0500 Subject: [PATCH 01/46] feat(Decide): Add Optimizely User Context --- .gitignore | 3 +- lib/optimizely.rb | 19 ++++++ lib/optimizely/optimizely_user_context.rb | 51 ++++++++++++++++ spec/optimizely_user_context_spec.rb | 71 +++++++++++++++++++++++ spec/project_spec.rb | 30 ++++++++++ 5 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 lib/optimizely/optimizely_user_context.rb create mode 100644 spec/optimizely_user_context_spec.rb diff --git a/.gitignore b/.gitignore index d37791c7..a1653a5c 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,5 @@ Gemfile.lock /tmp/ *.DS_Store .ruby-version -vendor/bundle \ No newline at end of file +vendor/bundle +*.code-workspace \ No newline at end of file diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 9ef4895d..cb1ac851 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -34,6 +34,7 @@ require_relative 'optimizely/logger' require_relative 'optimizely/notification_center' require_relative 'optimizely/optimizely_config' +require_relative 'optimizely/optimizely_user_context' module Optimizely class Project @@ -107,6 +108,24 @@ def initialize( end end + def create_user_context(user_id, attributes = nil) + # We do not check for is_valid here as a user context can be created successfully + # even when the SDK is not fully configured. + + # validate user_id + return nil unless Optimizely::Helpers::Validator.inputs_valid?( + { + user_id: user_id + }, @logger, Logger::ERROR + ) + + # validate attributes + return nil unless user_inputs_valid?(attributes) + + user_context = OptimizelyUserContext.new(self, user_id, attributes) + user_context + end + # Buckets visitor and sends impression event to Optimizely. # # @param experiment_key - Experiment which needs to be activated. diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb new file mode 100644 index 00000000..d4a74553 --- /dev/null +++ b/lib/optimizely/optimizely_user_context.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# +# Copyright 2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +module Optimizely + class OptimizelyUserContext + # Representation of an Optimizely User Context using which APIs are to be called. + + def initialize(optimizely_client, user_id, user_attributes) + @optimizely_client = optimizely_client + @user_id = user_id + @user_attributes = user_attributes + + @user_attributes = {} if @user_attributes.nil? + end + + def set_attribute(attribute_key, attribute_value) + @user_attributes[attribute_key] = attribute_value + end + + def decide(key, options = nil) + # TODO: call decide API in Optimizely class. + end + + def decide_for_keys(keys, options = nil) + # TODO: call decideForKeys in Optimizely class. + end + + def decide_all(options = nil) + # TODO: call decideForAll in optimizely class. + end + + def track_event(event_key, event_tags = nil) + @optimizely_client.track(event_key, @user_id, @user_attributes, event_tags) + end + end +end diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb new file mode 100644 index 00000000..4e1cc78f --- /dev/null +++ b/spec/optimizely_user_context_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +# +# Copyright 2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +require 'spec_helper' +require 'optimizely' +require 'optimizely/optimizely_user_context' + +describe 'Optimizely' do + let(:config_body) { OptimizelySpec::VALID_CONFIG_BODY } + let(:config_body_JSON) { OptimizelySpec::VALID_CONFIG_BODY_JSON } + let(:config_body_invalid_JSON) { OptimizelySpec::INVALID_CONFIG_BODY_JSON } + let(:error_handler) { Optimizely::RaiseErrorHandler.new } + let(:spy_logger) { spy('logger') } + let(:project_instance) { Optimizely::Project.new(config_body_JSON, nil, spy_logger, error_handler) } + + describe '#initialize' do + it 'should set passed value as expected' do + user_id = 'test_user' + attributes = {' browser' => 'firefox'} + user_context_obj = Optimizely::OptimizelyUserContext.new(project_instance, user_id, attributes) + + expect(user_context_obj.instance_variable_get(:@optimizely_client)). to eq(project_instance) + expect(user_context_obj.instance_variable_get(:@user_id)). to eq(user_id) + expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq(attributes) + end + + it 'should set user attributes to empty hash when passed nil' do + user_context_obj = Optimizely::OptimizelyUserContext.new(project_instance, 'test_user', nil) + expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq({}) + end + end + + describe '#set_attribute' do + it 'should add attribute key and value is attributes hash' do + user_id = 'test_user' + attributes = {' browser' => 'firefox'} + user_context_obj = Optimizely::OptimizelyUserContext.new(project_instance, user_id, attributes) + user_context_obj.set_attribute('id', 49) + + expected_attributes = attributes + expected_attributes['id'] = 49 + expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq(expected_attributes) + end + + it 'should override attribute value if key already exists in hash' do + user_id = 'test_user' + attributes = {' browser' => 'firefox', 'color' => ' red'} + user_context_obj = Optimizely::OptimizelyUserContext.new(project_instance, user_id, attributes) + user_context_obj.set_attribute('browser', 'chrome') + + expected_attributes = attributes + expected_attributes['browser'] = 'chrome' + + expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq(expected_attributes) + end + end +end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index deb6caaa..ee13b3f1 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -23,6 +23,7 @@ require 'optimizely/event/batch_event_processor' require 'optimizely/exceptions' require 'optimizely/helpers/validator' +require 'optimizely/optimizely_user_context' require 'optimizely/version' describe 'Optimizely' do @@ -135,6 +136,35 @@ class InvalidErrorHandler; end end end + describe '#create_user_context' do + it 'should log and return nil when user ID is non string' do + expect(project_instance.create_user_context(nil)).to eq(nil) + expect(project_instance.create_user_context(5)).to eq(nil) + expect(project_instance.create_user_context(5.5)).to eq(nil) + expect(project_instance.create_user_context(true)).to eq(nil) + expect(project_instance.create_user_context({})).to eq(nil) + expect(project_instance.create_user_context([])).to eq(nil) + expect(spy_logger).to have_received(:log).with(Logger::ERROR, 'User ID is invalid').exactly(6).times + end + + it 'should return nil when attributes are invalid' do + expect(Optimizely::Helpers::Validator).to receive(:attributes_valid?).once.with('invalid') + expect(error_handler).to receive(:handle_error).once.with(Optimizely::InvalidAttributeFormatError) + expect(project_instance.create_user_context( + 'test_user', + 'invalid' + )).to eq(nil) + expect(spy_logger).to have_received(:log).once.with(Logger::ERROR, 'Provided attributes are in an invalid format.') + end + + it 'should return OptimizelyUserContext with valid user ID and attributes' do + expect(project_instance.create_user_context( + 'test_user', + 'browser' => 'chrome' + )).to be_instance_of(Optimizely::OptimizelyUserContext) + end + end + describe '#activate' do before(:example) do allow(Time).to receive(:now).and_return(time_now) From a7b78d8a3bae6678680a04f9a26e434c7bc16b82 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 21 Oct 2020 19:34:55 +0500 Subject: [PATCH 02/46] line at EOF --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index a1653a5c..637df6ef 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,4 @@ Gemfile.lock *.DS_Store .ruby-version vendor/bundle -*.code-workspace \ No newline at end of file +*.code-workspace From c8d3a358336ed495bd152c06fa87d3aab2a39510 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 23 Oct 2020 18:14:54 +0500 Subject: [PATCH 03/46] feat(Decide): Add Decide API --- lib/optimizely.rb | 118 +++++++++++++++++- .../decide/optimizely_decide_option.rb | 28 +++++ lib/optimizely/decide/optimizely_decision.rb | 40 ++++++ .../decide/optimizely_decision_message.rb | 26 ++++ lib/optimizely/decision_service.rb | 30 ++--- lib/optimizely/helpers/constants.rb | 1 + lib/optimizely/optimizely_user_context.rb | 4 +- 7 files changed, 231 insertions(+), 16 deletions(-) create mode 100644 lib/optimizely/decide/optimizely_decide_option.rb create mode 100644 lib/optimizely/decide/optimizely_decision.rb create mode 100644 lib/optimizely/decide/optimizely_decision_message.rb diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 553cfd17..921b9a94 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -19,6 +19,9 @@ require_relative 'optimizely/config/datafile_project_config' require_relative 'optimizely/config_manager/http_project_config_manager' require_relative 'optimizely/config_manager/static_project_config_manager' +require_relative 'optimizely/decide/optimizely_decide_option' +require_relative 'optimizely/decide/optimizely_decision' +require_relative 'optimizely/decide/optimizely_decision_message' require_relative 'optimizely/decision_service' require_relative 'optimizely/error_handler' require_relative 'optimizely/event_builder' @@ -68,12 +71,21 @@ def initialize( sdk_key = nil, config_manager = nil, notification_center = nil, - event_processor = nil + event_processor = nil, + default_decide_options = [] ) @logger = logger || NoOpLogger.new @error_handler = error_handler || NoOpErrorHandler.new @event_dispatcher = event_dispatcher || EventDispatcher.new(logger: @logger, error_handler: @error_handler) @user_profile_service = user_profile_service + @default_decide_options = [] + + if default_decide_options.is_a? Array + @default_decide_options = default_decide_options.clone + else + @logger.log(Logger::DEBUG, 'Provided default decide options is not an array.') + @default_decide_options = [] + end begin validate_instantiation_options @@ -126,6 +138,110 @@ def create_user_context(user_id, attributes = nil) user_context 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('activate').message) + reasons.append(OptimizelyDecisionMessage::SDK_NOT_READY) + return OptimizelyDecision.new(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') + return OptimizelyDecision.new(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 '#{feature_flag_key}'.") + reasons.append(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) + return OptimizelyDecision.new(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 + + # 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 = nil + all_variables = {} + decision_event_dispatched = false + + decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes, decide_options) + + # Send impression event if Decision came from a feature test and decide options doesn't include disableDecisionEvent + if decision.is_a?(Optimizely::DecisionService::Decision) + variation = decision['variation'] + variation_key = variation['key'] + feature_enabled = variation['featureEnabled'] + flag_key = key + rule_key = decision.experiment['key'] + + if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT + source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + send_impression( + config, decision.experiment, variation_key, flag_key, rule_key, source_string, user_id, attributes + ) + decision_event_dispatched = true + end + end + end + + # Generate all variables map if decide options doesn't include excludeVariables + if decision.is_a?(Optimizely::DecisionService::Decision) + unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES + + feature_flag['variables'].each do |variable| + variable_value = get_feature_variable_for_variation(key, feature_enabled, variation, variable, user_id) + all_variables[variable['key']] = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) + end + end + end + + # Send notification + @notification_center.send_notifications( + NotificationCenter::NOTIFICATION_TYPES[:DECISION], + Helpers::Constants::DECISION_NOTIFICATION_TYPES['FLAG'], + user_id, (attributes || {}), + flag_key: flag_key, + enabled: feature_enabled, + variables: all_variables, + variation_key: variation_key, + rule_key: rule_key, + reasons: reasons, + decision_event_dispatched: decision_event_dispatched + ) + + Optimizely::Decide::OptimizelyDecision.new( + variation_key: variation_key, + enabled: feature_enabled, + variables: all_variables, + rule_key: rule_key, + flag_key: flag_key, + user_context: user_context, + reasons: reasons + ) + end + + # Buckets visitor and sends impression event to Optimizely. # # @param experiment_key - Experiment which needs to be activated. diff --git a/lib/optimizely/decide/optimizely_decide_option.rb b/lib/optimizely/decide/optimizely_decide_option.rb new file mode 100644 index 00000000..80816948 --- /dev/null +++ b/lib/optimizely/decide/optimizely_decide_option.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Copyright 2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +module Optimizely + module Decide + module OptimizelyDecideOption + DISABLE_DECISION_EVENT = 'disableDecisionEvent' + ENABLED_FLAGS_ONLY = 'enabledFlagsOnly' + IGNORE_USER_PROFILE_SERVICE = 'ignoreUserProfileService' + INCLUDE_REASONS = 'includeReasons' + EXCLUDE_VARIABLES = 'excludeVariables' + end + end +end diff --git a/lib/optimizely/decide/optimizely_decision.rb b/lib/optimizely/decide/optimizely_decision.rb new file mode 100644 index 00000000..79dedf85 --- /dev/null +++ b/lib/optimizely/decide/optimizely_decision.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# Copyright 2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +module Optimizely + module Decide + class OptimizelyDecision + def initialize( + variation_key: nil, + enabled: nil, + variables: nil, + rule_key: nil, + flag_key: nil, + user_context: nil, + reasons: nil + ) + @variation_key = variation_key + @enabled = enabled || false, + @variables = variables || {} + @rule_key = rule_key + @flag_key = flag_key + @user_context = user_context + @reasons = reasons || [] + end + end + end +end diff --git a/lib/optimizely/decide/optimizely_decision_message.rb b/lib/optimizely/decide/optimizely_decision_message.rb new file mode 100644 index 00000000..c7f26dfa --- /dev/null +++ b/lib/optimizely/decide/optimizely_decision_message.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# Copyright 2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +module Optimizely + module Decide + module OptimizelyDecisionMessage + SDK_NOT_READY = 'Optimizely SDK not configured properly yet.' + FLAG_KEY_INVALID = 'No flag was found for key \"%s\"."' + VARIABLE_VALUE_INVALID = 'Variable value for key \"%s\" is invalid or wrong type.' + end + end +end diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 2b8e0d67..4818c364 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) + 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 @@ -83,15 +83,17 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil) whitelisted_variation_id = get_whitelisted_variation_id(project_config, experiment_key, user_id) return whitelisted_variation_id if whitelisted_variation_id - # Check for saved bucketing decisions - user_profile = get_user_profile(user_id) - saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile) - if saved_variation_id - @logger.log( - Logger::INFO, - "Returning previously activated variation ID #{saved_variation_id} of experiment '#{experiment_key}' for user '#{user_id}' from user profile." - ) - return saved_variation_id + # Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService + unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE + user_profile = get_user_profile(user_id) + saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile) + if saved_variation_id + @logger.log( + Logger::INFO, + "Returning previously activated variation ID #{saved_variation_id} of experiment '#{experiment_key}' for user '#{user_id}' from user profile." + ) + return saved_variation_id + end end # Check audience conditions @@ -118,11 +120,11 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil) end # Persist bucketing decision - save_user_profile(user_profile, experiment_id, variation_id) + save_user_profile(user_profile, experiment_id, variation_id) unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE variation_id end - def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil) + def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil, decide_options = nil) # Get the variation the user is bucketed into for the given FeatureFlag. # # project_config - project_config - Instance of ProjectConfig @@ -133,7 +135,7 @@ 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) # 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) + decision = get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes, decide_options) return decision unless decision.nil? decision = get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes) @@ -141,7 +143,7 @@ def get_variation_for_feature(project_config, feature_flag, user_id, attributes decision end - def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil) + def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, _decide_options = nil) # Gets the variation the user is bucketed into for the feature flag's experiment. # # project_config - project_config - Instance of ProjectConfig diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb index ed5e1c6d..45dede71 100644 --- a/lib/optimizely/helpers/constants.rb +++ b/lib/optimizely/helpers/constants.rb @@ -369,6 +369,7 @@ module Constants 'FEATURE' => 'feature', 'FEATURE_TEST' => 'feature-test', 'FEATURE_VARIABLE' => 'feature-variable', + 'FLAG' => 'flag', 'ALL_FEATURE_VARIABLES' => 'all-feature-variables' }.freeze diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index d4a74553..36ebf866 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -20,6 +20,8 @@ module Optimizely class OptimizelyUserContext # Representation of an Optimizely User Context using which APIs are to be called. + attr_reader :user_id, :user_attributes + def initialize(optimizely_client, user_id, user_attributes) @optimizely_client = optimizely_client @user_id = user_id @@ -33,7 +35,7 @@ def set_attribute(attribute_key, attribute_value) end def decide(key, options = nil) - # TODO: call decide API in Optimizely class. + return @optimizely_client.decide(self, key, options) end def decide_for_keys(keys, options = nil) From f770ce7900d0c252cc3a6fc50bdad6aad056c517 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 23 Oct 2020 18:15:46 +0500 Subject: [PATCH 04/46] fix --- lib/optimizely.rb | 1 - lib/optimizely/optimizely_user_context.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 921b9a94..72157ebe 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -241,7 +241,6 @@ def decide(user_context, key, decide_options = []) ) end - # Buckets visitor and sends impression event to Optimizely. # # @param experiment_key - Experiment which needs to be activated. diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 36ebf866..817fa990 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -35,7 +35,7 @@ def set_attribute(attribute_key, attribute_value) end def decide(key, options = nil) - return @optimizely_client.decide(self, key, options) + @optimizely_client.decide(self, key, options) end def decide_for_keys(keys, options = nil) From 244c4f35df73bab4f495483a0ad356c3a411f41d Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 23 Oct 2020 18:17:40 +0500 Subject: [PATCH 05/46] nil by [] --- lib/optimizely/decision_service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 4818c364..13dfa946 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -124,7 +124,7 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec variation_id end - def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil, decide_options = 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 @@ -143,7 +143,7 @@ def get_variation_for_feature(project_config, feature_flag, user_id, attributes decision end - def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, _decide_options = 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 From 1bd6df7396a7612d65f5c2f2ff7d7d7fa815a278 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 23 Oct 2020 18:24:22 +0500 Subject: [PATCH 06/46] fix enabled --- lib/optimizely/decide/optimizely_decision.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/decide/optimizely_decision.rb b/lib/optimizely/decide/optimizely_decision.rb index 79dedf85..8e73ad7f 100644 --- a/lib/optimizely/decide/optimizely_decision.rb +++ b/lib/optimizely/decide/optimizely_decision.rb @@ -28,8 +28,8 @@ def initialize( reasons: nil ) @variation_key = variation_key - @enabled = enabled || false, - @variables = variables || {} + @enabled = enabled || false + @variables = variables || {} @rule_key = rule_key @flag_key = flag_key @user_context = user_context From ddd53494fcd852ff8553bcfb80fa291b3030db61 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Tue, 27 Oct 2020 12:54:11 +0500 Subject: [PATCH 07/46] add to_json in entities --- lib/optimizely/decide/optimizely_decision.rb | 17 +++++++++++++++++ lib/optimizely/optimizely_user_context.rb | 13 +++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/optimizely/decide/optimizely_decision.rb b/lib/optimizely/decide/optimizely_decision.rb index 8e73ad7f..4aeda998 100644 --- a/lib/optimizely/decide/optimizely_decision.rb +++ b/lib/optimizely/decide/optimizely_decision.rb @@ -15,6 +15,8 @@ # limitations under the License. # +require 'json' + module Optimizely module Decide class OptimizelyDecision @@ -35,6 +37,21 @@ def initialize( @user_context = user_context @reasons = reasons || [] end + + def as_json + { + variation_key: @variation_key, + enabled: @enabled, + variables: @variables, + rule_key: @rule_key, + flag_key: @flag_key, + user_context: @user_context.as_json + } + end + + def to_json(*args) + as_json.to_json(*args) + end end end end diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 817fa990..6846410b 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -16,6 +16,8 @@ # limitations under the License. # +require 'json' + module Optimizely class OptimizelyUserContext # Representation of an Optimizely User Context using which APIs are to be called. @@ -49,5 +51,16 @@ def decide_all(options = nil) def track_event(event_key, event_tags = nil) @optimizely_client.track(event_key, @user_id, @user_attributes, event_tags) end + + def as_json + { + user_id: @user_id, + attributes: @user_attributes + } + end + + def to_json(*args) + as_json.to_json(*args) + end end end From 98ced8c851d953b7022bef50f36746617304b206 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 30 Oct 2020 12:53:19 -0700 Subject: [PATCH 08/46] Made it work with https://github.com/optimizely/fullstack-sdk-compatibility-suite/pull/478 --- lib/optimizely.rb | 38 ++++++++++++------- .../decide/optimizely_decide_option.rb | 10 ++--- lib/optimizely/decide/optimizely_decision.rb | 3 +- lib/optimizely/decision_service.rb | 4 +- lib/optimizely/optimizely_user_context.rb | 9 ++++- 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 72157ebe..4c5bc6d8 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -41,6 +41,8 @@ module Optimizely class Project + include Optimizely::Decide + attr_reader :notification_center # @api no-doc attr_reader :config_manager, :decision_service, :error_handler, :event_dispatcher, @@ -148,27 +150,27 @@ def decide(user_context, key, decide_options = []) unless is_valid @logger.log(Logger::ERROR, InvalidProjectConfigError.new('activate').message) reasons.append(OptimizelyDecisionMessage::SDK_NOT_READY) - return OptimizelyDecision.new(key: key, user_context: user_context, reasons: reasons) + 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') - return OptimizelyDecision.new(key: key, user_context: user_context, reasons: reasons) + 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 '#{feature_flag_key}'.") + @logger.log(Logger::ERROR, "No feature flag was found for key '#{key}'.") reasons.append(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) - return OptimizelyDecision.new(key: key, user_context: user_context, reasons: reasons) + 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 + 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 @@ -180,7 +182,7 @@ def decide(user_context, key, decide_options = []) variation_key = nil feature_enabled = false rule_key = nil - flag_key = nil + flag_key = key all_variables = {} decision_event_dispatched = false @@ -206,13 +208,10 @@ def decide(user_context, key, decide_options = []) end # Generate all variables map if decide options doesn't include excludeVariables - if decision.is_a?(Optimizely::DecisionService::Decision) - unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES - - feature_flag['variables'].each do |variable| - variable_value = get_feature_variable_for_variation(key, feature_enabled, variation, variable, user_id) - all_variables[variable['key']] = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) - end + unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES + feature_flag['variables'].each do |variable| + variable_value = get_feature_variable_for_variation(key, feature_enabled, variation, variable, user_id) + all_variables[variable['key']] = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) end end @@ -230,7 +229,7 @@ def decide(user_context, key, decide_options = []) decision_event_dispatched: decision_event_dispatched ) - Optimizely::Decide::OptimizelyDecision.new( + OptimizelyDecision.new( variation_key: variation_key, enabled: feature_enabled, variables: all_variables, @@ -241,6 +240,17 @@ def decide(user_context, key, decide_options = []) ) 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 + + decisions = [] + project_config.feature_flags.each do |feature_flag| + decisions.push(decide(user_context, feature_flag['key'], decide_options)) + end + decisions + end + # Buckets visitor and sends impression event to Optimizely. # # @param experiment_key - Experiment which needs to be activated. diff --git a/lib/optimizely/decide/optimizely_decide_option.rb b/lib/optimizely/decide/optimizely_decide_option.rb index 80816948..f89dcd51 100644 --- a/lib/optimizely/decide/optimizely_decide_option.rb +++ b/lib/optimizely/decide/optimizely_decide_option.rb @@ -18,11 +18,11 @@ module Optimizely module Decide module OptimizelyDecideOption - DISABLE_DECISION_EVENT = 'disableDecisionEvent' - ENABLED_FLAGS_ONLY = 'enabledFlagsOnly' - IGNORE_USER_PROFILE_SERVICE = 'ignoreUserProfileService' - INCLUDE_REASONS = 'includeReasons' - EXCLUDE_VARIABLES = 'excludeVariables' + DISABLE_DECISION_EVENT = 'DISABLE_DECISION_EVENT' + ENABLED_FLAGS_ONLY = 'ENABLED_FLAGS_ONLY' + IGNORE_USER_PROFILE_SERVICE = 'IGNORE_USER_PROFILE_SERVICE' + INCLUDE_REASONS = 'INCLUDE_REASONS' + EXCLUDE_VARIABLES = 'EXCLUDE_VARIABLES' end end end diff --git a/lib/optimizely/decide/optimizely_decision.rb b/lib/optimizely/decide/optimizely_decision.rb index 4aeda998..1d5d329f 100644 --- a/lib/optimizely/decide/optimizely_decision.rb +++ b/lib/optimizely/decide/optimizely_decision.rb @@ -45,7 +45,8 @@ def as_json variables: @variables, rule_key: @rule_key, flag_key: @flag_key, - user_context: @user_context.as_json + user_context: @user_context.as_json, + reasons: @reasons } end diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 13dfa946..e7159e19 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -143,7 +143,7 @@ def get_variation_for_feature(project_config, feature_flag, user_id, attributes decision end - def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, _decide_options = []) + 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 @@ -174,7 +174,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_id, end experiment_key = experiment['key'] - variation_id = get_variation(project_config, experiment_key, user_id, attributes) + variation_id = get_variation(project_config, experiment_key, user_id, attributes, decide_options) next unless variation_id diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 6846410b..9ffeae59 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -41,11 +41,16 @@ def decide(key, options = nil) end def decide_for_keys(keys, options = nil) - # TODO: call decideForKeys in Optimizely class. + decisions = [] + keys.each do |key| + decisions = decisions.push(@optimizely_client.decide(self, key, options)) + end + decisions end def decide_all(options = nil) - # TODO: call decideForAll in optimizely class. + decisions = @optimizely_client.decide_all(self, options) + decisions end def track_event(event_key, event_tags = nil) From e7366f4ebe166f63b9bdf4c08c535fd2782fcb55 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 30 Oct 2020 16:07:25 -0700 Subject: [PATCH 09/46] fixed decide_all api --- lib/optimizely.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 4c5bc6d8..67c4b375 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -244,9 +244,9 @@ 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 - decisions = [] + decisions = {} project_config.feature_flags.each do |feature_flag| - decisions.push(decide(user_context, feature_flag['key'], decide_options)) + decisions[feature_flag['key']] = decide(user_context, feature_flag['key'], decide_options) end decisions end From 03a397f3177762cfd3e7860ed938f90898d42ebd Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 2 Nov 2020 13:21:49 -0800 Subject: [PATCH 10/46] fixed decide_for_keys --- lib/optimizely/optimizely_user_context.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 9ffeae59..ef84421c 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -41,9 +41,9 @@ def decide(key, options = nil) end def decide_for_keys(keys, options = nil) - decisions = [] + decisions = {} keys.each do |key| - decisions = decisions.push(@optimizely_client.decide(self, key, options)) + decisions[key] = @optimizely_client.decide(self, key, options) end decisions end From 3e9e417f45c5f4f0bf4d77cf2d67586a8d94ce58 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 2 Nov 2020 14:13:53 -0800 Subject: [PATCH 11/46] fixed some failing unit tests --- spec/decision_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 3d9537e8..9f415ab9 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -372,7 +372,7 @@ # 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) + .with(config, multivariate_experiment['key'], 'user_1', user_attributes, []) .and_return(nil) end @@ -431,10 +431,10 @@ 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) + .with(config, mutex_exp['key'], user_id, user_attributes, []) .and_return(nil) allow(decision_service).to receive(:get_variation) - .with(config, mutex_exp2['key'], user_id, user_attributes) + .with(config, mutex_exp2['key'], user_id, user_attributes, []) .and_return(nil) end From ab824c7a75d407f67b83908c9a09dbf08a8353f5 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 3 Nov 2020 12:55:50 -0800 Subject: [PATCH 12/46] added some test cases --- lib/optimizely.rb | 2 +- spec/decision_service_spec.rb | 8 ++++++++ spec/project_spec.rb | 38 +++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 67c4b375..dafe0c58 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -148,7 +148,7 @@ def decide(user_context, key, decide_options = []) # check if SDK is ready unless is_valid - @logger.log(Logger::ERROR, InvalidProjectConfigError.new('activate').message) + @logger.log(Logger::ERROR, InvalidProjectConfigError.new('decide').message) reasons.append(OptimizelyDecisionMessage::SDK_NOT_READY) return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) end diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 9f415ab9..7efdf9b2 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -337,6 +337,14 @@ 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 decide option', :decide do + it 'should ignore user profile service if this option is set' do + end + + it 'should not ignore user profile service if this option is not set' do + end + end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 10d91dd9..8a92cc4d 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3485,4 +3485,42 @@ def callback(_args); end .to eq(nil) end end + + describe '#decide', :decide do + describe 'should return empty decision object with correct reason when sdk is not ready' do + it 'when sdk is not ready' do + end + + it 'when flag key is invalid' do + end + + it 'when flag key is not available' do + end + end + + describe 'should return correct decision object' do + it 'when flag key is part of feature experiment' do + end + + # add more tests here + end + + describe 'decide options' do + describe 'DISABLE_DECISION_EVENT' do + it 'should send event when option is not set' do + end + + it 'should not send event when option is set' do + end + end + + describe 'EXCLUDE_VARIABLES' do + it 'should exclude variables if set' do + end + + it 'should include variables if not set' do + end + end + end + end end From 507dac6a9c55fbf2b99804cac17c5a707ebbed9c Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 3 Nov 2020 17:22:55 -0800 Subject: [PATCH 13/46] added decide temporary tags --- spec/optimizely_user_context_spec.rb | 5 ++++- spec/project_spec.rb | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb index 4e1cc78f..2af8b822 100644 --- a/spec/optimizely_user_context_spec.rb +++ b/spec/optimizely_user_context_spec.rb @@ -19,7 +19,7 @@ require 'optimizely' require 'optimizely/optimizely_user_context' -describe 'Optimizely' do +describe 'Optimizely', :decide do let(:config_body) { OptimizelySpec::VALID_CONFIG_BODY } let(:config_body_JSON) { OptimizelySpec::VALID_CONFIG_BODY_JSON } let(:config_body_invalid_JSON) { OptimizelySpec::INVALID_CONFIG_BODY_JSON } @@ -67,5 +67,8 @@ expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq(expected_attributes) end + + it 'should not alter original attributes object when attrubute is modifed' do + end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 8a92cc4d..ebc11666 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -136,7 +136,7 @@ class InvalidErrorHandler; end end end - describe '#create_user_context' do + describe '#create_user_context', :decide do it 'should log and return nil when user ID is non string' do expect(project_instance.create_user_context(nil)).to eq(nil) expect(project_instance.create_user_context(5)).to eq(nil) From 13833dea87ef6cee8f64fea07708ab9fbffcf20c Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 3 Nov 2020 17:41:37 -0800 Subject: [PATCH 14/46] added pending to forcefully fail unfinished tests --- spec/decision_service_spec.rb | 2 ++ spec/optimizely_user_context_spec.rb | 1 + spec/project_spec.rb | 8 ++++++++ 3 files changed, 11 insertions(+) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 7efdf9b2..3fbb9727 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -340,9 +340,11 @@ describe 'IGNORE_USER_PROFILE decide option', :decide do it 'should ignore user profile service if this option is set' do + pending end it 'should not ignore user profile service if this option is not set' do + pending end end end diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb index 2af8b822..17be2f52 100644 --- a/spec/optimizely_user_context_spec.rb +++ b/spec/optimizely_user_context_spec.rb @@ -69,6 +69,7 @@ end it 'should not alter original attributes object when attrubute is modifed' do + pending end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index ebc11666..8108dfaa 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3489,17 +3489,21 @@ def callback(_args); end describe '#decide', :decide do describe 'should return empty decision object with correct reason when sdk is not ready' do it 'when sdk is not ready' do + pending end it 'when flag key is invalid' do + pending end it 'when flag key is not available' do + pending end end describe 'should return correct decision object' do it 'when flag key is part of feature experiment' do + pending end # add more tests here @@ -3508,17 +3512,21 @@ def callback(_args); end describe 'decide options' do describe 'DISABLE_DECISION_EVENT' do it 'should send event when option is not set' do + pending end it 'should not send event when option is set' do + pending end end describe 'EXCLUDE_VARIABLES' do it 'should exclude variables if set' do + pending end it 'should include variables if not set' do + pending end end end From b981f64cac47bf5e53b5f0972598fbd8b1d2c83d Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 3 Nov 2020 18:17:00 -0800 Subject: [PATCH 15/46] added some unit tests for decide --- lib/optimizely.rb | 1 + .../decide/optimizely_decision_message.rb | 4 ++-- spec/project_spec.rb | 22 ++++++++++++++++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index dafe0c58..220d32f9 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -156,6 +156,7 @@ def decide(user_context, key, decide_options = []) # validate that key is a string unless key.is_a?(String) @logger.log(Logger::ERROR, 'Provided key is invalid') + reasons.append(OptimizelyDecisionMessage::VARIABLE_VALUE_INVALID) return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) end diff --git a/lib/optimizely/decide/optimizely_decision_message.rb b/lib/optimizely/decide/optimizely_decision_message.rb index c7f26dfa..d38c7aec 100644 --- a/lib/optimizely/decide/optimizely_decision_message.rb +++ b/lib/optimizely/decide/optimizely_decision_message.rb @@ -19,8 +19,8 @@ module Optimizely module Decide module OptimizelyDecisionMessage SDK_NOT_READY = 'Optimizely SDK not configured properly yet.' - FLAG_KEY_INVALID = 'No flag was found for key \"%s\"."' - VARIABLE_VALUE_INVALID = 'Variable value for key \"%s\" is invalid or wrong type.' + FLAG_KEY_INVALID = 'No flag was found for key "%s".' + VARIABLE_VALUE_INVALID = 'Variable value for key "%s" is invalid or wrong type.' end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 8108dfaa..237afb85 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3489,15 +3489,31 @@ def callback(_args); end describe '#decide', :decide do describe 'should return empty decision object with correct reason when sdk is not ready' do it 'when sdk is not ready' do - pending + invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) + user_context = project_instance.create_user_context('user1') + decision = invalid_project.decide(user_context, 'dummy_flag') + expect(decision.as_json).to include( + flag_key: 'dummy_flag', + reasons: ['Optimizely SDK not configured properly yet.'] + ) end it 'when flag key is invalid' do - pending + user_context = project_instance.create_user_context('user1') + decision = project_instance.decide(user_context, 123) + expect(decision.as_json).to include( + flag_key: 123, + reasons: ['Variable value for key "%s" is invalid or wrong type.'] + ) end it 'when flag key is not available' do - pending + user_context = project_instance.create_user_context('user1') + decision = project_instance.decide(user_context, 'not_found_key') + expect(decision.as_json).to include( + flag_key: 'not_found_key', + reasons: ['No flag was found for key "not_found_key".'] + ) end end From d102a149c640b4d30b8452e1aa5312c715cde028 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 4 Nov 2020 18:49:14 -0800 Subject: [PATCH 16/46] added unit tests for decide api --- spec/project_spec.rb | 145 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 139 insertions(+), 6 deletions(-) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 237afb85..909b8e11 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3518,8 +3518,43 @@ def callback(_args); end end describe 'should return correct decision object' do - it 'when flag key is part of feature experiment' do - pending + it 'when user is bucketed into a feature experiment' do + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + 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], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: true, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred', + rule_key: 'test_experiment_multivariate', + reasons: [], + decision_event_dispatched: true + ) + allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + 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) + user_context = project_instance.create_user_context('user1') + decision = project_instance.decide(user_context, 'multi_variate_feature') + expect(decision.as_json).to include( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred' + ) end # add more tests here @@ -3528,23 +3563,121 @@ def callback(_args); end describe 'decide options' do describe 'DISABLE_DECISION_EVENT' do it 'should send event when option is not set' do - pending + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + expect(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + 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) + user_context = project_instance.create_user_context('user1') + project_instance.decide(user_context, 'multi_variate_feature') end it 'should not send event when option is set' do - pending + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + expect(project_instance.event_dispatcher).not_to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + 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) + user_context = project_instance.create_user_context('user1') + project_instance.decide(user_context, 'multi_variate_feature', [Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT]) end end describe 'EXCLUDE_VARIABLES' do it 'should exclude variables if set' do - pending + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + 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) + 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( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {}, + variation_key: 'Fred' + ) end it 'should include variables if not set' do - pending + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + 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) + user_context = project_instance.create_user_context('user1') + decision = project_instance.decide(user_context, 'multi_variate_feature') + expect(decision.as_json).to include( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred' + ) end end + + it 'should pass on decide options to internal methods' do + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + 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) + 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 + 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 + 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) + .with(anything, 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 + ]).once + project_instance + .decide(user_context, 'multi_variate_feature', [ + 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 + ]) + end end end end From 7fb2ddfd7976ffd8a3f2fae648fc253b743dd979 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 5 Nov 2020 18:41:54 -0800 Subject: [PATCH 17/46] added tests for ignoring user profile service option --- lib/optimizely/decision_service.rb | 5 +++-- spec/decision_service_spec.rb | 34 ++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index e7159e19..08f7ddb5 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -83,8 +83,9 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec whitelisted_variation_id = get_whitelisted_variation_id(project_config, experiment_key, user_id) return whitelisted_variation_id 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 decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE + unless should_ignore_user_profile_service user_profile = get_user_profile(user_id) saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile) if saved_variation_id @@ -120,7 +121,7 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec end # Persist bucketing decision - save_user_profile(user_profile, experiment_id, variation_id) unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE + save_user_profile(user_profile, experiment_id, variation_id) unless should_ignore_user_profile_service variation_id end diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 3fbb9727..4a4d953c 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -340,11 +340,41 @@ describe 'IGNORE_USER_PROFILE decide option', :decide do it 'should ignore user profile service if this option is set' do - pending + saved_user_profile = { + user_id: 'test_user', + experiment_bucket_map: { + '111127' => { + variation_id: '111129' + } + } + } + allow(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', nil, [Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE])).to eq('111128') + + 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(:save) end it 'should not ignore user profile service if this option is not set' do - pending + saved_user_profile = { + user_id: 'test_user', + experiment_bucket_map: { + '111127' => { + variation_id: '111129' + } + } + } + allow(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') + + expect(decision_service.bucketer).not_to have_received(:bucket) + expect(Optimizely::Audience).not_to have_received(:user_meets_audience_conditions?) + expect(spy_user_profile_service).not_to have_received(:save) end end end From 16a509db66ab022c06f8c443a3e2bf9fc7fd83df Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 5 Nov 2020 18:53:37 -0800 Subject: [PATCH 18/46] added few more tests and some cleanup --- lib/optimizely/optimizely_user_context.rb | 2 +- spec/decision_service_spec.rb | 2 +- spec/optimizely_user_context_spec.rb | 14 +++++++++++--- spec/project_spec.rb | 4 ++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index ef84421c..cbfaf35d 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -27,7 +27,7 @@ class OptimizelyUserContext def initialize(optimizely_client, user_id, user_attributes) @optimizely_client = optimizely_client @user_id = user_id - @user_attributes = user_attributes + @user_attributes = user_attributes.clone @user_attributes = {} if @user_attributes.nil? end diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 4a4d953c..25f6107f 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -338,7 +338,7 @@ .with(Logger::ERROR, "Error while saving user profile for user ID 'test_user': uncaught throw :SaveError.") end - describe 'IGNORE_USER_PROFILE decide option', :decide do + describe 'IGNORE_USER_PROFILE decide option' do it 'should ignore user profile service if this option is set' do saved_user_profile = { user_id: 'test_user', diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb index 17be2f52..ed8b8bf6 100644 --- a/spec/optimizely_user_context_spec.rb +++ b/spec/optimizely_user_context_spec.rb @@ -19,7 +19,7 @@ require 'optimizely' require 'optimizely/optimizely_user_context' -describe 'Optimizely', :decide do +describe 'Optimizely' do let(:config_body) { OptimizelySpec::VALID_CONFIG_BODY } let(:config_body_JSON) { OptimizelySpec::VALID_CONFIG_BODY_JSON } let(:config_body_invalid_JSON) { OptimizelySpec::INVALID_CONFIG_BODY_JSON } @@ -68,8 +68,16 @@ expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq(expected_attributes) end - it 'should not alter original attributes object when attrubute is modifed' do - pending + it 'should not alter original attributes object when attrubute is modified in the user context' do + user_id = 'test_user' + original_attributes = {'browser' => 'firefox'} + user_context_obj = Optimizely::OptimizelyUserContext.new(project_instance, user_id, original_attributes) + user_context_obj.set_attribute('id', 49) + expect(user_context_obj.instance_variable_get(:@user_attributes)). to eq( + 'browser' => 'firefox', + 'id' => 49 + ) + expect(original_attributes).to eq('browser' => 'firefox') end end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 909b8e11..db0e0648 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -136,7 +136,7 @@ class InvalidErrorHandler; end end end - describe '#create_user_context', :decide do + describe '#create_user_context' do it 'should log and return nil when user ID is non string' do expect(project_instance.create_user_context(nil)).to eq(nil) expect(project_instance.create_user_context(5)).to eq(nil) @@ -3486,7 +3486,7 @@ def callback(_args); end end end - describe '#decide', :decide do + describe '#decide' do describe 'should return empty decision object with correct reason when sdk is not ready' do it 'when sdk is not ready' do invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) From f6437b7b798de59c1041bce9bd668fe48505f585 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 5 Nov 2020 21:45:16 -0800 Subject: [PATCH 19/46] added a null check before cloning attributes --- lib/optimizely/optimizely_user_context.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index cbfaf35d..823c42ba 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -27,7 +27,7 @@ class OptimizelyUserContext def initialize(optimizely_client, user_id, user_attributes) @optimizely_client = optimizely_client @user_id = user_id - @user_attributes = user_attributes.clone + @user_attributes = user_attributes.clone unless user_attributes == nil @user_attributes = {} if @user_attributes.nil? end From ac90cd06f121a431d2659888a7ce3f5f0a59231e Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 5 Nov 2020 21:53:03 -0800 Subject: [PATCH 20/46] simplified the null check --- lib/optimizely/optimizely_user_context.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 823c42ba..f08ad204 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -27,9 +27,7 @@ class OptimizelyUserContext def initialize(optimizely_client, user_id, user_attributes) @optimizely_client = optimizely_client @user_id = user_id - @user_attributes = user_attributes.clone unless user_attributes == nil - - @user_attributes = {} if @user_attributes.nil? + @user_attributes = user_attributes.nil? ? {} : user_attributes.clone end def set_attribute(attribute_key, attribute_value) From 4026855a5d0ad5fb8c90f6fdf915a53d54cc8e77 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 5 Nov 2020 22:09:37 -0800 Subject: [PATCH 21/46] changed array append to push to support old ruby versions --- lib/optimizely.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 220d32f9..552706c2 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -149,14 +149,14 @@ def decide(user_context, key, decide_options = []) # check if SDK is ready unless is_valid @logger.log(Logger::ERROR, InvalidProjectConfigError.new('decide').message) - reasons.append(OptimizelyDecisionMessage::SDK_NOT_READY) + 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.append(OptimizelyDecisionMessage::VARIABLE_VALUE_INVALID) + reasons.push(OptimizelyDecisionMessage::VARIABLE_VALUE_INVALID) return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) end @@ -165,7 +165,7 @@ def decide(user_context, key, decide_options = []) 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.append(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) + reasons.push(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) end From d6be7c2cc3f9a4fec8b6168a36c8760a7ceb4e6f Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 12 Nov 2020 14:17:50 -0800 Subject: [PATCH 22/46] added support for ENABLED_FLAGS_ONLY decide option --- lib/optimizely.rb | 17 +++++++++++++++-- lib/optimizely/optimizely_user_context.rb | 9 ++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 552706c2..33fd8b0f 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -245,9 +245,22 @@ 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 - decisions = {} + keys = [] project_config.feature_flags.each do |feature_flag| - decisions[feature_flag['key']] = decide(user_context, feature_flag['key'], decide_options) + keys.push(feature_flag['key']) + end + decide_for_keys(user_context, keys, decide_options) + end + + def decide_for_keys(user_context, keys, decide_options = []) + # raising on user context as it is internal and not provided directly by the user. + raise if user_context.class != OptimizelyUserContext + + enabled_flags_only = !decide_options.nil? && (decide_options.include? Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + decisions = {} + keys.each do |key| + decision = decide(user_context, key, decide_options) + decisions[key] = decision unless enabled_flags_only && !decision.as_json[:enabled] end decisions end diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index f08ad204..94bbbcdc 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -39,16 +39,11 @@ def decide(key, options = nil) end def decide_for_keys(keys, options = nil) - decisions = {} - keys.each do |key| - decisions[key] = @optimizely_client.decide(self, key, options) - end - decisions + @optimizely_client.decide_for_keys(self, keys, options) end def decide_all(options = nil) - decisions = @optimizely_client.decide_all(self, options) - decisions + @optimizely_client.decide_all(self, options) end def track_event(event_key, event_tags = nil) From 752114dd6f5b908afa1de2df370620a00dcda77d Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 12 Nov 2020 16:20:44 -0800 Subject: [PATCH 23/46] Added unit tests for decide_for_keys and decide_all --- spec/project_spec.rb | 116 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index db0e0648..0e1689f5 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3680,4 +3680,120 @@ def callback(_args); end end end end + + describe '#decide_all' do + it 'should get all the decisions' do + stub_request(:post, impression_log_url) + user_context = project_instance.create_user_context('user1') + decisions = project_instance.decide_all(user_context) + expect(decisions.length).to eq(10) + expect(decisions['boolean_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'boolean_single_variable_feature', + reasons: [], + rule_key: '177776', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'boolean_variable' => false}, + variation_key: '177778' + ) + expect(decisions['integer_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'integer_single_variable_feature', + reasons: [], + rule_key: 'test_experiment_integer_feature', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'integer_variable' => 42}, + variation_key: 'control' + ) + end + + it 'should get only enabled decisions for keys when ENABLED_FLAGS_ONLY is true' do + stub_request(:post, impression_log_url) + user_context = project_instance.create_user_context('user1') + decisions = project_instance.decide_all(user_context, [Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY]) + expect(decisions.length).to eq(6) + expect(decisions['boolean_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'boolean_single_variable_feature', + reasons: [], + rule_key: '177776', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'boolean_variable' => false}, + variation_key: '177778' + ) + expect(decisions['integer_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'integer_single_variable_feature', + reasons: [], + rule_key: 'test_experiment_integer_feature', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'integer_variable' => 42}, + variation_key: 'control' + ) + end + end + + describe '#decide_for_keys' do + it 'should get all the decisions for keys' do + keys = %w[ + boolean_single_variable_feature + integer_single_variable_feature + boolean_feature + empty_feature + ] + stub_request(:post, impression_log_url) + user_context = project_instance.create_user_context('user1') + decisions = project_instance.decide_for_keys(user_context, keys) + expect(decisions.length).to eq(4) + expect(decisions['boolean_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'boolean_single_variable_feature', + reasons: [], + rule_key: '177776', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'boolean_variable' => false}, + variation_key: '177778' + ) + expect(decisions['integer_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'integer_single_variable_feature', + reasons: [], + rule_key: 'test_experiment_integer_feature', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'integer_variable' => 42}, + variation_key: 'control' + ) + end + + it 'should get only enabled decisions for keys when ENABLED_FLAGS_ONLY is true' do + keys = %w[ + boolean_single_variable_feature + integer_single_variable_feature + boolean_feature + empty_feature + ] + stub_request(:post, impression_log_url) + user_context = project_instance.create_user_context('user1') + decisions = project_instance.decide_for_keys(user_context, keys, [Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY]) + expect(decisions.length).to eq(2) + expect(decisions['boolean_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'boolean_single_variable_feature', + reasons: [], + rule_key: '177776', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'boolean_variable' => false}, + variation_key: '177778' + ) + expect(decisions['integer_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'integer_single_variable_feature', + reasons: [], + rule_key: 'test_experiment_integer_feature', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'integer_variable' => 42}, + variation_key: 'control' + ) + end + end end From ee32ee1c096474dcd86707fb8158765d3b0c4600 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 12 Nov 2020 19:01:30 -0800 Subject: [PATCH 24/46] added flag decisions support in decide api --- lib/optimizely.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 33fd8b0f..1f1e5e73 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -197,17 +197,20 @@ def decide(user_context, key, decide_options = []) flag_key = key rule_key = decision.experiment['key'] - if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] - unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT - source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] - send_impression( - config, decision.experiment, variation_key, flag_key, rule_key, source_string, user_id, attributes - ) + unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT + if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || + (decision.source == Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] && config.send_flag_decisions) + send_impression(config, decision.experiment, variation_key, flag_key, rule_key, decision.source, user_id, attributes) decision_event_dispatched = true end end end + if decision.nil? && config.send_flag_decisions + send_impression(config, nil, '', flag_key, '', Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'], user_id, attributes) + decision_event_dispatched = true + end + # Generate all variables map if decide options doesn't include excludeVariables unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES feature_flag['variables'].each do |variable| From 57ac8a0343d469416f0efc338b0a22218d84cb9b Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 13 Nov 2020 10:00:26 -0800 Subject: [PATCH 25/46] added getters to OptimizelyDecision --- lib/optimizely.rb | 2 +- lib/optimizely/decide/optimizely_decision.rb | 2 ++ spec/project_spec.rb | 20 ++++++-------------- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 1f1e5e73..387eb416 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -263,7 +263,7 @@ def decide_for_keys(user_context, keys, decide_options = []) decisions = {} keys.each do |key| decision = decide(user_context, key, decide_options) - decisions[key] = decision unless enabled_flags_only && !decision.as_json[:enabled] + decisions[key] = decision unless enabled_flags_only && !decision.enabled end decisions end diff --git a/lib/optimizely/decide/optimizely_decision.rb b/lib/optimizely/decide/optimizely_decision.rb index 1d5d329f..06b109b3 100644 --- a/lib/optimizely/decide/optimizely_decision.rb +++ b/lib/optimizely/decide/optimizely_decision.rb @@ -20,6 +20,8 @@ module Optimizely module Decide class OptimizelyDecision + attr_reader :variation_key, :enabled, :variables, :rule_key, :flag_key, :user_context, :reasons + def initialize( variation_key: nil, enabled: nil, diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 0e1689f5..d0282a3a 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3492,28 +3492,22 @@ def callback(_args); end invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) user_context = project_instance.create_user_context('user1') decision = invalid_project.decide(user_context, 'dummy_flag') - expect(decision.as_json).to include( - flag_key: 'dummy_flag', - reasons: ['Optimizely SDK not configured properly yet.'] - ) + expect(decision.flag_key).to eq('dummy_flag') + expect(decision.reasons).to eq(['Optimizely SDK not configured properly yet.']) end it 'when flag key is invalid' do user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 123) - expect(decision.as_json).to include( - flag_key: 123, - reasons: ['Variable value for key "%s" is invalid or wrong type.'] - ) + expect(decision.flag_key).to eq(123) + expect(decision.reasons).to eq(['Variable value for key "%s" is invalid or wrong type.']) end it 'when flag key is not available' do user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'not_found_key') - expect(decision.as_json).to include( - flag_key: 'not_found_key', - reasons: ['No flag was found for key "not_found_key".'] - ) + expect(decision.flag_key).to eq('not_found_key') + expect(decision.reasons).to eq(['No flag was found for key "not_found_key".']) end end @@ -3556,8 +3550,6 @@ def callback(_args); end variation_key: 'Fred' ) end - - # add more tests here end describe 'decide options' do From 0070d3bdfb6aef06eba595747dbaf6bc1b4c1784 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 17 Nov 2020 10:33:33 -0800 Subject: [PATCH 26/46] added unit tests for flag decisionss support in decide API --- spec/project_spec.rb | 142 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index e1e7eb4e..27aa8243 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3559,6 +3559,148 @@ def callback(_args); end variation_key: 'Fred' ) end + + it 'when user is bucketed into a rollout and send_flag_decisions is true' do + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + 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], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: true, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred', + rule_key: 'test_experiment_multivariate', + reasons: [], + decision_event_dispatched: true + ) + allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + ) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_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( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred' + ) + end + + it 'when user is bucketed into a rollout and send_flag_decisions is false' do + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + expect(project_instance.notification_center).to receive(:send_notifications) + .once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: true, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred', + rule_key: 'test_experiment_multivariate', + reasons: [], + decision_event_dispatched: false + ) + allow(project_config).to receive(:send_flag_decisions).and_return(false) + allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + ) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_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( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred' + ) + end + + it 'when decision service returns nil and send_flag_decisions is false' do + expect(project_instance.notification_center).to receive(:send_notifications) + .once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: false, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil, + rule_key: nil, + reasons: [], + decision_event_dispatched: false + ) + allow(project_config).to receive(:send_flag_decisions).and_return(false) + allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = nil + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_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( + flag_key: 'multi_variate_feature', + enabled: false, + reasons: [], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil + ) + end + + it 'when decision service returns nil and send_flag_decisions is true' 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], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: false, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil, + rule_key: nil, + reasons: [], + decision_event_dispatched: true + ) + allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = nil + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_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( + flag_key: 'multi_variate_feature', + enabled: false, + reasons: [], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil + ) + end end describe 'decide options' do From 6727d30f2513d8608ae2bae5fb25d04685924a60 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 17 Nov 2020 10:43:17 -0800 Subject: [PATCH 27/46] added sdk ready check to all apis --- lib/optimizely.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index c6b28a70..81b21e9e 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -248,6 +248,12 @@ 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 + # check if SDK is ready + unless is_valid + @logger.log(Logger::ERROR, InvalidProjectConfigError.new('decide_all').message) + return {} + end + keys = [] project_config.feature_flags.each do |feature_flag| keys.push(feature_flag['key']) @@ -259,6 +265,12 @@ def decide_for_keys(user_context, keys, decide_options = []) # raising on user context as it is internal and not provided directly by the user. raise if user_context.class != OptimizelyUserContext + # check if SDK is ready + unless is_valid + @logger.log(Logger::ERROR, InvalidProjectConfigError.new('decide_for_keys').message) + return {} + end + enabled_flags_only = !decide_options.nil? && (decide_options.include? Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY) decisions = {} keys.each do |key| From d9a55498f7d6a28826a9cda9d34934ad3a2cbcbf Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 18 Nov 2020 11:51:18 -0800 Subject: [PATCH 28/46] added a check to include reasons --- lib/optimizely.rb | 11 ++++++----- spec/project_spec.rb | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 81b21e9e..56f98071 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -156,7 +156,7 @@ def decide(user_context, key, decide_options = []) # validate that key is a string unless key.is_a?(String) @logger.log(Logger::ERROR, 'Provided key is invalid') - reasons.push(OptimizelyDecisionMessage::VARIABLE_VALUE_INVALID) + reasons.push(format(OptimizelyDecisionMessage::FLAG_KEY_INVALID, key)) return OptimizelyDecision.new(flag_key: key, user_context: user_context, reasons: reasons) end @@ -197,7 +197,7 @@ def decide(user_context, key, decide_options = []) flag_key = key rule_key = decision.experiment['key'] - unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT + unless decide_options.include? OptimizelyDecideOption::DISABLE_DECISION_EVENT if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || (decision.source == Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] && config.send_flag_decisions) send_impression(config, decision.experiment, variation_key, flag_key, rule_key, decision.source, user_id, attributes) @@ -212,7 +212,7 @@ def decide(user_context, key, decide_options = []) end # Generate all variables map if decide options doesn't include excludeVariables - unless decide_options.include? Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES + 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) all_variables[variable['key']] = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) @@ -233,6 +233,7 @@ def decide(user_context, key, decide_options = []) decision_event_dispatched: decision_event_dispatched ) + reasons_to_include = decide_options.include? OptimizelyDecideOption::INCLUDE_REASONS ? reasons : [] OptimizelyDecision.new( variation_key: variation_key, enabled: feature_enabled, @@ -240,7 +241,7 @@ def decide(user_context, key, decide_options = []) rule_key: rule_key, flag_key: flag_key, user_context: user_context, - reasons: reasons + reasons: reasons_to_include ) end @@ -271,7 +272,7 @@ def decide_for_keys(user_context, keys, decide_options = []) return {} end - enabled_flags_only = !decide_options.nil? && (decide_options.include? Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + enabled_flags_only = !decide_options.nil? && (decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY) decisions = {} keys.each do |key| decision = decide(user_context, key, decide_options) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 27aa8243..bf57d1bf 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3509,7 +3509,7 @@ def callback(_args); end user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 123) expect(decision.flag_key).to eq(123) - expect(decision.reasons).to eq(['Variable value for key "%s" is invalid or wrong type.']) + expect(decision.reasons).to eq(['No flag was found for key "123".']) end it 'when flag key is not available' do From cd1648e06a91a1e61f22aa6d8bd819001aad1ab6 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 18 Nov 2020 15:36:20 -0800 Subject: [PATCH 29/46] added decide reasons --- lib/optimizely.rb | 2 +- lib/optimizely/bucketer.rb | 37 +++--- lib/optimizely/decision_service.rb | 191 +++++++++++++++-------------- spec/decision_service_spec.rb | 30 ++--- spec/project_spec.rb | 6 +- 5 files changed, 135 insertions(+), 131 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 56f98071..a1edbff8 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -187,7 +187,7 @@ def decide(user_context, key, decide_options = []) all_variables = {} decision_event_dispatched = false - decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes, decide_options) + decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes, decide_options, reasons) # Send impression event if Decision came from a feature test and decide options doesn't include disableDecisionEvent if decision.is_a?(Optimizely::DecisionService::Decision) diff --git a/lib/optimizely/bucketer.rb b/lib/optimizely/bucketer.rb index c3fddead..8b6c46ae 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) + def bucket(project_config, experiment, bucketing_id, user_id, decide_reasons = nil) # Determines ID of variation to be shown for a given experiment key and user ID. # # project_config - Instance of ProjectConfig @@ -58,29 +58,29 @@ def bucket(project_config, experiment, bucketing_id, user_id) bucketed_experiment_id = find_bucket(bucketing_id, user_id, group_id, traffic_allocations) # return if the user is not bucketed into any experiment unless bucketed_experiment_id - @logger.log(Logger::INFO, "User '#{user_id}' is in no experiment.") + message = "User '#{user_id}' is in no experiment." + @logger.log(Logger::INFO, message) + decide_reasons&.push(message) return nil end # return if the user is bucketed into a different experiment than the one specified if bucketed_experiment_id != experiment_id - @logger.log( - Logger::INFO, - "User '#{user_id}' is not in experiment '#{experiment_key}' of group #{group_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 end # continue bucketing if the user is bucketed into the experiment specified - @logger.log( - Logger::INFO, - "User '#{user_id}' is in experiment '#{experiment_key}' of group #{group_id}." - ) + message = "User '#{user_id}' is in experiment '#{experiment_key}' of group #{group_id}." + @logger.log(Logger::INFO, message) + decide_reasons&.push(message) end end traffic_allocations = experiment['trafficAllocation'] - variation_id = find_bucket(bucketing_id, user_id, experiment_id, traffic_allocations) + variation_id = find_bucket(bucketing_id, user_id, experiment_id, traffic_allocations, decide_reasons) if variation_id && variation_id != '' variation = project_config.get_variation_from_id(experiment_key, variation_id) return variation @@ -88,16 +88,15 @@ def bucket(project_config, experiment, bucketing_id, user_id) # Handle the case when the traffic range is empty due to sticky bucketing if variation_id == '' - @logger.log( - Logger::DEBUG, - 'Bucketed into an empty traffic range. Returning nil.' - ) + message = 'Bucketed into an empty traffic range. Returning nil.' + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(message) end nil end - def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations) + def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations, decide_reasons = nil) # 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 @@ -108,8 +107,10 @@ def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations) # Returns entity ID corresponding to the provided bucket value or nil if no match is found. bucketing_key = format(BUCKETING_ID_TEMPLATE, bucketing_id: bucketing_id, entity_id: parent_id) bucket_value = generate_bucket_value(bucketing_key) - @logger.log(Logger::DEBUG, "Assigned bucket #{bucket_value} to user '#{user_id}' "\ - "with bucketing ID: '#{bucketing_id}'.") + + message = "Assigned bucket #{bucket_value} to user '#{user_id}' with bucketing ID: '#{bucketing_id}'." + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(decide_reasons) traffic_allocations.each do |traffic_allocation| current_end_of_range = traffic_allocation['endOfRange'] diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 08f7ddb5..e596f4c7 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 = []) + def get_variation(project_config, experiment_key, user_id, attributes = nil, decide_options = [], decide_reasons = nil) # Determines variation into which user will be bucketed. # # project_config - project_config - Instance of ProjectConfig @@ -64,68 +64,68 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec # (nil if experiment is inactive or user does not meet audience conditions) # By default, the bucketing ID should be the user ID - bucketing_id = get_bucketing_id(user_id, attributes) + bucketing_id = get_bucketing_id(user_id, attributes, decide_reasons) # Check to make sure experiment is active experiment = project_config.get_experiment_from_key(experiment_key) return nil if experiment.nil? experiment_id = experiment['id'] unless project_config.experiment_running?(experiment) - @logger.log(Logger::INFO, "Experiment '#{experiment_key}' is not running.") + message = "Experiment '#{experiment_key}' is not running." + @logger.log(Logger::INFO, message) + decide_reasons&.push(message) return nil end # Check if a forced variation is set for the user - forced_variation = get_forced_variation(project_config, experiment_key, user_id) + forced_variation = get_forced_variation(project_config, experiment_key, user_id, decide_reasons) return forced_variation['id'] 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) + whitelisted_variation_id = get_whitelisted_variation_id(project_config, experiment_key, user_id, decide_reasons) return whitelisted_variation_id 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) - saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile) + user_profile = get_user_profile(user_id, decide_reasons) + saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile, decide_reasons) if saved_variation_id - @logger.log( - Logger::INFO, - "Returning previously activated variation ID #{saved_variation_id} of experiment '#{experiment_key}' for user '#{user_id}' from user profile." - ) + 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 end end # Check audience conditions unless Audience.user_meets_audience_conditions?(project_config, experiment, attributes, @logger) - @logger.log( - Logger::INFO, - "User '#{user_id}' does not meet the conditions to be in experiment '#{experiment_key}'." - ) + 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 end # Bucket normally - variation = @bucketer.bucket(project_config, experiment, bucketing_id, user_id) + variation = @bucketer.bucket(project_config, experiment, bucketing_id, user_id, decide_reasons) variation_id = variation ? variation['id'] : nil + message = '' if variation_id variation_key = variation['key'] - @logger.log( - Logger::INFO, - "User '#{user_id}' is in variation '#{variation_key}' of experiment '#{experiment_key}'." - ) + message = "User '#{user_id}' is in variation '#{variation_key}' of experiment '#{experiment_key}'." else - @logger.log(Logger::INFO, "User '#{user_id}' is in no variation.") + message = "User '#{user_id}' is in no variation." end + @logger.log(Logger::INFO, message) + decide_reasons&.push(message) # Persist bucketing decision - save_user_profile(user_profile, experiment_id, variation_id) unless should_ignore_user_profile_service + save_user_profile(user_profile, experiment_id, variation_id, decide_reasons) unless should_ignore_user_profile_service variation_id end - def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil, decide_options = []) + def get_variation_for_feature(project_config, feature_flag, user_id, attributes = nil, decide_options = [], decide_reasons = nil) # Get the variation the user is bucketed into for the given FeatureFlag. # # project_config - project_config - Instance of ProjectConfig @@ -136,15 +136,15 @@ 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) # 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) + decision = get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes, decide_options, decide_reasons) return decision unless decision.nil? - decision = get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes) + decision = get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes, decide_reasons) decision end - def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, decide_options = []) + def get_variation_for_feature_experiment(project_config, feature_flag, user_id, attributes = nil, decide_options = [], decide_reasons = nil) # Gets the variation the user is bucketed into for the feature flag's experiment. # # project_config - project_config - Instance of ProjectConfig @@ -156,10 +156,9 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_id, # or nil if the user is not bucketed into any of the experiments on the feature feature_flag_key = feature_flag['key'] if feature_flag['experimentIds'].empty? - @logger.log( - Logger::DEBUG, - "The feature flag '#{feature_flag_key}' is not used in any experiments." - ) + message = "The feature flag '#{feature_flag_key}' is not used in any experiments." + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(message) return nil end @@ -167,15 +166,14 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_id, feature_flag['experimentIds'].each do |experiment_id| experiment = project_config.experiment_id_map[experiment_id] unless experiment - @logger.log( - Logger::DEBUG, - "Feature flag experiment with ID '#{experiment_id}' is not in the datafile." - ) + message = "Feature flag experiment with ID '#{experiment_id}' is not in the datafile." + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(message) return nil end experiment_key = experiment['key'] - variation_id = get_variation(project_config, experiment_key, user_id, attributes, decide_options) + variation_id = get_variation(project_config, experiment_key, user_id, attributes, decide_options, decide_reasons) next unless variation_id @@ -184,15 +182,14 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_id, return Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST']) end - @logger.log( - Logger::INFO, - "The user '#{user_id}' is not bucketed into any of the experiments on the feature '#{feature_flag_key}'." - ) + 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) nil end - def get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes = nil) + def get_variation_for_feature_rollout(project_config, feature_flag, user_id, attributes = nil, decide_reasons = nil) # Determine which variation the user is in for a given rollout. # Returns the variation of the first experiment the user qualifies for. # @@ -202,23 +199,21 @@ 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) + bucketing_id = get_bucketing_id(user_id, attributes, decide_reasons) rollout_id = feature_flag['rolloutId'] if rollout_id.nil? || rollout_id.empty? feature_flag_key = feature_flag['key'] - @logger.log( - Logger::DEBUG, - "Feature flag '#{feature_flag_key}' is not used in a rollout." - ) + message = "Feature flag '#{feature_flag_key}' is not used in a rollout." + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(message) return nil end rollout = project_config.get_rollout_from_id(rollout_id) if rollout.nil? - @logger.log( - Logger::DEBUG, - "Rollout with ID '#{rollout_id}' is not in the datafile '#{feature_flag['key']}'" - ) + 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 end @@ -234,21 +229,19 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_id, att # 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) - @logger.log( - Logger::DEBUG, - "User '#{user_id}' does not meet the audience conditions for targeting rule '#{logging_key}'." - ) + message = "User '#{user_id}' does not meet the audience conditions for targeting rule '#{logging_key}'." + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(message) # move onto the next targeting rule next end - @logger.log( - Logger::DEBUG, - "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'." - ) + message = "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'." + @logger.log(Logger::DEBUG, 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) + 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? break @@ -259,18 +252,17 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_id, att logging_key = 'Everyone Else' # 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) - @logger.log( - Logger::DEBUG, - "User '#{user_id}' does not meet the audience conditions for targeting rule '#{logging_key}'." - ) + 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 end - @logger.log( - Logger::DEBUG, - "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'." - ) - variation = @bucketer.bucket(project_config, everyone_else_experiment, bucketing_id, user_id) + message = "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'." + @logger.log(Logger::DEBUG, 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? nil @@ -314,7 +306,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) + def get_forced_variation(project_config, experiment_key, user_id, decide_reasons = nil) # Gets the forced variation for the given user and experiment. # # project_config - Instance of ProjectConfig @@ -324,7 +316,9 @@ def get_forced_variation(project_config, experiment_key, user_id) # Returns Variation The variation which the given user and experiment should be forced into unless @forced_variation_map.key? user_id - @logger.log(Logger::DEBUG, "User '#{user_id}' is not in the forced variation map.") + message = "User '#{user_id}' is not in the forced variation map." + @logger.log(Logger::DEBUG, message) + decide_reasons&.push(message) return nil end @@ -336,8 +330,9 @@ def get_forced_variation(project_config, experiment_key, user_id) return nil if experiment_id.nil? || experiment_id.empty? unless experiment_to_variation_map.key? experiment_id - @logger.log(Logger::DEBUG, "No experiment '#{experiment_key}' mapped to user '#{user_id}' "\ - 'in the forced variation map.') + 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 end @@ -350,15 +345,16 @@ def get_forced_variation(project_config, experiment_key, user_id) # this case is logged in get_variation_from_id return nil if variation_key.empty? - @logger.log(Logger::DEBUG, "Variation '#{variation_key}' is mapped to experiment '#{experiment_key}' "\ - "and user '#{user_id}' in the forced variation map") + 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) variation end private - def get_whitelisted_variation_id(project_config, experiment_key, user_id) + def get_whitelisted_variation_id(project_config, experiment_key, user_id, decide_reasons = nil) # 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 @@ -378,21 +374,20 @@ def get_whitelisted_variation_id(project_config, experiment_key, user_id) whitelisted_variation_id = project_config.get_variation_id_from_key(experiment_key, whitelisted_variation_key) unless whitelisted_variation_id - @logger.log( - Logger::INFO, - "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}', which is not in the datafile." - ) + 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 end - @logger.log( - Logger::INFO, - "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}' of experiment '#{experiment_key}'." - ) + 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 end - def get_saved_variation_id(project_config, experiment_id, user_profile) + def get_saved_variation_id(project_config, experiment_id, user_profile, decide_reasons = nil) # Retrieve variation ID of stored bucketing decision for a given experiment from a given user profile # # project_config - project_config - Instance of ProjectConfig @@ -408,14 +403,14 @@ def get_saved_variation_id(project_config, experiment_id, user_profile) variation_id = decision[:variation_id] return variation_id if project_config.variation_id_exists?(experiment_id, variation_id) - @logger.log( - Logger::INFO, - "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 end - def get_user_profile(user_id) + def get_user_profile(user_id, decide_reasons = nil) # 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 @@ -432,13 +427,15 @@ def get_user_profile(user_id) begin user_profile = @user_profile_service.lookup(user_id) || user_profile rescue => e - @logger.log(Logger::ERROR, "Error while looking up user profile for user ID '#{user_id}': #{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 end - def save_user_profile(user_profile, experiment_id, variation_id) + def save_user_profile(user_profile, experiment_id, variation_id, decide_reasons = nil) # Save a given bucketing decision to a given user profile # # user_profile - Hash user profile @@ -453,13 +450,17 @@ def save_user_profile(user_profile, experiment_id, variation_id) variation_id: variation_id } @user_profile_service.save(user_profile) - @logger.log(Logger::INFO, "Saved variation ID #{variation_id} of experiment ID #{experiment_id} for user '#{user_id}'.") + message = "Saved variation ID #{variation_id} of experiment ID #{experiment_id} for user '#{user_id}'." + @logger.log(Logger::INFO, message) + decide_reasons&.push(message) rescue => e - @logger.log(Logger::ERROR, "Error while saving user profile for user ID '#{user_id}': #{e}.") + message = "Error while saving user profile for user ID '#{user_id}': #{e}." + @logger.log(Logger::ERROR, message) + decide_reasons&.push(message) end end - def get_bucketing_id(user_id, attributes) + def get_bucketing_id(user_id, attributes, decide_reasons = nil) # Gets the Bucketing Id for Bucketing # # user_id - String user ID @@ -473,7 +474,9 @@ def get_bucketing_id(user_id, attributes) if bucketing_id return bucketing_id if bucketing_id.is_a?(String) - @logger.log(Logger::WARN, 'Bucketing ID attribute is not a string. Defaulted to user ID.') + message = 'Bucketing ID attribute is not a string. Defaulted to user ID.' + @logger.log(Logger::WARN, message) + decide_reasons&.push(message) end user_id end diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 25f6107f..934524dd 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -181,7 +181,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') + expect(decision_service.bucketer).to have_received(:bucket).once.with(config, experiment, 'forced_user_with_invalid_variation', 'forced_user_with_invalid_variation', nil) end describe 'when a UserProfile service is provided' do @@ -412,13 +412,13 @@ # 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, []) + .with(config, multivariate_experiment['key'], 'user_1', user_attributes, [], nil) .and_return(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)).to eq(nil) + expect(decision_service.get_variation_for_feature_experiment(config, feature_flag, 'user_1', user_attributes, [], nil)).to eq(nil) 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'.") @@ -471,10 +471,10 @@ 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, []) + .with(config, mutex_exp['key'], user_id, user_attributes, [], nil) .and_return(nil) allow(decision_service).to receive(:get_variation) - .with(config, mutex_exp2['key'], user_id, user_attributes, []) + .with(config, mutex_exp2['key'], user_id, user_attributes, [], nil) .and_return(nil) end @@ -533,9 +533,9 @@ 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) + .with(config, rollout_experiment, user_id, user_id, nil) .and_return(variation) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(expected_decision) + expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(expected_decision) end end @@ -548,13 +548,13 @@ 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) + .with(config, rollout['experiments'][0], user_id, user_id, nil) .and_return(nil) allow(decision_service.bucketer).to receive(:bucket) - .with(config, everyone_else_experiment, user_id, user_id) + .with(config, everyone_else_experiment, user_id, user_id, nil) .and_return(nil) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(nil) + expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(nil) # make sure we only checked the audience for the first rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -573,13 +573,13 @@ 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) + .with(config, rollout['experiments'][0], user_id, user_id, nil) .and_return(nil) allow(decision_service.bucketer).to receive(:bucket) - .with(config, everyone_else_experiment, user_id, user_id) + .with(config, everyone_else_experiment, user_id, user_id, nil) .and_return(variation) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(expected_decision) + expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(expected_decision) # make sure we only checked the audience for the first rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -604,10 +604,10 @@ .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) + .with(config, everyone_else_experiment, user_id, user_id, nil) .and_return(variation) - expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes)).to eq(expected_decision) + expect(decision_service.get_variation_for_feature_rollout(config, feature_flag, user_id, user_attributes, nil)).to eq(expected_decision) # verify we tried to bucket in all targeting rules and the everyone else rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index bf57d1bf..bb61feed 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3795,11 +3795,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) @@ -3810,7 +3810,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, From 8ec1619c457f5284a102e768f7a2207e7318b1da Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 18 Nov 2020 16:36:27 -0800 Subject: [PATCH 30/46] fixed a minor issue with decide reasons --- lib/optimizely.rb | 4 ++-- lib/optimizely/bucketer.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index a1edbff8..1333c81e 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -233,7 +233,7 @@ def decide(user_context, key, decide_options = []) decision_event_dispatched: decision_event_dispatched ) - reasons_to_include = decide_options.include? OptimizelyDecideOption::INCLUDE_REASONS ? reasons : [] + should_include_reasons = decide_options.include? OptimizelyDecideOption::INCLUDE_REASONS OptimizelyDecision.new( variation_key: variation_key, enabled: feature_enabled, @@ -241,7 +241,7 @@ def decide(user_context, key, decide_options = []) rule_key: rule_key, flag_key: flag_key, user_context: user_context, - reasons: reasons_to_include + reasons: should_include_reasons ? reasons : [] ) end diff --git a/lib/optimizely/bucketer.rb b/lib/optimizely/bucketer.rb index 8b6c46ae..1a4994f6 100644 --- a/lib/optimizely/bucketer.rb +++ b/lib/optimizely/bucketer.rb @@ -110,7 +110,7 @@ def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations, decide_re message = "Assigned bucket #{bucket_value} to user '#{user_id}' with bucketing ID: '#{bucketing_id}'." @logger.log(Logger::DEBUG, message) - decide_reasons&.push(decide_reasons) + decide_reasons&.push(message) traffic_allocations.each do |traffic_allocation| current_end_of_range = traffic_allocation['endOfRange'] From 7d122a4f7d8f94d1389ab3c12e4756fd2ecb66f6 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 23 Nov 2020 17:40:54 -0800 Subject: [PATCH 31/46] removed some logs from decide reasons --- lib/optimizely/decision_service.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index e596f4c7..77df3635 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -121,7 +121,7 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec decide_reasons&.push(message) # Persist bucketing decision - save_user_profile(user_profile, experiment_id, variation_id, decide_reasons) unless should_ignore_user_profile_service + save_user_profile(user_profile, experiment_id, variation_id) unless should_ignore_user_profile_service variation_id end @@ -435,7 +435,7 @@ def get_user_profile(user_id, decide_reasons = nil) user_profile end - def save_user_profile(user_profile, experiment_id, variation_id, decide_reasons = nil) + def save_user_profile(user_profile, experiment_id, variation_id) # Save a given bucketing decision to a given user profile # # user_profile - Hash user profile @@ -450,13 +450,9 @@ def save_user_profile(user_profile, experiment_id, variation_id, decide_reasons variation_id: variation_id } @user_profile_service.save(user_profile) - message = "Saved variation ID #{variation_id} of experiment ID #{experiment_id} for user '#{user_id}'." - @logger.log(Logger::INFO, message) - decide_reasons&.push(message) + @logger.log(Logger::INFO, "Saved variation ID #{variation_id} of experiment ID #{experiment_id} for user '#{user_id}'.") rescue => e - message = "Error while saving user profile for user ID '#{user_id}': #{e}." - @logger.log(Logger::ERROR, message) - decide_reasons&.push(message) + @logger.log(Logger::ERROR, "Error while saving user profile for user ID '#{user_id}': #{e}.") end end From c40237fbd226ea22f9d36cde6f5055e402a5fc53 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 23 Nov 2020 17:45:06 -0800 Subject: [PATCH 32/46] additional null check for optimizely object --- lib/optimizely/optimizely_user_context.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 94bbbcdc..1f742d6e 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -35,19 +35,19 @@ def set_attribute(attribute_key, attribute_value) end def decide(key, options = nil) - @optimizely_client.decide(self, key, options) + @optimizely_client&.decide(self, key, options) end def decide_for_keys(keys, options = nil) - @optimizely_client.decide_for_keys(self, keys, options) + @optimizely_client&.decide_for_keys(self, keys, options) end def decide_all(options = nil) - @optimizely_client.decide_all(self, options) + @optimizely_client&.decide_all(self, options) end def track_event(event_key, event_tags = nil) - @optimizely_client.track(event_key, @user_id, @user_attributes, event_tags) + @optimizely_client&.track(event_key, @user_id, @user_attributes, event_tags) end def as_json From 7f6d985512918a089d32bc7998db484127ebbd9c Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 23 Nov 2020 18:17:25 -0800 Subject: [PATCH 33/46] fixed failing FSC test for some events --- lib/optimizely.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 1333c81e..09cea2db 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -200,14 +200,14 @@ def decide(user_context, key, decide_options = []) unless decide_options.include? OptimizelyDecideOption::DISABLE_DECISION_EVENT if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || (decision.source == Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] && config.send_flag_decisions) - send_impression(config, decision.experiment, variation_key, flag_key, rule_key, decision.source, user_id, attributes) + send_impression(config, decision.experiment, variation_key, flag_key, rule_key, feature_enabled, decision.source, user_id, attributes) decision_event_dispatched = true end end end if decision.nil? && config.send_flag_decisions - send_impression(config, nil, '', flag_key, '', Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'], user_id, attributes) + send_impression(config, nil, '', flag_key, '', feature_enabled, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'], user_id, attributes) decision_event_dispatched = true end From b7930127206fb991aa8f0699570979a0586834df Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 24 Nov 2020 17:29:34 -0800 Subject: [PATCH 34/46] merged the send impression calls under one check --- lib/optimizely.rb | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 09cea2db..5cd94bb5 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -186,6 +186,8 @@ def decide(user_context, key, decide_options = []) flag_key = key all_variables = {} decision_event_dispatched = false + 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) @@ -194,21 +196,15 @@ def decide(user_context, key, decide_options = []) variation = decision['variation'] variation_key = variation['key'] feature_enabled = variation['featureEnabled'] - flag_key = key rule_key = decision.experiment['key'] - - unless decide_options.include? OptimizelyDecideOption::DISABLE_DECISION_EVENT - if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || - (decision.source == Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] && config.send_flag_decisions) - send_impression(config, decision.experiment, variation_key, flag_key, rule_key, feature_enabled, decision.source, user_id, attributes) - decision_event_dispatched = true - end - end + decision_source = decision.source end - if decision.nil? && config.send_flag_decisions - send_impression(config, nil, '', flag_key, '', feature_enabled, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'], user_id, attributes) - decision_event_dispatched = true + unless decide_options.include? OptimizelyDecideOption::DISABLE_DECISION_EVENT + if decision_source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || config.send_flag_decisions + send_impression(config, experiment, variation_key || '', flag_key, rule_key || '', feature_enabled, decision_source, user_id, attributes) + decision_event_dispatched = true + end end # Generate all variables map if decide options doesn't include excludeVariables From 0a38bdddacd1857e38153a1e42ca8ae548fe2c69 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 24 Nov 2020 17:46:59 -0800 Subject: [PATCH 35/46] fixed failing FSC test --- lib/optimizely.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 5cd94bb5..70205ac1 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -193,10 +193,11 @@ def decide(user_context, key, decide_options = []) # Send impression event if Decision came from a feature test and decide options doesn't include disableDecisionEvent if decision.is_a?(Optimizely::DecisionService::Decision) + experiment = decision.experiment + rule_key = experiment['key'] variation = decision['variation'] variation_key = variation['key'] feature_enabled = variation['featureEnabled'] - rule_key = decision.experiment['key'] decision_source = decision.source end From c4015881883e092fc1b65ad94495e81ba2c64851 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 24 Nov 2020 18:00:59 -0800 Subject: [PATCH 36/46] modified some tests --- spec/project_spec.rb | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index bb61feed..e2246cf2 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3501,22 +3501,43 @@ def callback(_args); end invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) user_context = project_instance.create_user_context('user1') decision = invalid_project.decide(user_context, 'dummy_flag') - expect(decision.flag_key).to eq('dummy_flag') - expect(decision.reasons).to eq(['Optimizely SDK not configured properly yet.']) + expect(decision.as_json).to eq( + enabled: false, + flag_key: 'dummy_flag', + reasons: ['Optimizely SDK not configured properly yet.'], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {}, + variation_key: nil + ) end it 'when flag key is invalid' do user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 123) - expect(decision.flag_key).to eq(123) - expect(decision.reasons).to eq(['No flag was found for key "123".']) + expect(decision.as_json).to eq( + enabled: false, + flag_key: 123, + reasons: ['No flag was found for key "123".'], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {}, + variation_key: nil + ) end it 'when flag key is not available' do user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'not_found_key') - expect(decision.flag_key).to eq('not_found_key') - expect(decision.reasons).to eq(['No flag was found for key "not_found_key".']) + expect(decision.as_json).to eq( + enabled: false, + flag_key: 'not_found_key', + reasons: ['No flag was found for key "not_found_key".'], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {}, + variation_key: nil + ) end end From cffa963cb8c524c78b258b061f4093155afa585b Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 25 Nov 2020 18:27:57 -0800 Subject: [PATCH 37/46] added event payload tests --- spec/project_spec.rb | 84 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index e2246cf2..c236c5dc 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3584,6 +3584,8 @@ def callback(_args); end it 'when user is bucketed into a rollout and send_flag_decisions is true' do experiment_to_return = config_body['experiments'][3] variation_to_return = experiment_to_return['variations'][0] + allow(Time).to receive(:now).and_return(time_now) + allow(SecureRandom).to receive(:uuid).and_return('a68cf1ad-0393-4e18-af87-efe8f01a7c9c') 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) @@ -3618,6 +3620,45 @@ def callback(_args); end variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, variation_key: 'Fred' ) + expected_params = { + account_id: '12001', + project_id: '111001', + revision: '42', + client_name: 'ruby-sdk', + client_version: '3.7.0', + anonymize_ip: false, + enrich_decisions: true, + visitors: [{ + snapshots: [{ + events: [{ + entity_id: '4', + uuid: 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + key: 'campaign_activated', + timestamp: (time_now.to_f * 1000).to_i + }], + decisions: [{ + campaign_id: '4', + experiment_id: '122230', + variation_id: '122231', + metadata: { + flag_key: 'multi_variate_feature', + rule_key: 'test_experiment_multivariate', + rule_type: 'rollout', + variation_key: 'Fred', + enabled: true + } + }] + }], + visitor_id: 'user1', + attributes: [{ + entity_id: '$opt_bot_filtering', + key: '$opt_bot_filtering', + type: 'custom', + value: true + }] + }] + } + expect(project_instance.event_dispatcher).to have_received(:dispatch_event).with(Optimizely::Event.new(:post, impression_log_url, expected_params, post_headers)) end it 'when user is bucketed into a rollout and send_flag_decisions is false' do @@ -3656,6 +3697,7 @@ def callback(_args); end variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, variation_key: 'Fred' ) + expect(project_instance.event_dispatcher).to have_received(:dispatch_event).exactly(0).times end it 'when decision service returns nil and send_flag_decisions is false' do @@ -3688,9 +3730,12 @@ def callback(_args); end variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, variation_key: nil ) + expect(project_instance.event_dispatcher).to have_received(:dispatch_event).exactly(0).times end it 'when decision service returns nil and send_flag_decisions is true' do + allow(Time).to receive(:now).and_return(time_now) + allow(SecureRandom).to receive(:uuid).and_return('a68cf1ad-0393-4e18-af87-efe8f01a7c9c') 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) @@ -3721,6 +3766,45 @@ def callback(_args); end variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, variation_key: nil ) + expected_params = { + account_id: '12001', + project_id: '111001', + revision: '42', + client_name: 'ruby-sdk', + client_version: '3.7.0', + anonymize_ip: false, + enrich_decisions: true, + visitors: [{ + snapshots: [{ + events: [{ + entity_id: '', + uuid: 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + key: 'campaign_activated', + timestamp: (time_now.to_f * 1000).to_i + }], + decisions: [{ + campaign_id: '', + experiment_id: '', + variation_id: '', + metadata: { + flag_key: 'multi_variate_feature', + rule_key: '', + rule_type: 'rollout', + variation_key: '', + enabled: false + } + }] + }], + visitor_id: 'user1', + attributes: [{ + entity_id: '$opt_bot_filtering', + key: '$opt_bot_filtering', + type: 'custom', + value: true + }] + }] + } + expect(project_instance.event_dispatcher).to have_received(:dispatch_event).with(Optimizely::Event.new(:post, impression_log_url, expected_params, post_headers)) end end From 43b14ee292e438918111e38bc79b12f849b40deb Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 25 Nov 2020 19:15:39 -0800 Subject: [PATCH 38/46] Added tests for include_reasons --- lib/optimizely.rb | 5 +-- spec/project_spec.rb | 74 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 70205ac1..ddd97432 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -216,6 +216,8 @@ def decide(user_context, key, decide_options = []) end end + should_include_reasons = decide_options.include? OptimizelyDecideOption::INCLUDE_REASONS + # Send notification @notification_center.send_notifications( NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -226,11 +228,10 @@ def decide(user_context, key, decide_options = []) variables: all_variables, variation_key: variation_key, rule_key: rule_key, - reasons: reasons, + reasons: should_include_reasons ? reasons : [], decision_event_dispatched: decision_event_dispatched ) - should_include_reasons = decide_options.include? OptimizelyDecideOption::INCLUDE_REASONS OptimizelyDecision.new( variation_key: variation_key, enabled: feature_enabled, diff --git a/spec/project_spec.rb b/spec/project_spec.rb index c236c5dc..232a7b25 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -3887,6 +3887,80 @@ def callback(_args); end end 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], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: false, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil, + rule_key: nil, + reasons: [ + "User 'user1' is not in the forced variation map.", + "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." + ], + decision_event_dispatched: true + ) + 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]) + expect(decision.as_json).to include( + flag_key: 'multi_variate_feature', + enabled: false, + reasons: [ + "User 'user1' is not in the forced variation map.", + "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." + ], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil + ) + end + + it 'should not include reasons when the option is not 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], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: false, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil, + rule_key: nil, + reasons: [], + decision_event_dispatched: true + ) + 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') + expect(decision.as_json).to include( + flag_key: 'multi_variate_feature', + enabled: false, + reasons: [], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil + ) + end + end + it 'should pass on decide options to internal methods' do experiment_to_return = config_body['experiments'][3] variation_to_return = experiment_to_return['variations'][0] From 245637ed1b0266bb2c258034ab4b1a41c3e79708 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 25 Nov 2020 23:51:02 -0800 Subject: [PATCH 39/46] fixed a test title --- spec/decision_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index 934524dd..f2e6d997 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -338,7 +338,7 @@ .with(Logger::ERROR, "Error while saving user profile for user ID 'test_user': uncaught throw :SaveError.") end - describe 'IGNORE_USER_PROFILE decide option' do + describe 'IGNORE_USER_PROFILE_SERVICE decide option' do it 'should ignore user profile service if this option is set' do saved_user_profile = { user_id: 'test_user', From b76af55dbe7c6db06b1177d2047bf5c40c0bffd7 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 30 Nov 2020 09:43:44 -0800 Subject: [PATCH 40/46] added unit tests for default_decide_options --- spec/project_spec.rb | 169 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 232a7b25..258460c6 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -4118,4 +4118,173 @@ def callback(_args); end ) end end + + describe 'default_decide_options' do + describe 'EXCLUDE_VARIABLES' do + it 'should include variables when the option is not set in default_decide_options' do + custom_project_instance = Optimizely::Project.new(config_body_JSON, nil, spy_logger, error_handler) + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + 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) + 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( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'F', 'rest_of_name' => 'red'}, + variation_key: 'Fred' + ) + end + + it 'should exclude variables when the option is set in default_decide_options' do + custom_project_instance = Optimizely::Project.new( + config_body_JSON, nil, spy_logger, error_handler, + false, nil, nil, nil, nil, nil, [Optimizely::Decide::OptimizelyDecideOption::EXCLUDE_VARIABLES] + ) + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + 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) + 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( + flag_key: 'multi_variate_feature', + enabled: true, + reasons: [], + rule_key: 'test_experiment_multivariate', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {}, + variation_key: 'Fred' + ) + end + end + + describe 'INCLUDE_REASONS' do + it 'should include reasons when the option is set in default_decide_options' do + custom_project_instance = Optimizely::Project.new( + config_body_JSON, nil, spy_logger, error_handler, + false, nil, nil, nil, nil, nil, [Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS] + ) + expect(custom_project_instance.notification_center).to receive(:send_notifications) + .once.with(Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args) + expect(custom_project_instance.notification_center).to receive(:send_notifications) + .once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: false, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil, + rule_key: nil, + reasons: [ + "User 'user1' is not in the forced variation map.", + "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." + ], + decision_event_dispatched: true + ) + allow(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + 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( + flag_key: 'multi_variate_feature', + enabled: false, + reasons: [ + "User 'user1' is not in the forced variation map.", + "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." + ], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil + ) + end + + it 'should not include reasons when the option is not set in default_decide_options' do + custom_project_instance = Optimizely::Project.new(config_body_JSON, nil, spy_logger, error_handler) + expect(custom_project_instance.notification_center).to receive(:send_notifications) + .once.with(Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args) + expect(custom_project_instance.notification_center).to receive(:send_notifications) + .once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'flag', + 'user1', + {}, + flag_key: 'multi_variate_feature', + enabled: false, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil, + rule_key: nil, + reasons: [], + decision_event_dispatched: true + ) + allow(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + 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( + flag_key: 'multi_variate_feature', + enabled: false, + reasons: [], + rule_key: nil, + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'first_letter' => 'H', 'rest_of_name' => 'arry'}, + variation_key: nil + ) + end + end + + describe 'DISABLE_DECISION_EVENT' do + it 'should send event when option is not set in default_decide_options' do + custom_project_instance = Optimizely::Project.new(config_body_JSON, nil, spy_logger, error_handler) + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + expect(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + allow(custom_project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + user_context = custom_project_instance.create_user_context('user1') + custom_project_instance.decide(user_context, 'multi_variate_feature') + end + + it 'should not send event when option is set in default_decide_options' do + custom_project_instance = Optimizely::Project.new( + config_body_JSON, nil, spy_logger, error_handler, + false, nil, nil, nil, nil, nil, [Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT] + ) + experiment_to_return = config_body['experiments'][3] + variation_to_return = experiment_to_return['variations'][0] + expect(custom_project_instance.event_dispatcher).not_to receive(:dispatch_event).with(instance_of(Optimizely::Event)) + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ) + allow(custom_project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + user_context = custom_project_instance.create_user_context('user1') + custom_project_instance.decide(user_context, 'multi_variate_feature') + end + end + end end From 5057b3f0bdabf80e6d077524b5a0e2bf40e0170d Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 30 Nov 2020 12:39:02 -0800 Subject: [PATCH 41/46] added sdk not ready tests for decide_all and decide_for_keys --- spec/project_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 258460c6..9fb1f4a3 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -4004,6 +4004,13 @@ def callback(_args); end end describe '#decide_all' do + it 'should get empty object when sdk is not ready' do + invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) + user_context = project_instance.create_user_context('user1') + decisions = invalid_project.decide_all(user_context) + expect(decisions).to eq({}) + end + it 'should get all the decisions' do stub_request(:post, impression_log_url) user_context = project_instance.create_user_context('user1') @@ -4056,6 +4063,19 @@ def callback(_args); end end describe '#decide_for_keys' do + it 'should get empty object when sdk is not ready' do + keys = %w[ + boolean_single_variable_feature + integer_single_variable_feature + boolean_feature + empty_feature + ] + invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) + user_context = project_instance.create_user_context('user1') + decisions = invalid_project.decide_for_keys(user_context, keys) + expect(decisions).to eq({}) + end + it 'should get all the decisions for keys' do keys = %w[ boolean_single_variable_feature From 92afe269d8a9abd988d84e8328ca93c85c262428 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 7 Dec 2020 09:38:56 -0800 Subject: [PATCH 42/46] incorporated some review changes --- lib/optimizely.rb | 3 ++- spec/decision_service_spec.rb | 30 ++++++++---------------------- spec/project_spec.rb | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index ddd97432..f1fbcb33 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -270,7 +270,8 @@ def decide_for_keys(user_context, keys, decide_options = []) return {} end - enabled_flags_only = !decide_options.nil? && (decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + enabled_flags_only = (!decide_options.nil? && (decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY)) || (@default_decide_options.include? OptimizelyDecideOption::ENABLED_FLAGS_ONLY) + decisions = {} keys.each do |key| decision = decide(user_context, key, decide_options) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index f2e6d997..7cdc65b5 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -340,41 +340,27 @@ describe 'IGNORE_USER_PROFILE_SERVICE decide option' do it 'should ignore user profile service if this option is set' do - saved_user_profile = { - user_id: 'test_user', - experiment_bucket_map: { - '111127' => { - variation_id: '111129' - } - } - } allow(spy_user_profile_service).to receive(:lookup) - .with('test_user').once.and_return(saved_user_profile) + .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') 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 - saved_user_profile = { - user_id: 'test_user', - experiment_bucket_map: { - '111127' => { - variation_id: '111129' - } - } - } allow(spy_user_profile_service).to receive(:lookup) - .with('test_user').once.and_return(saved_user_profile) + .with('test_user').once.and_return(nil) - expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111129') + expect(decision_service.get_variation(config, 'test_experiment', 'test_user')).to eq('111128') - expect(decision_service.bucketer).not_to have_received(:bucket) - expect(Optimizely::Audience).not_to have_received(:user_meets_audience_conditions?) - expect(spy_user_profile_service).not_to have_received(:save) + 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 9fb1f4a3..b91685ce 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -4137,6 +4137,41 @@ def callback(_args); end variation_key: 'control' ) end + + it 'should get only enabled decisions for keys when ENABLED_FLAGS_ONLY is true in default_decide_options' do + custom_project_instance = Optimizely::Project.new( + config_body_JSON, nil, spy_logger, error_handler, + false, nil, nil, nil, nil, nil, [Optimizely::Decide::OptimizelyDecideOption::ENABLED_FLAGS_ONLY] + ) + keys = %w[ + boolean_single_variable_feature + integer_single_variable_feature + boolean_feature + empty_feature + ] + stub_request(:post, impression_log_url) + user_context = custom_project_instance.create_user_context('user1') + decisions = custom_project_instance.decide_for_keys(user_context, keys) + expect(decisions.length).to eq(2) + expect(decisions['boolean_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'boolean_single_variable_feature', + reasons: [], + rule_key: '177776', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'boolean_variable' => false}, + variation_key: '177778' + ) + expect(decisions['integer_single_variable_feature'].as_json).to eq( + enabled: true, + flag_key: 'integer_single_variable_feature', + reasons: [], + rule_key: 'test_experiment_integer_feature', + user_context: {attributes: {}, user_id: 'user1'}, + variables: {'integer_variable' => 42}, + variation_key: 'control' + ) + end end describe 'default_decide_options' do From 1ef9539d9c11361205fac24e048e76d4df876c57 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Mon, 7 Dec 2020 11:39:31 -0800 Subject: [PATCH 43/46] added a synchronize block when setting attributes --- lib/optimizely/optimizely_user_context.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 1f742d6e..5b24fb65 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -25,13 +25,14 @@ class OptimizelyUserContext attr_reader :user_id, :user_attributes def initialize(optimizely_client, user_id, user_attributes) + @set_attr_mutex = Mutex.new @optimizely_client = optimizely_client @user_id = user_id @user_attributes = user_attributes.nil? ? {} : user_attributes.clone end def set_attribute(attribute_key, attribute_value) - @user_attributes[attribute_key] = attribute_value + @set_attr_mutex.synchronize { @user_attributes[attribute_key] = attribute_value } end def decide(key, options = nil) From 89c3d6b9d547885756b844322dec4010cf74bf72 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 8 Dec 2020 13:47:24 -0800 Subject: [PATCH 44/46] Added doc comments --- lib/optimizely.rb | 10 +++++++ lib/optimizely/optimizely_user_context.rb | 34 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index f1fbcb33..504eeaf7 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -122,6 +122,16 @@ def initialize( end end + # Create a context of the user for which decision APIs will be called. + # + # A user context will be created successfully even when the SDK is not fully configured yet. + # + # @param user_id - The user ID to be used for bucketing. + # @param attributes - A Hash representing user attribute names and values. + # + # @return [OptimizelyUserContext] An OptimizelyUserContext associated with this OptimizelyClient. + # @return [nil] If user attributes are not in valid format. + def create_user_context(user_id, attributes = nil) # We do not check for is_valid here as a user context can be created successfully # even when the SDK is not fully configured. diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 5b24fb65..8e1d7db9 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -31,22 +31,56 @@ def initialize(optimizely_client, user_id, user_attributes) @user_attributes = user_attributes.nil? ? {} : user_attributes.clone end + # Set an attribute for a given key + # + # @param key - An attribute key + # @param value - An attribute value + def set_attribute(attribute_key, attribute_value) @set_attr_mutex.synchronize { @user_attributes[attribute_key] = attribute_value } end + # Returns a decision result (OptimizelyDecision) for a given flag key and a user context, which contains all data required to deliver the flag. + # + # If the SDK finds an error, it’ll return a `decision` with nil for `variation_key`. The decision will include an error message in `reasons` + # + # @param key -A flag key for which a decision will be made + # @param options - A list of options for decision making. + # + # @return [OptimizelyDecision] A decision result + def decide(key, options = nil) @optimizely_client&.decide(self, key, options) end + # Returns a hash of decision results (OptimizelyDecision) for multiple flag keys and a user context. + # + # If the SDK finds an error for a key, the response will include a decision for the key showing `reasons` for the error. + # The SDK will always return hash of decisions. When it can not process requests, it’ll return an empty hash after logging the errors. + # + # @param keys - A list of flag keys for which the decisions will be made. + # @param options - A list of options for decision making. + # + # @return - Hash of decisions containing flag keys as hash keys and corresponding decisions as their values. + def decide_for_keys(keys, options = nil) @optimizely_client&.decide_for_keys(self, keys, options) end + # Returns a hash of decision results (OptimizelyDecision) for all active flag keys. + # + # @param options - A list of options for decision making. + # + # @return - Hash of decisions containing flag keys as hash keys and corresponding decisions as their values. + def decide_all(options = nil) @optimizely_client&.decide_all(self, options) end + # Track an event + # + # @param event_key - Event key representing the event which needs to be recorded. + def track_event(event_key, event_tags = nil) @optimizely_client&.track(event_key, @user_id, @user_attributes, event_tags) end From 315d4aaa16953fc92892364350ee913e4e912e28 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 8 Dec 2020 17:24:26 -0800 Subject: [PATCH 45/46] lint fixes --- lib/optimizely/optimizely_user_context.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 8e1d7db9..3e9cefd8 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -42,7 +42,7 @@ def set_attribute(attribute_key, attribute_value) # Returns a decision result (OptimizelyDecision) for a given flag key and a user context, which contains all data required to deliver the flag. # - # If the SDK finds an error, it’ll return a `decision` with nil for `variation_key`. The decision will include an error message in `reasons` + # If the SDK finds an error, it'll return a `decision` with nil for `variation_key`. The decision will include an error message in `reasons` # # @param key -A flag key for which a decision will be made # @param options - A list of options for decision making. @@ -56,7 +56,7 @@ def decide(key, options = nil) # Returns a hash of decision results (OptimizelyDecision) for multiple flag keys and a user context. # # If the SDK finds an error for a key, the response will include a decision for the key showing `reasons` for the error. - # The SDK will always return hash of decisions. When it can not process requests, it’ll return an empty hash after logging the errors. + # The SDK will always return hash of decisions. When it can not process requests, it'll return an empty hash after logging the errors. # # @param keys - A list of flag keys for which the decisions will be made. # @param options - A list of options for decision making. From f58d8bd355df30ecaaec6b66611dca0fa1aeaf7b Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 10 Dec 2020 15:57:41 -0800 Subject: [PATCH 46/46] clone and sync user attributes and context --- lib/optimizely/optimizely_user_context.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index 3e9cefd8..928470a2 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -22,22 +22,30 @@ module Optimizely class OptimizelyUserContext # Representation of an Optimizely User Context using which APIs are to be called. - attr_reader :user_id, :user_attributes + attr_reader :user_id def initialize(optimizely_client, user_id, user_attributes) - @set_attr_mutex = Mutex.new + @attr_mutex = Mutex.new @optimizely_client = optimizely_client @user_id = user_id @user_attributes = user_attributes.nil? ? {} : user_attributes.clone end + def clone + OptimizelyUserContext.new(@optimizely_client, @user_id, user_attributes) + end + + def user_attributes + @attr_mutex.synchronize { @user_attributes.clone } + end + # Set an attribute for a given key # # @param key - An attribute key # @param value - An attribute value def set_attribute(attribute_key, attribute_value) - @set_attr_mutex.synchronize { @user_attributes[attribute_key] = attribute_value } + @attr_mutex.synchronize { @user_attributes[attribute_key] = attribute_value } end # Returns a decision result (OptimizelyDecision) for a given flag key and a user context, which contains all data required to deliver the flag. @@ -50,7 +58,7 @@ def set_attribute(attribute_key, attribute_value) # @return [OptimizelyDecision] A decision result def decide(key, options = nil) - @optimizely_client&.decide(self, key, options) + @optimizely_client&.decide(clone, key, options) end # Returns a hash of decision results (OptimizelyDecision) for multiple flag keys and a user context. @@ -64,7 +72,7 @@ def decide(key, options = nil) # @return - Hash of decisions containing flag keys as hash keys and corresponding decisions as their values. def decide_for_keys(keys, options = nil) - @optimizely_client&.decide_for_keys(self, keys, options) + @optimizely_client&.decide_for_keys(clone, keys, options) end # Returns a hash of decision results (OptimizelyDecision) for all active flag keys. @@ -74,7 +82,7 @@ def decide_for_keys(keys, options = nil) # @return - Hash of decisions containing flag keys as hash keys and corresponding decisions as their values. def decide_all(options = nil) - @optimizely_client&.decide_all(self, options) + @optimizely_client&.decide_all(clone, options) end # Track an event @@ -82,7 +90,7 @@ def decide_all(options = nil) # @param event_key - Event key representing the event which needs to be recorded. def track_event(event_key, event_tags = nil) - @optimizely_client&.track(event_key, @user_id, @user_attributes, event_tags) + @optimizely_client&.track(event_key, @user_id, user_attributes, event_tags) end def as_json