diff --git a/CHANGELOG.md b/CHANGELOG.md index 9806861d..c3bff28b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.1.0 +- Updated to send datafile revision information in log events. +- Gracefully handle empty entity IDs. +- Added event tags to track API to allow users to pass in event metadata. +- Deprecated the `eventValue` parameter from the track method. Should use `eventTags` to pass in event value instead. +- Relaxed restriction on monolog package. + ## 1.0.1 - Updated to support more versions of json-schema package. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9525e100..ca26a035 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,7 @@ -#Contributing to the Optimizely PHP SDK +# Contributing to the Optimizely PHP SDK We welcome contributions and feedback! All contributors must sign our [Contributor License Agreement (CLA)](https://docs.google.com/a/optimizely.com/forms/d/e/1FAIpQLSf9cbouWptIpMgukAKZZOIAhafvjFCV8hS00XJLWQnWDFtwtA/viewform) to be eligible to contribute. Please read the [README](README.md) to set up your development environment, then read the guidelines below for information on submitting your code. -##Development process +## Development process 1. Create a branch off of `devel`: `git checkout -b YOUR_NAME/branch_name`. 2. Commit your changes. Make sure to add tests! @@ -10,12 +10,12 @@ We welcome contributions and feedback! All contributors must sign our [Contribut 5. Open a pull request from `YOUR_NAME/branch_name` to `devel`. 6. A repository maintainer will review your pull request and, if all goes well, merge it! -##Pull request acceptance criteria +## Pull request acceptance criteria * **All code must have test coverage.** We use PHPUnit. Changes in functionality should have accompanying unit tests. Bug fixes should have accompanying regression tests. * Tests are located in `tests` with one file per class. -##License +## License All contributions are under the CLA mentioned above. For this project, Optimizely uses the Apache 2.0 license, and so asks that by contributing your code, you agree to license your contribution under the terms of the [Apache License v2.0](http://www.apache.org/licenses/LICENSE-2.0). Your contributions should also include the following header: @@ -39,5 +39,5 @@ All contributions are under the CLA mentioned above. For this project, Optimizel The YEAR above should be the year of the contribution. If work on the file has been done over multiple years, list each year in the section above. Example: Optimizely writes the file and releases it in 2014. No changes are made in 2015. Change made in 2016. YEAR should be “2014, 2016”. -##Contact +## Contact If you have questions, please contact developers@optimizely.com. diff --git a/README.md b/README.md index ad07d982..ff3e206d 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -#Optimizely PHP SDK +# Optimizely PHP SDK [![Build Status](https://travis-ci.org/optimizely/php-sdk.svg?branch=master)](https://travis-ci.org/optimizely/php-sdk) [![Coverage Status](https://coveralls.io/repos/github/optimizely/php-sdk/badge.svg?branch=master)](https://coveralls.io/github/optimizely/php-sdk?branch=master) [![Total Downloads](https://poser.pugx.org/optimizely/optimizely-sdk/downloads)](https://packagist.org/packages/optimizely/optimizely-sdk) @@ -6,9 +6,9 @@ This repository houses the PHP SDK for Optimizely Full Stack. -##Getting Started +## Getting Started -###Installing the SDK +### Installing the SDK The Optimizely PHP SDK can be installed through [Composer](https://getcomposer.org/). Please use the following command: @@ -16,20 +16,20 @@ The Optimizely PHP SDK can be installed through [Composer](https://getcomposer.o php composer.phar require optimizely/optimizely-sdk ``` -###Using the SDK +### Using the SDK See the Optimizely Full Stack [developer documentation](https://developers.optimizely.com/x/solutions/sdks/reference/?language=php) to learn how to set up your first Full Stack project and use the SDK. -##Development +## Development -###Unit tests +### Unit tests -#####Running all tests +##### Running all tests You can run all unit tests with: ``` ./vendor/bin/phpunit ``` -###Contributing +### Contributing Please see [CONTRIBUTING](CONTRIBUTING.md). diff --git a/composer.json b/composer.json index 77500a73..863daa63 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "justinrainbow/json-schema": "^1.6 || ^2.0 || ^4.0", "lastguest/murmurhash": "1.3.0", "guzzlehttp/guzzle": "~5.3|~6.2", - "monolog/monolog": "1.21.0" + "monolog/monolog": "~1.21" }, "require-dev": { "phpunit/phpunit": "~4.8|~5.0", diff --git a/src/Optimizely/Bucketer.php b/src/Optimizely/Bucketer.php index 3e94d116..03119c39 100644 --- a/src/Optimizely/Bucketer.php +++ b/src/Optimizely/Bucketer.php @@ -1,6 +1,6 @@ findBucket($userId, $group->getId(), $group->getTrafficAllocation()); - if (is_null($userExperimentId)) { + if (empty($userExperimentId)) { $this->_logger->log(Logger::INFO, sprintf('User "%s" is in no experiment.', $userId)); return new Variation(); } @@ -172,7 +172,7 @@ public function bucket(ProjectConfig $config, Experiment $experiment, $userId) // Bucket user if not in whitelist and in group (if any). $variationId = $this->findBucket($userId, $experiment->getId(), $experiment->getTrafficAllocation()); - if (!is_null($variationId)) { + if (!empty($variationId)) { $variation = $config->getVariationFromId($experiment->getKey(), $variationId); $this->_logger->log(Logger::INFO, sprintf('User "%s" is in variation %s of experiment %s.', diff --git a/src/Optimizely/Event/Builder/EventBuilder.php b/src/Optimizely/Event/Builder/EventBuilder.php index 3e626c04..6f2f7a98 100644 --- a/src/Optimizely/Event/Builder/EventBuilder.php +++ b/src/Optimizely/Event/Builder/EventBuilder.php @@ -1,6 +1,6 @@ _eventParams[PROJECT_ID] = $config->getProjectId(); $this->_eventParams[ACCOUNT_ID] = $config->getAccountId(); + $this->_eventParams[REVISION] = $config->getRevision(); $this->_eventParams[VISITOR_ID] = $userId; $this->_eventParams[CLIENT_ENGINE] = self::SDK_TYPE; $this->_eventParams[CLIENT_VERSION] = self::SDK_VERSION; @@ -153,20 +155,34 @@ private function setImpressionParams(Experiment $experiment, $variationId) * @param $eventKey string Key representing the event. * @param $experiments array Experiments for which conversion event needs to be recorded. * @param $userId string ID of user. - * @param $eventValue integer Value associated with the event. + * @param $eventTags array Hash representing metadata associated with the event. */ - private function setConversionParams($config, $eventKey, $experiments, $userId, $eventValue) + private function setConversionParams($config, $eventKey, $experiments, $userId, $eventTags) { $this->_eventParams[EVENT_FEATURES] = []; $this->_eventParams[EVENT_METRICS] = []; - if (!is_null($eventValue)) { - $this->_eventParams[EVENT_METRICS] = [ - [ - 'name' => 'revenue', - 'value' => $eventValue - ] - ]; + if (!is_null($eventTags)) { + forEach ($eventTags as $eventTagId => $eventTagValue) { + if (is_null($eventTagValue)) { + continue; + } + $eventFeature = array( + 'name' => $eventTagId, + 'type' => 'custom', + 'value' => $eventTagValue, + 'shouldIndex' => false, + ); + array_push($this->_eventParams[EVENT_FEATURES], $eventFeature); + } + $eventValue = EventTagUtils::getRevenueValue($eventTags); + if ($eventValue) { + $eventMetric = array( + 'name' => EventTagUtils::REVENUE_EVENT_METRIC_NAME, + 'value' => $eventValue, + ); + array_push($this->_eventParams[EVENT_METRICS], $eventMetric); + } } $eventEntity = $config->getEvent($eventKey); @@ -180,6 +196,7 @@ private function setConversionParams($config, $eventKey, $experiments, $userId, array_push($this->_eventParams[LAYER_STATES], [ LAYER_ID => $experiment->getLayerId(), ACTION_TRIGGERED => true, + REVISION => $config->getRevision(), DECISION => [ EXPERIMENT_ID => $experiment->getId(), VARIATION_ID => $variation->getId(), @@ -218,15 +235,15 @@ public function createImpressionEvent($config, Experiment $experiment, $variatio * @param $experiments array Experiments for which conversion event needs to be recorded. * @param $userId string ID of user. * @param $attributes array Attributes of the user. - * @param $eventValue integer Value associated with the event. + * @param $eventTags array Hash representing metadata associated with the event. * * @return LogEvent Event object to be sent to dispatcher. */ - public function createConversionEvent($config, $eventKey, $experiments, $userId, $attributes, $eventValue) + public function createConversionEvent($config, $eventKey, $experiments, $userId, $attributes, $eventTags) { $this->resetParams(); $this->setCommonParams($config, $userId, $attributes); - $this->setConversionParams($config, $eventKey, $experiments, $userId, $eventValue); + $this->setConversionParams($config, $eventKey, $experiments, $userId, $eventTags); return new LogEvent(self::$CONVERSION_ENDPOINT, $this->getParams(), self::$HTTP_VERB, self::$HTTP_HEADERS); } diff --git a/src/Optimizely/Event/Builder/Params.php b/src/Optimizely/Event/Builder/Params.php index d2baa635..4779adcd 100644 --- a/src/Optimizely/Event/Builder/Params.php +++ b/src/Optimizely/Event/Builder/Params.php @@ -17,6 +17,7 @@ define('ACCOUNT_ID', 'accountId'); define('PROJECT_ID', 'projectId'); +define('REVISION', 'revision'); define('LAYER_ID', 'layerId'); define('EXPERIMENT_ID', 'experimentId'); define('VARIATION_ID', 'variationId'); diff --git a/src/Optimizely/Exceptions/InvalidEventTagException.php b/src/Optimizely/Exceptions/InvalidEventTagException.php new file mode 100644 index 00000000..195c7a90 --- /dev/null +++ b/src/Optimizely/Exceptions/InvalidEventTagException.php @@ -0,0 +1,23 @@ +_isValid) { $this->_logger->log(Logger::ERROR, 'Datafile has invalid format. Failing "track".'); @@ -257,6 +259,24 @@ public function track($eventKey, $userId, $attributes = null, $eventValue = null return; } + if (!is_null($eventTags)) { + if (is_numeric($eventTags) && !is_string($eventTags)) { + $eventTags = array( + EventTagUtils::REVENUE_EVENT_METRIC_NAME => $eventTags, + ); + $this->_logger->log( + Logger::WARNING, + 'Event value is deprecated in track call. Use event tags to pass in revenue value instead.' + ); + } + if (!Validator::areEventTagsValid($eventTags)) { + $this->_logger->log(Logger::ERROR, 'Provided event tags are in an invalid format.'); + $this->_errorHandler->handleError( + new InvalidEventTagException('Provided event tags are in an invalid format.') + ); + } + } + $event = $this->_config->getEvent($eventKey); if (is_null($event->getKey())) { @@ -284,7 +304,7 @@ public function track($eventKey, $userId, $attributes = null, $eventValue = null $validExperiments, $userId, $attributes, - $eventValue + $eventTags ); $this->_logger->log(Logger::INFO, sprintf('Tracking event "%s" for user "%s".', $eventKey, $userId)); $this->_logger->log( diff --git a/src/Optimizely/ProjectConfig.php b/src/Optimizely/ProjectConfig.php index beed9a14..8a3f9216 100644 --- a/src/Optimizely/ProjectConfig.php +++ b/src/Optimizely/ProjectConfig.php @@ -184,6 +184,14 @@ public function getProjectId() return $this->_projectId; } + /** + * @return string String representing revision of the datafile. + */ + public function getRevision() + { + return $this->_revision; + } + /** * @param $groupId string ID of the group. * diff --git a/src/Optimizely/Utils/EventTagUtils.php b/src/Optimizely/Utils/EventTagUtils.php new file mode 100644 index 00000000..02b94196 --- /dev/null +++ b/src/Optimizely/Utils/EventTagUtils.php @@ -0,0 +1,52 @@ +loggerMock); - $bucketer->setBucketValues([3000, 7000, 9000]); + $bucketer->setBucketValues([1000, 3000, 7000, 9000]); // Total calls in this test - $this->loggerMock->expects($this->exactly(6)) + $this->loggerMock->expects($this->exactly(8)) ->method('log'); + // No variation (empty entity ID) + $this->loggerMock->expects($this->at(0)) + ->method('log') + ->with(Logger::DEBUG, 'Assigned bucket 1000 to user "testUserId".'); + $this->loggerMock->expects($this->at(1)) + ->method('log') + ->with(Logger::INFO, 'User "testUserId" is in no variation.'); + + $this->assertEquals( + new Variation(), + $bucketer->bucket( + $this->config, + $this->config->getExperimentFromKey('test_experiment'), + $this->testUserId + ) + ); + // control $this->loggerMock->expects($this->at(0)) ->method('log') @@ -145,10 +162,10 @@ public function testBucketValidExperimentInGroup() { $bucketer = new TestBucketer($this->loggerMock); // Total calls in this test - $this->loggerMock->expects($this->exactly(10)) + $this->loggerMock->expects($this->exactly(14)) ->method('log'); - // group_experiment_1 (20% experiment) + // group_experiment_1 (15% experiment) // variation 1 $bucketer->setBucketValues([1000, 4000]); $this->loggerMock->expects($this->at(0)) @@ -216,6 +233,41 @@ public function testBucketValidExperimentInGroup() $this->testUserId ) ); + + // User not in any experiment (previously allocated space) + $bucketer->setBucketValues([400]); + $this->loggerMock->expects($this->at(0)) + ->method('log') + ->with(Logger::DEBUG, 'Assigned bucket 400 to user "testUserId".'); + $this->loggerMock->expects($this->at(1)) + ->method('log') + ->with(Logger::INFO, 'User "testUserId" is in no experiment.'); + + $this->assertEquals( + new Variation(), + $bucketer->bucket( + $this->config, + $this->config->getExperimentFromKey('group_experiment_1'), + $this->testUserId + ) + ); + + // User not in any experiment (never allocated space) + $bucketer->setBucketValues([9000]); + $this->loggerMock->expects($this->at(0)) + ->method('log') + ->with(Logger::DEBUG, 'Assigned bucket 9000 to user "testUserId".'); + $this->loggerMock->expects($this->at(1)) + ->method('log') + ->with(Logger::INFO, 'User "testUserId" is in no experiment.'); + $this->assertEquals( + new Variation(), + $bucketer->bucket( + $this->config, + $this->config->getExperimentFromKey('group_experiment_1'), + $this->testUserId + ) + ); } public function testBucketInvalidExperiment() diff --git a/tests/EventTests/EventBuilderTest.php b/tests/EventTests/EventBuilderTest.php index c856160f..467c48eb 100644 --- a/tests/EventTests/EventBuilderTest.php +++ b/tests/EventTests/EventBuilderTest.php @@ -1,6 +1,6 @@ '7720880029', 'accountId' => '1592310167', + 'revision' => '15', 'layerId' => '7719770039', 'visitorId' => 'testUserId', 'clientEngine' => 'php-sdk', - 'clientVersion' => '1.0.1', + 'clientVersion' => '1.1.0', 'timestamp' => time() * 1000, 'isGlobalHoldback' => false, 'userFeatures' => [], @@ -79,10 +80,11 @@ public function testCreateImpressionEventWithAttributes() [ 'projectId' => '7720880029', 'accountId' => '1592310167', + 'revision' => '15', 'layerId' => '7719770039', 'visitorId' => 'testUserId', 'clientEngine' => 'php-sdk', - 'clientVersion' => '1.0.1', + 'clientVersion' => '1.1.0', 'timestamp' => time() * 1000, 'isGlobalHoldback' => false, 'userFeatures' => [[ @@ -125,8 +127,9 @@ public function testCreateConversionEventNoAttributesNoValue() 'projectId' => '7720880029', 'accountId' => '1592310167', 'visitorId' => 'testUserId', + 'revision' => '15', 'clientEngine' => 'php-sdk', - 'clientVersion' => '1.0.1', + 'clientVersion' => '1.1.0', 'userFeatures' => [], 'isGlobalHoldback' => false, 'timestamp' => time() * 1000, @@ -137,6 +140,7 @@ public function testCreateConversionEventNoAttributesNoValue() 'layerStates' => [[ 'layerId' => '7719770039', 'actionTriggered' => true, + 'revision' => '15', 'decision' => [ 'experimentId' => '7716830082', 'variationId' => '7722370027', @@ -167,8 +171,9 @@ public function testCreateConversionEventWithAttributesNoValue() 'projectId' => '7720880029', 'accountId' => '1592310167', 'visitorId' => 'testUserId', + 'revision' => '15', 'clientEngine' => 'php-sdk', - 'clientVersion' => '1.0.1', + 'clientVersion' => '1.1.0', 'isGlobalHoldback' => false, 'timestamp' => time() * 1000, 'eventFeatures' => [], @@ -178,6 +183,7 @@ public function testCreateConversionEventWithAttributesNoValue() 'layerStates' => [[ 'layerId' => '7719770039', 'actionTriggered' => true, + 'revision' => '15', 'decision' => [ 'experimentId' => '7716830082', 'variationId' => '7722370027', @@ -220,12 +226,20 @@ public function testCreateConversionEventNoAttributesWithValue() 'projectId' => '7720880029', 'accountId' => '1592310167', 'visitorId' => 'testUserId', + 'revision' => '15', 'clientEngine' => 'php-sdk', - 'clientVersion' => '1.0.1', + 'clientVersion' => '1.1.0', 'userFeatures' => [], 'isGlobalHoldback' => false, 'timestamp' => time() * 1000, - 'eventFeatures' => [], + 'eventFeatures' => [ + [ + 'name' => 'revenue', + 'type' => 'custom', + 'value' => 42, + 'shouldIndex' => false + ] + ], 'eventMetrics' => [[ 'name' => 'revenue', 'value' => 42 @@ -235,6 +249,7 @@ public function testCreateConversionEventNoAttributesWithValue() 'layerStates' => [[ 'layerId' => '7719770039', 'actionTriggered' => true, + 'revision' => '15', 'decision' => [ 'experimentId' => '7716830082', 'variationId' => '7722370027', @@ -251,7 +266,7 @@ public function testCreateConversionEventNoAttributesWithValue() [$this->config->getExperimentFromKey('test_experiment')], $this->testUserId, null, - 42 + array('revenue' => 42) ); $this->assertEquals($expectedLogEvent, $logEvent); @@ -265,11 +280,25 @@ public function testCreateConversionEventWithAttributesWithValue() 'projectId' => '7720880029', 'accountId' => '1592310167', 'visitorId' => 'testUserId', + 'revision' => '15', 'clientEngine' => 'php-sdk', - 'clientVersion' => '1.0.1', + 'clientVersion' => '1.1.0', 'isGlobalHoldback' => false, 'timestamp' => time() * 1000, - 'eventFeatures' => [], + 'eventFeatures' => [ + [ + 'name' => 'revenue', + 'type' => 'custom', + 'value' => 42, + 'shouldIndex' => false + ], + [ + 'name' => 'non-revenue', + 'type' => 'custom', + 'value' => 'definitely', + 'shouldIndex' => false + ] + ], 'eventMetrics' => [[ 'name' => 'revenue', 'value' => 42 @@ -279,6 +308,7 @@ public function testCreateConversionEventWithAttributesWithValue() 'layerStates' => [[ 'layerId' => '7719770039', 'actionTriggered' => true, + 'revision' => '15', 'decision' => [ 'experimentId' => '7716830082', 'variationId' => '7722370027', @@ -307,7 +337,70 @@ public function testCreateConversionEventWithAttributesWithValue() [$this->config->getExperimentFromKey('test_experiment')], $this->testUserId, $userAttributes, - 42 + array( + 'revenue' => 42, + 'non-revenue' => 'definitely' + ) + ); + + $this->assertEquals($expectedLogEvent, $logEvent); + } + + public function testCreateConversionEventNoAttributesWithInvalidValue() + { + $expectedLogEvent = new LogEvent( + 'https://logx.optimizely.com/log/event', + [ + 'projectId' => '7720880029', + 'accountId' => '1592310167', + 'visitorId' => 'testUserId', + 'revision' => '15', + 'clientEngine' => 'php-sdk', + 'clientVersion' => '1.1.0', + 'userFeatures' => [], + 'isGlobalHoldback' => false, + 'timestamp' => time() * 1000, + 'eventFeatures' => [ + [ + 'name' => 'revenue', + 'type' => 'custom', + 'value' => 42, + 'shouldIndex' => false + ], + [ + 'name' => 'non-revenue', + 'type' => 'custom', + 'value' => 'definitely', + 'shouldIndex' => false + ] + ], + 'eventMetrics' => [], + 'eventEntityId' => '7718020063', + 'eventName' => 'purchase', + 'layerStates' => [[ + 'layerId' => '7719770039', + 'actionTriggered' => true, + 'revision' => '15', + 'decision' => [ + 'experimentId' => '7716830082', + 'variationId' => '7722370027', + 'isLayerHoldback' => false + ] + ]] + ], + 'POST', + ['Content-Type' => 'application/json'] + ); + $logEvent = $this->eventBuilder->createConversionEvent( + $this->config, + 'purchase', + [$this->config->getExperimentFromKey('test_experiment')], + $this->testUserId, + null, + array( + 'revenue' => '42', + 'non-revenue' => 'definitely' + ) ); $this->assertEquals($expectedLogEvent, $logEvent); diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index fef5622f..d23dae19 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -1,6 +1,6 @@ track('purchase', 'test_user', $userAttributes); } + public function testTrackNoAttributesWithDeprecatedEventValue() + { + $this->eventBuilderMock->expects($this->once()) + ->method('createConversionEvent') + ->with( + $this->projectConfig, + 'purchase', + [$this->projectConfig->getExperimentFromKey('group_experiment_1'), + $this->projectConfig->getExperimentFromKey('group_experiment_2')], + 'test_user', + null, + array('revenue' => 42) + ) + ->willReturn(new LogEvent('logx.optimizely.com/track', ['param1' => 'val1'], 'POST', [])); + + $this->loggerMock->expects($this->exactly(7)) + ->method('log'); + $this->loggerMock->expects($this->at(0)) + ->method('log') + ->with(Logger::WARNING, + 'Event value is deprecated in track call. Use event tags to pass in revenue value instead.'); + $this->loggerMock->expects($this->at(1)) + ->method('log') + ->with(Logger::INFO, 'User "test_user" does not meet conditions to be in experiment "test_experiment".'); + $this->loggerMock->expects($this->at(2)) + ->method('log') + ->with(Logger::INFO, + 'Not tracking user "test_user" for experiment "test_experiment".'); + $this->loggerMock->expects($this->at(3)) + ->method('log') + ->with(Logger::INFO, + 'Experiment "paused_experiment" is not running.'); + $this->loggerMock->expects($this->at(4)) + ->method('log') + ->with(Logger::INFO, + 'Not tracking user "test_user" for experiment "paused_experiment".'); + $this->loggerMock->expects($this->at(5)) + ->method('log') + ->with(Logger::INFO, + 'Tracking event "purchase" for user "test_user".'); + $this->loggerMock->expects($this->at(6)) + ->method('log') + ->with(Logger::DEBUG, + 'Dispatching conversion event to URL logx.optimizely.com/track with params param1=val1.'); + + $optlyObject = new Optimizely($this->datafile, new ValidEventDispatcher(), $this->loggerMock); + + $eventBuilder = new \ReflectionProperty(Optimizely::class, '_eventBuilder'); + $eventBuilder->setAccessible(true); + $eventBuilder->setValue($optlyObject, $this->eventBuilderMock); + + // Call track + $optlyObject->track('purchase', 'test_user', null, 42); + } + public function testTrackNoAttributesWithEventValue() { $this->eventBuilderMock->expects($this->once()) @@ -660,7 +715,7 @@ public function testTrackNoAttributesWithEventValue() $this->projectConfig->getExperimentFromKey('group_experiment_2')], 'test_user', null, - 42 + array('revenue' => 42) ) ->willReturn(new LogEvent('logx.optimizely.com/track', ['param1' => 'val1'], 'POST', [])); @@ -697,10 +752,62 @@ public function testTrackNoAttributesWithEventValue() $eventBuilder->setValue($optlyObject, $this->eventBuilderMock); // Call track - $optlyObject->track('purchase', 'test_user', null, 42); + $optlyObject->track('purchase', 'test_user', null, array('revenue' => 42)); } - public function testTrackWithAttributesWithEventValue() + public function testTrackNoAttributesWithInvalidEventValue() + { + $this->eventBuilderMock->expects($this->once()) + ->method('createConversionEvent') + ->with( + $this->projectConfig, + 'purchase', + [$this->projectConfig->getExperimentFromKey('group_experiment_1'), + $this->projectConfig->getExperimentFromKey('group_experiment_2')], + 'test_user', + null, + array('revenue' => '4200') + ) + ->willReturn(new LogEvent('logx.optimizely.com/track', ['param1' => 'val1'], 'POST', [])); + + $callIndex = 0; + $this->loggerMock->expects($this->exactly(6)) + ->method('log'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, 'User "test_user" does not meet conditions to be in experiment "test_experiment".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Not tracking user "test_user" for experiment "test_experiment".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Experiment "paused_experiment" is not running.'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Not tracking user "test_user" for experiment "paused_experiment".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Tracking event "purchase" for user "test_user".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::DEBUG, + 'Dispatching conversion event to URL logx.optimizely.com/track with params param1=val1.'); + + $optlyObject = new Optimizely($this->datafile, new ValidEventDispatcher(), $this->loggerMock); + + $eventBuilder = new \ReflectionProperty(Optimizely::class, '_eventBuilder'); + $eventBuilder->setAccessible(true); + $eventBuilder->setValue($optlyObject, $this->eventBuilderMock); + + // Call track + $optlyObject->track('purchase', 'test_user', null, array('revenue' => '4200')); + } + + public function testTrackWithAttributesWithDeprecatedEventValue() { $userAttributes = [ 'device_type' => 'iPhone', @@ -716,32 +823,36 @@ public function testTrackWithAttributesWithEventValue() $this->projectConfig->getExperimentFromKey('group_experiment_2')], 'test_user', $userAttributes, - 42 + array('revenue' => 42) ) ->willReturn(new LogEvent('logx.optimizely.com/track', ['param1' => 'val1'], 'POST', [])); - $this->loggerMock->expects($this->exactly(6)) + $this->loggerMock->expects($this->exactly(7)) ->method('log'); $this->loggerMock->expects($this->at(0)) ->method('log') - ->with(Logger::INFO, 'User "test_user" does not meet conditions to be in experiment "test_experiment".'); + ->with(Logger::WARNING, + 'Event value is deprecated in track call. Use event tags to pass in revenue value instead.'); $this->loggerMock->expects($this->at(1)) + ->method('log') + ->with(Logger::INFO, 'User "test_user" does not meet conditions to be in experiment "test_experiment".'); + $this->loggerMock->expects($this->at(2)) ->method('log') ->with(Logger::INFO, 'Not tracking user "test_user" for experiment "test_experiment".'); - $this->loggerMock->expects($this->at(2)) + $this->loggerMock->expects($this->at(3)) ->method('log') ->with(Logger::INFO, 'Experiment "paused_experiment" is not running.'); - $this->loggerMock->expects($this->at(3)) + $this->loggerMock->expects($this->at(4)) ->method('log') ->with(Logger::INFO, 'Not tracking user "test_user" for experiment "paused_experiment".'); - $this->loggerMock->expects($this->at(4)) + $this->loggerMock->expects($this->at(5)) ->method('log') ->with(Logger::INFO, 'Tracking event "purchase" for user "test_user".'); - $this->loggerMock->expects($this->at(5)) + $this->loggerMock->expects($this->at(6)) ->method('log') ->with(Logger::DEBUG, 'Dispatching conversion event to URL logx.optimizely.com/track with params param1=val1.'); @@ -755,4 +866,61 @@ public function testTrackWithAttributesWithEventValue() // Call track $optlyObject->track('purchase', 'test_user', $userAttributes, 42); } + + public function testTrackWithAttributesWithEventValue() + { + $userAttributes = [ + 'device_type' => 'iPhone', + 'company' => 'Optimizely' + ]; + + $this->eventBuilderMock->expects($this->once()) + ->method('createConversionEvent') + ->with( + $this->projectConfig, + 'purchase', + [$this->projectConfig->getExperimentFromKey('group_experiment_1'), + $this->projectConfig->getExperimentFromKey('group_experiment_2')], + 'test_user', + $userAttributes, + array('revenue' => 42) + ) + ->willReturn(new LogEvent('logx.optimizely.com/track', ['param1' => 'val1'], 'POST', [])); + + $callIndex = 0; + $this->loggerMock->expects($this->exactly(6)) + ->method('log'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, 'User "test_user" does not meet conditions to be in experiment "test_experiment".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Not tracking user "test_user" for experiment "test_experiment".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Experiment "paused_experiment" is not running.'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Not tracking user "test_user" for experiment "paused_experiment".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::INFO, + 'Tracking event "purchase" for user "test_user".'); + $this->loggerMock->expects($this->at($callIndex++)) + ->method('log') + ->with(Logger::DEBUG, + 'Dispatching conversion event to URL logx.optimizely.com/track with params param1=val1.'); + + $optlyObject = new Optimizely($this->datafile, new ValidEventDispatcher(), $this->loggerMock); + + $eventBuilder = new \ReflectionProperty(Optimizely::class, '_eventBuilder'); + $eventBuilder->setAccessible(true); + $eventBuilder->setValue($optlyObject, $this->eventBuilderMock); + + // Call track + $optlyObject->track('purchase', 'test_user', $userAttributes, array('revenue' => 42)); + } } diff --git a/tests/ProjectConfigTest.php b/tests/ProjectConfigTest.php index 18c6f20a..c378c978 100644 --- a/tests/ProjectConfigTest.php +++ b/tests/ProjectConfigTest.php @@ -181,6 +181,11 @@ public function testGetProjectId() $this->assertEquals('7720880029', $this->config->getProjectId()); } + public function testGetRevision() + { + $this->assertEquals('15', $this->config->getRevision()); + } + public function testGetGroupValidId() { $group = $this->config->getGroup('7722400015'); diff --git a/tests/TestData.php b/tests/TestData.php index d5f62a4c..ee3f5df7 100644 --- a/tests/TestData.php +++ b/tests/TestData.php @@ -23,7 +23,7 @@ define('DATAFILE', '{"experiments": [{"status": "Running", "key": "test_experiment", "layerId": "7719770039", - "trafficAllocation": [{"entityId": "7722370027", "endOfRange": 4000}, + "trafficAllocation": [{"entityId": "", "endOfRange": 1500}, {"entityId": "7722370027", "endOfRange": 4000}, {"entityId": "7721010009", "endOfRange": 8000}], "audienceIds": ["7718080042"], "variations": [{"id": "7722370027", "key": "control"}, {"id": "7721010009", "key": "variation"}], "forcedVariations": {"user1": "control"}, "id": "7716830082"}, {"status": "Paused", "key": "paused_experiment", "layerId": "7719779139", @@ -32,7 +32,7 @@ "variations": [{"id": "7722370427", "key": "control"}, {"id": "7721010509", "key": "variation"}], "forcedVariations": {}, "id": "7716830585"}], "version": "2", "audiences": [{"conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"iPhone\"}]], [\"or\", [\"or\", {\"name\": \"location\", \"type\": \"custom_attribute\", \"value\": \"San Francisco\"}]]]", "id": "7718080042", "name": "iPhone users in San Francisco"}], - "groups": [{"policy": "random", "trafficAllocation": [{"entityId": "7723330021", "endOfRange": 2000}, {"entityId": "7718750065", "endOfRange": 6000}], "experiments": [{"status": "Running", "key": "group_experiment_1", "layerId": "7721010011", "trafficAllocation": [{"entityId": "7722260071", "endOfRange": 5000}, {"entityId": "7722360022", "endOfRange": 10000}], "audienceIds": [], "variations": [{"id": "7722260071", "key": "group_exp_1_var_1"}, {"id": "7722360022", "key": "group_exp_1_var_2"}], "forcedVariations": {"user1": "group_exp_1_var_1"}, "id": "7723330021"}, {"status": "Running", "key": "group_experiment_2", "layerId": "7721020020", "trafficAllocation": [{"entityId": "7713030086", "endOfRange": 5000}, {"entityId": "7725250007", "endOfRange": 10000}], "audienceIds": [], + "groups": [{"policy": "random", "trafficAllocation": [{"entityId": "", "endOfRange": 500}, {"entityId": "7723330021", "endOfRange": 2000}, {"entityId": "7718750065", "endOfRange": 6000}], "experiments": [{"status": "Running", "key": "group_experiment_1", "layerId": "7721010011", "trafficAllocation": [{"entityId": "7722260071", "endOfRange": 5000}, {"entityId": "7722360022", "endOfRange": 10000}], "audienceIds": [], "variations": [{"id": "7722260071", "key": "group_exp_1_var_1"}, {"id": "7722360022", "key": "group_exp_1_var_2"}], "forcedVariations": {"user1": "group_exp_1_var_1"}, "id": "7723330021"}, {"status": "Running", "key": "group_experiment_2", "layerId": "7721020020", "trafficAllocation": [{"entityId": "7713030086", "endOfRange": 5000}, {"entityId": "7725250007", "endOfRange": 10000}], "audienceIds": [], "variations": [{"id": "7713030086", "key": "group_exp_2_var_1"}, {"id": "7725250007", "key": "group_exp_2_var_2"}], "forcedVariations": {}, "id": "7718750065"}], "id": "7722400015"}], "attributes": [{"id": "7723280020", "key": "device_type"}, {"id": "7723340004", "key": "location"}], "projectId": "7720880029", "accountId": "1592310167", diff --git a/tests/UtilsTests/EventTagUtilsTest.php b/tests/UtilsTests/EventTagUtilsTest.php new file mode 100644 index 00000000..1e44b6b9 --- /dev/null +++ b/tests/UtilsTests/EventTagUtilsTest.php @@ -0,0 +1,50 @@ +assertNull(EventTagUtils::getRevenueValue(null)); + $this->assertNull(EventTagUtils::getRevenueValue(0.5)); + $this->assertNull(EventTagUtils::getRevenueValue(65536)); + $this->assertNull(EventTagUtils::getRevenueValue(9223372036854775807)); + $this->assertNull(EventTagUtils::getRevenueValue('65536')); + $this->assertNull(EventTagUtils::getRevenueValue(true)); + } + public function testGetRevenueValueNoRevenueTag() { + $this->assertNull(EventTagUtils::getRevenueValue(array())); + $this->assertNull(EventTagUtils::getRevenueValue(array('non-revenue' => 42))); + } + + public function testGetRevenueValueWithInvalidRevenueTag() { + $this->assertNull(EventTagUtils::getRevenueValue(array('revenue' => null))); + $this->assertNull(EventTagUtils::getRevenueValue(array('revenue' => 0.5))); + $this->assertNull(EventTagUtils::getRevenueValue(array('revenue' => '65536'))); + $this->assertNull(EventTagUtils::getRevenueValue(array('revenue' => true))); + $this->assertNull(EventTagUtils::getRevenueValue(array('revenue' => array(1, 2, 3)))); + } + + public function testGetRevenueValueWithRevenueTag() { + $this->assertEquals(65536, EventTagUtils::getRevenueValue(array('revenue' => 65536))); + $this->assertEquals(9223372036854775807, EventTagUtils::getRevenueValue(array('revenue' => 9223372036854775807))); + $this->assertEquals(0, EventTagUtils::getRevenueValue(array('revenue' => 0))); + } +} diff --git a/tests/UtilsTests/ValidatorTest.php b/tests/UtilsTests/ValidatorTest.php index bfdd156f..175f2bcd 100644 --- a/tests/UtilsTests/ValidatorTest.php +++ b/tests/UtilsTests/ValidatorTest.php @@ -1,6 +1,6 @@ assertFalse(Validator::areAttributesValid([0, 1, 2, 42, 'abc' => 'def'])); } + public function testAreEventTagsValidValidEventTags() + { + // Empty attributes + $this->assertTrue(Validator::areEventTagsValid([])); + + // Valid attributes + $this->assertTrue(Validator::areEventTagsValid([ + 'revenue' => 0, + 'location' => 'San Francisco', + 'browser' => 'Firefox' + ])); + } + + public function testAreEventTagsValidInvalidEventTags() + { + // String as attributes + $this->assertFalse(Validator::areEventTagsValid('Invalid string attributes.')); + + // Integer as attributes + $this->assertFalse(Validator::areEventTagsValid(42)); + + // Boolean as attributes + $this->assertFalse(Validator::areEventTagsValid(true)); + + // Sequential array as attributes + $this->assertFalse(Validator::areEventTagsValid([0, 1, 2, 42])); + + // Mixed array as attributes + $this->assertFalse(Validator::areEventTagsValid([0, 1, 2, 42, 'abc' => 'def'])); + } + public function testIsUserInExperimentNoAudienceUsedInExperiment() { $config = new ProjectConfig(DATAFILE, new NoOpLogger(), new NoOpErrorHandler());