diff --git a/lib/optimizely.rb b/lib/optimizely.rb index fe8997a9..8750fbac 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -96,15 +96,11 @@ def activate(experiment_key, user_id, attributes = nil) return nil unless Optimizely::Helpers::Validator.inputs_valid?( { - experiment_key: experiment_key + experiment_key: experiment_key, + user_id: user_id }, @logger, Logger::ERROR ) - unless user_id.is_a?(String) - @logger.log(Logger::ERROR, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return nil - end - variation_key = get_variation(experiment_key, user_id, attributes) if variation_key.nil? @@ -136,15 +132,11 @@ def get_variation(experiment_key, user_id, attributes = nil) return nil unless Optimizely::Helpers::Validator.inputs_valid?( { - experiment_key: experiment_key + experiment_key: experiment_key, + user_id: user_id }, @logger, Logger::ERROR ) - unless user_id.is_a?(String) - @logger.log(Logger::ERROR, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return nil - end - unless user_inputs_valid?(attributes) @logger.log(Logger::INFO, "Not activating user '#{user_id}.") return nil @@ -202,15 +194,11 @@ def track(event_key, user_id, attributes = nil, event_tags = nil) return nil unless Optimizely::Helpers::Validator.inputs_valid?( { - event_key: event_key + event_key: event_key, + user_id: user_id }, @logger, Logger::ERROR ) - unless user_id.is_a?(String) - @logger.log(Logger::ERROR, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return nil - end - return nil unless user_inputs_valid?(attributes, event_tags) experiment_ids = @config.get_experiment_ids_for_event(event_key) @@ -265,15 +253,11 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil) return false unless Optimizely::Helpers::Validator.inputs_valid?( { - feature_flag_key: feature_flag_key + feature_flag_key: feature_flag_key, + user_id: user_id }, @logger, Logger::ERROR ) - unless user_id.is_a?(String) - @logger.log(Logger::ERROR, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return false - end - return false unless user_inputs_valid?(attributes) feature_flag = @config.get_feature_flag_from_key(feature_flag_key) @@ -323,10 +307,11 @@ def get_enabled_features(user_id, attributes = nil) return enabled_features end - unless user_id.is_a?(String) - @logger.log(Logger::ERROR, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return enabled_features - end + return enabled_features unless Optimizely::Helpers::Validator.inputs_valid?( + { + user_id: user_id + }, @logger, Logger::ERROR + ) return enabled_features unless user_inputs_valid?(attributes) @@ -467,16 +452,12 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, { feature_flag_key: feature_flag_key, variable_key: variable_key, + user_id: user_id, variable_type: variable_type }, @logger, Logger::ERROR ) - unless user_id.is_a?(String) - @logger.log(Logger::ERROR, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return nil - end - return nil unless user_inputs_valid?(attributes) feature_flag = @config.get_feature_flag_from_key(feature_flag_key) diff --git a/lib/optimizely/helpers/validator.rb b/lib/optimizely/helpers/validator.rb index 041e31aa..3a8e0af1 100644 --- a/lib/optimizely/helpers/validator.rb +++ b/lib/optimizely/helpers/validator.rb @@ -108,13 +108,21 @@ def inputs_valid?(variables, logger = NoOpLogger.new, level = Logger::ERROR) return false unless variables.respond_to?(:each) && !variables.empty? is_valid = true + if variables.include? :user_id + unless variables[:user_id].is_a?(String) + is_valid = false + logger.log(level, "#{Constants::INPUT_VARIABLES['USER_ID']} is invalid") + end + variables.delete :user_id + end + variables.each do |key, value| next if value.is_a?(String) && !value.empty? is_valid = false next unless logger_valid?(logger) && level - logger.log(level, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES[key.to_s.upcase]} is invalid") + logger.log(level, "#{Constants::INPUT_VARIABLES[key.to_s.upcase]} is invalid") end is_valid end diff --git a/lib/optimizely/project_config.rb b/lib/optimizely/project_config.rb index fb178800..3a90c6be 100644 --- a/lib/optimizely/project_config.rb +++ b/lib/optimizely/project_config.rb @@ -282,15 +282,11 @@ def get_forced_variation(experiment_key, user_id) return nil unless Optimizely::Helpers::Validator.inputs_valid?( { - experiment_key: experiment_key + experiment_key: experiment_key, + user_id: user_id }, @logger, Logger::DEBUG ) - unless user_id.is_a?(String) - @logger.log(Logger::DEBUG, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return nil - end - unless @forced_variation_map.key? user_id @logger.log(Logger::DEBUG, "User '#{user_id}' is not in the forced variation map.") return nil @@ -333,15 +329,10 @@ def set_forced_variation(experiment_key, user_id, variation_key) # # Returns a boolean value that indicates if the set completed successfully. - input_values = {experiment_key: experiment_key} + input_values = {experiment_key: experiment_key, user_id: user_id} input_values[:variation_key] = variation_key unless variation_key.nil? return false unless Optimizely::Helpers::Validator.inputs_valid?(input_values, @logger, Logger::DEBUG) - unless user_id.is_a?(String) - @logger.log(Logger::DEBUG, "#{Optimizely::Helpers::Constants::INPUT_VARIABLES['USER_ID']} is invalid") - return false - end - experiment = get_experiment_from_key(experiment_key) experiment_id = experiment['id'] if experiment # check if the experiment exists in the datafile diff --git a/spec/project_config_spec.rb b/spec/project_config_spec.rb index 7116de54..50b341d0 100644 --- a/spec/project_config_spec.rb +++ b/spec/project_config_spec.rb @@ -947,6 +947,7 @@ expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( { experiment_key: @valid_experiment[:key], + user_id: @user_id, variation_key: @valid_variation[:key] }, spy_logger, Logger::DEBUG ) @@ -966,7 +967,8 @@ it 'should call inputs_valid? with the proper arguments in get_forced_variation' do expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( { - experiment_key: @valid_experiment[:key] + experiment_key: @valid_experiment[:key], + user_id: @user_id }, spy_logger, Logger::DEBUG ) config.get_forced_variation(@valid_experiment[:key], @user_id) diff --git a/spec/project_spec.rb b/spec/project_spec.rb index f6300088..b47a08d5 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -271,7 +271,8 @@ class InvalidErrorHandler; end it 'should call inputs_valid? with the proper arguments in activate' do expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( { - experiment_key: 'test_experiment_with_audience' + experiment_key: 'test_experiment_with_audience', + user_id: 'test_user' }, spy_logger, Logger::ERROR ) project_instance.activate('test_experiment_with_audience', 'test_user') @@ -454,7 +455,8 @@ class InvalidErrorHandler; end it 'should call inputs_valid? with the proper arguments in track' do expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( { - event_key: 'test_event' + event_key: 'test_event', + user_id: 'test_user' }, spy_logger, Logger::ERROR ) project_instance.track('test_event', 'test_user') @@ -646,7 +648,8 @@ class InvalidErrorHandler; end it 'should call inputs_valid? with the proper arguments in get_variation' do expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( { - experiment_key: 'test_experiment_with_audience' + experiment_key: 'test_experiment_with_audience', + user_id: 'test_user' }, spy_logger, Logger::ERROR ) project_instance.get_variation('test_experiment_with_audience', 'test_user', nil) @@ -799,7 +802,8 @@ class InvalidErrorHandler; end it 'should call inputs_valid? with the proper arguments in is_feature_enabled' do expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( { - feature_flag_key: 'multi_variate_feature' + feature_flag_key: 'multi_variate_feature', + user_id: 'test_user' }, spy_logger, Logger::ERROR ) project_instance.is_feature_enabled('multi_variate_feature', 'test_user') @@ -926,6 +930,15 @@ class InvalidErrorHandler; end expect(invalid_project.get_enabled_features('test_user')).to be_empty end + it 'should call inputs_valid? with the proper arguments in get_enabled_features' do + expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with( + { + user_id: 'test_user' + }, spy_logger, Logger::ERROR + ) + project_instance.get_enabled_features('test_user') + end + it 'should return empty when no feature flag is enabled' do allow(project_instance).to receive(:is_feature_enabled).and_return(false) expect(project_instance.get_enabled_features('test_user')).to be_empty @@ -1249,11 +1262,11 @@ class InvalidErrorHandler; end { feature_flag_key: 'integer_single_variable_feature', variable_key: 'integer_variable', + user_id: 'test_user', variable_type: 'integer' }, spy_logger, Logger::ERROR ) - expect(project_instance.get_feature_variable_integer('integer_single_variable_feature', 'integer_variable', nil, user_attributes)) - .to eq(nil) + project_instance.get_feature_variable_integer('integer_single_variable_feature', 'integer_variable', 'test_user', user_attributes) end it 'should log and return nil when user ID is non string' do