Skip to content

feat(attribute_value): Don't target NAN, INF, -INF and > 2^53 #148

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions lib/optimizely/helpers/validator.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

#
# Copyright 2016-2018, Optimizely and contributors
# Copyright 2016-2019, 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.
Expand Down Expand Up @@ -44,7 +44,9 @@ def attribute_valid?(attribute_key, attribute_value)

return false unless attribute_key.is_a?(String) || attribute_key.is_a?(Symbol)

Helpers::Constants::ATTRIBUTE_VALID_TYPES.any? { |type| attribute_value.is_a?(type) }
return true if (boolean? attribute_value) || (attribute_value.is_a? String)

finite_number?(attribute_value)
end

def event_tags_valid?(event_tags)
Expand Down
75 changes: 74 additions & 1 deletion spec/custom_attribute_condition_evaluator_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

#
# Copyright 2018, Optimizely and contributors
# Copyright 2019, 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.
Expand All @@ -17,6 +17,7 @@
#
require 'json'
require 'spec_helper'
require 'optimizely/helpers/validator'

describe Optimizely::CustomAttributeConditionEvaluator do
it 'should return true when the attributes pass the audience conditions and no match type is provided' do
Expand Down Expand Up @@ -168,6 +169,30 @@
expect(condition_evaluator.evaluate(@exact_integer_conditions)).to eq(nil)
expect(condition_evaluator.evaluate(@exact_float_conditions)).to eq(nil)
end

it 'should return nil when finite_number? returns false for provided arguments' do
# Returns false for user attribute value
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).once.with(10).and_return(false)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('sum' => 10)
expect(condition_evaluator.evaluate(@exact_integer_conditions)).to be nil
# finite_number? should not be called with condition value as user attribute value is failed
expect(Optimizely::Helpers::Validator).not_to have_received(:finite_number?).with(100)

# Returns false for condition value
@exact_integer_conditions['value'] = 101
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).twice.and_return(true, false)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('sum' => 100)
expect(condition_evaluator.evaluate(@exact_integer_conditions)).to be nil
# finite_number? should be called with condition value as it returns true for user attribute value
expect(Optimizely::Helpers::Validator).to have_received(:finite_number?).with(101)
end

it 'should not return nil when finite_number? returns true for provided arguments' do
@exact_integer_conditions['value'] = 10
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).twice.and_return(true, true)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('sum' => 10)
expect(condition_evaluator.evaluate(@exact_integer_conditions)).not_to be_nil
end
end

describe 'with a boolean condition value' do
Expand Down Expand Up @@ -286,6 +311,30 @@
expect(condition_evaluator.evaluate(@gt_integer_conditions)).to eq(nil)
expect(condition_evaluator.evaluate(@gt_float_conditions)).to eq(nil)
end

it 'should return nil when finite_number? returns false for provided arguments' do
# Returns false for user attribute value
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).once.with(5).and_return(false)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('input_value' => 5)
expect(condition_evaluator.evaluate(@gt_integer_conditions)).to be nil
# finite_number? should not be called with condition value as user attribute value is failed
expect(Optimizely::Helpers::Validator).not_to have_received(:finite_number?).with(10)

# Returns false for condition value
@gt_integer_conditions['value'] = 95
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).twice.and_return(true, false)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('input_value' => 10)
expect(condition_evaluator.evaluate(@gt_integer_conditions)).to be nil
# finite_number? should be called with condition value as it returns true for user attribute value
expect(Optimizely::Helpers::Validator).to have_received(:finite_number?).with(95)
end

it 'should not return nil when finite_number? returns true for provided arguments' do
@gt_integer_conditions['value'] = 81
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).twice.and_return(true, true)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('input_value' => 51)
expect(condition_evaluator.evaluate(@gt_integer_conditions)).not_to be_nil
end
end

describe 'less than match type' do
Expand Down Expand Up @@ -341,5 +390,29 @@
expect(condition_evaluator.evaluate(@lt_integer_conditions)).to eq(nil)
expect(condition_evaluator.evaluate(@lt_float_conditions)).to eq(nil)
end

it 'should return nil when finite_number? returns false for provided arguments' do
# Returns false for user attribute value
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).once.with(15).and_return(false)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('input_value' => 15)
expect(condition_evaluator.evaluate(@lt_integer_conditions)).to be nil
# finite_number? should not be called with condition value as user attribute value is failed
expect(Optimizely::Helpers::Validator).not_to have_received(:finite_number?).with(10)

# Returns false for condition value
@lt_integer_conditions['value'] = 25
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).twice.and_return(true, false)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('input_value' => 10)
expect(condition_evaluator.evaluate(@lt_integer_conditions)).to be nil
# finite_number? should be called with condition value as it returns true for user attribute value
expect(Optimizely::Helpers::Validator).to have_received(:finite_number?).with(25)
end

it 'should not return nil when finite_number? returns true for provided arguments' do
@lt_integer_conditions['value'] = 65
allow(Optimizely::Helpers::Validator).to receive(:finite_number?).twice.and_return(true, true)
condition_evaluator = Optimizely::CustomAttributeConditionEvaluator.new('input_value' => 75)
expect(condition_evaluator.evaluate(@lt_integer_conditions)).not_to be_nil
end
end
end
4 changes: 3 additions & 1 deletion spec/validator_helper_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

#
# Copyright 2018, Optimizely and contributors
# Copyright 2018-2019, 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.
Expand Down Expand Up @@ -122,9 +122,11 @@
expect(Optimizely::Helpers::Validator.finite_number?(5.5)).to eq(true)
# Upper limit
expect(Optimizely::Helpers::Validator.finite_number?((2**53) - 1)).to eq(true)
# float(2.0**53) + 1 evaluates to float(2.0**53)
expect(Optimizely::Helpers::Validator.finite_number?((2.0**53) + 1)).to eq(true)
# Lower limit
expect(Optimizely::Helpers::Validator.finite_number?((-2**53) + 1)).to eq(true)
# float(-2.0**53) - 1 evaluates to float(-2.0**53)
expect(Optimizely::Helpers::Validator.finite_number?((-2.0**53) - 1)).to eq(true)
# exact number integer
expect(Optimizely::Helpers::Validator.finite_number?(2**53)).to eq(true)
Expand Down