Skip to content

Refact(API): validates user_id inside validator. #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: rashid/input-validations-2
Choose a base branch
from
Open

Refact(API): validates user_id inside validator. #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 14 additions & 33 deletions lib/optimizely.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
10 changes: 9 additions & 1 deletion lib/optimizely/helpers/validator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Add a comment that empty str is a valid user ID

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
Expand Down
15 changes: 3 additions & 12 deletions lib/optimizely/project_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion spec/project_config_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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)
Expand Down
25 changes: 19 additions & 6 deletions spec/project_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down