diff --git a/optimizely/helpers/validator.py b/optimizely/helpers/validator.py index b8cd3f42..9efe25ac 100644 --- a/optimizely/helpers/validator.py +++ b/optimizely/helpers/validator.py @@ -187,20 +187,24 @@ def is_attribute_valid(attribute_key, attribute_value): if not isinstance(attribute_key, string_types): return False - if isinstance(attribute_value, string_types) or type(attribute_value) in (int, float, bool): + if isinstance(attribute_value, (string_types, bool)): return True + if isinstance(attribute_value, (numbers.Integral, float)): + return is_finite_number(attribute_value) + return False def is_finite_number(value): - """ Method to validate if the given value is a number and not one of NAN, INF, -INF. + """ Validates if the given value is a number, enforces + absolute limit of 2^53 and restricts NAN, INF, -INF. Args: value: Value to be validated. Returns: - Boolean: True if value is a number and not NAN, INF or -INF else False. + Boolean: True if value is a number and not NAN, INF, -INF or greater than 2^53 else False. """ if not isinstance(value, (numbers.Integral, float)): # numbers.Integral instead of int to accomodate long integer in python 2 @@ -214,6 +218,9 @@ def is_finite_number(value): if math.isnan(value) or math.isinf(value): return False + if abs(value) > (2**53): + return False + return True diff --git a/tests/base.py b/tests/base.py index 7f79697b..ba3b5e02 100644 --- a/tests/base.py +++ b/tests/base.py @@ -13,9 +13,14 @@ import json import unittest +from six import PY3 from optimizely import optimizely +if PY3: + def long(a): + raise NotImplementedError('Tests should only call `long` if running in PY2') + class BaseTest(unittest.TestCase): diff --git a/tests/helpers_tests/test_condition.py b/tests/helpers_tests/test_condition.py index 51021a02..f5fb9aae 100644 --- a/tests/helpers_tests/test_condition.py +++ b/tests/helpers_tests/test_condition.py @@ -12,16 +12,12 @@ # limitations under the License. import mock -from six import PY2, PY3 +from six import PY2 from optimizely.helpers import condition as condition_helper from tests import base -if PY3: - def long(a): - raise NotImplementedError('Tests should only call `long` if running in PY2') - browserConditionSafari = ['browser_type', 'safari', 'custom_attribute', 'exact'] booleanCondition = ['is_firefox', True, 'custom_attribute', 'exact'] integerCondition = ['num_users', 10, 'custom_attribute', 'exact'] @@ -286,6 +282,27 @@ def test_exact_float__returns_null__when_no_user_provided_value(self): self.assertIsNone(evaluator.evaluate(0)) + def test_exact__given_number_values__calls_is_finite_number(self): + """ Returns True if is_finite_number returns True. Returns None if is_finite_number returns False. """ + + evaluator = condition_helper.CustomAttributeConditionEvaluator( + exact_int_condition_list, {'lasers_count': 9000} + ) + + with mock.patch('optimizely.helpers.validator.is_finite_number', + return_value=True) as is_finite: + self.assertTrue(evaluator.evaluate(0)) + is_finite.assert_called_with(9000) + + evaluator = condition_helper.CustomAttributeConditionEvaluator( + exact_int_condition_list, {'lasers_count': 9000.0} + ) + + with mock.patch('optimizely.helpers.validator.is_finite_number', + return_value=False) as is_finite: + self.assertIsNone(evaluator.evaluate(0)) + is_finite.assert_called_with(9000.0) + def test_exact_bool__returns_true__when_user_provided_value_is_equal_to_condition_value(self): evaluator = condition_helper.CustomAttributeConditionEvaluator( @@ -594,6 +611,82 @@ def test_less_than_float__returns_null__when_no_user_provided_value(self): self.assertIsNone(evaluator.evaluate(0)) + def test_greater_than__calls_is_finite_number(self): + """ Returns True if is_finite_number returns True. Returns None if is_finite_number returns False. """ + + evaluator = condition_helper.CustomAttributeConditionEvaluator( + gt_int_condition_list, {'meters_travelled': 48.1} + ) + + def is_finite_number__rejecting_condition_value(value): + if value == 48: + return False + return True + + with mock.patch('optimizely.helpers.validator.is_finite_number', + side_effect=is_finite_number__rejecting_condition_value) as is_finite_mock: + self.assertIsNone(evaluator.evaluate(0)) + + # assert that isFiniteNumber only needs to reject condition value to stop evaluation. + is_finite_mock.assert_called_once_with(48) + + def is_finite_number__rejecting_user_attribute_value(value): + if value == 48.1: + return False + return True + + with mock.patch('optimizely.helpers.validator.is_finite_number', + side_effect=is_finite_number__rejecting_user_attribute_value) as is_finite_mock: + self.assertIsNone(evaluator.evaluate(0)) + + # assert that isFiniteNumber evaluates user value only if it has accepted condition value. + is_finite_mock.assert_has_calls([mock.call(48), mock.call(48.1)]) + + def is_finite_number__accepting_both_values(value): + return True + + with mock.patch('optimizely.helpers.validator.is_finite_number', + side_effect=is_finite_number__accepting_both_values): + self.assertTrue(evaluator.evaluate(0)) + + def test_less_than__calls_is_finite_number(self): + """ Returns True if is_finite_number returns True. Returns None if is_finite_number returns False. """ + + evaluator = condition_helper.CustomAttributeConditionEvaluator( + lt_int_condition_list, {'meters_travelled': 47} + ) + + def is_finite_number__rejecting_condition_value(value): + if value == 48: + return False + return True + + with mock.patch('optimizely.helpers.validator.is_finite_number', + side_effect=is_finite_number__rejecting_condition_value) as is_finite_mock: + self.assertIsNone(evaluator.evaluate(0)) + + # assert that isFiniteNumber only needs to reject condition value to stop evaluation. + is_finite_mock.assert_called_once_with(48) + + def is_finite_number__rejecting_user_attribute_value(value): + if value == 47: + return False + return True + + with mock.patch('optimizely.helpers.validator.is_finite_number', + side_effect=is_finite_number__rejecting_user_attribute_value) as is_finite_mock: + self.assertIsNone(evaluator.evaluate(0)) + + # assert that isFiniteNumber evaluates user value only if it has accepted condition value. + is_finite_mock.assert_has_calls([mock.call(48), mock.call(47)]) + + def is_finite_number__accepting_both_values(value): + return True + + with mock.patch('optimizely.helpers.validator.is_finite_number', + side_effect=is_finite_number__accepting_both_values): + self.assertTrue(evaluator.evaluate(0)) + class ConditionDecoderTests(base.BaseTest): diff --git a/tests/helpers_tests/test_validator.py b/tests/helpers_tests/test_validator.py index 5f63a072..cca4a0c5 100644 --- a/tests/helpers_tests/test_validator.py +++ b/tests/helpers_tests/test_validator.py @@ -12,6 +12,9 @@ # limitations under the License. import json +import mock + +from six import PY2 from optimizely import error_handler from optimizely import event_dispatcher @@ -168,6 +171,62 @@ def test_is_attribute_valid(self): self.assertTrue(validator.is_attribute_valid('test_attribute', "")) self.assertTrue(validator.is_attribute_valid('test_attribute', 'test_value')) + # test if attribute value is a number, it calls is_finite_number and returns it's result + with mock.patch('optimizely.helpers.validator.is_finite_number', + return_value=True) as is_finite: + self.assertTrue(validator.is_attribute_valid('test_attribute', 5)) + + is_finite.assert_called_once_with(5) + + with mock.patch('optimizely.helpers.validator.is_finite_number', + return_value=False) as is_finite: + self.assertFalse(validator.is_attribute_valid('test_attribute', 5.5)) + + is_finite.assert_called_once_with(5.5) + + if PY2: + with mock.patch('optimizely.helpers.validator.is_finite_number', + return_value=None) as is_finite: + self.assertIsNone(validator.is_attribute_valid('test_attribute', long(5))) + + is_finite.assert_called_once_with(long(5)) + + def test_is_finite_number(self): + """ Test that it returns true if value is a number and not NAN, INF, -INF or greater than 2^53. + Otherwise False. + """ + # test non number values + self.assertFalse(validator.is_finite_number('HelloWorld')) + self.assertFalse(validator.is_finite_number(True)) + self.assertFalse(validator.is_finite_number(False)) + self.assertFalse(validator.is_finite_number(None)) + self.assertFalse(validator.is_finite_number({})) + self.assertFalse(validator.is_finite_number([])) + self.assertFalse(validator.is_finite_number(())) + + # test invalid numbers + self.assertFalse(validator.is_finite_number(float('inf'))) + self.assertFalse(validator.is_finite_number(float('-inf'))) + self.assertFalse(validator.is_finite_number(float('nan'))) + self.assertFalse(validator.is_finite_number(int(2**53) + 1)) + self.assertFalse(validator.is_finite_number(-int(2**53) - 1)) + self.assertFalse(validator.is_finite_number(float(2**53) + 2.0)) + self.assertFalse(validator.is_finite_number(-float(2**53) - 2.0)) + if PY2: + self.assertFalse(validator.is_finite_number(long(2**53) + 1)) + self.assertFalse(validator.is_finite_number(-long(2**53) - 1)) + + # test valid numbers + self.assertTrue(validator.is_finite_number(0)) + self.assertTrue(validator.is_finite_number(5)) + self.assertTrue(validator.is_finite_number(5.5)) + # float(2**53) + 1.0 evaluates to float(2**53) + self.assertTrue(validator.is_finite_number(float(2**53) + 1.0)) + self.assertTrue(validator.is_finite_number(-float(2**53) - 1.0)) + self.assertTrue(validator.is_finite_number(int(2**53))) + if PY2: + self.assertTrue(validator.is_finite_number(long(2**53))) + class DatafileValidationTests(base.BaseTest):