From 11e7feb5c83556b9b346982bca3657069ec51071 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 6 May 2020 15:18:46 -0700 Subject: [PATCH 01/11] Implemented getFeatureVariableJson --- lib/optimizely.rb | 26 +++ .../config/datafile_project_config.rb | 11 ++ lib/optimizely/helpers/constants.rb | 3 +- lib/optimizely/helpers/variable_type.rb | 7 + spec/config/datafile_project_config_spec.rb | 31 +++- spec/optimizely_config_spec.rb | 2 +- spec/project_spec.rb | 163 ++++++++++++++++++ spec/spec_params.rb | 20 +++ 8 files changed, 259 insertions(+), 4 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index a21c87d2..b7a82bcb 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -430,6 +430,32 @@ def get_feature_variable_string(feature_flag_key, variable_key, user_id, attribu variable_value end + # Get the Json value of the specified variable in the feature flag in a Dict. + # + # @param feature_flag_key - String key of feature flag the variable belongs to + # @param variable_key - String key of variable for which we are getting the string value + # @param user_id - String user ID + # @param attributes - Hash representing visitor attributes and values which need to be recorded. + # + # @return [Dict] the Dict containing variable value. + # @return [nil] if the feature flag or variable are not found. + + def get_feature_variable_json(feature_flag_key, variable_key, user_id, attributes = nil) + unless is_valid + @logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_feature_variable_json').message) + return nil + end + variable_value = get_feature_variable_for_type( + feature_flag_key, + variable_key, + Optimizely::Helpers::Constants::VARIABLE_TYPES['JSON'], + user_id, + attributes + ) + + variable_value + end + # Get the Boolean value of the specified variable in the feature flag. # # @param feature_flag_key - String key of feature flag the variable belongs to diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index ff772788..55d24346 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/lib/optimizely/config/datafile_project_config.rb @@ -82,6 +82,17 @@ def initialize(datafile, logger, error_handler) @revision = config['revision'] @rollouts = config.fetch('rollouts', []) + # Json type is represented in datafile as a subtype of string for the sake of backwards compatibility. + # Converting it to a first-class json type while creating Project Config + @feature_flags.each do |feature_flag| + feature_flag['variables'].each do |variable| + if variable['type'] == 'string' && variable['subType'] == 'json' + variable['type'] = 'json' + variable.delete('subType') + end + end + end + # Utility maps for quick lookup @attribute_key_map = generate_key_map(@attributes, 'key') @event_key_map = generate_key_map(@events, 'key') diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb index 3c116442..dd94dd3b 100644 --- a/lib/optimizely/helpers/constants.rb +++ b/lib/optimizely/helpers/constants.rb @@ -304,7 +304,8 @@ module Constants 'BOOLEAN' => 'boolean', 'DOUBLE' => 'double', 'INTEGER' => 'integer', - 'STRING' => 'string' + 'STRING' => 'string', + 'JSON' => 'json' }.freeze INPUT_VARIABLES = { diff --git a/lib/optimizely/helpers/variable_type.rb b/lib/optimizely/helpers/variable_type.rb index 2a2cbc09..118215ba 100644 --- a/lib/optimizely/helpers/variable_type.rb +++ b/lib/optimizely/helpers/variable_type.rb @@ -48,6 +48,13 @@ def cast_value_to_type(value, variable_type, logger) logger.log(Logger::ERROR, "Unable to cast variable value '#{value}' to type "\ "'#{variable_type}': #{e.message}.") end + when 'json' + begin + return_value = JSON.parse(value) + rescue => e + logger.log(Logger::ERROR, "Unable to cast variable value '#{value}' to type "\ + "'#{variable_type}': #{e.message}.") + end else # default case is string return_value = value diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index fa4abeda..efe94d0e 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -31,12 +31,22 @@ it 'should initialize properties correctly upon creating project' do project_config = Optimizely::DatafileProjectConfig.new(config_body_JSON, logger, error_handler) + feature_flags_to_compare = config_body.fetch('featureFlags') + feature_flags_to_compare.each do |feature_flag| + feature_flag['variables'].each do |variable| + if variable['type'] == 'string' && variable['subType'] == 'json' + variable['type'] = 'json' + variable.delete('subType') + end + end + end + expect(project_config.account_id).to eq(config_body['accountId']) expect(project_config.attributes).to eq(config_body['attributes']) expect(project_config.audiences).to eq(config_body['audiences']) expect(project_config.bot_filtering).to eq(config_body['botFiltering']) expect(project_config.events).to eq(config_body['events']) - expect(project_config.feature_flags).to eq(config_body['featureFlags']) + expect(project_config.feature_flags).to eq(feature_flags_to_compare) expect(project_config.groups).to eq(config_body['groups']) expect(project_config.project_id).to eq(config_body['projectId']) expect(project_config.revision).to eq(config_body['revision']) @@ -475,7 +485,8 @@ 'string_single_variable_feature' => config_body['featureFlags'][4], 'multi_variate_feature' => config_body['featureFlags'][5], 'mutex_group_feature' => config_body['featureFlags'][6], - 'empty_feature' => config_body['featureFlags'][7] + 'empty_feature' => config_body['featureFlags'][7], + 'json_single_variable_feature' => config_body['featureFlags'][8] } expected_feature_variable_key_map = { @@ -512,6 +523,14 @@ 'defaultValue' => 'wingardium leviosa' } }, + 'json_single_variable_feature' => { + 'json_variable' => { + 'id' => '1555588', + 'key' => 'json_variable', + 'type' => 'json', + 'defaultValue' => '{ "val": "wingardium leviosa" }' + } + }, 'multi_variate_feature' => { 'first_letter' => { 'id' => '155560', @@ -582,12 +601,20 @@ '155558' => { 'id' => '155558', 'value' => 'cta_1' + }, + '1555588' => { + 'id' => '1555588', + 'value' => '{"value": "cta_1"}' } }, '122237' => { '155558' => { 'id' => '155558', 'value' => 'cta_2' + }, + '1555588' => { + 'id' => '1555588', + 'value' => '{"value": "cta_2"}' } }, '122239' => { diff --git a/spec/optimizely_config_spec.rb b/spec/optimizely_config_spec.rb index 10a3a519..097dd448 100644 --- a/spec/optimizely_config_spec.rb +++ b/spec/optimizely_config_spec.rb @@ -47,7 +47,7 @@ it 'should return all feature flags' do features_map = optimizely_config['featuresMap'] - expect(features_map.length).to eq(8) + expect(features_map.length).to eq(9) project_config.feature_flags.each do |feature_flag| expect(features_map[feature_flag['key']]).to include( 'id' => feature_flag['id'], diff --git a/spec/project_spec.rb b/spec/project_spec.rb index aa2fdd5c..d8ef1d04 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -1865,6 +1865,15 @@ def callback(_args); end source_info: {} ).ordered + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'feature', 'test_user', {'browser_type' => 'firefox'}, + feature_enabled: false, + feature_key: 'json_single_variable_feature', + source: 'rollout', + source_info: {} + ).ordered + expect(project_instance.get_enabled_features('test_user', 'browser_type' => 'firefox')).to eq(enabled_features) end end @@ -2024,6 +2033,160 @@ def callback(_args); end end end + describe '#get_feature_variable_json' do + user_id = 'test_user' + user_attributes = {} + + it 'should return nil when called with invalid project config' do + logger = double('logger') + allow(logger).to receive(:log) + allow(Optimizely::SimpleLogger).to receive(:new) { logger } + invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) + expect(invalid_project.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) + .to eq(nil) + expect(logger).to have_received(:log).once.with(Logger::ERROR, 'Provided datafile is in an invalid format.') + expect(spy_logger).to have_received(:log).once.with(Logger::ERROR, "Optimizely instance is not valid. Failing 'get_feature_variable_json'.") + end + + it 'should return nil and log an error when Config Manager returns nil config' do + allow(project_instance.config_manager).to receive(:config).and_return(nil) + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)).to eq(nil) + expect(spy_logger).to have_received(:log).once.with( + Logger::ERROR, + "Optimizely instance is not valid. Failing 'get_feature_variable_json'." + ) + end + + describe 'when the feature flag is enabled for the user' do + describe 'and a variable usage instance is not found' do + it 'should return the default variable value' do + variation_to_return = project_config.rollout_id_map['166661']['experiments'][0]['variations'][0] + decision_to_return = { + 'experiment' => nil, + 'variation' => variation_to_return + } + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) + .to eq('val' => 'wingardium leviosa') + expect(spy_logger).to have_received(:log).once + .with( + Logger::DEBUG, + "Variable 'json_variable' is not used in variation '177775'. Returning the default variable value '{ \"val\": \"wingardium leviosa\" }'." + ) + end + end + + describe 'and a variable usage instance is found' do + describe 'and the variable type boolean is not a json' do + it 'should log a warning' do + variation_to_return = project_config.rollout_id_map['166660']['experiments'][0]['variations'][0] + decision_to_return = { + 'experiment' => nil, + 'variation' => variation_to_return + } + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + + expect(project_instance.get_feature_variable_json('boolean_single_variable_feature', 'boolean_variable', user_id, user_attributes)) + .to eq(nil) + expect(spy_logger).to have_received(:log).once + .with( + Logger::WARN, + "Requested variable as type 'json' but variable 'boolean_variable' is of type 'boolean'." + ) + end + end + + describe 'and the variable type integer is not a json' do + it 'should log a warning' do + integer_feature = project_config.feature_flag_key_map['integer_single_variable_feature'] + experiment_to_return = project_config.experiment_id_map[integer_feature['experimentIds'][0]] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = { + 'experiment' => experiment_to_return, + 'variation' => variation_to_return + } + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + + expect(project_instance.get_feature_variable_json('integer_single_variable_feature', 'integer_variable', user_id, user_attributes)) + .to eq(nil) + expect(spy_logger).to have_received(:log).once + .with( + Logger::WARN, + "Requested variable as type 'json' but variable 'integer_variable' is of type 'integer'." + ) + end + end + + it 'should return the variable value for the variation for the user is bucketed into' do + experiment_to_return = project_config.experiment_key_map['test_experiment_with_feature_rollout'] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = { + 'experiment' => experiment_to_return, + 'variation' => variation_to_return + } + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) + .to eq('value' => 'cta_1') + + expect(spy_logger).to have_received(:log).once + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "Got variable value '{\"value\": \"cta_1\"}' for variable 'json_variable' of feature flag 'json_single_variable_feature'." + ) + end + end + end + + describe 'when the feature flag is not enabled for the user' do + it 'should return the default variable value' do + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) + .to eq('val' => 'wingardium leviosa') + expect(spy_logger).to have_received(:log).once + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "User 'test_user' was not bucketed into any variation for feature flag 'json_single_variable_feature'. Returning the default variable value '{ \"val\": \"wingardium leviosa\" }'." + ) + end + end + + describe 'when the specified feature flag is invalid' do + it 'should log an error message and return nil' do + expect(project_instance.get_feature_variable_json('totally_invalid_feature_key', 'json_variable', user_id, user_attributes)) + .to eq(nil) + expect(spy_logger).to have_received(:log).twice + expect(spy_logger).to have_received(:log).once + .with( + Logger::ERROR, + "Feature flag key 'totally_invalid_feature_key' is not in datafile." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "No feature flag was found for key 'totally_invalid_feature_key'." + ) + end + end + + describe 'when the specified feature variable is invalid' do + it 'should log an error message and return nil' do + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'invalid_json_variable', user_id, user_attributes)) + .to eq(nil) + expect(spy_logger).to have_received(:log).once + expect(spy_logger).to have_received(:log).once + .with( + Logger::ERROR, + "No feature variable was found for key 'invalid_json_variable' in feature flag 'json_single_variable_feature'." + ) + end + end + end + describe '#get_feature_variable_boolean' do user_id = 'test_user' user_attributes = {} diff --git a/spec/spec_params.rb b/spec/spec_params.rb index a97671bd..41c29fc9 100644 --- a/spec/spec_params.rb +++ b/spec/spec_params.rb @@ -218,6 +218,9 @@ module OptimizelySpec 'variables' => [{ 'id' => '155558', 'value' => 'cta_1' + }, { + 'id' => '1555588', + 'value' => '{"value": "cta_1"}' }] }, { 'id' => '122237', @@ -226,6 +229,9 @@ module OptimizelySpec 'variables' => [{ 'id' => '155558', 'value' => 'cta_2' + }, { + 'id' => '1555588', + 'value' => '{"value": "cta_2"}' }] }] }, { @@ -558,6 +564,20 @@ module OptimizelySpec 'rolloutId' => '', 'experimentIds' => [], 'variables' => [] + }, { + 'id' => '15555577', + 'key' => 'json_single_variable_feature', + 'rolloutId' => '166661', + 'experimentIds' => [], + 'variables' => [ + { + 'id' => '1555588', + 'key' => 'json_variable', + 'type' => 'string', + 'subType' => 'json', + 'defaultValue' => '{ "val": "wingardium leviosa" }' + } + ] }], 'rollouts' => [{ 'id' => '166660', From 894aa77b1d45f6586930738380e95bbe60d566d5 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 6 May 2020 18:52:27 -0700 Subject: [PATCH 02/11] moved common logic to a separate function to support get_all_feature_variables --- lib/optimizely.rb | 75 +++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index b7a82bcb..b925dd07 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -675,8 +675,6 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, # Error message logged in DatafileProjectConfig- get_feature_flag_from_key return nil if variable.nil? - feature_enabled = false - # If variable_type is nil, set it equal to variable['type'] variable_type ||= variable['type'] # Returns nil if type differs @@ -684,43 +682,24 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, @logger.log(Logger::WARN, "Requested variable as type '#{variable_type}' but variable '#{variable_key}' is of type '#{variable['type']}'.") return nil - else - source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] - decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes) - variable_value = variable['defaultValue'] - if decision - variation = decision['variation'] - if decision['source'] == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] - source_info = { - experiment_key: decision.experiment['key'], - variation_key: variation['key'] - } - source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] - end - feature_enabled = variation['featureEnabled'] - if feature_enabled == true - variation_variable_usages = config.variation_id_to_variable_usage_map[variation['id']] - variable_id = variable['id'] - if variation_variable_usages&.key?(variable_id) - variable_value = variation_variable_usages[variable_id]['value'] - @logger.log(Logger::INFO, - "Got variable value '#{variable_value}' for variable '#{variable_key}' of feature flag '#{feature_flag_key}'.") - else - @logger.log(Logger::DEBUG, - "Variable '#{variable_key}' is not used in variation '#{variation['key']}'. Returning the default variable value '#{variable_value}'.") - end - else - @logger.log(Logger::DEBUG, - "Feature '#{feature_flag_key}' for variation '#{variation['key']}' is not enabled. Returning the default variable value '#{variable_value}'.") - end - else - @logger.log(Logger::INFO, - "User '#{user_id}' was not bucketed into any variation for feature flag '#{feature_flag_key}'. Returning the default variable value '#{variable_value}'.") - end end + decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes) + variation = decision ? decision['variation'] : nil + feature_enabled = variation ? variation['featureEnabled'] : false + + variable_value = get_all_feature_variables_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) variable_value = Helpers::VariableType.cast_value_to_type(variable_value, variable_type, @logger) + source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + if decision && decision['source'] == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + source_info = { + experiment_key: decision.experiment['key'], + variation_key: variation['key'] + } + source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + end + @notification_center.send_notifications( NotificationCenter::NOTIFICATION_TYPES[:DECISION], Helpers::Constants::DECISION_NOTIFICATION_TYPES['FEATURE_VARIABLE'], user_id, (attributes || {}), @@ -736,6 +715,32 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, variable_value end + def get_all_feature_variables_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) + config = project_config + variable_value = variable['defaultValue'] + if variation + if feature_enabled == true + variation_variable_usages = config.variation_id_to_variable_usage_map[variation['id']] + variable_id = variable['id'] + if variation_variable_usages&.key?(variable_id) + variable_value = variation_variable_usages[variable_id]['value'] + @logger.log(Logger::INFO, + "Got variable value '#{variable_value}' for variable '#{variable['key']}' of feature flag '#{feature_flag_key}'.") + else + @logger.log(Logger::DEBUG, + "Variable '#{variable['key']}' is not used in variation '#{variation['key']}'. Returning the default variable value '#{variable_value}'.") + end + else + @logger.log(Logger::DEBUG, + "Feature '#{feature_flag_key}' for variation '#{variation['key']}' is not enabled. Returning the default variable value '#{variable_value}'.") + end + else + @logger.log(Logger::INFO, + "User '#{user_id}' was not bucketed into any variation for feature flag '#{feature_flag_key}'. Returning the default variable value '#{variable_value}'.") + end + variable_value + end + def user_inputs_valid?(attributes = nil, event_tags = nil) # Helper method to validate user inputs. # From 430f221dd1667bf3b6377872d0acb4b34553f073 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 7 May 2020 19:35:51 -0700 Subject: [PATCH 03/11] added get_all_feature_variables and its tests --- lib/optimizely.rb | 62 ++++++- spec/config/datafile_project_config_spec.rb | 37 +++- spec/optimizely_config_spec.rb | 2 +- spec/project_spec.rb | 187 ++++++++++++++++++++ spec/spec_params.rb | 34 ++++ 5 files changed, 317 insertions(+), 5 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index b925dd07..bda1c073 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -510,6 +510,64 @@ def get_feature_variable_double(feature_flag_key, variable_key, user_id, attribu variable_value end + def get_all_feature_variables(feature_flag_key, user_id, attributes = nil) + unless is_valid + @logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_all_feature_variables').message) + return nil + end + + return nil unless Optimizely::Helpers::Validator.inputs_valid?( + { + feature_flag_key: feature_flag_key, + user_id: user_id + }, + @logger, Logger::ERROR + ) + + return nil unless user_inputs_valid?(attributes) + + config = project_config + + feature_flag = config.get_feature_flag_from_key(feature_flag_key) + unless feature_flag + @logger.log(Logger::INFO, "No feature flag was found for key '#{feature_flag_key}'.") + return nil + end + + decision = @decision_service.get_variation_for_feature(config, feature_flag, user_id, attributes) + variation = decision ? decision['variation'] : nil + feature_enabled = variation ? variation['featureEnabled'] : false + all_variables = {} + + feature_flag['variables'].each do |variable| + variable_value = get_feature_variable_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) + variable_value = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) + all_variables[variable['key']] = variable_value + end + + source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + if decision && decision['source'] == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + source_info = { + experiment_key: decision.experiment['key'], + variation_key: variation['key'] + } + source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + end + + # @notification_center.send_notifications( + # NotificationCenter::NOTIFICATION_TYPES[:DECISION], + # Helpers::Constants::DECISION_NOTIFICATION_TYPES['FEATURE_VARIABLE'], user_id, (attributes || {}), + # feature_key: feature_flag_key, + # feature_enabled: feature_enabled, + # source: source_string, + # variable_key: variable_key, + # variable_type: variable_type, + # variable_value: variable_value, + # source_info: source_info || {} + # ) + all_variables + end + # Get the Integer value of the specified variable in the feature flag. # # @param feature_flag_key - String key of feature flag the variable belongs to @@ -688,7 +746,7 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, variation = decision ? decision['variation'] : nil feature_enabled = variation ? variation['featureEnabled'] : false - variable_value = get_all_feature_variables_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) + variable_value = get_feature_variable_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) variable_value = Helpers::VariableType.cast_value_to_type(variable_value, variable_type, @logger) source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] @@ -715,7 +773,7 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, variable_value end - def get_all_feature_variables_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) + def get_feature_variable_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) config = project_config variable_value = variable['defaultValue'] if variation diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index efe94d0e..ec0c96ae 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -486,7 +486,8 @@ 'multi_variate_feature' => config_body['featureFlags'][5], 'mutex_group_feature' => config_body['featureFlags'][6], 'empty_feature' => config_body['featureFlags'][7], - 'json_single_variable_feature' => config_body['featureFlags'][8] + 'json_single_variable_feature' => config_body['featureFlags'][8], + 'all_variables_feature' => config_body['featureFlags'][9] } expected_feature_variable_key_map = { @@ -553,7 +554,39 @@ 'defaultValue' => 'null' } }, - 'empty_feature' => {} + 'empty_feature' => {}, + 'all_variables_feature' => { + 'json_variable' => { + 'id' => '155558891', + 'key' => 'json_variable', + 'type' => 'json', + 'defaultValue' => '{ "val": "default json" }' + }, + 'string_variable' => { + 'id' => '155558892', + 'key' => 'string_variable', + 'type' => 'string', + 'defaultValue' => 'default string' + }, + 'boolean_variable' => { + 'id' => '155558893', + 'key' => 'boolean_variable', + 'type' => 'boolean', + 'defaultValue' => 'false' + }, + 'double_variable' => { + 'id' => '155558894', + 'key' => 'double_variable', + 'type' => 'double', + 'defaultValue' => '1.99' + }, + 'integer_variable' => { + 'id' => '155558895', + 'key' => 'integer_variable', + 'type' => 'integer', + 'defaultValue' => '10' + } + } } expected_variation_id_to_variable_usage_map = { diff --git a/spec/optimizely_config_spec.rb b/spec/optimizely_config_spec.rb index 097dd448..1a000419 100644 --- a/spec/optimizely_config_spec.rb +++ b/spec/optimizely_config_spec.rb @@ -47,7 +47,7 @@ it 'should return all feature flags' do features_map = optimizely_config['featuresMap'] - expect(features_map.length).to eq(9) + expect(features_map.length).to eq(10) project_config.feature_flags.each do |feature_flag| expect(features_map[feature_flag['key']]).to include( 'id' => feature_flag['id'], diff --git a/spec/project_spec.rb b/spec/project_spec.rb index d8ef1d04..6fcc5ced 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -2330,6 +2330,193 @@ def callback(_args); end end end + describe '#get_all_feature_variables' do + user_id = 'test_user' + user_attributes = {} + + it 'should return nil when called with invalid project config' do + logger = double('logger') + allow(logger).to receive(:log) + allow(Optimizely::SimpleLogger).to receive(:new) { logger } + invalid_project = Optimizely::Project.new('invalid', nil, spy_logger) + expect(invalid_project.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) + .to eq(nil) + expect(logger).to have_received(:log).once.with(Logger::ERROR, 'Provided datafile is in an invalid format.') + expect(spy_logger).to have_received(:log).once.with(Logger::ERROR, "Optimizely instance is not valid. Failing 'get_all_feature_variables'.") + end + + it 'should return nil and log an error when Config Manager returns nil config' do + allow(project_instance.config_manager).to receive(:config).and_return(nil) + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)).to eq(nil) + expect(spy_logger).to have_received(:log).once.with( + Logger::ERROR, + "Optimizely instance is not valid. Failing 'get_all_feature_variables'." + ) + end + + describe 'when the feature flag is enabled for the user' do + describe 'and a variable usage instance is not found' do + it 'should return the default variable value' do + variation_to_return = project_config.rollout_id_map['166661']['experiments'][0]['variations'][0] + decision_to_return = { + 'experiment' => nil, + 'variation' => variation_to_return + } + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) + .to eq( + 'json_variable' => {'val' => 'default json'}, + 'string_variable' => 'default string', + 'boolean_variable' => false, + 'double_variable' => 1.99, + 'integer_variable' => 10 + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::DEBUG, + "Variable 'json_variable' is not used in variation '177775'. Returning the default variable value '{ \"val\": \"default json\" }'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::DEBUG, + "Variable 'string_variable' is not used in variation '177775'. Returning the default variable value 'default string'." + ) + end + end + + describe 'and a variable usage instance is found' do + it 'should return the variable value for the variation for the user is bucketed into' do + experiment_to_return = project_config.experiment_key_map['test_experiment_with_feature_rollout'] + variation_to_return = { + 'id' => '12345678', + 'featureEnabled' => true + } + variation_id_to_variable_usage_map = { + '12345678' => { + '155558891' => { + 'value' => '{ "val": "feature enabled" }' + }, + '155558892' => { + 'value' => 'feature enabled' + }, + '155558893' => { + 'value' => 'true' + }, + '155558894' => { + 'value' => '14.99' + }, + '155558895' => { + 'value' => '99' + } + } + } + + decision_to_return = { + 'experiment' => experiment_to_return, + 'variation' => variation_to_return + } + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_config).to receive(:variation_id_to_variable_usage_map).and_return(variation_id_to_variable_usage_map) + + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) + .to eq( + 'json_variable' => {'val' => 'feature enabled'}, + 'string_variable' => 'feature enabled', + 'boolean_variable' => true, + 'double_variable' => 14.99, + 'integer_variable' => 99 + ) + + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "Got variable value '{ \"val\": \"feature enabled\" }' for variable 'json_variable' of feature flag 'all_variables_feature'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "Got variable value 'feature enabled' for variable 'string_variable' of feature flag 'all_variables_feature'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "Got variable value 'true' for variable 'boolean_variable' of feature flag 'all_variables_feature'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "Got variable value '14.99' for variable 'double_variable' of feature flag 'all_variables_feature'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "Got variable value '99' for variable 'integer_variable' of feature flag 'all_variables_feature'." + ) + end + end + end + + describe 'when the feature flag is not enabled for the user' do + it 'should return the default variable value' do + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) + .to eq( + 'json_variable' => {'val' => 'default json'}, + 'string_variable' => 'default string', + 'boolean_variable' => false, + 'double_variable' => 1.99, + 'integer_variable' => 10 + ) + + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "User 'test_user' was not bucketed into any variation for feature flag 'all_variables_feature'. Returning the default variable value '{ \"val\": \"default json\" }'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "User 'test_user' was not bucketed into any variation for feature flag 'all_variables_feature'. Returning the default variable value 'default string'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "User 'test_user' was not bucketed into any variation for feature flag 'all_variables_feature'. Returning the default variable value 'false'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "User 'test_user' was not bucketed into any variation for feature flag 'all_variables_feature'. Returning the default variable value '1.99'." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "User 'test_user' was not bucketed into any variation for feature flag 'all_variables_feature'. Returning the default variable value '10'." + ) + end + end + + describe 'when the specified feature flag is invalid' do + it 'should log an error message and return nil' do + expect(project_instance.get_all_feature_variables('totally_invalid_feature_key', user_id, user_attributes)) + .to eq(nil) + expect(spy_logger).to have_received(:log).twice + expect(spy_logger).to have_received(:log).once + .with( + Logger::ERROR, + "Feature flag key 'totally_invalid_feature_key' is not in datafile." + ) + expect(spy_logger).to have_received(:log).once + .with( + Logger::INFO, + "No feature flag was found for key 'totally_invalid_feature_key'." + ) + end + end + end + describe '#get_feature_variable' do user_id = 'test_user' user_attributes = {} diff --git a/spec/spec_params.rb b/spec/spec_params.rb index 41c29fc9..81d721f3 100644 --- a/spec/spec_params.rb +++ b/spec/spec_params.rb @@ -578,6 +578,40 @@ module OptimizelySpec 'defaultValue' => '{ "val": "wingardium leviosa" }' } ] + }, { + 'id' => '155555777', + 'key' => 'all_variables_feature', + 'rolloutId' => '1666611', + 'experimentIds' => [], + 'variables' => [ + { + 'id' => '155558891', + 'key' => 'json_variable', + 'type' => 'string', + 'subType' => 'json', + 'defaultValue' => '{ "val": "default json" }' + }, { + 'id' => '155558892', + 'key' => 'string_variable', + 'type' => 'string', + 'defaultValue' => 'default string' + }, { + 'id' => '155558893', + 'key' => 'boolean_variable', + 'type' => 'boolean', + 'defaultValue' => 'false' + }, { + 'id' => '155558894', + 'key' => 'double_variable', + 'type' => 'double', + 'defaultValue' => '1.99' + }, { + 'id' => '155558895', + 'key' => 'integer_variable', + 'type' => 'integer', + 'defaultValue' => '10' + } + ] }], 'rollouts' => [{ 'id' => '166660', From 222c4ad54b15fb1d03f4497b9fcae0a90b0f48c1 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 8 May 2020 13:28:17 -0700 Subject: [PATCH 04/11] added notifications for get_all_feature_variables and added unit tests --- lib/optimizely.rb | 21 +++++------ lib/optimizely/helpers/constants.rb | 3 +- spec/project_spec.rb | 57 +++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index bda1c073..bcaa4b53 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -554,17 +554,16 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil) source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] end - # @notification_center.send_notifications( - # NotificationCenter::NOTIFICATION_TYPES[:DECISION], - # Helpers::Constants::DECISION_NOTIFICATION_TYPES['FEATURE_VARIABLE'], user_id, (attributes || {}), - # feature_key: feature_flag_key, - # feature_enabled: feature_enabled, - # source: source_string, - # variable_key: variable_key, - # variable_type: variable_type, - # variable_value: variable_value, - # source_info: source_info || {} - # ) + @notification_center.send_notifications( + NotificationCenter::NOTIFICATION_TYPES[:DECISION], + Helpers::Constants::DECISION_NOTIFICATION_TYPES['ALL_FEATURE_VARIABLES'], user_id, (attributes || {}), + feature_key: feature_flag_key, + feature_enabled: feature_enabled, + source: source_string, + variable_values: all_variables, + source_info: source_info || {} + ) + all_variables end diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb index dd94dd3b..a0497ed3 100644 --- a/lib/optimizely/helpers/constants.rb +++ b/lib/optimizely/helpers/constants.rb @@ -358,7 +358,8 @@ module Constants 'AB_TEST' => 'ab-test', 'FEATURE' => 'feature', 'FEATURE_TEST' => 'feature-test', - 'FEATURE_VARIABLE' => 'feature-variable' + 'FEATURE_VARIABLE' => 'feature-variable', + 'ALL_FEATURE_VARIABLES' => 'all-feature-variables' }.freeze CONFIG_MANAGER = { diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 6fcc5ced..3169fe2a 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -1874,6 +1874,15 @@ def callback(_args); end source_info: {} ).ordered + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'feature', 'test_user', {'browser_type' => 'firefox'}, + feature_enabled: false, + feature_key: 'all_variables_feature', + source: 'rollout', + source_info: {} + ).ordered + expect(project_instance.get_enabled_features('test_user', 'browser_type' => 'firefox')).to eq(enabled_features) end end @@ -2364,6 +2373,22 @@ def callback(_args); end } allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'all-feature-variables', 'test_user', {}, + feature_enabled: true, + feature_key: 'all_variables_feature', + source: 'rollout', + variable_values: { + 'json_variable' => {'val' => 'default json'}, + 'string_variable' => 'default string', + 'boolean_variable' => false, + 'double_variable' => 1.99, + 'integer_variable' => 10 + }, + source_info: {} + ).ordered + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) .to eq( 'json_variable' => {'val' => 'default json'}, @@ -2419,6 +2444,22 @@ def callback(_args); end allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) allow(project_config).to receive(:variation_id_to_variable_usage_map).and_return(variation_id_to_variable_usage_map) + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'all-feature-variables', 'test_user', {}, + feature_enabled: true, + feature_key: 'all_variables_feature', + source: 'rollout', + variable_values: { + 'json_variable' => {'val' => 'feature enabled'}, + 'string_variable' => 'feature enabled', + 'boolean_variable' => true, + 'double_variable' => 14.99, + 'integer_variable' => 99 + }, + source_info: {} + ).ordered + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) .to eq( 'json_variable' => {'val' => 'feature enabled'}, @@ -2461,6 +2502,22 @@ def callback(_args); end it 'should return the default variable value' do allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'all-feature-variables', 'test_user', {}, + feature_enabled: false, + feature_key: 'all_variables_feature', + source: 'rollout', + variable_values: { + 'json_variable' => {'val' => 'default json'}, + 'string_variable' => 'default string', + 'boolean_variable' => false, + 'double_variable' => 1.99, + 'integer_variable' => 10 + }, + source_info: {} + ).ordered + expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) .to eq( 'json_variable' => {'val' => 'default json'}, From 41f5eacda3f0d621ad52ddca9b15c3a4ef128773 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 8 May 2020 14:22:06 -0700 Subject: [PATCH 05/11] added doc comments and updated copyright information --- lib/optimizely.rb | 22 +++++++++++++++++++ .../config/datafile_project_config.rb | 2 +- lib/optimizely/helpers/constants.rb | 2 +- lib/optimizely/helpers/variable_type.rb | 2 +- spec/config/datafile_project_config_spec.rb | 2 +- spec/optimizely_config_spec.rb | 2 +- spec/project_spec.rb | 2 +- spec/spec_params.rb | 2 +- 8 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index bcaa4b53..5cf30d9c 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -510,6 +510,15 @@ def get_feature_variable_double(feature_flag_key, variable_key, user_id, attribu variable_value end + # Get values of all the variables in the feature flag and returns them in a Dict + # + # @param feature_flag_key - String key of feature flag + # @param user_id - String user ID + # @param attributes - Hash representing visitor attributes and values which need to be recorded. + # + # @return [Dict] the Dict containing all the varible values + # @return [nil] if the feature flag is not found. + def get_all_feature_variables(feature_flag_key, user_id, attributes = nil) unless is_valid @logger.log(Logger::ERROR, InvalidProjectConfigError.new('get_all_feature_variables').message) @@ -773,6 +782,19 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, end def get_feature_variable_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) + # Helper method to get the non type-casted value for a variable attached to a + # feature flag. Returns appropriate variable value depending on whether there + # was a matching variation, feature was enabled or not or varible was part of the + # available variation or not. Also logs the appropriate message explaining how it + # evaluated the value of the variable. + # + # feature_flag_key - String key of feature flag the variable belongs to + # feature_enabled - Boolean indicating if feature is enabled or not + # variation - varition returned by decision service + # user_id - String user ID + # + # Returns string value of the variable. + config = project_config variable_value = variable['defaultValue'] if variation diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index 55d24346..9384ab30 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/lib/optimizely/config/datafile_project_config.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright 2019, Optimizely and contributors +# Copyright 2019-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. diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb index a0497ed3..1f962c0b 100644 --- a/lib/optimizely/helpers/constants.rb +++ b/lib/optimizely/helpers/constants.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2016-2019, Optimizely and contributors +# Copyright 2016-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. diff --git a/lib/optimizely/helpers/variable_type.rb b/lib/optimizely/helpers/variable_type.rb index 118215ba..d3049a31 100644 --- a/lib/optimizely/helpers/variable_type.rb +++ b/lib/optimizely/helpers/variable_type.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2017, Optimizely and contributors +# Copyright 2017, 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. diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index ec0c96ae..0875591a 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2019, Optimizely and contributors +# Copyright 2019-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. diff --git a/spec/optimizely_config_spec.rb b/spec/optimizely_config_spec.rb index 1a000419..eb97ef21 100644 --- a/spec/optimizely_config_spec.rb +++ b/spec/optimizely_config_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2019, Optimizely and contributors +# Copyright 2019-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. diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 3169fe2a..7d9d90a4 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2016-2019, Optimizely and contributors +# Copyright 2016-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. diff --git a/spec/spec_params.rb b/spec/spec_params.rb index 81d721f3..a24c7da1 100644 --- a/spec/spec_params.rb +++ b/spec/spec_params.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2016-2019, Optimizely and contributors +# Copyright 2016-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. From 22e4b6d021bbb60d99139b940680f5363413b98f Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 8 May 2020 14:26:09 -0700 Subject: [PATCH 06/11] removed whitespaces --- lib/optimizely.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 5cf30d9c..0b47e64c 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -782,19 +782,19 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, end def get_feature_variable_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) - # Helper method to get the non type-casted value for a variable attached to a - # feature flag. Returns appropriate variable value depending on whether there - # was a matching variation, feature was enabled or not or varible was part of the - # available variation or not. Also logs the appropriate message explaining how it + # Helper method to get the non type-casted value for a variable attached to a + # feature flag. Returns appropriate variable value depending on whether there + # was a matching variation, feature was enabled or not or varible was part of the + # available variation or not. Also logs the appropriate message explaining how it # evaluated the value of the variable. # # feature_flag_key - String key of feature flag the variable belongs to # feature_enabled - Boolean indicating if feature is enabled or not # variation - varition returned by decision service - # user_id - String user ID + # user_id - String user ID # # Returns string value of the variable. - + config = project_config variable_value = variable['defaultValue'] if variation From 1b1ed67f949710a97466860e9482bc4b6e03532d Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 8 May 2020 15:55:41 -0700 Subject: [PATCH 07/11] fixed a minor nit --- lib/optimizely.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 0b47e64c..ab3f72a0 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -550,8 +550,7 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil) feature_flag['variables'].each do |variable| variable_value = get_feature_variable_for_variation(feature_flag_key, feature_enabled, variation, variable, user_id) - variable_value = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) - all_variables[variable['key']] = variable_value + all_variables[variable['key']] = Helpers::VariableType.cast_value_to_type(variable_value, variable['type'], @logger) end source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] From 5e26a71ce409978e30afee4a96479425c55b1d09 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 8 May 2020 16:04:32 -0700 Subject: [PATCH 08/11] removed unnecessary order --- spec/project_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 7d9d90a4..d4315fcb 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -2387,7 +2387,7 @@ def callback(_args); end 'integer_variable' => 10 }, source_info: {} - ).ordered + ) expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) .to eq( @@ -2458,7 +2458,7 @@ def callback(_args); end 'integer_variable' => 99 }, source_info: {} - ).ordered + ) expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) .to eq( @@ -2516,7 +2516,7 @@ def callback(_args); end 'integer_variable' => 10 }, source_info: {} - ).ordered + ) expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes)) .to eq( From cd2ee7dff8797978d309d850306c513321d1303b Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 12 May 2020 14:00:17 -0700 Subject: [PATCH 09/11] refactored a test --- spec/config/datafile_project_config_spec.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index 0875591a..84d51892 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -32,14 +32,13 @@ project_config = Optimizely::DatafileProjectConfig.new(config_body_JSON, logger, error_handler) feature_flags_to_compare = config_body.fetch('featureFlags') - feature_flags_to_compare.each do |feature_flag| - feature_flag['variables'].each do |variable| - if variable['type'] == 'string' && variable['subType'] == 'json' - variable['type'] = 'json' - variable.delete('subType') - end - end - end + # modify json variable from datafile in the format expected by project config + variable_to_modify = feature_flags_to_compare[8]['variables'][0] + variable_to_modify['type'] = 'json' + variable_to_modify.delete('subType') + variable_to_modify = feature_flags_to_compare[9]['variables'][0] + variable_to_modify['type'] = 'json' + variable_to_modify.delete('subType') expect(project_config.account_id).to eq(config_body['accountId']) expect(project_config.attributes).to eq(config_body['attributes']) From 4ccea7893f61f0d5612bcbdeb2bdeed83cb1cefc Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 12 May 2020 14:20:49 -0700 Subject: [PATCH 10/11] added notification listener tests for get_feature_variable_json --- spec/project_spec.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 7941fa51..d2ee451c 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -2094,6 +2094,18 @@ def callback(_args); end } allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'feature-variable', 'test_user', {}, + feature_enabled: true, + feature_key: 'json_single_variable_feature', + source: 'rollout', + variable_key: 'json_variable', + variable_type: 'json', + variable_value: {'val' => 'wingardium leviosa'}, + source_info: {} + ) + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) .to eq('val' => 'wingardium leviosa') expect(spy_logger).to have_received(:log).once @@ -2154,6 +2166,18 @@ def callback(_args); end } allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'feature-variable', 'test_user', {}, + feature_enabled: true, + feature_key: 'json_single_variable_feature', + source: 'rollout', + variable_key: 'json_variable', + variable_type: 'json', + variable_value: {'value' => 'cta_1'}, + source_info: {} + ) + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) .to eq('value' => 'cta_1') @@ -2171,6 +2195,18 @@ def callback(_args); end it 'should return the default variable value' do allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], + 'feature-variable', 'test_user', {}, + feature_enabled: false, + feature_key: 'json_single_variable_feature', + source: 'rollout', + variable_key: 'json_variable', + variable_type: 'json', + variable_value: {'val' => 'wingardium leviosa'}, + source_info: {} + ) + expect(project_instance.get_feature_variable_json('json_single_variable_feature', 'json_variable', user_id, user_attributes)) .to eq('val' => 'wingardium leviosa') expect(spy_logger).to have_received(:log).once From 95b0de1b1ac70b674d0da94cce1326930cabff55 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 12 May 2020 16:57:59 -0700 Subject: [PATCH 11/11] added source info verification to the tests for feature tests --- spec/project_spec.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index d2ee451c..2b61076e 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -2420,11 +2420,9 @@ def callback(_args); end describe 'when the feature flag is enabled for the user' do describe 'and a variable usage instance is not found' do it 'should return the default variable value' do + Decision = Struct.new(:experiment, :variation, :source) variation_to_return = project_config.rollout_id_map['166661']['experiments'][0]['variations'][0] - decision_to_return = { - 'experiment' => nil, - 'variation' => variation_to_return - } + decision_to_return = Decision.new({'key' => 'test-exp'}, variation_to_return, 'feature-test') allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) expect(project_instance.notification_center).to receive(:send_notifications).once.with( @@ -2432,7 +2430,7 @@ def callback(_args); end 'all-feature-variables', 'test_user', {}, feature_enabled: true, feature_key: 'all_variables_feature', - source: 'rollout', + source: 'feature-test', variable_values: { 'json_variable' => {'val' => 'default json'}, 'string_variable' => 'default string', @@ -2440,7 +2438,10 @@ def callback(_args); end 'double_variable' => 1.99, 'integer_variable' => 10 }, - source_info: {} + source_info: { + experiment_key: 'test-exp', + variation_key: '177775' + } ) expect(project_instance.get_all_feature_variables('all_variables_feature', user_id, user_attributes))