From d514637a83f6d89c6f50d5c2824dceb8452618b6 Mon Sep 17 00:00:00 2001 From: Owais Date: Wed, 19 Dec 2018 11:51:32 +0500 Subject: [PATCH 1/2] feat(audience match types): Add support for typed audiences --- src/Optimizely/ProjectConfig.php | 15 +- src/Optimizely/Utils/ConditionDecoder.php | 44 --- .../Utils/ConditionTreeEvaluator.php | 2 +- .../CustomAttributeConditionEvaluator.php | 41 ++- src/Optimizely/Utils/Validator.php | 19 +- .../DecisionServiceTest.php | 2 +- tests/OptimizelyTest.php | 164 ++++++++++++ tests/ProjectConfigTest.php | 29 ++ tests/TestData.php | 251 ++++++++++++++++++ tests/UtilsTests/ConditionDecoderTest.php | 58 ---- .../UtilsTests/ConditionTreeEvaluatorTest.php | 6 +- .../CustomAttributeConditionEvaluatorTest.php | 34 +-- tests/UtilsTests/ValidatorTest.php | 42 ++- 13 files changed, 554 insertions(+), 153 deletions(-) delete mode 100644 src/Optimizely/Utils/ConditionDecoder.php delete mode 100644 tests/UtilsTests/ConditionDecoderTest.php diff --git a/src/Optimizely/ProjectConfig.php b/src/Optimizely/ProjectConfig.php index 674c2e96..fd172299 100644 --- a/src/Optimizely/ProjectConfig.php +++ b/src/Optimizely/ProjectConfig.php @@ -214,6 +214,7 @@ public function __construct($datafile, $logger, $errorHandler) $events = $config['events'] ?: []; $attributes = $config['attributes'] ?: []; $audiences = $config['audiences'] ?: []; + $typedAudiences = isset($config['typedAudiences']) ? $config['typedAudiences']: []; $rollouts = isset($config['rollouts']) ? $config['rollouts'] : []; $featureFlags = isset($config['featureFlags']) ? $config['featureFlags']: []; @@ -221,6 +222,7 @@ public function __construct($datafile, $logger, $errorHandler) $this->_experimentKeyMap = ConfigParser::generateMap($experiments, 'key', Experiment::class); $this->_eventKeyMap = ConfigParser::generateMap($events, 'key', Event::class); $this->_attributeKeyMap = ConfigParser::generateMap($attributes, 'key', Attribute::class); + $typedAudienceIdMap = ConfigParser::generateMap($typedAudiences, 'id', Audience::class); $this->_audienceIdMap = ConfigParser::generateMap($audiences, 'id', Audience::class); $this->_rollouts = ConfigParser::generateMap($rollouts, null, Rollout::class); $this->_featureFlags = ConfigParser::generateMap($featureFlags, null, FeatureFlag::class); @@ -249,12 +251,19 @@ public function __construct($datafile, $logger, $errorHandler) } } - $conditionDecoder = new ConditionDecoder(); foreach (array_values($this->_audienceIdMap) as $audience) { - $conditionDecoder->deserializeAudienceConditions($audience->getConditions()); - $audience->setConditionsList($conditionDecoder->getConditionsList()); + $audience->setConditionsList(json_decode($audience->getConditions(), true)); } + // Conditions in typedAudiences are not expected to be string-encoded so they don't need + // to be decoded unlike audiences. + foreach (array_values($typedAudienceIdMap) as $typedAudience) { + $typedAudience->setConditionsList($typedAudience->getConditions()); + } + + // Overwrite audiences by typedAudiences. + $this->_audienceIdMap = array_replace($this->_audienceIdMap, $typedAudienceIdMap); + $rolloutVariationIdMap = []; $rolloutVariationKeyMap = []; foreach ($this->_rollouts as $rollout) { diff --git a/src/Optimizely/Utils/ConditionDecoder.php b/src/Optimizely/Utils/ConditionDecoder.php deleted file mode 100644 index ba0df673..00000000 --- a/src/Optimizely/Utils/ConditionDecoder.php +++ /dev/null @@ -1,44 +0,0 @@ -_conditionsList = json_decode($conditions); - } - - /** - * @return array JSON decoded audience conditions. - */ - public function getConditionsList() - { - return $this->_conditionsList; - } -} diff --git a/src/Optimizely/Utils/ConditionTreeEvaluator.php b/src/Optimizely/Utils/ConditionTreeEvaluator.php index ecd77b62..fe84c2c1 100644 --- a/src/Optimizely/Utils/ConditionTreeEvaluator.php +++ b/src/Optimizely/Utils/ConditionTreeEvaluator.php @@ -140,7 +140,7 @@ protected function notEvaluator(array $condition, callable $leafEvaluator) */ public function evaluate($conditions, callable $leafEvaluator) { - if (is_array($conditions)) { + if (!Validator::doesArrayContainOnlyStringKeys($conditions)) { if(in_array($conditions[0], $this->getOperators())) { $operator = array_shift($conditions); diff --git a/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php b/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php index c4195e48..c557200d 100644 --- a/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php +++ b/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php @@ -44,6 +44,21 @@ public function __construct(array $userAttributes) $this->userAttributes = $userAttributes; } + /** + * Sets null for missing keys in a leaf condition. + * + * @param array $leafCondition The leaf condition node of an audience. + */ + protected function setNullForMissingKeys(array $leafCondition) + { + $keys = ['type', 'match', 'value']; + foreach($keys as $key) { + $leafCondition[$key] = isset($leafCondition[$key]) ? $leafCondition[$key]: null; + } + + return $leafCondition; + } + /** * Gets the supported match types for condition evaluation. * @@ -103,8 +118,8 @@ protected function isValueValidForExactConditions($value) */ protected function exactEvaluator($condition) { - $conditionName = $condition->{'name'}; - $conditionValue = $condition->{'value'}; + $conditionName = $condition['name']; + $conditionValue = $condition['value']; $userValue = isset($this->userAttributes[$conditionName]) ? $this->userAttributes[$conditionName]: null; if(!$this->isValueValidForExactConditions($userValue) || @@ -128,7 +143,7 @@ protected function exactEvaluator($condition) */ protected function existsEvaluator($condition) { - $conditionName = $condition->{'name'}; + $conditionName = $condition['name']; return isset($this->userAttributes[$conditionName]); } @@ -144,8 +159,8 @@ protected function existsEvaluator($condition) */ protected function greaterThanEvaluator($condition) { - $conditionName = $condition->{'name'}; - $conditionValue = $condition->{'value'}; + $conditionName = $condition['name']; + $conditionValue = $condition['value']; $userValue = isset($this->userAttributes[$conditionName]) ? $this->userAttributes[$conditionName]: null; if(!Validator::isFiniteNumber($userValue) || !Validator::isFiniteNumber($conditionValue)) { @@ -167,8 +182,8 @@ protected function greaterThanEvaluator($condition) */ protected function lessThanEvaluator($condition) { - $conditionName = $condition->{'name'}; - $conditionValue = $condition->{'value'}; + $conditionName = $condition['name']; + $conditionValue = $condition['value']; $userValue = isset($this->userAttributes[$conditionName]) ? $this->userAttributes[$conditionName]: null; if(!Validator::isFiniteNumber($userValue) || !Validator::isFiniteNumber($conditionValue)) { @@ -190,8 +205,8 @@ protected function lessThanEvaluator($condition) */ protected function substringEvaluator($condition) { - $conditionName = $condition->{'name'}; - $conditionValue = $condition->{'value'}; + $conditionName = $condition['name']; + $conditionValue = $condition['value']; $userValue = isset($this->userAttributes[$conditionName]) ? $this->userAttributes[$conditionName]: null; if(!is_string($userValue) || !is_string($conditionValue)) { @@ -211,14 +226,16 @@ protected function substringEvaluator($condition) */ public function evaluate($leafCondition) { - if($leafCondition->{'type'} !== self::CUSTOM_ATTRIBUTE_CONDITION_TYPE) { + $leafCondition = $this->setNullForMissingKeys($leafCondition); + + if($leafCondition['type'] !== self::CUSTOM_ATTRIBUTE_CONDITION_TYPE) { return null; } - if(!isset($leafCondition->{'match'})) { + if(($leafCondition['match']) == null) { $conditionMatch = self::EXACT_MATCH_TYPE; } else { - $conditionMatch = $leafCondition->{'match'}; + $conditionMatch = $leafCondition['match']; } if(!in_array($conditionMatch, $this->getMatchTypes())) { diff --git a/src/Optimizely/Utils/Validator.php b/src/Optimizely/Utils/Validator.php index 57becd54..311fc440 100644 --- a/src/Optimizely/Utils/Validator.php +++ b/src/Optimizely/Utils/Validator.php @@ -111,9 +111,8 @@ public static function isUserInExperiment($config, $experiment, $userAttributes) return true; } - // Return false if there is audience, but no user attributes. - if (empty($userAttributes)) { - return false; + if ($userAttributes == null) { + $userAttributes = []; } $customAttrCondEval = new CustomAttributeConditionEvaluator($userAttributes); @@ -223,4 +222,18 @@ public static function areValuesSameType($firstVal, $secondVal) return $firstValType == $secondValType; } + + /** + * Returns true only if given input is an array with all of it's keys of type string. + * @param mixed $arr + * @return bool True if array contains all string keys. Otherwise, false. + */ + public static function doesArrayContainOnlyStringKeys($arr) + { + if(empty($arr)) { + return false; + } + + return count(array_filter(array_keys($arr), 'is_string')) == count(array_keys($arr)); + } } diff --git a/tests/DecisionServiceTests/DecisionServiceTest.php b/tests/DecisionServiceTests/DecisionServiceTest.php index 7af23d98..eb0e349c 100644 --- a/tests/DecisionServiceTests/DecisionServiceTest.php +++ b/tests/DecisionServiceTests/DecisionServiceTest.php @@ -1138,7 +1138,7 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar $experiment2 = $rollout->getExperiments()[2]; // Set an AudienceId for everyone else/last rule so that user does not qualify for audience - $experiment2->setAudienceIds(["11154"]); + $experiment2->setAudienceIds(["11155"]); $expected_variation = $experiment2->getVariations()[0]; // Provide null attributes so that user does not qualify for audience diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index 94cdc050..0f283397 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -49,6 +49,7 @@ class OptimizelyTest extends \PHPUnit_Framework_TestCase public function setUp() { $this->datafile = DATAFILE; + $this->typedAudiencesDataFile = DATAFILE_WITH_TYPED_AUDIENCES; $this->testBucketingIdControl = 'testBucketingIdControl!'; // generates bucketing number 3741 $this->testBucketingIdVariation = '123456789'; // generates bucketing number 4567 $this->variationKeyControl = 'control'; @@ -61,8 +62,12 @@ public function setUp() ->setMethods(array('log')) ->getMock(); $this->optimizelyObject = new Optimizely($this->datafile, null, $this->loggerMock); + $this->optimizelyTypedAudienceObject = new Optimizely( + $this->typedAudiencesDataFile, null, $this->loggerMock + ); $this->projectConfig = new ProjectConfig($this->datafile, $this->loggerMock, new NoOpErrorHandler()); + $this->projectConfigForTypedAudience = new ProjectConfig($this->typedAudiencesDataFile, $this->loggerMock, new NoOpErrorHandler()); // Mock EventBuilder $this->eventBuilderMock = $this->getMockBuilder(EventBuilder::class) @@ -602,6 +607,58 @@ public function testActivateWithAttributesOfDifferentTypes() $this->assertEquals('control', $optimizelyMock->activate('test_experiment', 'test_user', $userAttributes)); } + public function testActivateWithAttributesTypedAudienceMatch() + { + $optimizelyMock = $this->getMockBuilder(Optimizely::class) + ->setConstructorArgs(array($this->typedAudiencesDataFile , null, null)) + ->setMethods(array('sendImpressionEvent')) + ->getMock(); + + $userAttributes = [ + 'house' => 'Gryffindor' + ]; + + // Verify that sendImpressionEvent is called with expected attributes + $optimizelyMock->expects($this->at(0)) + ->method('sendImpressionEvent') + ->with('typed_audience_experiment', 'A', 'test_user', $userAttributes); + + // Should be included via exact match string audience with id '3468206642' + $this->assertEquals('A', $optimizelyMock->activate('typed_audience_experiment', 'test_user', $userAttributes)); + + $userAttributes = [ + 'lasers' => 45.5 + ]; + + // Verify that sendImpressionEvent is called with expected attributes + $optimizelyMock->expects($this->at(0)) + ->method('sendImpressionEvent') + ->with('typed_audience_experiment', 'A', 'test_user', $userAttributes); + + //Should be included via exact match number audience with id '3468206646' + $this->assertEquals('A', $optimizelyMock->activate('typed_audience_experiment', 'test_user', $userAttributes)); + + } + + public function testActivateWithAttributesTypedAudienceMismatch() + { + $userAttributes = [ + 'house' => 'Hufflepuff' + ]; + + $optimizelyMock = $this->getMockBuilder(Optimizely::class) + ->setConstructorArgs(array($this->typedAudiencesDataFile , null, $this->loggerMock)) + ->setMethods(array('sendImpressionEvent')) + ->getMock(); + + // Verify that sendImpressionEvent is not called + $optimizelyMock->expects($this->never()) + ->method('sendImpressionEvent'); + + // Call activate + $this->assertNull($optimizelyMock->activate('typed_audience_experiment', 'test_user', $userAttributes)); + } + public function testActivateExperimentNotRunning() { $optimizelyMock = $this->getMockBuilder(Optimizely::class) @@ -1863,6 +1920,56 @@ public function testTrackWithAttributesWithEventValue() $optlyObject->track('purchase', 'test_user', $userAttributes, array('revenue' => 42)); } + public function testTrackWithAttributesTypedAudienceMatch() + { + $userAttributes = [ + 'house' => 'Welcome to Slytherin!' + ]; + + $this->eventBuilderMock->expects($this->once()) + ->method('createConversionEvent') + ->with( + $this->projectConfigForTypedAudience, + 'item_bought', + [ + '11564051718' => '11617170975', + '1323241597' => '1423767503' + ], + 'test_user', + $userAttributes, + array('revenue' => 42) + ) + ->willReturn(new LogEvent('logx.optimizely.com/track', ['param1' => 'val1'], 'POST', [])); + + $optlyObject = new Optimizely($this->typedAudiencesDataFile, new ValidEventDispatcher(), $this->loggerMock); + + $eventBuilder = new \ReflectionProperty(Optimizely::class, '_eventBuilder'); + $eventBuilder->setAccessible(true); + $eventBuilder->setValue($optlyObject, $this->eventBuilderMock); + + // Should be included via substring match string audience with id '3988293898' + $optlyObject->track('item_bought', 'test_user', $userAttributes, array('revenue' => 42)); + } + + public function testTrackWithAttributesTypedAudienceMismatch() + { + $userAttributes = [ + 'house' => 'Hufflepuff!' + ]; + + $this->eventBuilderMock->expects($this->never()) + ->method('createConversionEvent'); + + $optlyObject = new Optimizely($this->typedAudiencesDataFile, new ValidEventDispatcher(), $this->loggerMock); + + $eventBuilder = new \ReflectionProperty(Optimizely::class, '_eventBuilder'); + $eventBuilder->setAccessible(true); + $eventBuilder->setValue($optlyObject, $this->eventBuilderMock); + + // Call track + $optlyObject->track('item_bought', 'test_user', $userAttributes, array('revenue' => 42)); + } + public function testTrackWithEmptyUserID() { $userAttributes = [ @@ -2654,6 +2761,36 @@ public function testIsFeatureEnabledGivenFeatureRolloutAndFeatureEnabledIsFalse( $this->assertFalse($optimizelyMock->isFeatureEnabled('boolean_single_variable_feature', 'user_id', [])); } + public function testIsFeatureEnabledGivenFeatureRolloutTypedAudienceMatch() + { + $userAttributes = [ + 'favorite_ice_cream' => 'chocolate' + ]; + + // Should be included via exists match audience with id '3988293899' + $this->assertTrue( + $this->optimizelyTypedAudienceObject->isFeatureEnabled('feat', 'test_user', $userAttributes) + ); + + $userAttributes = [ + 'lasers' => -3 + ]; + + // Should be included via less-than match audience with id '3468206644' + $this->assertTrue( + $this->optimizelyTypedAudienceObject->isFeatureEnabled('feat', 'test_user', $userAttributes) + ); + } + + public function testIsFeatureEnabledGivenFeatureRolloutTypedAudienceMismatch() + { + $userAttributes = []; + + $this->assertFalse( + $this->optimizelyTypedAudienceObject->isFeatureEnabled('feat', 'test_user', $userAttributes) + ); + } + public function testIsFeatureEnabledWithEmptyUserID() { $optimizelyMock = $this->getMockBuilder(Optimizely::class) @@ -3259,6 +3396,33 @@ public function testGetFeatureVariableMethodsReturnNullWhenGetVariableValueForTy ); } + public function testGetFeatureVariableReturnsVariableValueForTypedAudienceMatch() + { + $userAttributes = [ + 'lasers' => 71 + ]; + + // Should be included in the feature test via greater-than match audience with id '3468206647' + $this->assertEquals('xyz', $this->optimizelyTypedAudienceObject->getFeatureVariableString('feat_with_var', 'x', 'user1', $userAttributes)); + + $userAttributes = [ + 'should_do_it' => true + ]; + + // Should be included in the feature test via exact match boolean audience with id '3468206643' + $this->assertEquals('xyz', $this->optimizelyTypedAudienceObject->getFeatureVariableString('feat_with_var', 'x', 'user1', $userAttributes)); + } + + public function testGetFeatureVariableReturnsDefaultValueForTypedAudienceMismatch() + { + $userAttributes = [ + 'lasers' => 50 + ]; + + // Should be included in the feature test via greater-than match audience with id '3468206647' + $this->assertEquals('x', $this->optimizelyTypedAudienceObject->getFeatureVariableString('feat_with_var', 'x', 'user1', $userAttributes)); + } + public function testSendImpressionEventWithNoAttributes() { $optlyObject = new OptimizelyTester($this->datafile, new ValidEventDispatcher(), $this->loggerMock); diff --git a/tests/ProjectConfigTest.php b/tests/ProjectConfigTest.php index e84d6538..b19c7526 100644 --- a/tests/ProjectConfigTest.php +++ b/tests/ProjectConfigTest.php @@ -508,6 +508,35 @@ public function testGetAudienceValidId() $this->assertEquals('iPhone users in San Francisco', $audience->getName()); } + public function testGetAudiencePrefersTypedAudiencesOverAudiences() + { + $projectConfig = new ProjectConfig( + DATAFILE_WITH_TYPED_AUDIENCES, $this->loggerMock, $this->errorHandlerMock + ); + + // test that typedAudience is returned when an audience exists with the same ID. + $audience = $projectConfig->getAudience('3988293898'); + + $this->assertEquals('3988293898', $audience->getId()); + $this->assertEquals('substringString', $audience->getName()); + + $expectedConditions = json_decode('["and", ["or", ["or", {"name": "house", "type": "custom_attribute", + "match": "substring", "value": "Slytherin"}]]]', true); + $this->assertEquals($expectedConditions, $audience->getConditions()); + $this->assertEquals($expectedConditions, $audience->getConditionsList()); + + // test that normal audience is returned if no typedAudience exists with the same ID. + $audience = $projectConfig->getAudience('3468206642'); + + $this->assertEquals('3468206642', $audience->getId()); + $this->assertEquals('exactString', $audience->getName()); + + $expectedConditions = '["and", ["or", ["or", {"name": "house", "type": "custom_attribute", "value": "Gryffindor"}]]]'; + $this->assertEquals($expectedConditions, $audience->getConditions()); + $expectedConditionsList = json_decode($expectedConditions, true); + $this->assertEquals($expectedConditionsList, $audience->getConditionsList()); + } + public function testGetAudienceInvalidKey() { $this->loggerMock->expects($this->once()) diff --git a/tests/TestData.php b/tests/TestData.php index 79f4b627..ac739474 100644 --- a/tests/TestData.php +++ b/tests/TestData.php @@ -843,6 +843,257 @@ }' ); +define( + 'DATAFILE_WITH_TYPED_AUDIENCES', + '{ + "version": "4", + "rollouts": [ + { + "experiments": [ + { + "status": "Running", + "key": "11488548027", + "layerId": "11551226731", + "trafficAllocation": [ + { + "entityId": "11557362669", + "endOfRange": 10000 + } + ], + "audienceIds": ["3468206642", "3988293898", "3988293899", "3468206646", + "3468206647", "3468206644", "3468206643"], + "variations": [ + { + "variables": [], + "id": "11557362669", + "key": "11557362669", + "featureEnabled":true + } + ], + "forcedVariations": {}, + "id": "11488548027" + } + ], + "id": "11551226731" + }, + { + "experiments": [ + { + "status": "Paused", + "key": "11630490911", + "layerId": "11638870867", + "trafficAllocation": [ + { + "entityId": "11475708558", + "endOfRange": 0 + } + ], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "11475708558", + "key": "11475708558", + "featureEnabled":false + } + ], + "forcedVariations": {}, + "id": "11630490911" + } + ], + "id": "11638870867" + } + ], + "anonymizeIP": false, + "projectId": "11624721371", + "variables": [], + "featureFlags": [ + { + "experimentIds": [], + "rolloutId": "11551226731", + "variables": [], + "id": "11477755619", + "key": "feat" + }, + { + "experimentIds": [ + "11564051718" + ], + "rolloutId": "11638870867", + "variables": [ + { + "defaultValue": "x", + "type": "string", + "id": "11535264366", + "key": "x" + } + ], + "id": "11567102051", + "key": "feat_with_var" + } + ], + "experiments": [ + { + "status": "Running", + "key": "feat_with_var_test", + "layerId": "11504144555", + "trafficAllocation": [ + { + "entityId": "11617170975", + "endOfRange": 10000 + } + ], + "audienceIds": ["3468206642", "3988293898", "3988293899", "3468206646", + "3468206647", "3468206644", "3468206643"], + "variations": [ + { + "variables": [ + { + "id": "11535264366", + "value": "xyz" + } + ], + "id": "11617170975", + "key": "variation_2", + "featureEnabled": true + } + ], + "forcedVariations": {}, + "id": "11564051718" + }, + { + "id": "1323241597", + "key": "typed_audience_experiment", + "layerId": "1630555627", + "status": "Running", + "variations": [ + { + "id": "1423767503", + "key": "A", + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "1423767503", + "endOfRange": 10000 + } + ], + "audienceIds": ["3468206642", "3988293898", "3988293899", "3468206646", + "3468206647", "3468206644", "3468206643"], + "forcedVariations": {} + } + ], + "audiences": [ + { + "id": "3468206642", + "name": "exactString", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"house\", \"type\": \"custom_attribute\", \"value\": \"Gryffindor\"}]]]" + }, + { + "id": "3988293898", + "name": "$$dummySubstringString", + "conditions": "{ \"type\": \"custom_attribute\", \"name\": \"$opt_dummy_attribute\", \"value\": \"impossible_value\" }" + }, + { + "id": "3988293899", + "name": "$$dummyExists", + "conditions": "{ \"type\": \"custom_attribute\", \"name\": \"$opt_dummy_attribute\", \"value\": \"impossible_value\" }" + }, + { + "id": "3468206646", + "name": "$$dummyExactNumber", + "conditions": "{ \"type\": \"custom_attribute\", \"name\": \"$opt_dummy_attribute\", \"value\": \"impossible_value\" }" + }, + { + "id": "3468206647", + "name": "$$dummyGtNumber", + "conditions": "{ \"type\": \"custom_attribute\", \"name\": \"$opt_dummy_attribute\", \"value\": \"impossible_value\" }" + }, + { + "id": "3468206644", + "name": "$$dummyLtNumber", + "conditions": "{ \"type\": \"custom_attribute\", \"name\": \"$opt_dummy_attribute\", \"value\": \"impossible_value\" }" + }, + { + "id": "3468206643", + "name": "$$dummyExactBoolean", + "conditions": "{ \"type\": \"custom_attribute\", \"name\": \"$opt_dummy_attribute\", \"value\": \"impossible_value\" }" + } + ], + "typedAudiences": [ + { + "id": "3988293898", + "name": "substringString", + "conditions": ["and", ["or", ["or", {"name": "house", "type": "custom_attribute", + "match": "substring", "value": "Slytherin"}]]] + }, + { + "id": "3988293899", + "name": "exists", + "conditions": ["and", ["or", ["or", {"name": "favorite_ice_cream", "type": "custom_attribute", + "match": "exists"}]]] + }, + { + "id": "3468206646", + "name": "exactNumber", + "conditions": ["and", ["or", ["or", {"name": "lasers", "type": "custom_attribute", + "match": "exact", "value": 45.5}]]] + }, + { + "id": "3468206647", + "name": "gtNumber", + "conditions": ["and", ["or", ["or", {"name": "lasers", "type": "custom_attribute", + "match": "gt", "value": 70}]]] + }, + { + "id": "3468206644", + "name": "ltNumber", + "conditions": ["and", ["or", ["or", {"name": "lasers", "type": "custom_attribute", + "match": "lt", "value": 1.0}]]] + }, + { + "id": "3468206643", + "name": "exactBoolean", + "conditions": ["and", ["or", ["or", {"name": "should_do_it", "type": "custom_attribute", + "match": "exact", "value": true}]]] + } + ], + "groups": [], + "attributes": [ + { + "key": "house", + "id": "594015" + }, + { + "key": "lasers", + "id": "594016" + }, + { + "key": "should_do_it", + "id": "594017" + }, + { + "key": "favorite_ice_cream", + "id": "594018" + } + ], + "botFiltering": false, + "accountId": "4879520872", + "events": [ + { + "key": "item_bought", + "id": "594089", + "experimentIds": [ + "11564051718", + "1323241597" + ] + } + ], + "revision": "3" + }' +); + /** * Class TestBucketer * Extending Bucketer for the sake of tests. diff --git a/tests/UtilsTests/ConditionDecoderTest.php b/tests/UtilsTests/ConditionDecoderTest.php deleted file mode 100644 index a94298e3..00000000 --- a/tests/UtilsTests/ConditionDecoderTest.php +++ /dev/null @@ -1,58 +0,0 @@ -conditionDecoder = new ConditionDecoder(); - $conditions = "[\"and\", [\"or\", [\"or\", {\"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"iPhone\"}]], [\"or\", [\"or\", {\"name\": \"location\", \"type\": \"custom_attribute\", \"value\": \"San Francisco\"}]]]"; - $this->conditionDecoder->deserializeAudienceConditions($conditions); - } - - public function testGetConditionsList() - { - $this->assertEquals( - [ - 'and', [ - 'or', [ - 'or', (object)[ - 'name' => 'device_type', - 'type' => 'custom_attribute', - 'value' => 'iPhone' - ] - ] - ], [ - 'or', [ - 'or', (object)[ - 'name' => 'location', - 'type' => 'custom_attribute', - 'value' => 'San Francisco' - ] - ] - ] - ], - $this->conditionDecoder->getConditionsList() - ); - } -} diff --git a/tests/UtilsTests/ConditionTreeEvaluatorTest.php b/tests/UtilsTests/ConditionTreeEvaluatorTest.php index d63f61ad..e9c5d3de 100644 --- a/tests/UtilsTests/ConditionTreeEvaluatorTest.php +++ b/tests/UtilsTests/ConditionTreeEvaluatorTest.php @@ -23,19 +23,19 @@ class ConditionTreeEvaluatorTest extends \PHPUnit_Framework_TestCase { public function setUp() { - $this->conditionA = (object)[ + $this->conditionA = [ 'name' => 'browser_type', 'value' => 'safari', 'type' => 'custom_attribute' ]; - $this->conditionB = (object)[ + $this->conditionB = [ 'name' => 'device_model', 'value' => 'iphone6', 'type' => 'custom_attribute' ]; - $this->conditionC = (object)[ + $this->conditionC = [ 'name' => 'location', 'match' => 'exact', 'type' => 'custom_attribute', diff --git a/tests/UtilsTests/CustomAttributeConditionEvaluatorTest.php b/tests/UtilsTests/CustomAttributeConditionEvaluatorTest.php index 4f308085..2577d8f0 100644 --- a/tests/UtilsTests/CustomAttributeConditionEvaluatorTest.php +++ b/tests/UtilsTests/CustomAttributeConditionEvaluatorTest.php @@ -23,85 +23,85 @@ class CustomAttributeConditionEvaluatorTest extends \PHPUnit_Framework_TestCase { public function setUp() { - $this->browserConditionSafari = (object)[ + $this->browserConditionSafari = [ 'type' => 'custom_attribute', 'name' => 'browser_type', 'value' => 'safari', 'match' => 'exact' ]; - $this->booleanCondition = (object)[ + $this->booleanCondition = [ 'type' => 'custom_attribute', 'name' => 'is_firefox', 'value' => true, 'match' => 'exact' ]; - $this->integerCondition = (object)[ + $this->integerCondition = [ 'type' => 'custom_attribute', 'name' => 'num_users', 'value' => 10, 'match' => 'exact' ]; - $this->doubleCondition = (object)[ + $this->doubleCondition = [ 'type' => 'custom_attribute', 'name' => 'pi_value', 'value' => 3.14, 'match' => 'exact' ]; - $this->existsCondition = (object)[ + $this->existsCondition = [ 'type' => 'custom_attribute', 'name' => 'input_value', 'value' => null, 'match' => 'exists' ]; - $this->exactStringCondition = (object)[ + $this->exactStringCondition = [ 'name' => 'favorite_constellation', 'value' =>'Lacerta', 'type' => 'custom_attribute', 'match' =>'exact' ]; - $this->exactIntCondition = (object)[ + $this->exactIntCondition = [ 'name' => 'lasers_count', 'value' => 9000, 'type' => 'custom_attribute', 'match' => 'exact' ]; - $this->exactFloatCondition = (object)[ + $this->exactFloatCondition = [ 'name' => 'lasers_count', 'value' => 9000.0, 'type' => 'custom_attribute', 'match' => 'exact' ]; - $this->exactBoolCondition = (object)[ + $this->exactBoolCondition = [ 'name' => 'did_register_user', 'value' => false, 'type' => 'custom_attribute', 'match' => 'exact' ]; - $this->substringCondition = (object)[ + $this->substringCondition = [ 'name' => 'headline_text', 'value' => 'buy now', 'type' => 'custom_attribute', 'match' => 'substring' ]; - $this->gtIntCondition = (object)[ + $this->gtIntCondition = [ 'name' => 'meters_travelled', 'value' => 48, 'type' => 'custom_attribute', 'match' => 'gt' ]; - $this->gtFloatCondition = (object)[ + $this->gtFloatCondition = [ 'name' => 'meters_travelled', 'value' => 48.2, 'type' => 'custom_attribute', 'match' => 'gt' ]; - $this->ltIntCondition = (object)[ + $this->ltIntCondition = [ 'name' => 'meters_travelled', 'value' => 48, 'type' => 'custom_attribute', 'match' => 'lt' ]; - $this->ltFloatCondition = (object)[ + $this->ltFloatCondition = [ 'name' => 'meters_travelled', 'value' => 48.2, 'type' => 'custom_attribute', @@ -181,7 +181,7 @@ public function testEvaluateReturnsNullForInvalidMatchProperty() $this->assertNull( $customAttrConditionEvaluator->evaluate( - (object)[ + [ 'type' => 'custom_attribute', 'name' => 'weird_condition', 'value' => 'hi', @@ -200,7 +200,7 @@ public function testEvaluateAssumesExactWhenConditionMatchPropertyIsNull() $this->assertTrue( $customAttrConditionEvaluator->evaluate( - (object)[ + [ 'type' => 'custom_attribute', 'name' => 'favorite_constellation', 'value' => 'Lacerta', @@ -218,7 +218,7 @@ public function testEvaluateReturnsNullWhenConditionHasInvalidTypeProperty() $this->assertNull( $customAttrConditionEvaluator->evaluate( - (object)[ + [ 'type' => 'weird_type', 'name' => 'weird_condition', 'value' => 'hi', diff --git a/tests/UtilsTests/ValidatorTest.php b/tests/UtilsTests/ValidatorTest.php index 715b54a9..27c5d4f9 100644 --- a/tests/UtilsTests/ValidatorTest.php +++ b/tests/UtilsTests/ValidatorTest.php @@ -18,6 +18,7 @@ namespace Optimizely\Tests; use Monolog\Logger; +use Optimizely\Entity\Audience; use Optimizely\ErrorHandler\NoOpErrorHandler; use Optimizely\Logger\NoOpLogger; use Optimizely\ProjectConfig; @@ -170,25 +171,44 @@ public function testIsUserInExperimentNoAudienceUsedInExperiment() ); } + // test that Audience evaluation proceeds if provided attributes are empty or null. public function testIsUserInExperimentAudienceUsedInExperimentNoAttributesProvided() { - $config = new ProjectConfig(DATAFILE, new NoOpLogger(), new NoOpErrorHandler()); + $configMock = $this->getMockBuilder(ProjectConfig::class) + ->setConstructorArgs(array(DATAFILE, $this->loggerMock, new NoOpErrorHandler())) + ->setMethods(array('getAudience')) + ->getMock(); - // Test with empty attributes - $this->assertFalse( + $existsCondition = [ + 'type' => 'custom_attribute', + 'name' => 'input_value', + 'match' => 'exists', + 'value' => null + ]; + + $experiment = $configMock->getExperimentFromKey('test_experiment'); + $experiment->setAudienceIds(['007']); + $audience = new Audience(); + $audience->setConditionsList(['not', $existsCondition]); + + $configMock + ->method('getAudience') + ->with('007') + ->will($this->returnValue($audience)); + + $this->assertTrue( Validator::isUserInExperiment( - $config, - $config->getExperimentFromKey('test_experiment'), - [] + $configMock, + $experiment, + null ) ); - // Test with null attributes - $this->assertFalse( + $this->assertTrue( Validator::isUserInExperiment( - $config, - $config->getExperimentFromKey('test_experiment'), - null + $configMock, + $experiment, + [] ) ); } From 8ac59ca60c172866dcd5dcd071018a43452b08da Mon Sep 17 00:00:00 2001 From: Owais Date: Wed, 19 Dec 2018 16:57:08 +0500 Subject: [PATCH 2/2] :pen: Unit Tests for pure assoc array --- .../CustomAttributeConditionEvaluator.php | 2 +- src/Optimizely/Utils/Validator.php | 2 +- tests/UtilsTests/ValidatorTest.php | 22 +++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php b/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php index c557200d..1ead3749 100644 --- a/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php +++ b/src/Optimizely/Utils/CustomAttributeConditionEvaluator.php @@ -232,7 +232,7 @@ public function evaluate($leafCondition) return null; } - if(($leafCondition['match']) == null) { + if(($leafCondition['match']) === null) { $conditionMatch = self::EXACT_MATCH_TYPE; } else { $conditionMatch = $leafCondition['match']; diff --git a/src/Optimizely/Utils/Validator.php b/src/Optimizely/Utils/Validator.php index 311fc440..00cde806 100644 --- a/src/Optimizely/Utils/Validator.php +++ b/src/Optimizely/Utils/Validator.php @@ -230,7 +230,7 @@ public static function areValuesSameType($firstVal, $secondVal) */ public static function doesArrayContainOnlyStringKeys($arr) { - if(empty($arr)) { + if(!is_array($arr) || empty($arr)) { return false; } diff --git a/tests/UtilsTests/ValidatorTest.php b/tests/UtilsTests/ValidatorTest.php index 27c5d4f9..f14dfcd9 100644 --- a/tests/UtilsTests/ValidatorTest.php +++ b/tests/UtilsTests/ValidatorTest.php @@ -264,4 +264,26 @@ public function testIsFeatureFlagValid() $this->assertFalse(Validator::isFeatureFlagValid($config, $featureFlag)); } + + public function testDoesArrayContainOnlyStringKeys() + { + // Valid values + $this->assertTrue(Validator::doesArrayContainOnlyStringKeys( + ["name"=> "favorite_ice_cream", "type"=> "custom_attribute", "match"=> "exists"]) + ); + + // Invalid values + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys([])); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys((object)[])); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys( + ["and", ["or", ["or", ["name"=> "favorite_ice_cream", "type"=> "custom_attribute","match"=> "exists"]]]] + )); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys(['hello' => 'world', 0 => 'bye'])); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys(['hello' => 'world', '0' => 'bye'])); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys(['hello' => 'world', 'and'])); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys('helloworld')); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys(12)); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys('12.5')); + $this->assertFalse(Validator::doesArrayContainOnlyStringKeys(true)); + } }