feat(attribute_value): Don't target NAN, INF, -INF and > 2^53 #147
Conversation
tests/UtilsTests/ValidatorTest.php
Outdated
| $this->assertTrue(Validator::IsFiniteNumber(0)); | ||
| $this->assertTrue(Validator::IsFiniteNumber(5)); | ||
| $this->assertTrue(Validator::IsFiniteNumber(5.5)); | ||
| // float(2**53) + 1.0 evaluates to float(2**53) |
There was a problem hiding this comment.
looks like copied from python, please change it.
There was a problem hiding this comment.
LGTM. Minor feedback.
Please address @msohailhussain 's comment. I can help merge once ready.
src/Optimizely/Utils/Validator.php
Outdated
| * @return boolean Representing whether attribute's value is | ||
| * a number and not NAN, INF, -INF or greater than 2^53. | ||
| */ | ||
| public static function isFiniteNumber($value) |
There was a problem hiding this comment.
nit. Just a code arrangement recommendation.
I would prefer if you defined isFiniteNumber before referencing it on line 91.
src/Optimizely/Utils/Validator.php
Outdated
| */ | ||
| public static function isFiniteNumber($value) | ||
| { | ||
| if (!(is_int($value) || is_float($value))) { |
There was a problem hiding this comment.
Technically you have already performed this check on line 91. If you want you can call isFiniteNumber directly on line 91.
There was a problem hiding this comment.
isFiniteNumber is also called from conditionEvaluator when determining valid values in exact, gt, lt match types. So we need this check here.
src/Optimizely/Utils/Validator.php
Outdated
| return false; | ||
| } | ||
|
|
||
| if (abs($value) > pow(2,53)) { |
There was a problem hiding this comment.
nit. Insert space after ,
Summary
Don't target NAN, INF or -INF doubles
Don't target Values > 2^53.
Both in EventBuilder payload and Audience Evaluation
Test plan
Issues