From e32e2b8ae9db18247fbb794200e88e3a6114aac0 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 24 Jun 2020 09:59:01 +0500 Subject: [PATCH 01/10] relocate bucketer log messages --- src/Optimizely/Bucketer.php | 13 ++----- .../DecisionService/DecisionService.php | 14 +++++++- tests/BucketerTest.php | 34 ++----------------- 3 files changed, 17 insertions(+), 44 deletions(-) diff --git a/src/Optimizely/Bucketer.php b/src/Optimizely/Bucketer.php index a1e88a04..3b236117 100644 --- a/src/Optimizely/Bucketer.php +++ b/src/Optimizely/Bucketer.php @@ -133,7 +133,7 @@ private function findBucket($bucketingId, $userId, $parentId, $trafficAllocation * Determine variation the user should be put in. * * @param $config ProjectConfigInterface Configuration for the project. - * @param $experiment Experiment Experiment in which user is to be bucketed. + * @param $experiment Experiment Experiment or Rollout rule in which user is to be bucketed. * @param $bucketingId string A customer-assigned value used to create the key for the murmur hash. * @param $userId string User identifier. * @@ -146,6 +146,7 @@ public function bucket(ProjectConfigInterface $config, Experiment $experiment, $ } // Determine if experiment is in a mutually exclusive group. + // This will not affect evaluation of rollout rules. if ($experiment->isInMutexGroup()) { $group = $config->getGroup($experiment->getGroupId()); @@ -188,19 +189,9 @@ public function bucket(ProjectConfigInterface $config, Experiment $experiment, $ if (!empty($variationId)) { $variation = $config->getVariationFromId($experiment->getKey(), $variationId); - $this->_logger->log( - Logger::INFO, - sprintf( - 'User "%s" is in variation %s of experiment %s.', - $userId, - $variation->getKey(), - $experiment->getKey() - ) - ); return $variation; } - $this->_logger->log(Logger::INFO, sprintf('User "%s" is in no variation.', $userId)); return null; } } diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index d00e7f84..f60e1109 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -161,9 +161,21 @@ public function getVariation(ProjectConfigInterface $projectConfig, Experiment $ } $variation = $this->_bucketer->bucket($projectConfig, $experiment, $bucketingId, $userId); - if (!is_null($variation)) { + if ($variation === null) { + $this->_logger->log(Logger::INFO, sprintf('User "%s" is in no variation.', $userId)); + } else { $this->saveVariation($experiment, $variation, $userProfile); + $this->_logger->log( + Logger::INFO, + sprintf( + 'User "%s" is in variation %s of experiment %s.', + $userId, + $variation->getKey(), + $experiment->getKey() + ) + ); } + return $variation; } diff --git a/tests/BucketerTest.php b/tests/BucketerTest.php index 97fc1c51..0c3f11c6 100644 --- a/tests/BucketerTest.php +++ b/tests/BucketerTest.php @@ -96,16 +96,13 @@ public function testBucketValidExperimentNotInGroup() $bucketer = new TestBucketer($this->loggerMock); $bucketer->setBucketValues([1000, 3000, 7000, 9000]); // Total calls in this test - $this->loggerMock->expects($this->exactly(8)) + $this->loggerMock->expects($this->exactly(4)) ->method('log'); // No variation (empty entity ID) $this->loggerMock->expects($this->at(0)) ->method('log') ->with(Logger::DEBUG, sprintf('Assigned bucket 1000 to user "%s" with bucketing ID "%s".', $this->testUserId, $this->testBucketingIdControl)); - $this->loggerMock->expects($this->at(1)) - ->method('log') - ->with(Logger::INFO, 'User "testUserId" is in no variation.'); $this->assertNull( $bucketer->bucket( @@ -120,12 +117,6 @@ public function testBucketValidExperimentNotInGroup() $this->loggerMock->expects($this->at(0)) ->method('log') ->with(Logger::DEBUG, sprintf('Assigned bucket 3000 to user "%s" with bucketing ID "%s".', $this->testUserId, $this->testBucketingIdControl)); - $this->loggerMock->expects($this->at(1)) - ->method('log') - ->with( - Logger::INFO, - 'User "testUserId" is in variation control of experiment test_experiment.' - ); $this->assertEquals( new Variation('7722370027', 'control'), @@ -141,12 +132,6 @@ public function testBucketValidExperimentNotInGroup() $this->loggerMock->expects($this->at(0)) ->method('log') ->with(Logger::DEBUG, sprintf('Assigned bucket 7000 to user "%s" with bucketing ID "%s".', $this->testUserId, $this->testBucketingIdControl)); - $this->loggerMock->expects($this->at(1)) - ->method('log') - ->with( - Logger::INFO, - 'User "testUserId" is in variation variation of experiment test_experiment.' - ); $this->assertEquals( new Variation('7721010009', 'variation'), @@ -162,9 +147,6 @@ public function testBucketValidExperimentNotInGroup() $this->loggerMock->expects($this->at(0)) ->method('log') ->with(Logger::DEBUG, sprintf('Assigned bucket 9000 to user "%s" with bucketing ID "%s".', $this->testUserId, $this->testBucketingIdControl)); - $this->loggerMock->expects($this->at(1)) - ->method('log') - ->with(Logger::INFO, 'User "testUserId" is in no variation.'); $this->assertNull( $bucketer->bucket( @@ -180,7 +162,7 @@ public function testBucketValidExperimentInGroup() { $bucketer = new TestBucketer($this->loggerMock); // Total calls in this test - $this->loggerMock->expects($this->exactly(14)) + $this->loggerMock->expects($this->exactly(12)) ->method('log'); // group_experiment_1 (15% experiment) @@ -195,12 +177,6 @@ public function testBucketValidExperimentInGroup() $this->loggerMock->expects($this->at(2)) ->method('log') ->with(Logger::DEBUG, sprintf('Assigned bucket 4000 to user "%s" with bucketing ID "%s".', $this->testUserId, $this->testBucketingIdControl)); - $this->loggerMock->expects($this->at(3)) - ->method('log') - ->with( - Logger::INFO, - 'User "testUserId" is in variation group_exp_1_var_1 of experiment group_experiment_1.' - ); $this->assertEquals( new Variation( @@ -233,12 +209,6 @@ public function testBucketValidExperimentInGroup() $this->loggerMock->expects($this->at(2)) ->method('log') ->with(Logger::DEBUG, sprintf('Assigned bucket 7000 to user "%s" with bucketing ID "%s".', $this->testUserId, $this->testBucketingIdControl)); - $this->loggerMock->expects($this->at(3)) - ->method('log') - ->with( - Logger::INFO, - 'User "testUserId" is in variation group_exp_1_var_2 of experiment group_experiment_1.' - ); $this->assertEquals( new Variation( From 2b32343bea0efd81a43bda2731717a57526325ef Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 24 Jun 2020 11:34:00 +0500 Subject: [PATCH 02/10] refact: Fix log messages for rollout rule --- .../DecisionService/DecisionService.php | 22 +++---- .../Enums/AudienceEvaluationLogs.php | 6 +- src/Optimizely/Utils/Validator.php | 15 +++-- .../DecisionServiceTest.php | 58 +++++++++++++++++-- 4 files changed, 78 insertions(+), 23 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index f60e1109..a8080662 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -1,6 +1,6 @@ _logger)) { + if (!Validator::isUserInExperiment($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, $i+1)) { $this->_logger->log( Logger::DEBUG, - sprintf("User '%s' did not meet the audience conditions to be in rollout rule '%s'.", $userId, $experiment->getKey()) + sprintf("User '%s' does not meet conditions for targeting rule %s.", $userId, $i+1) ); // Evaluate this user for the next rule continue; } // Evaluate if user satisfies the traffic allocation for this rollout rule - $variation = $this->_bucketer->bucket($projectConfig, $experiment, $bucketing_id, $userId); + $variation = $this->_bucketer->bucket($projectConfig, $rolloutRule, $bucketing_id, $userId); if ($variation && $variation->getKey()) { - return new FeatureDecision($experiment, $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT); + return new FeatureDecision($rolloutRule, $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT); } break; } // Evaluate Everyone Else Rule / Last Rule now - $experiment = $rolloutRules[sizeof($rolloutRules)-1]; + $rolloutRule = $rolloutRules[sizeof($rolloutRules)-1]; // Evaluate if user meets the audience condition of Everyone Else Rule / Last Rule now - if (!Validator::isUserInExperiment($projectConfig, $experiment, $userAttributes, $this->_logger)) { + if (!Validator::isUserInExperiment($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, 'Everyone Else')) { $this->_logger->log( Logger::DEBUG, - sprintf("User '%s' did not meet the audience conditions to be in rollout rule '%s'.", $userId, $experiment->getKey()) + sprintf("User '%s' does not meet conditions for targeting rule 'Everyone Else'.", $userId) ); return null; } - $variation = $this->_bucketer->bucket($projectConfig, $experiment, $bucketing_id, $userId); + $variation = $this->_bucketer->bucket($projectConfig, $rolloutRule, $bucketing_id, $userId); if ($variation && $variation->getKey()) { - return new FeatureDecision($experiment, $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT); + return new FeatureDecision($rolloutRule, $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT); } return null; } diff --git a/src/Optimizely/Enums/AudienceEvaluationLogs.php b/src/Optimizely/Enums/AudienceEvaluationLogs.php index b1b4b5d9..c8c12c58 100644 --- a/src/Optimizely/Enums/AudienceEvaluationLogs.php +++ b/src/Optimizely/Enums/AudienceEvaluationLogs.php @@ -1,6 +1,6 @@ getKey()}\""; + + if ($isRollout) { + $loggingStr = "rule {$rolloutRule}"; + } + $audienceConditions = $experiment->getAudienceConditions(); if ($audienceConditions === null) { $audienceConditions = $experiment->getAudienceIds(); @@ -148,7 +155,7 @@ public static function isUserInExperiment($config, $experiment, $userAttributes, $logger->log(Logger::DEBUG, sprintf( AudienceEvaluationLogs::EVALUATING_AUDIENCES_COMBINED, - $experiment->getKey(), + $loggingStr, json_encode($audienceConditions) )); @@ -156,7 +163,7 @@ public static function isUserInExperiment($config, $experiment, $userAttributes, if (empty($audienceConditions)) { $logger->log(Logger::INFO, sprintf( AudienceEvaluationLogs::AUDIENCE_EVALUATION_RESULT_COMBINED, - $experiment->getKey(), + $loggingStr, 'TRUE' )); return true; @@ -202,7 +209,7 @@ public static function isUserInExperiment($config, $experiment, $userAttributes, $logger->log(Logger::INFO, sprintf( AudienceEvaluationLogs::AUDIENCE_EVALUATION_RESULT_COMBINED, - $experiment->getKey(), + $loggingStr, strtoupper(var_export($evalResult, true)) )); diff --git a/tests/DecisionServiceTests/DecisionServiceTest.php b/tests/DecisionServiceTests/DecisionServiceTest.php index 151a295a..72a41181 100644 --- a/tests/DecisionServiceTests/DecisionServiceTest.php +++ b/tests/DecisionServiceTests/DecisionServiceTest.php @@ -1125,8 +1125,8 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar ); // Verify Logs - $this->assertContains([Logger::DEBUG, "User 'user_1' did not meet the audience conditions to be in rollout rule '{$experiment0->getKey()}'."], $this->collectedLogs); - $this->assertContains([Logger::DEBUG, "User 'user_1' did not meet the audience conditions to be in rollout rule '{$experiment1->getKey()}'."], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, "User 'user_1' does not meet conditions for targeting rule 1."], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, "User 'user_1' does not meet conditions for targeting rule 2."], $this->collectedLogs); } public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTargetingRuleOrEveryoneElseRule() @@ -1161,11 +1161,59 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar $this->assertNull($this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes)); // Verify Logs - $this->assertContains([Logger::DEBUG, "User 'user_1' did not meet the audience conditions to be in rollout rule '{$experiment0->getKey()}'."], $this->collectedLogs); - $this->assertContains([Logger::DEBUG, "User 'user_1' did not meet the audience conditions to be in rollout rule '{$experiment1->getKey()}'."], $this->collectedLogs); - $this->assertContains([Logger::DEBUG, "User 'user_1' did not meet the audience conditions to be in rollout rule '{$experiment2->getKey()}'."], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, "User 'user_1' does not meet conditions for targeting rule 1."], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, "User 'user_1' does not meet conditions for targeting rule 2."], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, "User 'user_1' does not meet conditions for targeting rule 'Everyone Else'."], $this->collectedLogs); } + + public function testGetVariationForFeatureRolloutLogging() + { + $featureFlag = $this->config->getFeatureFlagFromKey('boolean_single_variable_feature'); + $rollout_id = $featureFlag->getRolloutId(); + $rollout = $this->config->getRolloutFromId($rollout_id); + $experiment0 = $rollout->getExperiments()[0]; + $experiment1 = $rollout->getExperiments()[1]; + // Everyone Else Rule + $experiment2 = $rollout->getExperiments()[2]; + $expected_variation = $experiment2->getVariations()[0]; + $expected_decision = new FeatureDecision( + $experiment2, + $expected_variation, + FeatureDecision::DECISION_SOURCE_ROLLOUT + ); + + // Provide null attributes so that user does not qualify for audience + $user_attributes = []; + $this->decisionService = new DecisionService($this->loggerMock); + $bucketer = new \ReflectionProperty(DecisionService::class, '_bucketer'); + $bucketer->setAccessible(true); + $bucketer->setValue($this->decisionService, $this->bucketerMock); + + // Expect bucket to be called exactly once for the everyone else/last rule. + $this->bucketerMock->expects($this->exactly(1)) + ->method('bucket') + ->willReturn($expected_variation); + + $this->loggerMock->expects($this->any()) + ->method('log') + ->will($this->returnCallback($this->collectLogsForAssertion)); + + $this->assertEquals( + $expected_decision, + $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes) + ); + + // Verify Logs + $this->assertContains([Logger::DEBUG, 'Evaluating audiences for rule 1: ["11155"].'], $this->collectedLogs); + $this->assertContains([Logger::INFO, 'Audiences for rule 1 collectively evaluated to FALSE.'], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, 'Evaluating audiences for rule 2: ["11155"].'], $this->collectedLogs); + $this->assertContains([Logger::INFO,'Audiences for rule 2 collectively evaluated to FALSE.'], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, 'Evaluating audiences for rule Everyone Else: [].'], $this->collectedLogs); + $this->assertContains([Logger::INFO, 'Audiences for rule Everyone Else collectively evaluated to TRUE.'], $this->collectedLogs); + } + + // test set/get forced variation for the following cases: // - valid and invalid user ID // - valid and invalid experiment key From ef9269792cf2ebfc98c72d4e51ff7898cc8605d3 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 24 Jun 2020 12:14:26 +0500 Subject: [PATCH 03/10] update headers --- src/Optimizely/Bucketer.php | 2 +- tests/BucketerTest.php | 2 +- tests/DecisionServiceTests/DecisionServiceTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Optimizely/Bucketer.php b/src/Optimizely/Bucketer.php index 3b236117..1cc6a864 100644 --- a/src/Optimizely/Bucketer.php +++ b/src/Optimizely/Bucketer.php @@ -1,6 +1,6 @@ Date: Wed, 24 Jun 2020 12:31:23 +0500 Subject: [PATCH 04/10] add docs --- src/Optimizely/Utils/Validator.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Optimizely/Utils/Validator.php b/src/Optimizely/Utils/Validator.php index a6106fdd..3b3f3bf2 100644 --- a/src/Optimizely/Utils/Validator.php +++ b/src/Optimizely/Utils/Validator.php @@ -136,6 +136,8 @@ public static function areEventTagsValid($eventTags) * @param $experiment Experiment Entity representing the experiment. * @param $userAttributes array Attributes of the user. * @param $logger LoggerInterface. + * @param $isRollout Boolean true if experiment to evaluate is a rollout rule. + * @param $rolloutRule String Rollout rule identifier to log. * * @return boolean Representing whether user meets audience conditions to be in experiment or not. */ From 5481ce4a660ec76cbb80e4bc63f67e3b53b5896a Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Tue, 30 Jun 2020 18:09:10 +0500 Subject: [PATCH 05/10] rename isUserInExperiment --- .../DecisionService/DecisionService.php | 6 +- src/Optimizely/Utils/Validator.php | 2 +- tests/UtilsTests/ValidatorLoggingTest.php | 10 ++-- tests/UtilsTests/ValidatorTest.php | 56 +++++++++---------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index a8080662..6af08e6e 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -152,7 +152,7 @@ public function getVariation(ProjectConfigInterface $projectConfig, Experiment $ } } - if (!Validator::isUserInExperiment($projectConfig, $experiment, $attributes, $this->_logger)) { + if (!Validator::doesUserMeetAudienceConditions($projectConfig, $experiment, $attributes, $this->_logger)) { $this->_logger->log( Logger::INFO, sprintf('User "%s" does not meet conditions to be in experiment "%s".', $userId, $experiment->getKey()) @@ -313,7 +313,7 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon $rolloutRule = $rolloutRules[$i]; // Evaluate if user meets the audience condition of this rollout rule - if (!Validator::isUserInExperiment($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, $i+1)) { + if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, $i + 1)) { $this->_logger->log( Logger::DEBUG, sprintf("User '%s' does not meet conditions for targeting rule %s.", $userId, $i+1) @@ -333,7 +333,7 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon $rolloutRule = $rolloutRules[sizeof($rolloutRules)-1]; // Evaluate if user meets the audience condition of Everyone Else Rule / Last Rule now - if (!Validator::isUserInExperiment($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, 'Everyone Else')) { + if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, 'Everyone Else')) { $this->_logger->log( Logger::DEBUG, sprintf("User '%s' does not meet conditions for targeting rule 'Everyone Else'.", $userId) diff --git a/src/Optimizely/Utils/Validator.php b/src/Optimizely/Utils/Validator.php index 3b3f3bf2..572e9e2e 100644 --- a/src/Optimizely/Utils/Validator.php +++ b/src/Optimizely/Utils/Validator.php @@ -141,7 +141,7 @@ public static function areEventTagsValid($eventTags) * * @return boolean Representing whether user meets audience conditions to be in experiment or not. */ - public static function isUserInExperiment($config, $experiment, $userAttributes, $logger, $isRollout = null, $rolloutRule = null) + public static function doesUserMeetAudienceConditions($config, $experiment, $userAttributes, $logger, $isRollout = null, $rolloutRule = null) { $loggingStr = "experiment \"{$experiment->getKey()}\""; diff --git a/tests/UtilsTests/ValidatorLoggingTest.php b/tests/UtilsTests/ValidatorLoggingTest.php index 7c4d52d5..3549b505 100644 --- a/tests/UtilsTests/ValidatorLoggingTest.php +++ b/tests/UtilsTests/ValidatorLoggingTest.php @@ -42,7 +42,7 @@ protected function setUp() }; } - public function testIsUserInExperimentWithNoAudience() + public function testdoesUserMeetAudienceConditionsWithNoAudience() { $experiment = $this->config->getExperimentFromKey('test_experiment'); $experiment->setAudienceIds([]); @@ -56,10 +56,10 @@ public function testIsUserInExperimentWithNoAudience() ->method('log') ->with(Logger::INFO, "Audiences for experiment \"test_experiment\" collectively evaluated to TRUE."); - $this->assertTrue(Validator::isUserInExperiment($this->config, $experiment, [], $this->loggerMock)); + $this->assertTrue(Validator::doesUserMeetAudienceConditions($this->config, $experiment, [], $this->loggerMock)); } - public function testIsUserInExperimentEvaluatesAudienceIds() + public function testdoesUserMeetAudienceConditionsEvaluatesAudienceIds() { $userAttributes = [ "test_attribute" => "test_value_1" @@ -73,7 +73,7 @@ public function testIsUserInExperimentEvaluatesAudienceIds() ->method('log') ->will($this->returnCallback($this->collectLogsForAssertion)); - Validator::isUserInExperiment($this->config, $experiment, $userAttributes, $this->loggerMock); + Validator::doesUserMeetAudienceConditions($this->config, $experiment, $userAttributes, $this->loggerMock); $this->assertContains([Logger::DEBUG, "Evaluating audiences for experiment \"test_experiment\": [\"11155\"]."], $this->collectedLogs); $this->assertContains( @@ -94,7 +94,7 @@ public function testIsUserInExperimenEvaluatesAudienceConditions() ->method('log') ->will($this->returnCallback($this->collectLogsForAssertion)); - Validator::isUserInExperiment($this->typedConfig, $experiment, ["house" => "I am in Slytherin"], $this->loggerMock); + Validator::doesUserMeetAudienceConditions($this->typedConfig, $experiment, ["house" => "I am in Slytherin"], $this->loggerMock); $this->assertContains( [Logger::DEBUG, "Evaluating audiences for experiment \"audience_combinations_experiment\": [\"or\",[\"or\",\"3468206642\",\"3988293898\"]]."], diff --git a/tests/UtilsTests/ValidatorTest.php b/tests/UtilsTests/ValidatorTest.php index 27d84aea..09d38e0d 100644 --- a/tests/UtilsTests/ValidatorTest.php +++ b/tests/UtilsTests/ValidatorTest.php @@ -1,6 +1,6 @@ getMockBuilder(DatafileProjectConfig::class) ->setConstructorArgs(array(DATAFILE, $this->loggerMock, new NoOpErrorHandler())) @@ -213,7 +213,7 @@ public function testIsUserInExperimentAudienceUsedInExperimentNoAttributesProvid ->will($this->returnValue($audience)); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $configMock, $experiment, null, @@ -222,7 +222,7 @@ public function testIsUserInExperimentAudienceUsedInExperimentNoAttributesProvid ); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $configMock, $experiment, [], @@ -231,11 +231,11 @@ public function testIsUserInExperimentAudienceUsedInExperimentNoAttributesProvid ); } - public function testIsUserInExperimentAudienceMatch() + public function testdoesUserMeetAudienceConditionsAudienceMatch() { $config = new DatafileProjectConfig(DATAFILE, new NoOpLogger(), new NoOpErrorHandler()); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $config->getExperimentFromKey('test_experiment'), ['device_type' => 'iPhone', 'location' => 'San Francisco'], @@ -244,11 +244,11 @@ public function testIsUserInExperimentAudienceMatch() ); } - public function testIsUserInExperimentAudienceNoMatch() + public function testdoesUserMeetAudienceConditionsAudienceNoMatch() { $config = new DatafileProjectConfig(DATAFILE, new NoOpLogger(), new NoOpErrorHandler()); $this->assertFalse( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $config->getExperimentFromKey('test_experiment'), ['device_type' => 'Android', 'location' => 'San Francisco'], @@ -257,8 +257,8 @@ public function testIsUserInExperimentAudienceNoMatch() ); } - // test that isUserInExperiment returns true when no audience is attached to experiment. - public function testIsUserInExperimentNoAudienceUsedInExperiment() + // test that doesUserMeetAudienceConditions returns true when no audience is attached to experiment. + public function testdoesUserMeetAudienceConditionsNoAudienceUsedInExperiment() { $config = new DatafileProjectConfig(DATAFILE, null, null); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -267,7 +267,7 @@ public function testIsUserInExperimentNoAudienceUsedInExperiment() $experiment->setAudienceIds([]); $experiment->setAudienceConditions([]); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, [], @@ -279,7 +279,7 @@ public function testIsUserInExperimentNoAudienceUsedInExperiment() $experiment->setAudienceIds(['7718080042']); $experiment->setAudienceConditions([]); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, [], @@ -291,7 +291,7 @@ public function testIsUserInExperimentNoAudienceUsedInExperiment() $experiment->setAudienceIds([]); $experiment->setAudienceConditions(null); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, [], @@ -300,9 +300,9 @@ public function testIsUserInExperimentNoAudienceUsedInExperiment() ); } - // test that isUserInExperiment returns false when some audience is attached to experiment + // test that doesUserMeetAudienceConditions returns false when some audience is attached to experiment // and user attributes do not match. - public function testIsUserInExperimentSomeAudienceUsedInExperiment() + public function testdoesUserMeetAudienceConditionsSomeAudienceUsedInExperiment() { $config = new DatafileProjectConfig(DATAFILE, null, null); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -312,7 +312,7 @@ public function testIsUserInExperimentSomeAudienceUsedInExperiment() $experiment->setAudienceConditions(['11155']); $this->assertFalse( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, ['device_type' => 'Android', 'location' => 'San Francisco'], @@ -326,7 +326,7 @@ public function testIsUserInExperimentSomeAudienceUsedInExperiment() $experiment->setAudienceConditions(null); $this->assertFalse( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, ['device_type' => 'iPhone', 'location' => 'San Francisco'], @@ -335,8 +335,8 @@ public function testIsUserInExperimentSomeAudienceUsedInExperiment() ); } - // test that isUserInExperiment evaluates audience when audienceConditions is an audience leaf node. - public function testIsUserInExperimentWithAudienceConditionsSetToAudienceIdString() + // test that doesUserMeetAudienceConditions evaluates audience when audienceConditions is an audience leaf node. + public function testdoesUserMeetAudienceConditionsWithAudienceConditionsSetToAudienceIdString() { $config = new DatafileProjectConfig(DATAFILE, null, null); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -346,7 +346,7 @@ public function testIsUserInExperimentWithAudienceConditionsSetToAudienceIdStrin $experiment->setAudienceConditions('7718080042'); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, ['device_type' => 'iPhone', 'location' => 'San Francisco'], @@ -355,7 +355,7 @@ public function testIsUserInExperimentWithAudienceConditionsSetToAudienceIdStrin ); } - public function testIsUserInExperimentWithUnknownAudienceId() + public function testdoesUserMeetAudienceConditionsWithUnknownAudienceId() { $config = new DatafileProjectConfig(DATAFILE, $this->loggerMock, new NoOpErrorHandler()); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -366,7 +366,7 @@ public function testIsUserInExperimentWithUnknownAudienceId() // User qualifies for audience with ID "7718080042". $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $config, $experiment, ['device_type' => 'iPhone', 'location' => 'San Francisco'], @@ -375,8 +375,8 @@ public function testIsUserInExperimentWithUnknownAudienceId() ); } - // test that isUserInExperiment evaluates simple audience. - public function testIsUserInExperimentWithSimpleAudience() + // test that doesUserMeetAudienceConditions evaluates simple audience. + public function testdoesUserMeetAudienceConditionsWithSimpleAudience() { $config = new DatafileProjectConfig(DATAFILE, null, null); $configMock = $this->getMockBuilder(DatafileProjectConfig::class) @@ -399,7 +399,7 @@ public function testIsUserInExperimentWithSimpleAudience() ->will($this->returnValue($config->getAudience('7718080042'))); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $configMock, $experiment, ['device_type' => 'iPhone', 'location' => 'San Francisco'], @@ -408,8 +408,8 @@ public function testIsUserInExperimentWithSimpleAudience() ); } - // test that isUserInExperiment evaluates complex audience. - public function testIsUserInExperimentWithComplexAudience() + // test that doesUserMeetAudienceConditions evaluates complex audience. + public function testdoesUserMeetAudienceConditionsWithComplexAudience() { $config = new DatafileProjectConfig(DATAFILE_WITH_TYPED_AUDIENCES, null, null); $configMock = $this->getMockBuilder(DatafileProjectConfig::class) @@ -456,7 +456,7 @@ public function testIsUserInExperimentWithComplexAudience() ->will($this->returnValue($config->getAudience('3468206643'))); $this->assertTrue( - Validator::isUserInExperiment( + Validator::doesUserMeetAudienceConditions( $configMock, $experiment, ['should_do_it' => true, 'house' => 'foo'], From 45e7ed89dcdccad0333e558c7af6aba022f6ec52 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Tue, 30 Jun 2020 19:10:52 +0500 Subject: [PATCH 06/10] break up log messages --- ...s.php => CommonAudienceEvaluationLogs.php} | 4 +--- .../ExperimentAudienceEvaluationLogs.php | 24 +++++++++++++++++++ .../Enums/RolloutAudienceEvaluationLogs.php | 24 +++++++++++++++++++ .../CustomAttributeConditionEvaluator.php | 4 ++-- src/Optimizely/Utils/Validator.php | 22 +++++++++-------- 5 files changed, 63 insertions(+), 15 deletions(-) rename src/Optimizely/Enums/{AudienceEvaluationLogs.php => CommonAudienceEvaluationLogs.php} (90%) create mode 100644 src/Optimizely/Enums/ExperimentAudienceEvaluationLogs.php create mode 100644 src/Optimizely/Enums/RolloutAudienceEvaluationLogs.php diff --git a/src/Optimizely/Enums/AudienceEvaluationLogs.php b/src/Optimizely/Enums/CommonAudienceEvaluationLogs.php similarity index 90% rename from src/Optimizely/Enums/AudienceEvaluationLogs.php rename to src/Optimizely/Enums/CommonAudienceEvaluationLogs.php index c8c12c58..6a83e15e 100644 --- a/src/Optimizely/Enums/AudienceEvaluationLogs.php +++ b/src/Optimizely/Enums/CommonAudienceEvaluationLogs.php @@ -17,11 +17,9 @@ namespace Optimizely\Enums; -class AudienceEvaluationLogs +class CommonAudienceEvaluationLogs { const AUDIENCE_EVALUATION_RESULT = "Audience \"%s\" evaluated to %s."; - const AUDIENCE_EVALUATION_RESULT_COMBINED = "Audiences for %s collectively evaluated to %s."; - const EVALUATING_AUDIENCES_COMBINED = "Evaluating audiences for %s: %s."; const EVALUATING_AUDIENCE = "Starting to evaluate audience \"%s\" with conditions: %s."; const INFINITE_ATTRIBUTE_VALUE = "Audience condition %s evaluated to UNKNOWN because the number value for user attribute \"%s\" is not in the range [-2^53, +2^53]."; const MISSING_ATTRIBUTE_VALUE = "Audience condition %s evaluated to UNKNOWN because no value was passed for user attribute \"%s\"."; diff --git a/src/Optimizely/Enums/ExperimentAudienceEvaluationLogs.php b/src/Optimizely/Enums/ExperimentAudienceEvaluationLogs.php new file mode 100644 index 00000000..00d7021a --- /dev/null +++ b/src/Optimizely/Enums/ExperimentAudienceEvaluationLogs.php @@ -0,0 +1,24 @@ +getKey()}\""; + $loggingClass = 'Optimizely\Enums\ExperimentAudienceEvaluationLogs'; + $loggingStr = $experiment->getKey(); if ($isRollout) { - $loggingStr = "rule {$rolloutRule}"; + $loggingClass = 'Optimizely\Enums\RolloutAudienceEvaluationLogs'; + $loggingStr = $rolloutRule; } $audienceConditions = $experiment->getAudienceConditions(); @@ -156,7 +158,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use } $logger->log(Logger::DEBUG, sprintf( - AudienceEvaluationLogs::EVALUATING_AUDIENCES_COMBINED, + $loggingClass::EVALUATING_AUDIENCES_COMBINED, $loggingStr, json_encode($audienceConditions) )); @@ -164,7 +166,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use // Return true if experiment is not targeted to any audience. if (empty($audienceConditions)) { $logger->log(Logger::INFO, sprintf( - AudienceEvaluationLogs::AUDIENCE_EVALUATION_RESULT_COMBINED, + $loggingClass::AUDIENCE_EVALUATION_RESULT_COMBINED, $loggingStr, 'TRUE' )); @@ -180,14 +182,14 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use return $customAttrCondEval->evaluate($leafCondition); }; - $evaluateAudience = function ($audienceId) use ($config, $evaluateCustomAttr, $logger) { + $evaluateAudience = function ($audienceId) use ($config, $evaluateCustomAttr, $logger, $loggingClass) { $audience = $config->getAudience($audienceId); if ($audience === null) { return null; } $logger->log(Logger::DEBUG, sprintf( - AudienceEvaluationLogs::EVALUATING_AUDIENCE, + $loggingClass::EVALUATING_AUDIENCE, $audienceId, json_encode($audience->getConditionsList()) )); @@ -197,7 +199,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use $resultStr = $result === null ? 'UNKNOWN' : strtoupper(var_export($result, true)); $logger->log(Logger::DEBUG, sprintf( - AudienceEvaluationLogs::AUDIENCE_EVALUATION_RESULT, + $loggingClass::AUDIENCE_EVALUATION_RESULT, $audienceId, $resultStr )); @@ -210,7 +212,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use $evalResult = $evalResult || false; $logger->log(Logger::INFO, sprintf( - AudienceEvaluationLogs::AUDIENCE_EVALUATION_RESULT_COMBINED, + $loggingClass::AUDIENCE_EVALUATION_RESULT_COMBINED, $loggingStr, strtoupper(var_export($evalResult, true)) )); From 16b4f6d4335944b56fabc6d29a8b8e8d4ddb3c11 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 1 Jul 2020 14:14:49 +0500 Subject: [PATCH 07/10] address comments --- .../DecisionService/DecisionService.php | 4 +-- src/Optimizely/Utils/Validator.php | 26 +++++++++---------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 6af08e6e..5b07c398 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -313,7 +313,7 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon $rolloutRule = $rolloutRules[$i]; // Evaluate if user meets the audience condition of this rollout rule - if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, $i + 1)) { + if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, 'Optimizely\Enums\RolloutAudienceEvaluationLogs', $i + 1)) { $this->_logger->log( Logger::DEBUG, sprintf("User '%s' does not meet conditions for targeting rule %s.", $userId, $i+1) @@ -333,7 +333,7 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon $rolloutRule = $rolloutRules[sizeof($rolloutRules)-1]; // Evaluate if user meets the audience condition of Everyone Else Rule / Last Rule now - if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, true, 'Everyone Else')) { + if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, 'Optimizely\Enums\RolloutAudienceEvaluationLogs', 'Everyone Else')) { $this->_logger->log( Logger::DEBUG, sprintf("User '%s' does not meet conditions for targeting rule 'Everyone Else'.", $userId) diff --git a/src/Optimizely/Utils/Validator.php b/src/Optimizely/Utils/Validator.php index ed54df77..e76cc0bb 100644 --- a/src/Optimizely/Utils/Validator.php +++ b/src/Optimizely/Utils/Validator.php @@ -20,8 +20,6 @@ use Monolog\Logger; use Optimizely\Config\ProjectConfigInterface; use Optimizely\Entity\Experiment; -use Optimizely\Enums\ExperimentAudienceEvaluationLogs; -use Optimizely\Enums\RolloutAudienceEvaluationLogs; use Optimizely\Logger\LoggerInterface; use Optimizely\Utils\ConditionTreeEvaluator; use Optimizely\Utils\CustomAttributeConditionEvaluator; @@ -134,22 +132,22 @@ public static function areEventTagsValid($eventTags) /** * @param $config ProjectConfigInterface Configuration for the project. - * @param $experiment Experiment Entity representing the experiment. + * @param $experiment Experiment Entity representing the experiment or rollout rule. * @param $userAttributes array Attributes of the user. * @param $logger LoggerInterface. - * @param $isRollout Boolean true if experiment to evaluate is a rollout rule. - * @param $rolloutRule String Rollout rule identifier to log. + * @param $loggingClass String Class holding log strings with placeholders. + * @param $loggingKey String Identifier of an experiment/rollout rule. * * @return boolean Representing whether user meets audience conditions to be in experiment or not. */ - public static function doesUserMeetAudienceConditions($config, $experiment, $userAttributes, $logger, $isRollout = null, $rolloutRule = null) + public static function doesUserMeetAudienceConditions($config, $experiment, $userAttributes, $logger, $loggingClass = null, $loggingKey = null) { - $loggingClass = 'Optimizely\Enums\ExperimentAudienceEvaluationLogs'; - $loggingStr = $experiment->getKey(); + if ($loggingClass === null) { + $loggingClass = 'Optimizely\Enums\ExperimentAudienceEvaluationLogs'; + } - if ($isRollout) { - $loggingClass = 'Optimizely\Enums\RolloutAudienceEvaluationLogs'; - $loggingStr = $rolloutRule; + if ($loggingKey === null) { + $loggingKey = $experiment->getKey(); } $audienceConditions = $experiment->getAudienceConditions(); @@ -159,7 +157,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use $logger->log(Logger::DEBUG, sprintf( $loggingClass::EVALUATING_AUDIENCES_COMBINED, - $loggingStr, + $loggingKey, json_encode($audienceConditions) )); @@ -167,7 +165,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use if (empty($audienceConditions)) { $logger->log(Logger::INFO, sprintf( $loggingClass::AUDIENCE_EVALUATION_RESULT_COMBINED, - $loggingStr, + $loggingKey, 'TRUE' )); return true; @@ -213,7 +211,7 @@ public static function doesUserMeetAudienceConditions($config, $experiment, $use $logger->log(Logger::INFO, sprintf( $loggingClass::AUDIENCE_EVALUATION_RESULT_COMBINED, - $loggingStr, + $loggingKey, strtoupper(var_export($evalResult, true)) )); From 6b9a7a6c3d83b711e6a0d0201bfa65516a51f5fe Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 8 Jul 2020 18:51:15 +0500 Subject: [PATCH 08/10] first comment --- src/Optimizely/Optimizely.php | 4 ++-- tests/OptimizelyTest.php | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index da7c44aa..fafd0725 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -954,8 +954,8 @@ private function getFeatureVariableValueFromVariation($featureFlagKey, $variable } else { $this->_logger->log( Logger::INFO, - "Feature '{$featureFlagKey}' for variation '{$variation->getKey()}' is not enabled, ". - "returning default value '{$variableValue}'." + "Feature '{$featureFlagKey}' is not enabled for user '{$userId}'. ". + "Returning the default variable value '{$variableValue}'." ); } } diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index 5225df49..c12e385e 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -3634,8 +3634,8 @@ public function testGetFeatureVariableValueForTypeReturnDefaultVariableValueGive ->method('log') ->with( Logger::INFO, - "Feature 'double_single_variable_feature' for variation 'control' is not enabled, ". - "returning default value '14.99'." + "Feature 'double_single_variable_feature' is not enabled for user 'user_id'. ". + "Returning the default variable value '14.99'." ); $this->assertSame( @@ -3714,8 +3714,8 @@ public function testGetFeatureVariableValueReturnDefaultVariableValueGivenUserIn ->method('log') ->with( Logger::INFO, - "Feature 'boolean_single_variable_feature' for variation '177771' is not enabled, ". - "returning default value 'true'." + "Feature 'boolean_single_variable_feature' is not enabled for user 'user_id'. ". + "Returning the default variable value 'true'." ); $this->assertTrue($this->optimizelyObject->getFeatureVariableBoolean('boolean_single_variable_feature', 'boolean_variable', 'user_id', [])); From ea3fa488356ece49f6922231208b8b68a9357231 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 10 Jul 2020 11:47:42 +0500 Subject: [PATCH 09/10] nits --- .../DecisionService/DecisionService.php | 2 +- tests/UtilsTests/ValidatorTest.php | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 5b07c398..f4c7d410 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -330,7 +330,7 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon break; } // Evaluate Everyone Else Rule / Last Rule now - $rolloutRule = $rolloutRules[sizeof($rolloutRules)-1]; + $rolloutRule = $rolloutRules[sizeof($rolloutRules) - 1]; // Evaluate if user meets the audience condition of Everyone Else Rule / Last Rule now if (!Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, 'Optimizely\Enums\RolloutAudienceEvaluationLogs', 'Everyone Else')) { diff --git a/tests/UtilsTests/ValidatorTest.php b/tests/UtilsTests/ValidatorTest.php index 09d38e0d..a71f46b0 100644 --- a/tests/UtilsTests/ValidatorTest.php +++ b/tests/UtilsTests/ValidatorTest.php @@ -188,7 +188,7 @@ public function testAreEventTagsValidInvalidEventTags() } // test that Audience evaluation proceeds if provided attributes are empty or null. - public function testdoesUserMeetAudienceConditionsAudienceUsedInExperimentNoAttributesProvided() + public function testDoesUserMeetAudienceConditionsAudienceUsedInExperimentNoAttributesProvided() { $configMock = $this->getMockBuilder(DatafileProjectConfig::class) ->setConstructorArgs(array(DATAFILE, $this->loggerMock, new NoOpErrorHandler())) @@ -231,7 +231,7 @@ public function testdoesUserMeetAudienceConditionsAudienceUsedInExperimentNoAttr ); } - public function testdoesUserMeetAudienceConditionsAudienceMatch() + public function testDoesUserMeetAudienceConditionsAudienceMatch() { $config = new DatafileProjectConfig(DATAFILE, new NoOpLogger(), new NoOpErrorHandler()); $this->assertTrue( @@ -244,7 +244,7 @@ public function testdoesUserMeetAudienceConditionsAudienceMatch() ); } - public function testdoesUserMeetAudienceConditionsAudienceNoMatch() + public function testDoesUserMeetAudienceConditionsAudienceNoMatch() { $config = new DatafileProjectConfig(DATAFILE, new NoOpLogger(), new NoOpErrorHandler()); $this->assertFalse( @@ -258,7 +258,7 @@ public function testdoesUserMeetAudienceConditionsAudienceNoMatch() } // test that doesUserMeetAudienceConditions returns true when no audience is attached to experiment. - public function testdoesUserMeetAudienceConditionsNoAudienceUsedInExperiment() + public function testDoesUserMeetAudienceConditionsNoAudienceUsedInExperiment() { $config = new DatafileProjectConfig(DATAFILE, null, null); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -302,7 +302,7 @@ public function testdoesUserMeetAudienceConditionsNoAudienceUsedInExperiment() // test that doesUserMeetAudienceConditions returns false when some audience is attached to experiment // and user attributes do not match. - public function testdoesUserMeetAudienceConditionsSomeAudienceUsedInExperiment() + public function testDoesUserMeetAudienceConditionsSomeAudienceUsedInExperiment() { $config = new DatafileProjectConfig(DATAFILE, null, null); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -336,7 +336,7 @@ public function testdoesUserMeetAudienceConditionsSomeAudienceUsedInExperiment() } // test that doesUserMeetAudienceConditions evaluates audience when audienceConditions is an audience leaf node. - public function testdoesUserMeetAudienceConditionsWithAudienceConditionsSetToAudienceIdString() + public function testDoesUserMeetAudienceConditionsWithAudienceConditionsSetToAudienceIdString() { $config = new DatafileProjectConfig(DATAFILE, null, null); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -355,7 +355,7 @@ public function testdoesUserMeetAudienceConditionsWithAudienceConditionsSetToAud ); } - public function testdoesUserMeetAudienceConditionsWithUnknownAudienceId() + public function testDoesUserMeetAudienceConditionsWithUnknownAudienceId() { $config = new DatafileProjectConfig(DATAFILE, $this->loggerMock, new NoOpErrorHandler()); $experiment = $config->getExperimentFromKey('test_experiment'); @@ -376,7 +376,7 @@ public function testdoesUserMeetAudienceConditionsWithUnknownAudienceId() } // test that doesUserMeetAudienceConditions evaluates simple audience. - public function testdoesUserMeetAudienceConditionsWithSimpleAudience() + public function testDoesUserMeetAudienceConditionsWithSimpleAudience() { $config = new DatafileProjectConfig(DATAFILE, null, null); $configMock = $this->getMockBuilder(DatafileProjectConfig::class) @@ -409,7 +409,7 @@ public function testdoesUserMeetAudienceConditionsWithSimpleAudience() } // test that doesUserMeetAudienceConditions evaluates complex audience. - public function testdoesUserMeetAudienceConditionsWithComplexAudience() + public function testDoesUserMeetAudienceConditionsWithComplexAudience() { $config = new DatafileProjectConfig(DATAFILE_WITH_TYPED_AUDIENCES, null, null); $configMock = $this->getMockBuilder(DatafileProjectConfig::class) From f4b577e87ac7ef0a7bbec06ddd8601353145b39b Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 10 Jul 2020 12:15:30 +0500 Subject: [PATCH 10/10] update logs --- src/Optimizely/Optimizely.php | 9 ++++----- tests/OptimizelyTest.php | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index fafd0725..222cf750 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -931,7 +931,7 @@ private function getFeatureVariableValueFromVariation($featureFlagKey, $variable if ($variation === null) { $this->_logger->log( Logger::INFO, - "User '{$userId}'is not in any variation, ". + "User '{$userId}' is not in experiment or rollout, ". "returning default value '{$variableValue}'." ); } else { @@ -941,14 +941,13 @@ private function getFeatureVariableValueFromVariation($featureFlagKey, $variable $variableValue = $variableUsage->getValue(); $this->_logger->log( Logger::INFO, - "Returning variable value '{$variableValue}' for variation '{$variation->getKey()}' ". - "of feature flag '{$featureFlagKey}'" + "Returning variable value '{$variableValue}' for variable key '{$variableKey}' ". + "of feature flag '{$featureFlagKey}'." ); } else { $this->_logger->log( Logger::INFO, - "Variable '{$variableKey}' is not used in variation '{$variation->getKey()}', ". - "returning default value '{$variableValue}'." + "Variable value is not defined. Returning the default variable value '{$variableValue}'." ); } } else { diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index c12e385e..04eff148 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -3558,7 +3558,7 @@ public function testGetFeatureVariableValueForTypeGivenFeatureFlagIsNotEnabledFo ->method('log') ->with( Logger::INFO, - "User 'user_id'is not in any variation, returning default value '14.99'." + "User 'user_id' is not in experiment or rollout, returning default value '14.99'." ); $this->assertSame( @@ -3595,8 +3595,8 @@ public function testGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUs ->method('log') ->with( Logger::INFO, - "Returning variable value '42.42' for variation 'control' ". - "of feature flag 'double_single_variable_feature'" + "Returning variable value '42.42' for variable key 'double_variable' ". + "of feature flag 'double_single_variable_feature'." ); $this->assertSame( @@ -3675,8 +3675,8 @@ public function testGetFeatureVariableValueForTypeWithRolloutRule() ->method('log') ->with( Logger::INFO, - "Returning variable value 'true' for variation '177771' ". - "of feature flag 'boolean_single_variable_feature'" + "Returning variable value 'true' for variable key 'boolean_variable' ". + "of feature flag 'boolean_single_variable_feature'." ); $this->assertTrue($this->optimizelyObject->getFeatureVariableBoolean('boolean_single_variable_feature', 'boolean_variable', 'user_id', [])); @@ -3751,7 +3751,7 @@ public function testGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUs ->method('log') ->with( Logger::INFO, - "Variable 'double_variable' is not used in variation 'control', returning default value '14.99'." + "Variable value is not defined. Returning the default variable value '14.99'." ); $this->assertSame( @@ -3793,11 +3793,11 @@ public function testGetFeatureVariableValueForTypeWithEmptyUserID() ->method('log') ->with( Logger::INFO, - "User ''is not in any variation, returning default value '14.99'." + "User 'test_user' is not in experiment or rollout, returning default value '14.99'." ); $this->assertSame( - $this->optimizelyObject->getFeatureVariableValueForType('double_single_variable_feature', 'double_variable', '', [], 'double'), + $this->optimizelyObject->getFeatureVariableValueForType('double_single_variable_feature', 'double_variable', 'test_user', [], 'double'), 14.99 ); } @@ -4254,8 +4254,8 @@ public function testGetAllFeatureVariablesGivenFeatureFlagIsEnabledForUserAndVar ->method('log') ->with( Logger::INFO, - "Returning variable value '42.42' for variation 'control' ". - "of feature flag 'double_single_variable_feature'" + "Returning variable value '42.42' for variable key 'double_variable'". + " of feature flag 'double_single_variable_feature'." ); $this->assertSame(