From 8607ff266521c2be0532e088ceda07105f876ce1 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 21 Sep 2021 19:25:50 +0500 Subject: [PATCH 01/28] Work in progress --- .../Config/DatafileProjectConfig.php | 9 +- .../DecisionService/DecisionService.php | 52 +++++++--- src/Optimizely/Optimizely.php | 8 ++ src/Optimizely/OptimizelyUserContext.php | 95 ++++++++++++++++++- 4 files changed, 149 insertions(+), 15 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index 9663a924..e5320b45 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -245,6 +245,7 @@ class DatafileProjectConfig implements ProjectConfigInterface */ private $_sendFlagDecisions; + private $_flagVariationsMap; /** * DatafileProjectConfig constructor to load and set project configuration data. * @@ -376,7 +377,7 @@ public function __construct($datafile, $logger, $errorHandler) } } } - + $_flagVariationsMap // Add variations for rollout experiments to variationIdMap and variationKeyMap $this->_variationIdMap = $this->_variationIdMap + $rolloutVariationIdMap; $this->_variationKeyMap = $this->_variationKeyMap + $rolloutVariationKeyMap; @@ -404,6 +405,12 @@ public function __construct($datafile, $logger, $errorHandler) } } + function getAllRulesForFlag(_ flag: FeatureFlag) -> [Experiment] { + var rules = flag.experimentIds.compactMap { experimentIdMap[$0] } +let rollout = self.rolloutIdMap[flag.rolloutId] + rules.append(contentsOf: rollout?.experiments ?? []) + return rules + } /** * Create ProjectConfig based on datafile string. * diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 9ab91c0f..08d8f60a 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -28,6 +28,7 @@ use Optimizely\Enums\ControlAttributes; use Optimizely\Logger\LoggerInterface; use Optimizely\Optimizely; +use Optimizely\OptimizelyUserContext; use Optimizely\UserProfile\Decision; use Optimizely\UserProfile\UserProfileServiceInterface; use Optimizely\UserProfile\UserProfile; @@ -118,16 +119,17 @@ protected function getBucketingId($userId, $userAttributes) * Determine which variation to show the user. * * @param $projectConfig ProjectConfigInterface ProjectConfigInterface instance. - * @param $experiment Experiment Experiment to get the variation for. - * @param $userId string User identifier. - * @param $attributes array Attributes of the user. - * @param $decideOptions array Options to customize evaluation. + * @param $experiment Experiment Experiment to get the variation for. + * @param $user OptimizelyUserContext User identifier. + * @param $decideOptions array Options to customize evaluation. * * @return [ Variation, array ] Variation which the user is bucketed into and array of log messages representing decision making. */ - public function getVariation(ProjectConfigInterface $projectConfig, Experiment $experiment, $userId, $attributes = null, $decideOptions = []) + public function getVariation(ProjectConfigInterface $projectConfig, Experiment $experiment, OptimizelyUserContext $user, $decideOptions = []) { $decideReasons = []; + $userId = $user->getUserId(); + $attributes = $user->getAttributes(); list($bucketingId, $reasons) = $this->getBucketingId($userId, $attributes); $decideReasons = array_merge($decideReasons, $reasons); @@ -219,7 +221,7 @@ public function getVariation(ProjectConfigInterface $projectConfig, Experiment $ * * @return FeatureDecision representing decision. */ - public function getVariationForFeature(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, $userId, $userAttributes, $decideOptions = []) + public function getVariationForFeature(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user, $decideOptions = []) { $decideReasons = []; @@ -228,7 +230,7 @@ public function getVariationForFeature(ProjectConfigInterface $projectConfig, Fe //2. Attempt to bucket user into rollout using the feature flag. // Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments - $decision = $this->getVariationForFeatureExperiment($projectConfig, $featureFlag, $userId, $userAttributes, $decideOptions); + $decision = $this->getVariationForFeatureExperiment($projectConfig, $featureFlag, $user, $decideOptions); if ($decision->getVariation()) { return $decision; } @@ -236,7 +238,7 @@ public function getVariationForFeature(ProjectConfigInterface $projectConfig, Fe $decideReasons = array_merge($decideReasons, $decision->getReasons()); // Check if the feature flag has rollout and the user is bucketed into one of it's rules - $decision = $this->getVariationForFeatureRollout($projectConfig, $featureFlag, $userId, $userAttributes); + $decision = $this->getVariationForFeatureRollout($projectConfig, $featureFlag, $user); $decideReasons = array_merge($decideReasons, $decision->getReasons()); if ($decision->getVariation()) { @@ -272,7 +274,7 @@ public function getVariationForFeature(ProjectConfigInterface $projectConfig, Fe * * @return FeatureDecision representing decision. */ - public function getVariationForFeatureExperiment(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, $userId, $userAttributes, $decideOptions = []) + public function getVariationForFeatureExperiment(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user, $decideOptions = []) { $decideReasons = []; $featureFlagKey = $featureFlag->getKey(); @@ -297,7 +299,7 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project continue; } - list($variation, $reasons) = $this->getVariation($projectConfig, $experiment, $userId, $userAttributes, $decideOptions); + list($variation, $reasons) = $this->getVariation($projectConfig, $experiment, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); if ($variation && $variation->getKey()) { $message = "The user '{$userId}' is bucketed into experiment '{$experiment->getKey()}' of feature '{$featureFlagKey}'."; @@ -332,10 +334,10 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project * @param array $userAttributes user userAttributes * @return FeatureDecision representing decision. */ - public function getVariationForFeatureRollout(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, $userId, $userAttributes) + public function getVariationForFeatureRollout(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user) { $decideReasons = []; - list($bucketing_id, $reasons) = $this->getBucketingId($userId, $userAttributes); +// list($bucketing_id, $reasons) = $this->getBucketingId($userId, $userAttributes); $featureFlagKey = $featureFlag->getKey(); $rollout_id = $featureFlag->getRolloutId(); if (empty($rollout_id)) { @@ -357,7 +359,11 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon if (sizeof($rolloutRules) == 0) { return new FeatureDecision(null, null, null, $decideReasons); } + $index = 0; + while($index < sizeof($rolloutRules)) { + $decisionResponse = + } // Evaluate all rollout rules except for last one for ($i = 0; $i < sizeof($rolloutRules) - 1; $i++) { $rolloutRule = $rolloutRules[$i]; @@ -408,6 +414,28 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon return new FeatureDecision(null, null, null, $decideReasons); } + /** + * Gets the forced variation key for the given user and experiment. + * + * @param $projectConfig ProjectConfigInterface ProjectConfigInterface instance. + * @param $experimentKey string Key for experiment. + * @param $userId string The user Id. + * + * @return [ Variation, array ] The variation which the given user and experiment should be forced into and + * array of log messages representing decision making. + */ + public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConfig, $flagKey, array $rules, $ruleIndex, $user, array $options = []) + { + $decideReasons = []; + $skipToEveryoneElse = false; + // check forced-decision first + $rule = rules[ruleIndex]; + list($forcedDecisionResponse, $reasons) = user.findValidatedForcedDecision($flagKey, $rule->getKey(), $options); + + $decideReasons = array_merge($decideReasons, $reasons); + + + } /** * Gets the forced variation key for the given user and experiment. diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 5dcacbe6..a0ef1096 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -1281,4 +1281,12 @@ protected function validateInputs(array $values, $logLevel = Logger::ERROR) return $isValid; } + + public function getFlagVariationByKey($flagKey, $variationKey) + { + if(array_key_exists($flagKey, $this->_config)) + { + + } + } } diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index c1b50264..75fb7347 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -22,8 +22,8 @@ class OptimizelyUserContext implements \JsonSerializable private $optimizelyClient; private $userId; private $attributes; - - + private $forcedDecisions; + public function __construct(Optimizely $optimizelyClient, $userId, array $attributes = []) { $this->optimizelyClient = $optimizelyClient; @@ -31,6 +31,60 @@ public function __construct(Optimizely $optimizelyClient, $userId, array $attrib $this->attributes = $attributes; } + public function setForcedDecision($flagKey, $ruleKey = null, $variationKey) + { + $index = $this->findExisitingRuleAndFlagKey($flagKey, $ruleKey); + if($index != -1) + { + $this->forcedDecisions[$index]->variationKey = $variationKey; + } else { + array_push($this->forcedDecisions, new ForcedDecision($flagKey,$ruleKey, $variationKey)); + } + return true; + } + + public function getForcedDecision($flagKey, $ruleKey = null) + { + return findForcedDecision($flagKey, $ruleKey); + } + + private function findValidatedForcedDecision($flagKey, $ruleKey, array $options = []) + { + $decideReasons = []; + $variationKey = $this->findForcedDecision($flagKey, $ruleKey); + if($variationKey != null) + { + $variation = ($this->optimizelyClient)->getF + if() + } + } + private function findExistingRuleAndFlagKey($flagKey, $ruleKey) + { + for ($index = 0; count($this->forcedDecisions); $index++) + { + if ($this->forcedDecisions[$index]->getFlagKey() == $flagKey && $this->forcedDecisions[$index]->getRuleKey() == $ruleKey) { + return $index; + } + } + return -1; + } + + private function findForcedDecision($flagKey, $ruleKey) + { + if(count($this->forcedDecisions) == 0) + { + return null; + } + + $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); + + if ($index != -1) + { + return $this->forcedDecisions[$index]->getVariationKey(); + } + + return null; + } protected function copy() { return new OptimizelyUserContext($this->optimizelyClient, $this->userId, $this->attributes); @@ -85,3 +139,40 @@ public function jsonSerialize() ]; } } +class ForcedDecision +{ + private $flagKey; + private $ruleKey; + public $variationKey; + + public function __construct($flagKey, $ruleKey, $variationKey) + { + $this->flagKey = $flagKey; + $this->ruleKey = $ruleKey; + $this->variationKey = $variationKey; + } + + /** + * @return mixed + */ + public function getFlagKey() + { + return $this->flagKey; + } + + /** + * @return mixed + */ + public function getRuleKey() + { + return $this->ruleKey; + } + + /** + * @return mixed + */ + public function getVariationKey() + { + return $this->variationKey; + } +} From e013feadb68ce880fc435e2df1173aa06a893c14 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 21 Sep 2021 19:34:16 +0500 Subject: [PATCH 02/28] fix --- src/Optimizely/Config/DatafileProjectConfig.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index e5320b45..a80f1dbc 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -405,11 +405,11 @@ public function __construct($datafile, $logger, $errorHandler) } } - function getAllRulesForFlag(_ flag: FeatureFlag) -> [Experiment] { - var rules = flag.experimentIds.compactMap { experimentIdMap[$0] } -let rollout = self.rolloutIdMap[flag.rolloutId] - rules.append(contentsOf: rollout?.experiments ?? []) - return rules + function getAllRulesForFlag(FeatureFlag $flag) { + $rules = $flag->getExperimentIds(); + $rollout = $this->_rolloutIdMap[$flag->getRolloutId()]; + array_push($rules , rollout); + return rules; } /** * Create ProjectConfig based on datafile string. From 0b1f517acc55deaacf0da62ae550bc5c1da68c6f Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 22 Sep 2021 21:32:25 +0500 Subject: [PATCH 03/28] implemented forcedDecision --- .../Config/DatafileProjectConfig.php | 32 ++++- .../DecisionService/DecisionService.php | 117 +++++++++++------- src/Optimizely/Optimizely.php | 33 +++-- src/Optimizely/OptimizelyUserContext.php | 14 ++- 4 files changed, 134 insertions(+), 62 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index a80f1dbc..2a7b2dc7 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -377,7 +377,19 @@ public function __construct($datafile, $logger, $errorHandler) } } } - $_flagVariationsMap + $this->_flagVariationsMap = array(); + foreach ($this->_featureFlags as $flag) + { + $variations = array(); + + foreach ($this->getAllRulesForFlag($flag) as $rule) + { + $variations = array_filter(array_values($rule->getVariations()), function ($variation) use ($variations) { + return !in_array($variation->getId(), $variations); + }); + } + $this->_flagVariationsMap[$flag->getKey()] = $variations; + } // Add variations for rollout experiments to variationIdMap and variationKeyMap $this->_variationIdMap = $this->_variationIdMap + $rolloutVariationIdMap; $this->_variationKeyMap = $this->_variationKeyMap + $rolloutVariationKeyMap; @@ -406,9 +418,13 @@ public function __construct($datafile, $logger, $errorHandler) } function getAllRulesForFlag(FeatureFlag $flag) { - $rules = $flag->getExperimentIds(); + $rules = array(); + foreach (flag.experimentIds as $experimentId) + { + array_push($rules, $this->_experimentIdMap[$experimentId]); + } $rollout = $this->_rolloutIdMap[$flag->getRolloutId()]; - array_push($rules , rollout); + array_push($rules, $rollout->getExperiments()); return rules; } /** @@ -875,6 +891,16 @@ public function isFeatureExperiment($experimentId) return array_key_exists($experimentId, $this->_experimentFeatureMap); } + /** + * Returns map array of Flag key as key and Variations as value + * + * @return array + */ + public function getFlagVariationsMap() + { + return $this->_flagVariationsMap; + } + /** * Returns if flag decisions should be sent to server or not * diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 08d8f60a..44b54b65 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -26,6 +26,7 @@ use Optimizely\Entity\Rollout; use Optimizely\Entity\Variation; use Optimizely\Enums\ControlAttributes; +use Optimizely\ForcedDecision; use Optimizely\Logger\LoggerInterface; use Optimizely\Optimizely; use Optimizely\OptimizelyUserContext; @@ -240,7 +241,7 @@ public function getVariationForFeature(ProjectConfigInterface $projectConfig, Fe // Check if the feature flag has rollout and the user is bucketed into one of it's rules $decision = $this->getVariationForFeatureRollout($projectConfig, $featureFlag, $user); $decideReasons = array_merge($decideReasons, $decision->getReasons()); - + $userId = $user->getUserId(); if ($decision->getVariation()) { $message = "User '{$userId}' is bucketed into rollout for feature flag '{$featureFlag->getKey()}'."; $this->_logger->log( @@ -290,7 +291,7 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project $decideReasons[] = $message; return new FeatureDecision(null, null, null, $decideReasons); } - + $userId = $user->getUserId(); // Evaluate each experiment ID and return the first bucketed experiment variation foreach ($experimentIds as $experiment_id) { $experiment = $projectConfig->getExperimentFromId($experiment_id); @@ -334,7 +335,7 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project * @param array $userAttributes user userAttributes * @return FeatureDecision representing decision. */ - public function getVariationForFeatureRollout(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user) + public function getVariationForFeatureRollout(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user, $decideOptions = []) { $decideReasons = []; // list($bucketing_id, $reasons) = $this->getBucketingId($userId, $userAttributes); @@ -361,59 +362,41 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon } $index = 0; while($index < sizeof($rolloutRules)) { - $decisionResponse = - - } - // Evaluate all rollout rules except for last one - for ($i = 0; $i < sizeof($rolloutRules) - 1; $i++) { - $rolloutRule = $rolloutRules[$i]; - - // Evaluate if user meets the audience condition of this rollout rule - list($evalResult, $reasons) = Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, 'Optimizely\Enums\RolloutAudienceEvaluationLogs', $i + 1); + list($decisionResponse, $reasons) = $this->getVariationFromDeliveryRule($projectConfig, $featureFlagKey, $rolloutRules, $index, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); - if (!$evalResult) { - $message = sprintf("User '%s' does not meet conditions for targeting rule %s.", $userId, $i+1); - $this->_logger->log( - Logger::DEBUG, - $message - ); - $decideReasons[] = $message; - // Evaluate this user for the next rule - continue; + list($variation, $skipToEveryoneElse) = $decisionResponse == null? [null, true] : [$decisionResponse, false]; + if($variation != null) { + return new FeatureDecision($rolloutRules[$index], $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT, $decideReasons); } + // the last rule is special for "Everyone Else" + $index = $skipToEveryoneElse ? (sizeof($rolloutRules) - 1) : ($index + 1); + } + return new FeatureDecision(null, null, null, $decideReasons); + } - // Evaluate if user satisfies the traffic allocation for this rollout rule - list($variation, $reasons) = $this->_bucketer->bucket($projectConfig, $rolloutRule, $bucketing_id, $userId); - $decideReasons = array_merge($decideReasons, $reasons); - if ($variation && $variation->getKey()) { - return new FeatureDecision($rolloutRule, $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT, $decideReasons); - } - break; + private function getVariationFromExperimentRule(ProjectConfigInterface $projectConfig, $flagKey, Experiment $rule, OptimizelyUserContext $user, $decideOptions = []) + { + $decideReasons = []; + // check forced-decision first + list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $this->_logger); + $decideReasons = array_merge($decideReasons, $reasons); + if($decisionResponse != null) { + return new FeatureDecision(null, $decisionResponse, FeatureDecision::DECISION_SOURCE_EXPERIMENT, $decideReasons); } - // Evaluate Everyone Else Rule / Last Rule now - $rolloutRule = $rolloutRules[sizeof($rolloutRules) - 1]; - // Evaluate if user meets the audience condition of Everyone Else Rule / Last Rule now - list($evalResult, $reasons) = Validator::doesUserMeetAudienceConditions($projectConfig, $rolloutRule, $userAttributes, $this->_logger, 'Optimizely\Enums\RolloutAudienceEvaluationLogs', 'Everyone Else'); + // regular decision + list($variation, $reasons) = $this->getVariation($projectConfig, $rule, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); - if (!$evalResult) { - $message = sprintf("User '%s' does not meet conditions for targeting rule 'Everyone Else'.", $userId); + if ($variation && $variation->getKey()) { + $message = "The user '{$user->getUserId()}' is bucketed into experiment '{$rule->getKey()}' of feature '{$flagKey}'."; $this->_logger->log( - Logger::DEBUG, + Logger::INFO, $message ); $decideReasons[] = $message; - return new FeatureDecision(null, null, null, $decideReasons); + return new FeatureDecision($rule, $variation, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); } - - list($variation, $reasons) = $this->_bucketer->bucket($projectConfig, $rolloutRule, $bucketing_id, $userId); - $decideReasons = array_merge($decideReasons, $reasons); - if ($variation && $variation->getKey()) { - return new FeatureDecision($rolloutRule, $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT); - } - return new FeatureDecision(null, null, null, $decideReasons); } - /** * Gets the forced variation key for the given user and experiment. * @@ -424,17 +407,57 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon * @return [ Variation, array ] The variation which the given user and experiment should be forced into and * array of log messages representing decision making. */ - public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConfig, $flagKey, array $rules, $ruleIndex, $user, array $options = []) + public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConfig, $flagKey, array $rules, $ruleIndex, OptimizelyUserContext $user, array $options = []) { $decideReasons = []; $skipToEveryoneElse = false; // check forced-decision first - $rule = rules[ruleIndex]; - list($forcedDecisionResponse, $reasons) = user.findValidatedForcedDecision($flagKey, $rule->getKey(), $options); + $rule = $rules[$ruleIndex]; + list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $options, $this->_logger); $decideReasons = array_merge($decideReasons, $reasons); + if($forcedDecisionResponse != null) { + return new FeatureDecision($rule, $forcedDecisionResponse, null, $decideReasons); + } + // regular decision + $userId = $user->getUserId(); + $attributes = $user->getAttributes(); + $bucketingId = $this->getBucketingId($userId, $attributes); + + $everyoneElse = $ruleIndex == sizeof($rules) - 1; + $loggingKey = $everyoneElse ? "Everyone Else" : $ruleIndex + 1; + + list($evalResult, $reasons) = Validator::doesUserMeetAudienceConditions($projectConfig, $rule, $attributes, $this->_logger); + $decideReasons = array_merge($decideReasons, $reasons); + if(!$evalResult) { + $message = sprintf('User "%s" meets condition for targeting rule "%s".', $userId, $rule->getKey()); + $this->_logger->log( + Logger::INFO, + $message + ); + $decideReasons[] = $message; + list($bucketedVariation, $reasons) = $this->_bucketer->bucket($projectConfig, $rule, $bucketingId, $userId); + $decideReasons = array_merge($decideReasons, $reasons); + if ($bucketedVariation != null) { + $message = sprintf('User "%s" is in the traffic group of targeting rule "%s".', $userId, $rule->getKey()); + $this->_logger->log(Logger::INFO, $message); + $decideReasons[] = $message; + } else if (!$everyoneElse) { + // skip this logging for EveryoneElse since this has a message not for EveryoneElse + $message = sprintf('User "%s" is not in the traffic group for targeting rule "%s". Checking Everyone Else rule now.', $userId, $rule->getKey()); + $this->_logger->log(Logger::INFO, $message); + $decideReasons[] = $message; + // skip the rest of rollout rules to the everyone-else rule if audience matches but not bucketed. + $skipToEveryoneElse = true; + } + } else { + $message = sprintf('User "%s" does not meet conditions for targeting rule "%s".', $userId, $rule->getKey()); + $this->_logger->log(Logger::INFO, $message); + $decideReasons[] = $message; + } + return new FeatureDecision($rule, $bucketedVariation, null, $decideReasons); } /** diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index a0ef1096..48e26b77 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -347,13 +347,22 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp $decisionEventDispatched = false; // get decision - $decision = $this->_decisionService->getVariationForFeature( - $config, - $featureFlag, - $userId, - $userAttributes, - $decideOptions - ); + $decision = null; + // check forced-decisions first + list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($flagKey, $ruleKey, $decideOptions); + $decideReasons = array_merge($decideReasons, $reasons); + if($forcedDecisionResponse != null) { + $decision = new FeatureDecision(null, $forcedDecisionResponse, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); + } else { + // regular decision + $decision = $this->_decisionService->getVariationForFeature( + $config, + $featureFlag, + $userId, + $userAttributes, + $decideOptions + ); + } $decideReasons = $decision->getReasons(); $variation = $decision->getVariation(); @@ -1284,9 +1293,15 @@ protected function validateInputs(array $values, $logLevel = Logger::ERROR) public function getFlagVariationByKey($flagKey, $variationKey) { - if(array_key_exists($flagKey, $this->_config)) + if(array_key_exists($flagKey, $this->_config->getFlagVariationsMap())) { - + $variations = array_filter(array_values($this->_config->getFlagVariationsMap()[$flagKey]), function ($variations) use ($variationKey) { + return in_array($variationKey, $variations->getKey()); + }); + if(!empty($variations)) { + return $variations[0]; + } } + return null; } } diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 75fb7347..0b67742e 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -48,15 +48,23 @@ public function getForcedDecision($flagKey, $ruleKey = null) return findForcedDecision($flagKey, $ruleKey); } - private function findValidatedForcedDecision($flagKey, $ruleKey, array $options = []) + public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = [], $logger) { $decideReasons = []; $variationKey = $this->findForcedDecision($flagKey, $ruleKey); if($variationKey != null) { - $variation = ($this->optimizelyClient)->getF - if() + $variation = ($this->optimizelyClient)->getFlagVariationByKey($flagKey, $variationKey); + $message = sprintf('Variation "%s" is mapped to "%s" and user "%s" in the forced decision map.', $variationKey, $flagKey, $this->userId); + $logger->log(Logger::INFO, $message); + $decideReasons[] = $message; + return [$variation, $decideReasons]; + } else { + $message = sprintf('Invalid variation is mapped to "%s" and user "%s" in the forced decision map.', $flagKey, $this->userId); + $logger->log(Logger::INFO, $message); + $decideReasons[] = $message; } + return [null, $decideReasons]; } private function findExistingRuleAndFlagKey($flagKey, $ruleKey) { From cd4176cd9ae44d1df9e8e910b9d9be3ce9c6b995 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 24 Sep 2021 21:54:03 +0500 Subject: [PATCH 04/28] fixed issues --- .../Config/DatafileProjectConfig.php | 22 +++--- .../DecisionService/DecisionService.php | 15 ++-- src/Optimizely/Optimizely.php | 16 ++-- src/Optimizely/OptimizelyUserContext.php | 34 ++++---- .../DecisionServiceTest.php | 78 ++++++++++--------- 5 files changed, 86 insertions(+), 79 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index 2a7b2dc7..d68c6068 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -378,12 +378,10 @@ public function __construct($datafile, $logger, $errorHandler) } } $this->_flagVariationsMap = array(); - foreach ($this->_featureFlags as $flag) - { + foreach ($this->_featureFlags as $flag) { $variations = array(); - - foreach ($this->getAllRulesForFlag($flag) as $rule) - { + $flagRules = $this->getAllRulesForFlag($flag); + foreach ($flagRules as $ruleList) { $variations = array_filter(array_values($rule->getVariations()), function ($variation) use ($variations) { return !in_array($variation->getId(), $variations); }); @@ -417,15 +415,17 @@ public function __construct($datafile, $logger, $errorHandler) } } - function getAllRulesForFlag(FeatureFlag $flag) { + private function getAllRulesForFlag(FeatureFlag $flag) + { $rules = array(); - foreach (flag.experimentIds as $experimentId) - { + foreach ($flag->getExperimentIds() as $experimentId) { array_push($rules, $this->_experimentIdMap[$experimentId]); } - $rollout = $this->_rolloutIdMap[$flag->getRolloutId()]; - array_push($rules, $rollout->getExperiments()); - return rules; + if ($this->_rolloutIdMap && key_exists($flag->getRolloutId(), $this->_rolloutIdMap)) { + $rollout = $this->_rolloutIdMap[$flag->getRolloutId()]; + $rules = array_merge($rules, $rollout->getExperiments()); + } + return $rules; } /** * Create ProjectConfig based on datafile string. diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 44b54b65..d15306ee 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -361,11 +361,11 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon return new FeatureDecision(null, null, null, $decideReasons); } $index = 0; - while($index < sizeof($rolloutRules)) { + while ($index < sizeof($rolloutRules)) { list($decisionResponse, $reasons) = $this->getVariationFromDeliveryRule($projectConfig, $featureFlagKey, $rolloutRules, $index, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); list($variation, $skipToEveryoneElse) = $decisionResponse == null? [null, true] : [$decisionResponse, false]; - if($variation != null) { + if ($variation != null) { return new FeatureDecision($rolloutRules[$index], $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT, $decideReasons); } // the last rule is special for "Everyone Else" @@ -378,9 +378,9 @@ private function getVariationFromExperimentRule(ProjectConfigInterface $projectC { $decideReasons = []; // check forced-decision first - list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $this->_logger); + list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey()); $decideReasons = array_merge($decideReasons, $reasons); - if($decisionResponse != null) { + if ($decisionResponse != null) { return new FeatureDecision(null, $decisionResponse, FeatureDecision::DECISION_SOURCE_EXPERIMENT, $decideReasons); } @@ -413,10 +413,10 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $skipToEveryoneElse = false; // check forced-decision first $rule = $rules[$ruleIndex]; - list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $options, $this->_logger); + list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $options); $decideReasons = array_merge($decideReasons, $reasons); - if($forcedDecisionResponse != null) { + if ($forcedDecisionResponse != null) { return new FeatureDecision($rule, $forcedDecisionResponse, null, $decideReasons); } @@ -427,10 +427,11 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $everyoneElse = $ruleIndex == sizeof($rules) - 1; $loggingKey = $everyoneElse ? "Everyone Else" : $ruleIndex + 1; + $bucketedVariation = null; list($evalResult, $reasons) = Validator::doesUserMeetAudienceConditions($projectConfig, $rule, $attributes, $this->_logger); $decideReasons = array_merge($decideReasons, $reasons); - if(!$evalResult) { + if (!$evalResult) { $message = sprintf('User "%s" meets condition for targeting rule "%s".', $userId, $rule->getKey()); $this->_logger->log( Logger::INFO, diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 48e26b77..c712322e 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -351,15 +351,14 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp // check forced-decisions first list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($flagKey, $ruleKey, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); - if($forcedDecisionResponse != null) { + if ($forcedDecisionResponse != null) { $decision = new FeatureDecision(null, $forcedDecisionResponse, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); } else { // regular decision $decision = $this->_decisionService->getVariationForFeature( $config, $featureFlag, - $userId, - $userAttributes, + $this->createUserContext($userId, $userAttributes), $decideOptions ); } @@ -696,7 +695,7 @@ public function getVariation($experimentKey, $userId, $attributes = null) return null; } - list($variation, $reasons) = $this->_decisionService->getVariation($config, $experiment, $userId, $attributes); + list($variation, $reasons) = $this->_decisionService->getVariation($config, $experiment, $this->createUserContext($userId, $attributes = [])); $variationKey = ($variation === null) ? null : $variation->getKey(); if ($config->isFeatureExperiment($experiment->getId())) { @@ -824,7 +823,8 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null) } $featureEnabled = false; - $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $userId, $attributes); + $attributes = $attributes? $attributes : []; + $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $this->createUserContext($userId, $attributes)); $variation = $decision->getVariation(); if ($config->getSendFlagDecisions() && ($decision->getSource() == FeatureDecision::DECISION_SOURCE_ROLLOUT || !$variation)) { @@ -957,8 +957,8 @@ public function getFeatureVariableValueForType( // Error logged in DatafileProjectConfig - getFeatureFlagFromKey return null; } - - $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $userId, $attributes); + $attributes = $attributes? $attributes : []; + $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $this->createUserContext($userId, $attributes)); $variation = $decision->getVariation(); $experiment = $decision->getExperiment(); $featureEnabled = $variation !== null ? $variation->getFeatureEnabled() : false; @@ -1133,7 +1133,7 @@ public function getAllFeatureVariables($featureFlagKey, $userId, $attributes = n return null; } - $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $userId, $attributes); + $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $this->createUserContext($userId, $attributes)); $variation = $decision->getVariation(); $experiment = $decision->getExperiment(); $featureEnabled = $variation !== null ? $variation->getFeatureEnabled() : false; diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 0b67742e..3520ac9b 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -17,6 +17,9 @@ namespace Optimizely; +use Optimizely\Logger\LoggerInterface; +use Optimizely\Logger\NoOpLogger; + class OptimizelyUserContext implements \JsonSerializable { private $optimizelyClient; @@ -31,14 +34,13 @@ public function __construct(Optimizely $optimizelyClient, $userId, array $attrib $this->attributes = $attributes; } - public function setForcedDecision($flagKey, $ruleKey = null, $variationKey) + public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) { $index = $this->findExisitingRuleAndFlagKey($flagKey, $ruleKey); - if($index != -1) - { + if ($index != -1) { $this->forcedDecisions[$index]->variationKey = $variationKey; } else { - array_push($this->forcedDecisions, new ForcedDecision($flagKey,$ruleKey, $variationKey)); + array_push($this->forcedDecisions, new ForcedDecision($flagKey, $ruleKey, $variationKey)); } return true; } @@ -48,30 +50,30 @@ public function getForcedDecision($flagKey, $ruleKey = null) return findForcedDecision($flagKey, $ruleKey); } - public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = [], $logger) + public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = []) { $decideReasons = []; $variationKey = $this->findForcedDecision($flagKey, $ruleKey); - if($variationKey != null) - { + if ($variationKey != null) { $variation = ($this->optimizelyClient)->getFlagVariationByKey($flagKey, $variationKey); $message = sprintf('Variation "%s" is mapped to "%s" and user "%s" in the forced decision map.', $variationKey, $flagKey, $this->userId); - $logger->log(Logger::INFO, $message); + $decideReasons[] = $message; return [$variation, $decideReasons]; } else { $message = sprintf('Invalid variation is mapped to "%s" and user "%s" in the forced decision map.', $flagKey, $this->userId); - $logger->log(Logger::INFO, $message); + $decideReasons[] = $message; } return [null, $decideReasons]; } private function findExistingRuleAndFlagKey($flagKey, $ruleKey) { - for ($index = 0; count($this->forcedDecisions); $index++) - { - if ($this->forcedDecisions[$index]->getFlagKey() == $flagKey && $this->forcedDecisions[$index]->getRuleKey() == $ruleKey) { - return $index; + if ($this->forcedDecisions) { + for ($index = 0; $index < count($this->forcedDecisions); $index++) { + if ($this->forcedDecisions[$index]->getFlagKey() == $flagKey && $this->forcedDecisions[$index]->getRuleKey() == $ruleKey) { + return $index; + } } } return -1; @@ -79,15 +81,13 @@ private function findExistingRuleAndFlagKey($flagKey, $ruleKey) private function findForcedDecision($flagKey, $ruleKey) { - if(count($this->forcedDecisions) == 0) - { + if ($this->forcedDecisions && count($this->forcedDecisions) == 0) { return null; } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); - if ($index != -1) - { + if ($index != -1) { return $this->forcedDecisions[$index]->getVariationKey(); } diff --git a/tests/DecisionServiceTests/DecisionServiceTest.php b/tests/DecisionServiceTests/DecisionServiceTest.php index 6c4669f8..db4446f1 100644 --- a/tests/DecisionServiceTests/DecisionServiceTest.php +++ b/tests/DecisionServiceTests/DecisionServiceTest.php @@ -28,6 +28,7 @@ use Optimizely\Logger\DefaultLogger; use Optimizely\Logger\NoOpLogger; use Optimizely\Optimizely; +use Optimizely\OptimizelyUserContext; use Optimizely\UserProfile\UserProfileServiceInterface; use Optimizely\Utils\Validator; @@ -40,7 +41,7 @@ class DecisionServiceTest extends \PHPUnit_Framework_TestCase private $loggerMock; private $testUserId; private $userProvideServiceMock; - + private $optimizely; public function setUp() { $this->testUserId = 'testUserId'; @@ -84,6 +85,7 @@ public function setUp() ->setConstructorArgs(array($this->loggerMock)) ->setMethods(array('getVariation')) ->getMock(); + $this->optimizely = new Optimizely(DATAFILE, new ValidEventDispatcher(), $this->loggerMock); } public function compareFeatureDecisionsExceptReasons(FeatureDecision $expectedObj, FeatureDecision $actualObj) @@ -104,13 +106,15 @@ public function testGetVariationReturnsNullWhenExperimentIsNotRunning() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $pausedExperiment, $this->testUserId); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $pausedExperiment, $this->optimizely->createUserContext($this->testUserId)); $this->assertNull($variation); } public function testGetVariationBucketsUserWhenExperimentIsRunning() { + $optimizely = new Optimizely(DATAFILE, new ValidEventDispatcher(), $this->loggerMock); + $expectedVariation = new Variation('7722370027', 'control'); $this->bucketerMock->expects($this->once()) ->method('bucket') @@ -122,7 +126,7 @@ public function testGetVariationBucketsUserWhenExperimentIsRunning() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->testUserId, $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($this->testUserId, $this->testUserAttributes)); $this->assertEquals( $expectedVariation, @@ -219,7 +223,7 @@ public function testGetVariationReturnsWhitelistedVariation() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, 'user1'); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext('user1')); $this->assertEquals( $expectedVariation, @@ -260,7 +264,7 @@ public function testGetVariationReturnsWhitelistedVariationForGroupedExperiment( $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, 'user1'); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext('user1')); $this->assertEquals( $expectedVariation, @@ -290,7 +294,7 @@ public function testGetVariationBucketsWhenForcedVariationsIsEmpty() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, 'user1', $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext('user1', $this->testUserAttributes)); $this->assertEquals( $expectedVariation, @@ -321,7 +325,7 @@ public function testGetVariationBucketsWhenWhitelistedVariationIsInvalid() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, 'user1', $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext('user1', $this->testUserAttributes)); $this->assertEquals( $expectedVariation, @@ -342,7 +346,7 @@ public function testGetVariationBucketsUserWhenUserIsNotWhitelisted() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, 'not_whitelisted_user', $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext('not_whitelisted_user', $this->testUserAttributes)); $this->assertEquals( $expectedVariation, @@ -352,6 +356,8 @@ public function testGetVariationBucketsUserWhenUserIsNotWhitelisted() public function testGetVariationReturnsNullIfUserDoesNotMeetAudienceConditions() { + $optimizely = new Optimizely(DATAFILE, new ValidEventDispatcher(), $this->loggerMock); + $this->bucketerMock->expects($this->never()) ->method('bucket'); @@ -361,7 +367,7 @@ public function testGetVariationReturnsNullIfUserDoesNotMeetAudienceConditions() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->testUserId); // no matching attributes + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($this->testUserId)); // no matching attributes $this->assertNull($variation); } @@ -399,12 +405,14 @@ public function testGetVariationReturnsStoredVariationIfAvailable() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $userId); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($userId)); $this->assertEquals($expectedVariation, $variation); } public function testGetVariationBucketsIfNoStoredVariation() { + $optimizely = new Optimizely(DATAFILE, new ValidEventDispatcher(), $this->loggerMock); + $userId = $this->testUserId; $runningExperiment = $this->config->getExperimentFromKey('test_experiment'); $expectedVariation = new Variation('7722370027', 'control'); @@ -443,7 +451,7 @@ public function testGetVariationBucketsIfNoStoredVariation() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $userId, $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($userId, $this->testUserAttributes)); $this->assertEquals($expectedVariation, $variation); // Verify Logs @@ -496,7 +504,7 @@ public function testGetVariationBucketsIfStoredVariationIsInvalid() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $userId, $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($userId, $this->testUserAttributes)); $this->assertEquals($expectedVariation, $variation); // Verify Logs @@ -511,6 +519,8 @@ public function testGetVariationBucketsIfStoredVariationIsInvalid() public function testGetVariationBucketsIfUserProfileServiceLookupThrows() { + $optimizely = new Optimizely(DATAFILE, new ValidEventDispatcher(), $this->loggerMock); + $userId = $this->testUserId; $runningExperiment = $this->config->getExperimentFromKey('test_experiment'); $expectedVariation = new Variation('7722370027', 'control'); @@ -553,7 +563,7 @@ public function testGetVariationBucketsIfUserProfileServiceLookupThrows() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $userId, $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($userId, $this->testUserAttributes)); $this->assertEquals($expectedVariation, $variation); // Verify Logs @@ -601,7 +611,7 @@ public function testGetVariationBucketsIfUserProfileServiceSaveThrows() $bucketer->setAccessible(true); $bucketer->setValue($this->decisionService, $this->bucketerMock); - list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $userId, $this->testUserAttributes); + list($variation, $reasons) = $this->decisionService->getVariation($this->config, $runningExperiment, $this->optimizely->createUserContext($userId, $this->testUserAttributes)); $this->assertEquals($expectedVariation, $variation); // Verify Logs @@ -724,7 +734,7 @@ public function testGetVariationForFeatureExperimentGivenNullExperimentIds() ->method('log') ->with(Logger::DEBUG, "The feature flag 'empty_feature' is not used in any experiments."); - $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, 'user1', []); + $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, $this->optimizely->createUserContext('user1', [])); $this->assertNull($actualDecision->getVariation()); } @@ -747,7 +757,7 @@ public function testGetVariationForFeatureExperimentGivenExperimentNotInDataFile "The user 'user1' is not bucketed into any of the experiments using the feature 'boolean_feature'." ); - $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, 'user1', []); + $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, $this->optimizely->createUserContext('user1', [])); $this->assertNull($actualDecision->getVariation()); } @@ -769,7 +779,7 @@ public function testGetVariationForFeatureExperimentGivenNonMutexGroupAndUserNot ); $featureFlag = $this->config->getFeatureFlagFromKey('multi_variate_feature'); - $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, 'user1', []); + $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, $this->optimizely->createUserContext('user1', [])); $this->assertNull($actualDecision->getVariation()); } @@ -793,7 +803,7 @@ public function testGetVariationForFeatureExperimentGivenNonMutexGroupAndUserIsB "The user 'user_1' is bucketed into experiment 'test_experiment_multivariate' of feature 'multi_variate_feature'." ); - $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, 'user_1', []); + $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', [])); $this->compareFeatureDecisionsExceptReasons($expected_decision, $actualDecision); } @@ -818,7 +828,7 @@ public function testGetVariationForFeatureExperimentGivenMutexGroupAndUserIsBuck "The user 'user_1' is bucketed into experiment 'group_experiment_1' of feature 'mutex_group_feature'." ); - $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, 'user_1', []); + $actualDecision = $this->decisionServiceMock->getVariationForFeatureExperiment($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', [])); $this->compareFeatureDecisionsExceptReasons($expected_decision, $actualDecision); } @@ -844,8 +854,7 @@ public function testGetVariationForFeatureExperimentGivenMutexGroupAndUserNotBuc $actualFeatureDecision = $this->decisionServiceMock->getVariationForFeatureExperiment( $this->config, $featureFlag, - 'user_1', - [] + $this->optimizely->createUserContext('user_1', []) ); $this->assertNull($actualFeatureDecision->getVariation()); } @@ -874,7 +883,7 @@ public function testGetVariationForFeatureWhenTheUserIsBucketedIntoFeatureExperi $this->assertEquals( $expected_decision, - $decisionServiceMock->getVariationForFeature($this->config, $featureFlag, 'user_1', []) + $decisionServiceMock->getVariationForFeature($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', [])) ); } @@ -912,7 +921,7 @@ public function testGetVariationForFeatureWhenBucketedToFeatureRollout() "User 'user_1' is bucketed into rollout for feature flag 'string_single_variable_feature'." ); - $actualFeatureDecision = $decisionServiceMock->getVariationForFeature($this->config, $featureFlag, 'user_1', []); + $actualFeatureDecision = $decisionServiceMock->getVariationForFeature($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', [])); $this->compareFeatureDecisionsExceptReasons($expected_decision, $actualFeatureDecision); } @@ -948,7 +957,7 @@ public function testGetVariationForFeatureWhenTheUserIsNeitherBucketedIntoFeatur FeatureDecision::DECISION_SOURCE_ROLLOUT ); - $actualFeatureDecision = $decisionServiceMock->getVariationForFeature($this->config, $featureFlag, 'user_1', []); + $actualFeatureDecision = $decisionServiceMock->getVariationForFeature($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', [])); $this->compareFeatureDecisionsExceptReasons($expectedDecision, $actualFeatureDecision); } @@ -969,8 +978,7 @@ public function testGetVariationForFeatureRolloutWhenNoRolloutIsAssociatedToFeat $actualFeatureDecision = $this->decisionServiceMock->getVariationForFeatureRollout( $this->config, $featureFlag, - 'user_1', - [] + $this->optimizely->createUserContext('user_1', []) ); $this->assertNull($actualFeatureDecision->getVariation()); } @@ -993,8 +1001,7 @@ public function testGetVariationForFeatureRolloutWhenRolloutIsNotInDataFile() $actualFeatureDecision = $this->decisionServiceMock->getVariationForFeatureRollout( $this->config, $featureFlag, - 'user_1', - [] + $this->optimizely->createUserContext('user_1', []) ); $this->assertNull($actualFeatureDecision->getVariation()); } @@ -1020,7 +1027,7 @@ public function testGetVariationForFeatureRolloutWhenRolloutDoesNotHaveExperimen ->method('getRolloutFromId') ->will($this->returnValue($experiment_less_rollout)); - $actualFeatureDecision = $this->decisionService->getVariationForFeatureRollout($configMock, $featureFlag, 'user_1', []); + $actualFeatureDecision = $this->decisionService->getVariationForFeatureRollout($configMock, $featureFlag, $this->optimizely->createUserContext('user_1', [])); $this->assertNull($actualFeatureDecision->getVariation()); } @@ -1053,7 +1060,7 @@ public function testGetVariationForFeatureRolloutWhenUserIsBucketedInTheTargetin $this->compareFeatureDecisionsExceptReasons( $expected_decision, - $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes) + $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', $user_attributes)) ); } @@ -1091,7 +1098,7 @@ public function testGetVariationForFeatureRolloutWhenUserIsNotBucketedInTheTarge $this->assertEquals( $expected_decision, - $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes) + $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', $user_attributes)) ); } @@ -1124,8 +1131,7 @@ public function testGetVariationForFeatureRolloutWhenUserIsNeitherBucketedInTheT $actualFeatureDecision = $this->decisionService->getVariationForFeatureRollout( $this->config, $featureFlag, - 'user_1', - $user_attributes + $this->optimizely->createUserContext('user_1', $user_attributes) ); $this->assertNull($actualFeatureDecision->getVariation()); @@ -1171,7 +1177,7 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar $this->assertEquals( $expected_decision, - $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes) + $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', $user_attributes)) ); // Verify Logs @@ -1208,7 +1214,7 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar ->method('log') ->will($this->returnCallback($this->collectLogsForAssertion)); - $actualFeatureDecision = $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes); + $actualFeatureDecision = $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', $user_attributes)); $this->assertNull($actualFeatureDecision->getVariation()); @@ -1253,7 +1259,7 @@ public function testGetVariationForFeatureRolloutLogging() $this->assertEquals( $expected_decision, - $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, 'user_1', $user_attributes) + $this->decisionService->getVariationForFeatureRollout($this->config, $featureFlag, $this->optimizely->createUserContext('user_1', $user_attributes)) ); // Verify Logs From 5c753aa1295b5c6fef9566c5c2853f1cff54d4f5 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 27 Sep 2021 20:30:53 +0500 Subject: [PATCH 05/28] fixed all errors --- src/Optimizely/Config/DatafileProjectConfig.php | 2 +- src/Optimizely/DecisionService/DecisionService.php | 13 +++++++------ src/Optimizely/Optimizely.php | 13 +++++++------ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index d68c6068..05bcf946 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -381,7 +381,7 @@ public function __construct($datafile, $logger, $errorHandler) foreach ($this->_featureFlags as $flag) { $variations = array(); $flagRules = $this->getAllRulesForFlag($flag); - foreach ($flagRules as $ruleList) { + foreach ($flagRules as $rule) { $variations = array_filter(array_values($rule->getVariations()), function ($variation) use ($variations) { return !in_array($variation->getId(), $variations); }); diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index d15306ee..26329906 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -362,9 +362,9 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon } $index = 0; while ($index < sizeof($rolloutRules)) { - list($decisionResponse, $reasons) = $this->getVariationFromDeliveryRule($projectConfig, $featureFlagKey, $rolloutRules, $index, $user, $decideOptions); - $decideReasons = array_merge($decideReasons, $reasons); - list($variation, $skipToEveryoneElse) = $decisionResponse == null? [null, true] : [$decisionResponse, false]; + list($decisionResponses, $skipToEveryoneElse) = $this->getVariationFromDeliveryRule($projectConfig, $featureFlagKey, $rolloutRules, $index, $user, $decideOptions); + $decideReasons = array_merge($decideReasons, $decisionResponses->getReasons()); + $variation = $decisionResponses->getVariation(); if ($variation != null) { return new FeatureDecision($rolloutRules[$index], $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT, $decideReasons); } @@ -417,13 +417,14 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $decideReasons = array_merge($decideReasons, $reasons); if ($forcedDecisionResponse != null) { - return new FeatureDecision($rule, $forcedDecisionResponse, null, $decideReasons); + return [new FeatureDecision($rule, $forcedDecisionResponse, null, $decideReasons), $skipToEveryoneElse]; } // regular decision $userId = $user->getUserId(); $attributes = $user->getAttributes(); - $bucketingId = $this->getBucketingId($userId, $attributes); + list($bucketingId, $reasons) = $this->getBucketingId($userId, $attributes); + $decideReasons = array_merge($decideReasons, $reasons); $everyoneElse = $ruleIndex == sizeof($rules) - 1; $loggingKey = $everyoneElse ? "Everyone Else" : $ruleIndex + 1; @@ -458,7 +459,7 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $decideReasons[] = $message; } - return new FeatureDecision($rule, $bucketedVariation, null, $decideReasons); + return [new FeatureDecision($rule, $bucketedVariation, null, $decideReasons), $skipToEveryoneElse]; } /** diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index c712322e..3d0fd210 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -358,7 +358,7 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp $decision = $this->_decisionService->getVariationForFeature( $config, $featureFlag, - $this->createUserContext($userId, $userAttributes), + $userContext, $decideOptions ); } @@ -695,7 +695,8 @@ public function getVariation($experimentKey, $userId, $attributes = null) return null; } - list($variation, $reasons) = $this->_decisionService->getVariation($config, $experiment, $this->createUserContext($userId, $attributes = [])); + $userContext = $this->createUserContext($userId, $attributes ? $attributes : []); + list($variation, $reasons) = $this->_decisionService->getVariation($config, $experiment, $userContext); $variationKey = ($variation === null) ? null : $variation->getKey(); if ($config->isFeatureExperiment($experiment->getId())) { @@ -823,8 +824,8 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null) } $featureEnabled = false; - $attributes = $attributes? $attributes : []; - $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $this->createUserContext($userId, $attributes)); + $userContext = $this->createUserContext($userId, $attributes?: []); + $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $userContext); $variation = $decision->getVariation(); if ($config->getSendFlagDecisions() && ($decision->getSource() == FeatureDecision::DECISION_SOURCE_ROLLOUT || !$variation)) { @@ -957,8 +958,8 @@ public function getFeatureVariableValueForType( // Error logged in DatafileProjectConfig - getFeatureFlagFromKey return null; } - $attributes = $attributes? $attributes : []; - $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $this->createUserContext($userId, $attributes)); + $userContext = $this->createUserContext($userId, $attributes? $attributes : []); + $decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $userContext); $variation = $decision->getVariation(); $experiment = $decision->getExperiment(); $featureEnabled = $variation !== null ? $variation->getFeatureEnabled() : false; From 0fd717bf223776650436173299f2a9237772ae28 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 27 Sep 2021 22:21:19 +0500 Subject: [PATCH 06/28] fixed old tests and logs errors --- .../DecisionService/DecisionService.php | 16 +++--- .../DecisionServiceTest.php | 49 ++++++++++++++++--- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 26329906..bf70ccc6 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -338,7 +338,6 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project public function getVariationForFeatureRollout(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user, $decideOptions = []) { $decideReasons = []; -// list($bucketing_id, $reasons) = $this->getBucketingId($userId, $userAttributes); $featureFlagKey = $featureFlag->getKey(); $rollout_id = $featureFlag->getRolloutId(); if (empty($rollout_id)) { @@ -430,10 +429,11 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $loggingKey = $everyoneElse ? "Everyone Else" : $ruleIndex + 1; $bucketedVariation = null; - list($evalResult, $reasons) = Validator::doesUserMeetAudienceConditions($projectConfig, $rule, $attributes, $this->_logger); + // Evaluate if user meets the audience condition of this rollout rule + list($evalResult, $reasons) = Validator::doesUserMeetAudienceConditions($projectConfig, $rule, $attributes, $this->_logger, 'Optimizely\Enums\RolloutAudienceEvaluationLogs', $loggingKey); $decideReasons = array_merge($decideReasons, $reasons); - if (!$evalResult) { - $message = sprintf('User "%s" meets condition for targeting rule "%s".', $userId, $rule->getKey()); + if ($evalResult) { + $message = sprintf('User "%s" meets condition for targeting rule "%s".', $userId, $loggingKey); $this->_logger->log( Logger::INFO, $message @@ -442,20 +442,20 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf list($bucketedVariation, $reasons) = $this->_bucketer->bucket($projectConfig, $rule, $bucketingId, $userId); $decideReasons = array_merge($decideReasons, $reasons); if ($bucketedVariation != null) { - $message = sprintf('User "%s" is in the traffic group of targeting rule "%s".', $userId, $rule->getKey()); + $message = sprintf('User "%s" is in the traffic group of targeting rule "%s".', $userId, $loggingKey); $this->_logger->log(Logger::INFO, $message); $decideReasons[] = $message; } else if (!$everyoneElse) { // skip this logging for EveryoneElse since this has a message not for EveryoneElse - $message = sprintf('User "%s" is not in the traffic group for targeting rule "%s". Checking Everyone Else rule now.', $userId, $rule->getKey()); + $message = sprintf('User "%s" is not in the traffic group for targeting rule "%s". Checking Everyone Else rule now.', $userId, $loggingKey); $this->_logger->log(Logger::INFO, $message); $decideReasons[] = $message; // skip the rest of rollout rules to the everyone-else rule if audience matches but not bucketed. $skipToEveryoneElse = true; } } else { - $message = sprintf('User "%s" does not meet conditions for targeting rule "%s".', $userId, $rule->getKey()); - $this->_logger->log(Logger::INFO, $message); + $message = sprintf('User "%s" does not meet conditions for targeting rule "%s".', $userId, $loggingKey); + $this->_logger->log(Logger::DEBUG, $message); $decideReasons[] = $message; } diff --git a/tests/DecisionServiceTests/DecisionServiceTest.php b/tests/DecisionServiceTests/DecisionServiceTest.php index db4446f1..219d5d92 100644 --- a/tests/DecisionServiceTests/DecisionServiceTest.php +++ b/tests/DecisionServiceTests/DecisionServiceTest.php @@ -1078,7 +1078,16 @@ public function testGetVariationForFeatureRolloutWhenUserIsNotBucketedInTheTarge $expected_decision = new FeatureDecision( $experiment2, $expected_variation, - FeatureDecision::DECISION_SOURCE_ROLLOUT + FeatureDecision::DECISION_SOURCE_ROLLOUT, + ['Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule 1 collectively evaluated to TRUE.', + 'User "user_1" meets condition for targeting rule "1".', + 'User "user_1" is not in the traffic group for targeting rule "1". Checking Everyone Else rule now.', + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule Everyone Else collectively evaluated to TRUE.', + 'User "user_1" meets condition for targeting rule "Everyone Else".', + 'User "user_1" is in the traffic group of targeting rule "Everyone Else".' + ] ); // Provide attributes such that user qualifies for audience @@ -1156,7 +1165,19 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar $expected_decision = new FeatureDecision( $experiment2, $expected_variation, - FeatureDecision::DECISION_SOURCE_ROLLOUT + FeatureDecision::DECISION_SOURCE_ROLLOUT, + [ + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule 1 collectively evaluated to FALSE.' , + 'User "user_1" does not meet conditions for targeting rule "1".', + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule 2 collectively evaluated to FALSE.', + 'User "user_1" does not meet conditions for targeting rule "2".', + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule Everyone Else collectively evaluated to TRUE.', + 'User "user_1" meets condition for targeting rule "Everyone Else".', + 'User "user_1" is in the traffic group of targeting rule "Everyone Else".' + ] ); // Provide null attributes so that user does not qualify for audience @@ -1181,8 +1202,8 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar ); // Verify Logs - $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 "1".'], $this->collectedLogs); + $this->assertContains([Logger::DEBUG, 'User "user_1" does not meet conditions for targeting rule "2".'], $this->collectedLogs); } public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTargetingRuleOrEveryoneElseRule() @@ -1219,9 +1240,9 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar $this->assertNull($actualFeatureDecision->getVariation()); // Verify Logs - $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); + $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); } @@ -1238,7 +1259,19 @@ public function testGetVariationForFeatureRolloutLogging() $expected_decision = new FeatureDecision( $experiment2, $expected_variation, - FeatureDecision::DECISION_SOURCE_ROLLOUT + FeatureDecision::DECISION_SOURCE_ROLLOUT, + [ + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule 1 collectively evaluated to FALSE.', + 'User "user_1" does not meet conditions for targeting rule "1".', + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule 2 collectively evaluated to FALSE.', + 'User "user_1" does not meet conditions for targeting rule "2".', + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "user_1" in the forced decision map.', + 'Audiences for rule Everyone Else collectively evaluated to TRUE.', + 'User "user_1" meets condition for targeting rule "Everyone Else".', + 'User "user_1" is in the traffic group of targeting rule "Everyone Else".' + ] ); // Provide null attributes so that user does not qualify for audience From 7e2400f25a15ee918bb78b1fda07d24bc458cad6 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 28 Sep 2021 20:27:54 +0500 Subject: [PATCH 07/28] fix --- src/Optimizely/OptimizelyUserContext.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 3520ac9b..f54d3b20 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -47,7 +47,7 @@ public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) public function getForcedDecision($flagKey, $ruleKey = null) { - return findForcedDecision($flagKey, $ruleKey); + return $this->findForcedDecision($flagKey, $ruleKey); } public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = []) From 9cb1d037b873c53c4182754e2c935a321e8a686c Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 28 Sep 2021 20:41:43 +0500 Subject: [PATCH 08/28] fix --- src/Optimizely/OptimizelyUserContext.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index f54d3b20..1b018c45 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -36,7 +36,7 @@ public function __construct(Optimizely $optimizelyClient, $userId, array $attrib public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) { - $index = $this->findExisitingRuleAndFlagKey($flagKey, $ruleKey); + $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { $this->forcedDecisions[$index]->variationKey = $variationKey; } else { From ae48a616cd291d5e1bc9055e210dabfc012e6070 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 29 Sep 2021 00:18:48 +0500 Subject: [PATCH 09/28] added remove forcedecision removeall force decision apis --- src/Optimizely/OptimizelyUserContext.php | 32 +++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 1b018c45..d288f8d4 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -34,12 +34,31 @@ public function __construct(Optimizely $optimizelyClient, $userId, array $attrib $this->attributes = $attributes; } + public function removeForcedDecision($flagKey, $ruleKey = null) + { + $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); + if ($index != -1) { + unset($this->forcedDecisions[$index]); + return true; + } + return false; + } + + public function removeAllForcedDecisions() + { + $this->forcedDecisions = []; + return true; + } + public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) { $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { - $this->forcedDecisions[$index]->variationKey = $variationKey; + $this->forcedDecisions[$index]->setVariationKey($variationKey); } else { + if (!$this->forcedDecisions) { + $this->forcedDecisions = array(); + } array_push($this->forcedDecisions, new ForcedDecision($flagKey, $ruleKey, $variationKey)); } return true; @@ -151,13 +170,20 @@ class ForcedDecision { private $flagKey; private $ruleKey; - public $variationKey; + private $variationKey; public function __construct($flagKey, $ruleKey, $variationKey) { $this->flagKey = $flagKey; $this->ruleKey = $ruleKey; - $this->variationKey = $variationKey; + $this->setVariationKey($variationKey); + } + + public function setVariationKey($variationKey) + { + if (isset($variationKey) && trim($variationKey) !== '') { + $this->variationKey = $variationKey; + } } /** From 6a9a5f04591c6dcefb22ab9db69d230cfda2db8e Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 1 Oct 2021 21:30:38 +0500 Subject: [PATCH 10/28] Unit tests added and fixed some bugs --- .../Config/DatafileProjectConfig.php | 12 +- .../DecisionService/DecisionService.php | 16 +-- src/Optimizely/Optimizely.php | 18 +-- src/Optimizely/OptimizelyUserContext.php | 34 +++-- tests/OptimizelyTest.php | 2 + tests/OptimizelyUserContextTest.php | 118 ++++++++++++++++++ 6 files changed, 169 insertions(+), 31 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index 05bcf946..3aa6c4c3 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -245,7 +245,13 @@ class DatafileProjectConfig implements ProjectConfigInterface */ private $_sendFlagDecisions; + /** + * Map indicating variations of flag decisions + * + * @return map + */ private $_flagVariationsMap; + /** * DatafileProjectConfig constructor to load and set project configuration data. * @@ -381,11 +387,13 @@ public function __construct($datafile, $logger, $errorHandler) foreach ($this->_featureFlags as $flag) { $variations = array(); $flagRules = $this->getAllRulesForFlag($flag); + foreach ($flagRules as $rule) { - $variations = array_filter(array_values($rule->getVariations()), function ($variation) use ($variations) { + $variations = array_merge($variations, array_filter(array_values($rule->getVariations()), function ($variation) use ($variations) { return !in_array($variation->getId(), $variations); - }); + })); } + $this->_flagVariationsMap[$flag->getKey()] = $variations; } // Add variations for rollout experiments to variationIdMap and variationKeyMap diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index bf70ccc6..39c7939f 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -300,7 +300,7 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project continue; } - list($variation, $reasons) = $this->getVariation($projectConfig, $experiment, $user, $decideOptions); + list($variation, $reasons) = $this->getVariationFromExperimentRule($projectConfig, $featureFlagKey, $experiment, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); if ($variation && $variation->getKey()) { $message = "The user '{$userId}' is bucketed into experiment '{$experiment->getKey()}' of feature '{$featureFlagKey}'."; @@ -380,22 +380,16 @@ private function getVariationFromExperimentRule(ProjectConfigInterface $projectC list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey()); $decideReasons = array_merge($decideReasons, $reasons); if ($decisionResponse != null) { - return new FeatureDecision(null, $decisionResponse, FeatureDecision::DECISION_SOURCE_EXPERIMENT, $decideReasons); + return [$decisionResponse, $decideReasons]; } // regular decision list($variation, $reasons) = $this->getVariation($projectConfig, $rule, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); - if ($variation && $variation->getKey()) { - $message = "The user '{$user->getUserId()}' is bucketed into experiment '{$rule->getKey()}' of feature '{$flagKey}'."; - $this->_logger->log( - Logger::INFO, - $message - ); - $decideReasons[] = $message; - return new FeatureDecision($rule, $variation, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); - } + + return [$variation, $decideReasons]; } + /** * Gets the forced variation key for the given user and experiment. * diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 3d0fd210..25467828 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -369,8 +369,10 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp if ($variation) { $variationKey = $variation->getKey(); $featureEnabled = $variation->getFeatureEnabled(); - $ruleKey = $decision->getExperiment()->getKey(); - $experimentId = $decision->getExperiment()->getId(); + if ($decision->getExperiment()) { + $ruleKey = $decision->getExperiment()->getKey(); + $experimentId = $decision->getExperiment()->getId(); + } } else { $variationKey = null; $ruleKey = null; @@ -1294,13 +1296,11 @@ protected function validateInputs(array $values, $logLevel = Logger::ERROR) public function getFlagVariationByKey($flagKey, $variationKey) { - if(array_key_exists($flagKey, $this->_config->getFlagVariationsMap())) - { - $variations = array_filter(array_values($this->_config->getFlagVariationsMap()[$flagKey]), function ($variations) use ($variationKey) { - return in_array($variationKey, $variations->getKey()); - }); - if(!empty($variations)) { - return $variations[0]; + if (array_key_exists($flagKey, $this->getConfig()->getFlagVariationsMap())) { + foreach ($this->getConfig()->getFlagVariationsMap()[$flagKey] as $variation) { + if ($variation->getKey() == $variationKey) { + return $variation; + } } } return null; diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index d288f8d4..38e56fc8 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -27,15 +27,20 @@ class OptimizelyUserContext implements \JsonSerializable private $attributes; private $forcedDecisions; - public function __construct(Optimizely $optimizelyClient, $userId, array $attributes = []) + public function __construct(Optimizely $optimizelyClient, $userId, array $attributes = [], $forcedDecisions = null) { $this->optimizelyClient = $optimizelyClient; $this->userId = $userId; $this->attributes = $attributes; + $this->forcedDecisions = $forcedDecisions; } public function removeForcedDecision($flagKey, $ruleKey = null) { + // check if SDK is ready + if (!$this->optimizelyClient->isValid()) { + return false; + } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { unset($this->forcedDecisions[$index]); @@ -46,12 +51,20 @@ public function removeForcedDecision($flagKey, $ruleKey = null) public function removeAllForcedDecisions() { + // check if SDK is ready + if (!$this->optimizelyClient->isValid()) { + return false; + } $this->forcedDecisions = []; return true; } public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) { + // check if SDK is ready + if (!$this->optimizelyClient->isValid()) { + return false; + } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { $this->forcedDecisions[$index]->setVariationKey($variationKey); @@ -66,6 +79,10 @@ public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) public function getForcedDecision($flagKey, $ruleKey = null) { + // check if SDK is ready + if (!$this->optimizelyClient->isValid()) { + return null; + } return $this->findForcedDecision($flagKey, $ruleKey); } @@ -74,7 +91,7 @@ public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = $decideReasons = []; $variationKey = $this->findForcedDecision($flagKey, $ruleKey); if ($variationKey != null) { - $variation = ($this->optimizelyClient)->getFlagVariationByKey($flagKey, $variationKey); + $variation = $this->optimizelyClient->getFlagVariationByKey($flagKey, $variationKey); $message = sprintf('Variation "%s" is mapped to "%s" and user "%s" in the forced decision map.', $variationKey, $flagKey, $this->userId); $decideReasons[] = $message; @@ -86,6 +103,7 @@ public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = } return [null, $decideReasons]; } + private function findExistingRuleAndFlagKey($flagKey, $ruleKey) { if ($this->forcedDecisions) { @@ -98,23 +116,21 @@ private function findExistingRuleAndFlagKey($flagKey, $ruleKey) return -1; } - private function findForcedDecision($flagKey, $ruleKey) + public function findForcedDecision($flagKey, $ruleKey) { + $foundVariationKey = null; if ($this->forcedDecisions && count($this->forcedDecisions) == 0) { return null; } - $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); - if ($index != -1) { - return $this->forcedDecisions[$index]->getVariationKey(); + $foundVariationKey = $this->forcedDecisions[$index]->getVariationKey(); } - - return null; + return $foundVariationKey; } protected function copy() { - return new OptimizelyUserContext($this->optimizelyClient, $this->userId, $this->attributes); + return new OptimizelyUserContext($this->optimizelyClient, $this->userId, $this->attributes, $this->forcedDecisions); } public function setAttribute($key, $value) diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index 51504308..86d61abe 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -1652,6 +1652,7 @@ public function testDecideOptionIncludeReasons() ->getMock(); $expectedReasons = [ + 'Invalid variation is mapped to "double_single_variable_feature" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_double_feature" collectively evaluated to TRUE.', 'Assigned bucket 4513 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation control of experiment test_experiment_double_feature.', @@ -1724,6 +1725,7 @@ public function testDecideLogsWhenAudienceEvalFails() ->getMock(); $expectedReasons = [ + 'Invalid variation is mapped to "multi_variate_feature" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_multivariate" collectively evaluated to FALSE.', 'User "test_user" does not meet conditions to be in experiment "test_experiment_multivariate".', "The user 'test_user' is not bucketed into any of the experiments using the feature 'multi_variate_feature'.", diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index c41667b0..d5ffa24c 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -258,4 +258,122 @@ public function testJsonSerialize() 'attributes' => $attributes ], json_decode(json_encode($optUserContext), true)); } + + // Forced decision tests + + public function testForcedDecisionInvalidDatafileReturnStatus() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $invalidOptlyObject = new Optimizely("Invalid datafile"); + + $optUserContext = new OptimizelyUserContext($invalidOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("flag1", "variation1", "targeted_delivery"); + $this->assertFalse($setForcedDecision); + + $getForcedDecision = $optUserContext->getForcedDecision("flag1", "targeted_delivery"); + $this->assertNull($getForcedDecision); + + $removeForcedDecision = $optUserContext->removeForcedDecision("flag1", "targeted_delivery"); + $this->assertFalse($removeForcedDecision); + + $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions("flag1", "targeted_delivery"); + $this->assertFalse($removeAllForcedDecision); + } + + public function testForcedDecisionValidDatafileReturnStatus() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("flag1", "variation1", "targeted_delivery"); + $this->assertTrue($setForcedDecision); + + $getForcedDecision = $optUserContext->getForcedDecision("flag1", "targeted_delivery"); + $this->assertEquals($getForcedDecision, "variation1"); + + $removeForcedDecision = $optUserContext->removeForcedDecision("flag1", "targeted_delivery"); + $this->assertTrue($removeForcedDecision); + + $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions("flag1", "targeted_delivery"); + $this->assertTrue($removeAllForcedDecision); + } + + public function testForcedDecisionFlagToDecision() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "177773"); + $this->assertTrue($setForcedDecision); + + $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature"); + $this->assertEquals($getForcedDecision, "177773"); + + $decision = $optUserContext->decide('boolean_single_variable_feature'); + $this->assertEquals($decision->getVariationKey(), "177773"); + $this->assertNull($decision->getRuleKey()); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), []); + + // Removing forced decision to test + $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_single_variable_feature"); + $this->assertTrue($removeForcedDecision); + + $decision = $optUserContext->decide('boolean_single_variable_feature'); + $this->assertEquals($decision->getVariationKey(), "177778"); + $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), []); + } + + public function testForcedDecisionRuleToDecision() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "test_variation_1", "test_experiment_2"); + $this->assertTrue($setForcedDecision); + + $getForcedDecision = $optUserContext->getForcedDecision("boolean_feature", "test_experiment_2"); + $this->assertEquals($getForcedDecision, "test_variation_1"); + + $decision = $optUserContext->decide('boolean_feature'); + $this->assertEquals($decision->getVariationKey(), "test_variation_1"); + $this->assertEquals($decision->getRuleKey(), "test_experiment_2"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), []); + + // Removing forced decision to test + $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_feature", "test_experiment_2"); + $this->assertTrue($removeForcedDecision); + + $decision = $optUserContext->decide('boolean_feature'); + $this->assertEquals($decision->getVariationKey(), "test_variation_2"); + $this->assertEquals($decision->getRuleKey(), "test_experiment_2"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), []); + } } From 60ae438634fd2e4909f07681dc0ca70fa29dec12 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 4 Oct 2021 22:12:35 +0500 Subject: [PATCH 11/28] Added unit tests of forceddecision --- src/Optimizely/OptimizelyUserContext.php | 5 +- .../ConfigTests/DatafileProjectConfigTest.php | 49 ++++ tests/OptimizelyTest.php | 262 ++++++++++++++++++ tests/OptimizelyUserContextTest.php | 192 ++++++++++++- 4 files changed, 505 insertions(+), 3 deletions(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 38e56fc8..cd3ca75b 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -43,7 +43,7 @@ public function removeForcedDecision($flagKey, $ruleKey = null) } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { - unset($this->forcedDecisions[$index]); + array_splice($this->forcedDecisions, $index, 1); return true; } return false; @@ -55,6 +55,9 @@ public function removeAllForcedDecisions() if (!$this->optimizelyClient->isValid()) { return false; } + if (!$this->forcedDecisions || count($this->forcedDecisions) == 0) { + return false; + } $this->forcedDecisions = []; return true; } diff --git a/tests/ConfigTests/DatafileProjectConfigTest.php b/tests/ConfigTests/DatafileProjectConfigTest.php index f7bd7938..b96196de 100644 --- a/tests/ConfigTests/DatafileProjectConfigTest.php +++ b/tests/ConfigTests/DatafileProjectConfigTest.php @@ -189,6 +189,55 @@ public function testInit() $audienceIdMap->getValue($this->config) ); + // Check flag key variations map + $flagVariationsMap = new \ReflectionProperty(DatafileProjectConfig::class, '_flagVariationsMap'); + $flagVariationsMap->setAccessible(true); + + $this->assertEquals( + [ + 'boolean_feature' => [ + $this->config->getVariationFromKey('test_experiment_2', 'test_variation_1'), + $this->config->getVariationFromKey('test_experiment_2', 'test_variation_2') + ], + 'double_single_variable_feature' => [ + $this->config->getVariationFromKey('test_experiment_double_feature', 'control'), + $this->config->getVariationFromKey('test_experiment_double_feature', 'variation') + ], + 'integer_single_variable_feature' => [ + $this->config->getVariationFromKey('test_experiment_integer_feature', 'control'), + $this->config->getVariationFromKey('test_experiment_integer_feature', 'variation') + ], + 'boolean_single_variable_feature' => [ + $this->config->getVariationFromKey('rollout_1_exp_1', '177771'), + $this->config->getVariationFromKey('rollout_1_exp_2', '177773'), + $this->config->getVariationFromKey('rollout_1_exp_3', '177778') + ], + 'string_single_variable_feature' => [ + $this->config->getVariationFromKey('test_experiment_with_feature_rollout', 'control'), + $this->config->getVariationFromKey('test_experiment_with_feature_rollout', 'variation'), + $this->config->getVariationFromKey('rollout_2_exp_1', '177775'), + $this->config->getVariationFromKey('rollout_2_exp_2', '177780') + ], + 'multiple_variables_feature' => [ + $this->config->getVariationFromKey('test_experiment_json_feature', 'json_variation') + ], + 'multi_variate_feature' => [ + $this->config->getVariationFromKey('test_experiment_multivariate', 'Fred'), + $this->config->getVariationFromKey('test_experiment_multivariate', 'Feorge'), + $this->config->getVariationFromKey('test_experiment_multivariate', 'Gred'), + $this->config->getVariationFromKey('test_experiment_multivariate', 'George') + ], + 'mutex_group_feature' => [ + $this->config->getVariationFromKey('group_experiment_1', 'group_exp_1_var_1'), + $this->config->getVariationFromKey('group_experiment_1', 'group_exp_1_var_2'), + $this->config->getVariationFromKey('group_experiment_2', 'group_exp_2_var_1'), + $this->config->getVariationFromKey('group_experiment_2', 'group_exp_2_var_2') + ], + 'empty_feature' => [] + ], + $flagVariationsMap->getValue($this->config) + ); + // Check variation key map $variationKeyMap = new \ReflectionProperty(DatafileProjectConfig::class, '_variationKeyMap'); $variationKeyMap->setAccessible(true); diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index 86d61abe..fa50197f 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -475,6 +475,268 @@ public function testDecide() $this->compareOptimizelyDecisions($expectedOptimizelyDecision, $optimizelyDecision); } + public function testSetForcedDecisionExperimentRuleToDecisionSendImpressionAndNotification() + { + $optimizely = new Optimizely($this->datafile); + $userContext = $optimizely->createUserContext('test_user', ['device_type' => 'iPhone']); + + $optimizelyMock = $this->getMockBuilder(Optimizely::class) + ->setConstructorArgs(array($this->datafile, null, $this->loggerMock)) + ->setMethods(array('sendImpressionEvent')) + ->getMock(); + + $decisionServiceMock = $this->getMockBuilder(DecisionService::class) + ->setConstructorArgs(array($this->loggerMock)) + ->setMethods(array('getVariationForFeature')) + ->getMock(); + + $decisionService = new \ReflectionProperty(Optimizely::class, '_decisionService'); + $decisionService->setAccessible(true); + $decisionService->setValue($optimizelyMock, $decisionServiceMock); + + // Mock getVariationForFeature to return a valid decision with experiment and variation keys + $experiment = $this->projectConfig->getExperimentFromKey('test_experiment_double_feature'); + $variation = $this->projectConfig->getVariationFromKey('test_experiment_double_feature', 'variation'); + + // assert that featureEnabled for $variation is true + $this->assertFalse($variation->getFeatureEnabled()); + + $expected_decision = new FeatureDecision( + $experiment, + $variation, + FeatureDecision::DECISION_SOURCE_FEATURE_TEST + ); + + $decisionServiceMock->expects($this->exactly(1)) + ->method('getVariationForFeature') + ->will($this->returnValue($expected_decision)); + + // Verify that sendNotifications is called with expected params + $arrayParam = array( + DecisionNotificationTypes::FLAG, + 'test_user', + ['device_type' => 'iPhone'], + (object) array( + 'flagKey'=>'double_single_variable_feature', + 'enabled'=> false, + 'variables'=> ["double_variable" => 14.99], + 'variationKey' => 'variation', + 'ruleKey' => 'test_experiment_double_feature', + 'reasons' => [], + 'decisionEventDispatched' => true + ) + ); + + $this->notificationCenterMock->expects($this->once()) + ->method('sendNotifications') + ->with( + NotificationType::DECISION, + $arrayParam + ); + + //assert that sendImpressionEvent is called with expected params + $optimizelyMock->expects($this->exactly(1)) + ->method('sendImpressionEvent') + ->with( + $this->projectConfig, + '122238', + 'variation', + 'double_single_variable_feature', + 'test_experiment_double_feature', + FeatureDecision::DECISION_SOURCE_FEATURE_TEST, + false, + 'test_user', + ['device_type' => 'iPhone'] + ); + + $optimizelyMock->notificationCenter = $this->notificationCenterMock; + + $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', 'variation', 'test_experiment_double_feature')); + $optimizelyDecision = $optimizelyMock->decide($userContext, 'double_single_variable_feature'); + $expectedOptimizelyDecision = new OptimizelyDecision( + 'variation', + false, + ['double_variable' => 14.99], + 'test_experiment_double_feature', + 'double_single_variable_feature', + $userContext, + [] + ); + + $this->compareOptimizelyDecisions($expectedOptimizelyDecision, $optimizelyDecision); + } + + public function testSetForcedDecisionDeliveryRuleToDecisionSendImpressionAndNotification() + { + $optimizely = new Optimizely($this->datafile); + $userContext = $optimizely->createUserContext('test_user', ['device_type' => 'iPhone']); + + $optimizelyMock = $this->getMockBuilder(Optimizely::class) + ->setConstructorArgs(array($this->datafile, null, $this->loggerMock)) + ->setMethods(array('sendImpressionEvent')) + ->getMock(); + + $decisionServiceMock = $this->getMockBuilder(DecisionService::class) + ->setConstructorArgs(array($this->loggerMock)) + ->setMethods(array('getVariationForFeature')) + ->getMock(); + + $decisionService = new \ReflectionProperty(Optimizely::class, '_decisionService'); + $decisionService->setAccessible(true); + $decisionService->setValue($optimizelyMock, $decisionServiceMock); + + // Mock getVariationForFeature to return a valid decision with experiment and variation keys + $experiment = $this->projectConfig->getExperimentFromKey('rollout_1_exp_2'); + $variation = $this->projectConfig->getVariationFromKey('rollout_1_exp_2', '177773'); + + // assert that featureEnabled for $variation is true + $this->assertTrue($variation->getFeatureEnabled()); + + $expected_decision = new FeatureDecision( + $experiment, + $variation, + FeatureDecision::DECISION_SOURCE_FEATURE_TEST + ); + + $decisionServiceMock->expects($this->exactly(1)) + ->method('getVariationForFeature') + ->will($this->returnValue($expected_decision)); + + // Verify that sendNotifications is called with expected params + $arrayParam = array( + DecisionNotificationTypes::FLAG, + 'test_user', + ['device_type' => 'iPhone'], + (object) array( + 'flagKey'=>'boolean_single_variable_feature', + 'enabled'=> true, + 'variables'=> ['boolean_variable' => false], + 'variationKey' => '177773', + 'ruleKey' => 'rollout_1_exp_2', + 'reasons' => [], + 'decisionEventDispatched' => true + ) + ); + + $this->notificationCenterMock->expects($this->once()) + ->method('sendNotifications') + ->with( + NotificationType::DECISION, + $arrayParam + ); + + //assert that sendImpressionEvent is called with expected params + $optimizelyMock->expects($this->exactly(1)) + ->method('sendImpressionEvent') + ->with( + $this->projectConfig, + '177772', + '177773', + 'boolean_single_variable_feature', + 'rollout_1_exp_2', + FeatureDecision::DECISION_SOURCE_FEATURE_TEST, + true, + 'test_user', + ['device_type' => 'iPhone'] + ); + + $optimizelyMock->notificationCenter = $this->notificationCenterMock; + + $this->assertTrue($userContext->setForcedDecision('boolean_single_variable_feature', '177773', 'rollout_1_exp_2')); + $optimizelyDecision = $optimizelyMock->decide($userContext, 'boolean_single_variable_feature'); + $expectedOptimizelyDecision = new OptimizelyDecision( + '177773', + true, + ['boolean_variable' => false], + 'rollout_1_exp_2', + 'boolean_single_variable_feature', + $userContext, + [] + ); + + $this->compareOptimizelyDecisions($expectedOptimizelyDecision, $optimizelyDecision); + } + + public function testSetForcedDecisionFlagToDecisionSendImpressionAndNotification() + { + $optimizely = new Optimizely($this->datafile); + $userContext = $optimizely->createUserContext('test_user', ['device_type' => 'iPhone']); + + $optimizelyMock = $this->getMockBuilder(Optimizely::class) + ->setConstructorArgs(array($this->datafile, null, $this->loggerMock)) + ->setMethods(array('sendImpressionEvent')) + ->getMock(); + + $decisionServiceMock = $this->getMockBuilder(DecisionService::class) + ->setConstructorArgs(array($this->loggerMock)) + ->setMethods(array('getVariationForFeature')) + ->getMock(); + + $decisionService = new \ReflectionProperty(Optimizely::class, '_decisionService'); + $decisionService->setAccessible(true); + $decisionService->setValue($optimizelyMock, $decisionServiceMock); + + // Mock getVariationForFeature to return a valid decision with experiment and variation keys + $variation = $this->projectConfig->getVariationFromKey('test_experiment_double_feature', 'variation'); + + // assert that featureEnabled for $variation is true + $this->assertFalse($variation->getFeatureEnabled()); + + // Verify that sendNotifications is called with expected params + $arrayParam = array( + DecisionNotificationTypes::FLAG, + 'test_user', + ['device_type' => 'iPhone'], + (object) array( + 'flagKey'=>'double_single_variable_feature', + 'enabled'=> false, + 'variables'=> ["double_variable" => 14.99], + 'variationKey' => 'variation', + 'ruleKey' => null, + 'reasons' => [], + 'decisionEventDispatched' => true + ) + ); + + $this->notificationCenterMock->expects($this->once()) + ->method('sendNotifications') + ->with( + NotificationType::DECISION, + $arrayParam + ); + + //assert that sendImpressionEvent is called with expected params + $optimizelyMock->expects($this->exactly(1)) + ->method('sendImpressionEvent') + ->with( + $this->projectConfig, + '', + 'variation', + 'double_single_variable_feature', + '', + FeatureDecision::DECISION_SOURCE_FEATURE_TEST, + false, + 'test_user', + ['device_type' => 'iPhone'] + ); + + $optimizelyMock->notificationCenter = $this->notificationCenterMock; + + $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', 'variation')); + $optimizelyDecision = $optimizelyMock->decide($userContext, 'double_single_variable_feature'); + $expectedOptimizelyDecision = new OptimizelyDecision( + 'variation', + false, + ['double_variable' => 14.99], + null, + 'double_single_variable_feature', + $userContext, + [] + ); + + $this->compareOptimizelyDecisions($expectedOptimizelyDecision, $optimizelyDecision); + } + public function testDecideWhenSdkNotReady() { $optimizelyMock = $this->getMockBuilder(Optimizely::class) diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index d5ffa24c..cb917c68 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -17,6 +17,7 @@ namespace Optimizely\Tests; use Exception; +use Optimizely\Decide\OptimizelyDecideOption; use TypeError; use Optimizely\Logger\NoOpLogger; @@ -299,8 +300,8 @@ public function testForcedDecisionValidDatafileReturnStatus() $removeForcedDecision = $optUserContext->removeForcedDecision("flag1", "targeted_delivery"); $this->assertTrue($removeForcedDecision); - $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions("flag1", "targeted_delivery"); - $this->assertTrue($removeAllForcedDecision); + $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions(); + $this->assertFalse($removeAllForcedDecision); } public function testForcedDecisionFlagToDecision() @@ -376,4 +377,191 @@ public function testForcedDecisionRuleToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), []); } + + public function testForcedDecisionInvalidFlagToDecision() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "invalid"); + $this->assertTrue($setForcedDecision); + + $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); + $this->assertEquals($decision->getVariationKey(), "test_variation_2"); + $this->assertEquals($decision->getRuleKey(), "test_experiment_2"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), [ + 'Invalid variation is mapped to "boolean_feature" and user "test_user" in the forced decision map.', + 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', + 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', + 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', + "The user 'test_user' is bucketed into experiment 'test_experiment_2' of feature 'boolean_feature'." + ]); + } + + + public function testForcedDecisionInvalidDeliveryRuleToDecision() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "invalid", "rollout_1_exp_3"); + $this->assertTrue($setForcedDecision); + + $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); + $this->assertEquals($decision->getVariationKey(), "177778"); + $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), [ + "The feature flag 'boolean_single_variable_feature' is not used in any experiments.", + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "test_user" in the forced decision map.', + 'Audiences for rule 1 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "1".', + 'Invalid variation is mapped to "boolean_single_variable_feature" and user "test_user" in the forced decision map.', + 'Audiences for rule 2 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "2".', + 'Variation "invalid" is mapped to "boolean_single_variable_feature" and user "test_user" in the forced decision map.', + 'Audiences for rule Everyone Else collectively evaluated to TRUE.', + 'User "test_user" meets condition for targeting rule "Everyone Else".', + 'Assigned bucket 3041 to user "test_user" with bucketing ID "test_user".', + 'User "test_user" is in the traffic group of targeting rule "Everyone Else".', + "User 'test_user' is bucketed into rollout for feature flag 'boolean_single_variable_feature'." + ]); + } + + public function testForcedDecisionInvalidExperimentRuleToDecision() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "invalid"); + $this->assertTrue($setForcedDecision); + + $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); + $this->assertEquals("test_variation_2", $decision->getVariationKey()); + $this->assertEquals("test_experiment_2", $decision->getRuleKey()); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals("boolean_feature", $decision->getFlagKey()); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(1, count($decision->getUserContext()->getAttributes())); + $this->assertEquals([ + 'Invalid variation is mapped to "boolean_feature" and user "test_user" in the forced decision map.', + 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', + 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', + 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', + "The user 'test_user' is bucketed into experiment 'test_experiment_2' of feature 'boolean_feature'." + ], $decision->getReasons()); + } + + public function testForcedDecisionConflicts() + { + $featureKey = "boolean_feature"; + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision1 = $optUserContext->setForcedDecision($featureKey, "test_variation_1"); + $this->assertTrue($setForcedDecision1); + + $setForcedDecision2 = $optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2"); + $this->assertTrue($setForcedDecision2); + + // flag-to-decision is the 1st priority + + $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); + $this->assertEquals("test_variation_1", $decision->getVariationKey()); + $this->assertNull($decision->getRuleKey()); + } + + public function testGetForcedDecision() + { + $featureKey = "boolean_feature"; + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); + + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2")); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey)); + + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1", "test_experiment_2")); + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey)); + } + + public function testRemoveForcedDecision() + { + $featureKey = "boolean_feature"; + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); + + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + + $this->assertTrue($optUserContext->removeForcedDecision($featureKey)); + $this->assertNull($optUserContext->getForcedDecision($featureKey)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + + $this->assertTrue($optUserContext->removeForcedDecision($featureKey, "test_experiment_2")); + $this->assertNull($optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + $this->assertNull($optUserContext->getForcedDecision($featureKey)); + + $this->assertFalse($optUserContext->removeForcedDecision($featureKey)); // no more saved decisions + } + + public function testRemoveAllForcedDecisions() + { + $featureKey = "boolean_feature"; + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $this->assertFalse($optUserContext->removeAllForcedDecisions()); // no saved decisions + + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); + + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + + $this->assertTrue($optUserContext->removeAllForcedDecisions()); + $this->assertNull($optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + $this->assertNull($optUserContext->getForcedDecision($featureKey)); + + $this->assertFalse($optUserContext->removeAllForcedDecisions()); // no more saved decisions + } } + From df1c82f20c4ce42a28b73c242a8e152a77f5735f Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 6 Oct 2021 16:30:31 +0500 Subject: [PATCH 12/28] Comments fixed --- .../DecisionService/DecisionService.php | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 39c7939f..d2d3957e 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -215,10 +215,9 @@ public function getVariation(ProjectConfigInterface $projectConfig, Experiment $ * Get the variation the user is bucketed into for the given FeatureFlag * * @param ProjectConfigInterface $projectConfig ProjectConfigInterface instance. - * @param FeatureFlag $featureFlag The feature flag the user wants to access - * @param string $userId user ID - * @param array $userAttributes user attributes - * @param array $decideOptions Options to customize evaluation. + * @param FeatureFlag $featureFlag The feature flag the user wants to access + * @param OptimizelyUserContext $user Optimizely User context containing user id and attribute + * @param array $decideOptions Options to customize evaluation. * * @return FeatureDecision representing decision. */ @@ -269,8 +268,7 @@ public function getVariationForFeature(ProjectConfigInterface $projectConfig, Fe * * @param ProjectConfigInterface $projectConfig ProjectConfigInterface instance. * @param FeatureFlag $featureFlag The feature flag the user wants to access - * @param string $userId user id - * @param array $userAttributes user userAttributes + * @param OptimizelyUserContext $user Optimizely User context containing user id and attribute * @param array $decideOptions Options to customize evaluation. * * @return FeatureDecision representing decision. @@ -331,8 +329,8 @@ public function getVariationForFeatureExperiment(ProjectConfigInterface $project * * @param ProjectConfigInterface $projectConfig ProjectConfigInterface instance. * @param FeatureFlag $featureFlag The feature flag the user wants to access - * @param string $userId user id - * @param array $userAttributes user userAttributes + * @param OptimizelyUserContext $user Optimizely User context containing user id and attribute + * @param array $decideOptions Options to customize evaluation. * @return FeatureDecision representing decision. */ public function getVariationForFeatureRollout(ProjectConfigInterface $projectConfig, FeatureFlag $featureFlag, OptimizelyUserContext $user, $decideOptions = []) @@ -394,11 +392,14 @@ private function getVariationFromExperimentRule(ProjectConfigInterface $projectC * Gets the forced variation key for the given user and experiment. * * @param $projectConfig ProjectConfigInterface ProjectConfigInterface instance. - * @param $experimentKey string Key for experiment. - * @param $userId string The user Id. + * @param $flagKey string Key of feature flag. + * @param $rules array Array of delivery rules. + * @param $ruleIndex integer Index of delivery rule of which validation of forced decision is needed. + * @param $user OptimizelyUserContext Optimizely User context containing user id and attribute + * @param $options array Options to customize evaluation. * - * @return [ Variation, array ] The variation which the given user and experiment should be forced into and - * array of log messages representing decision making. + * @return [ FeatureDecision, Boolean ] The variation which the given user and experiment should be forced into and + * skipToEveryone boolean to decision making. */ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConfig, $flagKey, array $rules, $ruleIndex, OptimizelyUserContext $user, array $options = []) { From 80e502607c2a34ab2910e66c3835fbfc827ce6f6 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 6 Oct 2021 16:35:30 +0500 Subject: [PATCH 13/28] return true in removeallforcedecision even if forceddecision list is empty --- src/Optimizely/OptimizelyUserContext.php | 4 +--- tests/OptimizelyUserContextTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index cd3ca75b..ac9cd347 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -55,9 +55,7 @@ public function removeAllForcedDecisions() if (!$this->optimizelyClient->isValid()) { return false; } - if (!$this->forcedDecisions || count($this->forcedDecisions) == 0) { - return false; - } + $this->forcedDecisions = []; return true; } diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index cb917c68..ba87e42e 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -279,7 +279,7 @@ public function testForcedDecisionInvalidDatafileReturnStatus() $removeForcedDecision = $optUserContext->removeForcedDecision("flag1", "targeted_delivery"); $this->assertFalse($removeForcedDecision); - $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions("flag1", "targeted_delivery"); + $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions(); $this->assertFalse($removeAllForcedDecision); } @@ -301,7 +301,7 @@ public function testForcedDecisionValidDatafileReturnStatus() $this->assertTrue($removeForcedDecision); $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions(); - $this->assertFalse($removeAllForcedDecision); + $this->assertTrue($removeAllForcedDecision); } public function testForcedDecisionFlagToDecision() @@ -549,7 +549,7 @@ public function testRemoveAllForcedDecisions() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $this->assertFalse($optUserContext->removeAllForcedDecisions()); // no saved decisions + $this->assertTrue($optUserContext->removeAllForcedDecisions()); // no saved decisions $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); @@ -561,7 +561,7 @@ public function testRemoveAllForcedDecisions() $this->assertNull($optUserContext->getForcedDecision($featureKey, "test_experiment_2")); $this->assertNull($optUserContext->getForcedDecision($featureKey)); - $this->assertFalse($optUserContext->removeAllForcedDecisions()); // no more saved decisions + $this->assertTrue($optUserContext->removeAllForcedDecisions()); // no more saved decisions } } From 979a0e76095a4992a2425e929c6d143e705930ec Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 6 Oct 2021 16:44:37 +0500 Subject: [PATCH 14/28] Added depreciation warning in isFeatureEnabled getFeatureVariable{Type} getAllFeatureVariables getEnabledFeatures --- src/Optimizely/Optimizely.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 25467828..e1b0c5c2 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -795,6 +795,7 @@ public function getForcedVariation($experimentKey, $userId) * @param array Associative array of user attributes * * @return boolean + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null) { @@ -886,6 +887,7 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null) * @param string User ID * @param array Associative array of user attributes * @return array List of feature flag keys + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getEnabledFeatures($userId, $attributes = null) { @@ -1008,6 +1010,7 @@ public function getFeatureVariableValueForType( * @param array Associative array of user attributes * * @return string boolean variable value / null + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableBoolean($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1029,6 +1032,7 @@ public function getFeatureVariableBoolean($featureFlagKey, $variableKey, $userId * @param array Associative array of user attributes * * @return string integer variable value / null + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableInteger($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1050,6 +1054,7 @@ public function getFeatureVariableInteger($featureFlagKey, $variableKey, $userId * @param array Associative array of user attributes * * @return string double variable value / null + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableDouble($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1071,6 +1076,7 @@ public function getFeatureVariableDouble($featureFlagKey, $variableKey, $userId, * @param array Associative array of user attributes * * @return string variable value / null + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableString($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1092,6 +1098,7 @@ public function getFeatureVariableString($featureFlagKey, $variableKey, $userId, * @param array Associative array of user attributes * * @return array Associative array of json variable including key and value + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableJSON($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1112,6 +1119,7 @@ public function getFeatureVariableJSON($featureFlagKey, $variableKey, $userId, $ * @param array Associative array of user attributes * * @return array|null array of all the variables, or null if the feature key is invalid + * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getAllFeatureVariables($featureFlagKey, $userId, $attributes = null) { From d22ad11adda5dc807bf1d46aad4194890aec9ff7 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 6 Oct 2021 22:05:24 +0500 Subject: [PATCH 15/28] Fixed experimentId null or empty error in impression event --- .../Config/DatafileProjectConfig.php | 20 ++++++++++++++ .../Config/ProjectConfigInterface.php | 18 ++++++++++++- src/Optimizely/Event/Builder/EventBuilder.php | 26 +++++++++++++++---- src/Optimizely/Optimizely.php | 23 +++++++++------- src/Optimizely/OptimizelyUserContext.php | 3 +++ 5 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index 3aa6c4c3..bbd9f85c 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -645,6 +645,26 @@ public function getExperimentFromId($experimentId) return new Experiment(); } + /** + * Gets the variation associated with experiment or rollout in instance of given feature flag key + * + * @param string Feature flag key + * @param string variation key + * + * @return Variation / null + */ + public function getFlagVariationByKey($flagKey, $variationKey) + { + if (array_key_exists($flagKey, $this->_flagVariationsMap)) { + foreach ($this->_flagVariationsMap[$flagKey] as $variation) { + if ($variation->getKey() == $variationKey) { + return $variation; + } + } + } + return null; + } + /** * @param String $featureKey Key of the feature flag * diff --git a/src/Optimizely/Config/ProjectConfigInterface.php b/src/Optimizely/Config/ProjectConfigInterface.php index 887e706c..bbd02e94 100644 --- a/src/Optimizely/Config/ProjectConfigInterface.php +++ b/src/Optimizely/Config/ProjectConfigInterface.php @@ -199,7 +199,23 @@ public function getVariationFromKeyByExperimentId($experimentId, $variationKey); * @return FeatureVariable / null */ public function getFeatureVariableFromKey($featureFlagKey, $variableKey); - + + /** + * Gets the variation associated with experiment or rollout in instance of given feature flag key + * + * @param string Feature flag key + * @param string variation key + * + * @return Variation / null + */ + public function getFlagVariationByKey($flagKey, $variationKey); + + /** + * Returns map array of Flag key as key and Variations as value + * + * @return array + */ + public function getFlagVariationsMap(); /** * Determines if given experiment is a feature test. * diff --git a/src/Optimizely/Event/Builder/EventBuilder.php b/src/Optimizely/Event/Builder/EventBuilder.php index 8f2144cb..39ef68ab 100644 --- a/src/Optimizely/Event/Builder/EventBuilder.php +++ b/src/Optimizely/Event/Builder/EventBuilder.php @@ -27,6 +27,7 @@ use Optimizely\Utils\EventTagUtils; use Optimizely\Utils\GeneratorUtils; use Optimizely\Utils\Validator; +use phpDocumentor\Reflection\Types\This; class EventBuilder { @@ -139,18 +140,28 @@ private function getCommonParams($config, $userId, $attributes) * Helper function to get parameters specific to impression event. * * @param $experiment Experiment Experiment being activated. - * @param $variationId String ID representing the variation for the user. + * @param $variation Variation representing the variation for the user is allocated. + * @param $flagKey string feature flag key. + * @param $ruleKey string feature or rollout experiment key. + * @param $ruleType string feature or rollout experiment source type. + * @param $enabled Boolean feature enabled. * * @return array Hash representing parameters particular to impression event. */ private function getImpressionParams(Experiment $experiment, $variation, $flagKey, $ruleKey, $ruleType, $enabled) { $variationKey = $variation->getKey() ? $variation->getKey() : ''; + $experimentID = ''; + $campaignID = ''; + if ($experiment->getId()) { + $experimentID = $experiment->getId(); + $campaignID = $experiment->getLayerId(); + } $impressionParams = [ DECISIONS => [ [ - CAMPAIGN_ID => $experiment->getLayerId(), - EXPERIMENT_ID => $experiment->getId(), + CAMPAIGN_ID => $campaignID, + EXPERIMENT_ID => $experimentID, VARIATION_ID => $variation->getId(), METADATA => [ FLAG_KEY => $flagKey, @@ -232,9 +243,14 @@ private function getConversionParams($eventEntity, $eventTags) public function createImpressionEvent($config, $experimentId, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes) { $eventParams = $this->getCommonParams($config, $userId, $attributes); - $experiment = $config->getExperimentFromId($experimentId); - $variation = $config->getVariationFromKeyByExperimentId($experimentId, $variationKey); + + if (empty($experimentId)) { + $variation = $config->getFlagVariationByKey($flagKey, $variationKey); + } else { + $variation = $config->getVariationFromKeyByExperimentId($experimentId, $variationKey); + } + $impressionParams = $this->getImpressionParams($experiment, $variation, $flagKey, $ruleKey, $ruleType, $enabled); $eventParams[VISITORS][0][SNAPSHOTS][] = $impressionParams; diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index e1b0c5c2..a556feef 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -201,11 +201,15 @@ private function validateUserInputs($attributes, $eventTags = null) } /** + * @param DatafileProjectConfig DatafileProjectConfig instance * @param string Experiment ID * @param string Variation key + * @param string Flag key + * @param string Rule key + * @param string Rule type + * @param boolean Feature enabled * @param string User ID * @param array Associative array of user attributes - * @param DatafileProjectConfig DatafileProjectConfig instance */ protected function sendImpressionEvent($config, $experimentId, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes) { @@ -1302,15 +1306,16 @@ protected function validateInputs(array $values, $logLevel = Logger::ERROR) return $isValid; } + /** + * Gets the variation associated with experiment or rollout in instance of given feature flag key + * + * @param string Feature flag key + * @param string variation key + * + * @return Variation / null + */ public function getFlagVariationByKey($flagKey, $variationKey) { - if (array_key_exists($flagKey, $this->getConfig()->getFlagVariationsMap())) { - foreach ($this->getConfig()->getFlagVariationsMap()[$flagKey] as $variation) { - if ($variation->getKey() == $variationKey) { - return $variation; - } - } - } - return null; + return $this->getConfig()->getFlagVariationByKey($flagKey, $variationKey); } } diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index ac9cd347..93561cf2 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -66,6 +66,9 @@ public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) if (!$this->optimizelyClient->isValid()) { return false; } + if (empty($flagKey)) { + return false; + } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { $this->forcedDecisions[$index]->setVariationKey($variationKey); From a5843c2ffe925cc9838e114753c06fc67512ef97 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 6 Oct 2021 22:41:03 +0500 Subject: [PATCH 16/28] fixed logging and !=null comments --- .../DecisionService/DecisionService.php | 8 ++++---- src/Optimizely/Optimizely.php | 2 +- src/Optimizely/OptimizelyUserContext.php | 6 +++--- tests/ConfigTests/DatafileProjectConfigTest.php | 2 +- .../DecisionServiceTest.php | 17 +++++++++-------- tests/OptimizelyTest.php | 4 ++-- tests/OptimizelyUserContextTest.php | 10 +++++----- 7 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index d2d3957e..798da193 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -362,7 +362,7 @@ public function getVariationForFeatureRollout(ProjectConfigInterface $projectCon list($decisionResponses, $skipToEveryoneElse) = $this->getVariationFromDeliveryRule($projectConfig, $featureFlagKey, $rolloutRules, $index, $user, $decideOptions); $decideReasons = array_merge($decideReasons, $decisionResponses->getReasons()); $variation = $decisionResponses->getVariation(); - if ($variation != null) { + if ($variation) { return new FeatureDecision($rolloutRules[$index], $variation, FeatureDecision::DECISION_SOURCE_ROLLOUT, $decideReasons); } // the last rule is special for "Everyone Else" @@ -377,7 +377,7 @@ private function getVariationFromExperimentRule(ProjectConfigInterface $projectC // check forced-decision first list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey()); $decideReasons = array_merge($decideReasons, $reasons); - if ($decisionResponse != null) { + if ($decisionResponse) { return [$decisionResponse, $decideReasons]; } @@ -410,7 +410,7 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $options); $decideReasons = array_merge($decideReasons, $reasons); - if ($forcedDecisionResponse != null) { + if ($forcedDecisionResponse) { return [new FeatureDecision($rule, $forcedDecisionResponse, null, $decideReasons), $skipToEveryoneElse]; } @@ -436,7 +436,7 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $decideReasons[] = $message; list($bucketedVariation, $reasons) = $this->_bucketer->bucket($projectConfig, $rule, $bucketingId, $userId); $decideReasons = array_merge($decideReasons, $reasons); - if ($bucketedVariation != null) { + if ($bucketedVariation) { $message = sprintf('User "%s" is in the traffic group of targeting rule "%s".', $userId, $loggingKey); $this->_logger->log(Logger::INFO, $message); $decideReasons[] = $message; diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index a556feef..76cdc234 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -355,7 +355,7 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp // check forced-decisions first list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($flagKey, $ruleKey, $decideOptions); $decideReasons = array_merge($decideReasons, $reasons); - if ($forcedDecisionResponse != null) { + if ($forcedDecisionResponse) { $decision = new FeatureDecision(null, $forcedDecisionResponse, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); } else { // regular decision diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 93561cf2..3acf078a 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -94,14 +94,14 @@ public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = { $decideReasons = []; $variationKey = $this->findForcedDecision($flagKey, $ruleKey); - if ($variationKey != null) { + if ($variationKey) { $variation = $this->optimizelyClient->getFlagVariationByKey($flagKey, $variationKey); - $message = sprintf('Variation "%s" is mapped to "%s" and user "%s" in the forced decision map.', $variationKey, $flagKey, $this->userId); + $message = sprintf('Variation "%s" is mapped to %s and user "%s" in the forced decision map.', $variationKey, $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId); $decideReasons[] = $message; return [$variation, $decideReasons]; } else { - $message = sprintf('Invalid variation is mapped to "%s" and user "%s" in the forced decision map.', $flagKey, $this->userId); + $message = sprintf('Invalid variation is mapped to %s and user "%s" in the forced decision map.', $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId); $decideReasons[] = $message; } diff --git a/tests/ConfigTests/DatafileProjectConfigTest.php b/tests/ConfigTests/DatafileProjectConfigTest.php index b96196de..61cc3536 100644 --- a/tests/ConfigTests/DatafileProjectConfigTest.php +++ b/tests/ConfigTests/DatafileProjectConfigTest.php @@ -1,6 +1,6 @@ getMock(); $expectedReasons = [ - 'Invalid variation is mapped to "double_single_variable_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "double_single_variable_feature", rule "test_experiment_double_feature" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_double_feature" collectively evaluated to TRUE.', 'Assigned bucket 4513 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation control of experiment test_experiment_double_feature.', @@ -1987,7 +1987,7 @@ public function testDecideLogsWhenAudienceEvalFails() ->getMock(); $expectedReasons = [ - 'Invalid variation is mapped to "multi_variate_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "multi_variate_feature", rule "test_experiment_multivariate" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_multivariate" collectively evaluated to FALSE.', 'User "test_user" does not meet conditions to be in experiment "test_experiment_multivariate".', "The user 'test_user' is not bucketed into any of the experiments using the feature 'multi_variate_feature'.", diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index ba87e42e..2124ae62 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -397,7 +397,7 @@ public function testForcedDecisionInvalidFlagToDecision() $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), [ - 'Invalid variation is mapped to "boolean_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "boolean_feature", rule "test_experiment_2" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', @@ -426,13 +426,13 @@ public function testForcedDecisionInvalidDeliveryRuleToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), [ "The feature flag 'boolean_single_variable_feature' is not used in any experiments.", - 'Invalid variation is mapped to "boolean_single_variable_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_1" and user "test_user" in the forced decision map.', 'Audiences for rule 1 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "1".', - 'Invalid variation is mapped to "boolean_single_variable_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_2" and user "test_user" in the forced decision map.', 'Audiences for rule 2 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "2".', - 'Variation "invalid" is mapped to "boolean_single_variable_feature" and user "test_user" in the forced decision map.', + 'Variation "invalid" is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', 'Audiences for rule Everyone Else collectively evaluated to TRUE.', 'User "test_user" meets condition for targeting rule "Everyone Else".', 'Assigned bucket 3041 to user "test_user" with bucketing ID "test_user".', @@ -460,7 +460,7 @@ public function testForcedDecisionInvalidExperimentRuleToDecision() $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(1, count($decision->getUserContext()->getAttributes())); $this->assertEquals([ - 'Invalid variation is mapped to "boolean_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "boolean_feature", rule "test_experiment_2" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', From f387f8964e6cbc9f645dec085d3b3aa2bde3d8f6 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 6 Oct 2021 22:46:26 +0500 Subject: [PATCH 17/28] lint fix --- src/Optimizely/DecisionService/DecisionService.php | 2 +- tests/OptimizelyUserContextTest.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 798da193..1d42d6a9 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -440,7 +440,7 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $message = sprintf('User "%s" is in the traffic group of targeting rule "%s".', $userId, $loggingKey); $this->_logger->log(Logger::INFO, $message); $decideReasons[] = $message; - } else if (!$everyoneElse) { + } elseif (!$everyoneElse) { // skip this logging for EveryoneElse since this has a message not for EveryoneElse $message = sprintf('User "%s" is not in the traffic group for targeting rule "%s". Checking Everyone Else rule now.', $userId, $loggingKey); $this->_logger->log(Logger::INFO, $message); diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index 2124ae62..1170d4c6 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -564,4 +564,3 @@ public function testRemoveAllForcedDecisions() $this->assertTrue($optUserContext->removeAllForcedDecisions()); // no more saved decisions } } - From 3557793996381f1e0be637bc6ead4928dd8190f3 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 7 Oct 2021 20:37:50 +0500 Subject: [PATCH 18/28] removed depreciation warning --- src/Optimizely/Optimizely.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 76cdc234..d278f10f 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -799,7 +799,6 @@ public function getForcedVariation($experimentKey, $userId) * @param array Associative array of user attributes * * @return boolean - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null) { @@ -891,7 +890,6 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null) * @param string User ID * @param array Associative array of user attributes * @return array List of feature flag keys - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getEnabledFeatures($userId, $attributes = null) { @@ -1014,7 +1012,6 @@ public function getFeatureVariableValueForType( * @param array Associative array of user attributes * * @return string boolean variable value / null - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableBoolean($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1036,7 +1033,6 @@ public function getFeatureVariableBoolean($featureFlagKey, $variableKey, $userId * @param array Associative array of user attributes * * @return string integer variable value / null - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableInteger($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1058,7 +1054,6 @@ public function getFeatureVariableInteger($featureFlagKey, $variableKey, $userId * @param array Associative array of user attributes * * @return string double variable value / null - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableDouble($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1080,7 +1075,6 @@ public function getFeatureVariableDouble($featureFlagKey, $variableKey, $userId, * @param array Associative array of user attributes * * @return string variable value / null - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableString($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1102,7 +1096,6 @@ public function getFeatureVariableString($featureFlagKey, $variableKey, $userId, * @param array Associative array of user attributes * * @return array Associative array of json variable including key and value - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getFeatureVariableJSON($featureFlagKey, $variableKey, $userId, $attributes = null) { @@ -1123,7 +1116,6 @@ public function getFeatureVariableJSON($featureFlagKey, $variableKey, $userId, $ * @param array Associative array of user attributes * * @return array|null array of all the variables, or null if the feature key is invalid - * @deprecated Use 'decide' methods of 'OptimizelyUserContext' instead. */ public function getAllFeatureVariables($featureFlagKey, $userId, $attributes = null) { From 87971955318fa4945542abee03f799aa2e31e10e Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 11 Oct 2021 20:11:52 +0500 Subject: [PATCH 19/28] made rule key compulsory instead of optional and moved it to 2nd arg instead of third --- src/Optimizely/OptimizelyUserContext.php | 2 +- tests/OptimizelyTest.php | 6 ++--- tests/OptimizelyUserContextTest.php | 34 ++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 3acf078a..0105b2c2 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -60,7 +60,7 @@ public function removeAllForcedDecisions() return true; } - public function setForcedDecision($flagKey, $variationKey, $ruleKey = null) + public function setForcedDecision($flagKey, $ruleKey, $variationKey) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index b4879e7b..778c8c6e 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -551,7 +551,7 @@ public function testSetForcedDecisionExperimentRuleToDecisionSendImpressionAndNo $optimizelyMock->notificationCenter = $this->notificationCenterMock; - $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', 'variation', 'test_experiment_double_feature')); + $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', 'test_experiment_double_feature', 'variation')); $optimizelyDecision = $optimizelyMock->decide($userContext, 'double_single_variable_feature'); $expectedOptimizelyDecision = new OptimizelyDecision( 'variation', @@ -642,7 +642,7 @@ public function testSetForcedDecisionDeliveryRuleToDecisionSendImpressionAndNoti $optimizelyMock->notificationCenter = $this->notificationCenterMock; - $this->assertTrue($userContext->setForcedDecision('boolean_single_variable_feature', '177773', 'rollout_1_exp_2')); + $this->assertTrue($userContext->setForcedDecision('boolean_single_variable_feature', 'rollout_1_exp_2', '177773')); $optimizelyDecision = $optimizelyMock->decide($userContext, 'boolean_single_variable_feature'); $expectedOptimizelyDecision = new OptimizelyDecision( '177773', @@ -722,7 +722,7 @@ public function testSetForcedDecisionFlagToDecisionSendImpressionAndNotification $optimizelyMock->notificationCenter = $this->notificationCenterMock; - $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', 'variation')); + $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', null, 'variation')); $optimizelyDecision = $optimizelyMock->decide($userContext, 'double_single_variable_feature'); $expectedOptimizelyDecision = new OptimizelyDecision( 'variation', diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index 1170d4c6..9f3be553 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -270,7 +270,7 @@ public function testForcedDecisionInvalidDatafileReturnStatus() $invalidOptlyObject = new Optimizely("Invalid datafile"); $optUserContext = new OptimizelyUserContext($invalidOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("flag1", "variation1", "targeted_delivery"); + $setForcedDecision = $optUserContext->setForcedDecision("flag1", "targeted_delivery", "variation1"); $this->assertFalse($setForcedDecision); $getForcedDecision = $optUserContext->getForcedDecision("flag1", "targeted_delivery"); @@ -291,7 +291,7 @@ public function testForcedDecisionValidDatafileReturnStatus() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("flag1", "variation1", "targeted_delivery"); + $setForcedDecision = $optUserContext->setForcedDecision("flag1", "targeted_delivery", "variation1"); $this->assertTrue($setForcedDecision); $getForcedDecision = $optUserContext->getForcedDecision("flag1", "targeted_delivery"); @@ -312,7 +312,7 @@ public function testForcedDecisionFlagToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "177773"); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", null, "177773"); $this->assertTrue($setForcedDecision); $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature"); @@ -349,7 +349,7 @@ public function testForcedDecisionRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "test_variation_1", "test_experiment_2"); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "test_experiment_2", "test_variation_1"); $this->assertTrue($setForcedDecision); $getForcedDecision = $optUserContext->getForcedDecision("boolean_feature", "test_experiment_2"); @@ -386,7 +386,7 @@ public function testForcedDecisionInvalidFlagToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "invalid"); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", null, "invalid"); $this->assertTrue($setForcedDecision); $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -414,7 +414,7 @@ public function testForcedDecisionInvalidDeliveryRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "invalid", "rollout_1_exp_3"); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3", "invalid"); $this->assertTrue($setForcedDecision); $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -449,7 +449,7 @@ public function testForcedDecisionInvalidExperimentRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "invalid"); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", null, "invalid"); $this->assertTrue($setForcedDecision); $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -477,10 +477,10 @@ public function testForcedDecisionConflicts() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision1 = $optUserContext->setForcedDecision($featureKey, "test_variation_1"); + $setForcedDecision1 = $optUserContext->setForcedDecision($featureKey, null, "test_variation_1"); $this->assertTrue($setForcedDecision1); - $setForcedDecision2 = $optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2"); + $setForcedDecision2 = $optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2"); $this->assertTrue($setForcedDecision2); // flag-to-decision is the 1st priority @@ -499,16 +499,16 @@ public function testGetForcedDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_1")); $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_2")); $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey)); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1", "test_experiment_2")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_1")); $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2")); $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey)); @@ -523,8 +523,8 @@ public function testRemoveForcedDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_1")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2")); $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); @@ -551,8 +551,8 @@ public function testRemoveAllForcedDecisions() $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); $this->assertTrue($optUserContext->removeAllForcedDecisions()); // no saved decisions - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_1")); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_variation_2", "test_experiment_2")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_1")); + $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2")); $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); From 83ef326fc80b586c3188bb63079c168e955c30bd Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 11 Oct 2021 22:17:38 +0500 Subject: [PATCH 20/28] - moved the two methods of remove and remove all forceddecision after getForcedDecision - Added logging of invalid project config while checking if optlyclient is valid - added variation is not null check to make sure valid variation is getting mapped and correct log is getting print --- src/Optimizely/Optimizely.php | 9 +++- src/Optimizely/OptimizelyUserContext.php | 59 +++++++++++++----------- tests/OptimizelyUserContextTest.php | 41 ++++++++++++++-- 3 files changed, 77 insertions(+), 32 deletions(-) diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index d278f10f..721375af 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -1263,7 +1263,14 @@ private function getFeatureVariableValueFromVariation($featureFlagKey, $variable */ public function isValid() { - return $this->getConfig() !== null; + if (!$this->getConfig()) { + $this->_logger->log( + Logger::ERROR, + "Optimizely SDK not configured properly yet." + ); + return false; + } + return true; } /** diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 0105b2c2..69204333 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -35,77 +35,77 @@ public function __construct(Optimizely $optimizelyClient, $userId, array $attrib $this->forcedDecisions = $forcedDecisions; } - public function removeForcedDecision($flagKey, $ruleKey = null) + public function setForcedDecision($flagKey, $ruleKey, $variationKey) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { return false; } + if (empty($flagKey)) { + return false; + } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { - array_splice($this->forcedDecisions, $index, 1); - return true; + $this->forcedDecisions[$index]->setVariationKey($variationKey); + } else { + if (!$this->forcedDecisions) { + $this->forcedDecisions = array(); + } + array_push($this->forcedDecisions, new ForcedDecision($flagKey, $ruleKey, $variationKey)); } - return false; + return true; } - public function removeAllForcedDecisions() + public function getForcedDecision($flagKey, $ruleKey = null) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { - return false; + return null; } - - $this->forcedDecisions = []; - return true; + return $this->findForcedDecision($flagKey, $ruleKey); } - public function setForcedDecision($flagKey, $ruleKey, $variationKey) + public function removeForcedDecision($flagKey, $ruleKey = null) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { return false; } - if (empty($flagKey)) { - return false; - } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); if ($index != -1) { - $this->forcedDecisions[$index]->setVariationKey($variationKey); - } else { - if (!$this->forcedDecisions) { - $this->forcedDecisions = array(); - } - array_push($this->forcedDecisions, new ForcedDecision($flagKey, $ruleKey, $variationKey)); + array_splice($this->forcedDecisions, $index, 1); + return true; } - return true; + return false; } - public function getForcedDecision($flagKey, $ruleKey = null) + public function removeAllForcedDecisions() { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { - return null; + return false; } - return $this->findForcedDecision($flagKey, $ruleKey); + + $this->forcedDecisions = []; + return true; } public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = []) { $decideReasons = []; $variationKey = $this->findForcedDecision($flagKey, $ruleKey); + $variation = null; if ($variationKey) { $variation = $this->optimizelyClient->getFlagVariationByKey($flagKey, $variationKey); + } + if ($variation) { $message = sprintf('Variation "%s" is mapped to %s and user "%s" in the forced decision map.', $variationKey, $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId); - $decideReasons[] = $message; - return [$variation, $decideReasons]; } else { $message = sprintf('Invalid variation is mapped to %s and user "%s" in the forced decision map.', $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId); - $decideReasons[] = $message; } - return [null, $decideReasons]; + return [$variation, $decideReasons]; } private function findExistingRuleAndFlagKey($flagKey, $ruleKey) @@ -123,7 +123,10 @@ private function findExistingRuleAndFlagKey($flagKey, $ruleKey) public function findForcedDecision($flagKey, $ruleKey) { $foundVariationKey = null; - if ($this->forcedDecisions && count($this->forcedDecisions) == 0) { + if (!$this->forcedDecisions) { + return null; + } + if (count($this->forcedDecisions) == 0) { return null; } $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index 9f3be553..dbd35eaa 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -340,8 +340,7 @@ public function testForcedDecisionFlagToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), []); } - - public function testForcedDecisionRuleToDecision() + public function testForcedDecisionExperimentRuleToDecision() { $userId = 'test_user'; $attributes = [ "browser" => "chrome"]; @@ -377,6 +376,42 @@ public function testForcedDecisionRuleToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), []); } + public function testForcedDecisionRuleDeliveryRuleToDecision() + { + $userId = 'test_user'; + $attributes = [ "browser" => "chrome"]; + + $validOptlyObject = new Optimizely($this->datafile); + + $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); + $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3", "177773"); + $this->assertTrue($setForcedDecision); + + $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3"); + $this->assertEquals($getForcedDecision, "177773"); + + $decision = $optUserContext->decide('boolean_single_variable_feature'); + $this->assertEquals($decision->getVariationKey(), "177773"); + $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), []); + + // Removing forced decision to test + $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3"); + $this->assertTrue($removeForcedDecision); + + $decision = $optUserContext->decide('boolean_single_variable_feature'); + $this->assertEquals($decision->getVariationKey(), "177778"); + $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); + $this->assertTrue($decision->getEnabled()); + $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); + $this->assertEquals($decision->getUserContext()->getUserId(), $userId); + $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); + $this->assertEquals($decision->getReasons(), []); + } public function testForcedDecisionInvalidFlagToDecision() { @@ -432,7 +467,7 @@ public function testForcedDecisionInvalidDeliveryRuleToDecision() 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_2" and user "test_user" in the forced decision map.', 'Audiences for rule 2 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "2".', - 'Variation "invalid" is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', 'Audiences for rule Everyone Else collectively evaluated to TRUE.', 'User "test_user" meets condition for targeting rule "Everyone Else".', 'Assigned bucket 3041 to user "test_user" with bucketing ID "test_user".', From b7b980dbbca6549d28c348a9257b7d972c5ddb0d Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 12 Oct 2021 18:47:01 +0500 Subject: [PATCH 21/28] Added Decided by forceddecision reason fixed log messages --- .../DecisionService/DecisionService.php | 2 +- src/Optimizely/Optimizely.php | 7 +- src/Optimizely/OptimizelyUserContext.php | 15 ++-- .../ConfigTests/DatafileProjectConfigTest.php | 2 +- .../DecisionServiceTest.php | 8 -- tests/OptimizelyTest.php | 2 - tests/OptimizelyUserContextTest.php | 77 ++++++++++--------- 7 files changed, 52 insertions(+), 61 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 1d42d6a9..2b843649 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -407,7 +407,7 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $skipToEveryoneElse = false; // check forced-decision first $rule = $rules[$ruleIndex]; - list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey(), $options); + list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey()); $decideReasons = array_merge($decideReasons, $reasons); if ($forcedDecisionResponse) { diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 721375af..6ff9fba4 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -353,8 +353,7 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp // get decision $decision = null; // check forced-decisions first - list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($flagKey, $ruleKey, $decideOptions); - $decideReasons = array_merge($decideReasons, $reasons); + list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($flagKey, $ruleKey); if ($forcedDecisionResponse) { $decision = new FeatureDecision(null, $forcedDecisionResponse, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); } else { @@ -366,8 +365,8 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp $decideOptions ); } - - $decideReasons = $decision->getReasons(); + $decideReasons = array_merge($decideReasons, $reasons); + $decideReasons = array_merge($decideReasons, $decision->getReasons()); $variation = $decision->getVariation(); if ($variation) { diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 69204333..d87937e4 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -90,20 +90,19 @@ public function removeAllForcedDecisions() return true; } - public function findValidatedForcedDecision($flagKey, $ruleKey, array $options = []) + public function findValidatedForcedDecision($flagKey, $ruleKey) { $decideReasons = []; $variationKey = $this->findForcedDecision($flagKey, $ruleKey); $variation = null; if ($variationKey) { $variation = $this->optimizelyClient->getFlagVariationByKey($flagKey, $variationKey); - } - if ($variation) { - $message = sprintf('Variation "%s" is mapped to %s and user "%s" in the forced decision map.', $variationKey, $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId); - $decideReasons[] = $message; - } else { - $message = sprintf('Invalid variation is mapped to %s and user "%s" in the forced decision map.', $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId); - $decideReasons[] = $message; + if ($variation) { + array_push($decideReasons, 'Decided by forced decision.'); + array_push($decideReasons, sprintf('Variation "%s" is mapped to %s and user "%s" in the forced decision map.', $variationKey, $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId)); + } else { + array_push($decideReasons, sprintf('Invalid variation is mapped to %s and user "%s" in the forced decision map.', $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId)); + } } return [$variation, $decideReasons]; } diff --git a/tests/ConfigTests/DatafileProjectConfigTest.php b/tests/ConfigTests/DatafileProjectConfigTest.php index 61cc3536..ccbefa50 100644 --- a/tests/ConfigTests/DatafileProjectConfigTest.php +++ b/tests/ConfigTests/DatafileProjectConfigTest.php @@ -1,6 +1,6 @@ getMock(); $expectedReasons = [ - 'Invalid variation is mapped to flag "double_single_variable_feature", rule "test_experiment_double_feature" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_double_feature" collectively evaluated to TRUE.', 'Assigned bucket 4513 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation control of experiment test_experiment_double_feature.', @@ -1987,7 +1986,6 @@ public function testDecideLogsWhenAudienceEvalFails() ->getMock(); $expectedReasons = [ - 'Invalid variation is mapped to flag "multi_variate_feature", rule "test_experiment_multivariate" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_multivariate" collectively evaluated to FALSE.', 'User "test_user" does not meet conditions to be in experiment "test_experiment_multivariate".', "The user 'test_user' is not bucketed into any of the experiments using the feature 'multi_variate_feature'.", diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index dbd35eaa..971da0f5 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -354,27 +354,36 @@ public function testForcedDecisionExperimentRuleToDecision() $getForcedDecision = $optUserContext->getForcedDecision("boolean_feature", "test_experiment_2"); $this->assertEquals($getForcedDecision, "test_variation_1"); - $decision = $optUserContext->decide('boolean_feature'); + $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals($decision->getVariationKey(), "test_variation_1"); $this->assertEquals($decision->getRuleKey(), "test_experiment_2"); $this->assertTrue($decision->getEnabled()); $this->assertEquals($decision->getFlagKey(), "boolean_feature"); $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); - $this->assertEquals($decision->getReasons(), []); + $this->assertEquals($decision->getReasons(), [ + 'Decided by forced decision.', + 'Variation "test_variation_1" is mapped to flag "boolean_feature", rule "test_experiment_2" and user "test_user" in the forced decision map.', + "The user 'test_user' is bucketed into experiment 'test_experiment_2' of feature 'boolean_feature'." + ]); // Removing forced decision to test $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_feature", "test_experiment_2"); $this->assertTrue($removeForcedDecision); - $decision = $optUserContext->decide('boolean_feature'); + $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals($decision->getVariationKey(), "test_variation_2"); $this->assertEquals($decision->getRuleKey(), "test_experiment_2"); $this->assertTrue($decision->getEnabled()); $this->assertEquals($decision->getFlagKey(), "boolean_feature"); $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); - $this->assertEquals($decision->getReasons(), []); + $this->assertEquals($decision->getReasons(), [ + 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', + 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', + 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', + "The user 'test_user' is bucketed into experiment 'test_experiment_2' of feature 'boolean_feature'." + ]); } public function testForcedDecisionRuleDeliveryRuleToDecision() { @@ -390,57 +399,49 @@ public function testForcedDecisionRuleDeliveryRuleToDecision() $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3"); $this->assertEquals($getForcedDecision, "177773"); - $decision = $optUserContext->decide('boolean_single_variable_feature'); + $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals($decision->getVariationKey(), "177773"); $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); $this->assertTrue($decision->getEnabled()); $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); - $this->assertEquals($decision->getReasons(), []); + $this->assertEquals($decision->getReasons(), [ + "The feature flag 'boolean_single_variable_feature' is not used in any experiments.", + 'Audiences for rule 1 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "1".', + 'Audiences for rule 2 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "2".', + 'Decided by forced decision.', + 'Variation "177773" is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', + "User 'test_user' is bucketed into rollout for feature flag 'boolean_single_variable_feature'." + ]); // Removing forced decision to test $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3"); $this->assertTrue($removeForcedDecision); - $decision = $optUserContext->decide('boolean_single_variable_feature'); + $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals($decision->getVariationKey(), "177778"); $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); $this->assertTrue($decision->getEnabled()); $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); - $this->assertEquals($decision->getReasons(), []); - } - - public function testForcedDecisionInvalidFlagToDecision() - { - $userId = 'test_user'; - $attributes = [ "browser" => "chrome"]; - - $validOptlyObject = new Optimizely($this->datafile); - - $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", null, "invalid"); - $this->assertTrue($setForcedDecision); - - $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); - $this->assertEquals($decision->getVariationKey(), "test_variation_2"); - $this->assertEquals($decision->getRuleKey(), "test_experiment_2"); - $this->assertTrue($decision->getEnabled()); - $this->assertEquals($decision->getFlagKey(), "boolean_feature"); - $this->assertEquals($decision->getUserContext()->getUserId(), $userId); - $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), [ - 'Invalid variation is mapped to flag "boolean_feature", rule "test_experiment_2" and user "test_user" in the forced decision map.', - 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', - 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', - 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', - "The user 'test_user' is bucketed into experiment 'test_experiment_2' of feature 'boolean_feature'." + "The feature flag 'boolean_single_variable_feature' is not used in any experiments.", + 'Audiences for rule 1 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "1".', + 'Audiences for rule 2 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "2".', + 'Audiences for rule Everyone Else collectively evaluated to TRUE.', + 'User "test_user" meets condition for targeting rule "Everyone Else".', + 'Assigned bucket 3041 to user "test_user" with bucketing ID "test_user".', + 'User "test_user" is in the traffic group of targeting rule "Everyone Else".', + "User 'test_user' is bucketed into rollout for feature flag 'boolean_single_variable_feature'." ]); } - public function testForcedDecisionInvalidDeliveryRuleToDecision() { $userId = 'test_user'; @@ -461,10 +462,8 @@ public function testForcedDecisionInvalidDeliveryRuleToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), [ "The feature flag 'boolean_single_variable_feature' is not used in any experiments.", - 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_1" and user "test_user" in the forced decision map.', 'Audiences for rule 1 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "1".', - 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_2" and user "test_user" in the forced decision map.', 'Audiences for rule 2 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "2".', 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', @@ -495,7 +494,7 @@ public function testForcedDecisionInvalidExperimentRuleToDecision() $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(1, count($decision->getUserContext()->getAttributes())); $this->assertEquals([ - 'Invalid variation is mapped to flag "boolean_feature", rule "test_experiment_2" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag "boolean_feature" and user "test_user" in the forced decision map.', 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', @@ -523,6 +522,10 @@ public function testForcedDecisionConflicts() $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals("test_variation_1", $decision->getVariationKey()); $this->assertNull($decision->getRuleKey()); + $this->assertEquals([ + 'Decided by forced decision.', + 'Variation "test_variation_1" is mapped to flag "boolean_feature" and user "test_user" in the forced decision map.' + ], $decision->getReasons()); } public function testGetForcedDecision() From 280371c66afbfd2d309e57aed3da33cc7da476df Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 13 Oct 2021 19:53:49 +0500 Subject: [PATCH 22/28] Added reasons in test case --- tests/OptimizelyUserContextTest.php | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index 971da0f5..fe2fd183 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -318,27 +318,41 @@ public function testForcedDecisionFlagToDecision() $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature"); $this->assertEquals($getForcedDecision, "177773"); - $decision = $optUserContext->decide('boolean_single_variable_feature'); + $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals($decision->getVariationKey(), "177773"); $this->assertNull($decision->getRuleKey()); $this->assertTrue($decision->getEnabled()); $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); - $this->assertEquals($decision->getReasons(), []); + $this->assertEquals($decision->getReasons(), [ + 'Decided by forced decision.', + 'Variation "177773" is mapped to flag "boolean_single_variable_feature" and user "test_user" in the forced decision map.' + ]); // Removing forced decision to test $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_single_variable_feature"); $this->assertTrue($removeForcedDecision); - $decision = $optUserContext->decide('boolean_single_variable_feature'); + $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); $this->assertEquals($decision->getVariationKey(), "177778"); $this->assertEquals($decision->getRuleKey(), "rollout_1_exp_3"); $this->assertTrue($decision->getEnabled()); $this->assertEquals($decision->getFlagKey(), "boolean_single_variable_feature"); $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); - $this->assertEquals($decision->getReasons(), []); + $this->assertEquals($decision->getReasons(), [ + "The feature flag 'boolean_single_variable_feature' is not used in any experiments.", + 'Audiences for rule 1 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "1".', + 'Audiences for rule 2 collectively evaluated to FALSE.', + 'User "test_user" does not meet conditions for targeting rule "2".', + 'Audiences for rule Everyone Else collectively evaluated to TRUE.', + 'User "test_user" meets condition for targeting rule "Everyone Else".', + 'Assigned bucket 3041 to user "test_user" with bucketing ID "test_user".', + 'User "test_user" is in the traffic group of targeting rule "Everyone Else".', + "User 'test_user' is bucketed into rollout for feature flag 'boolean_single_variable_feature'." + ]); } public function testForcedDecisionExperimentRuleToDecision() { From 9168430cd878451d4ac61a5238707108cf9498e8 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 14 Oct 2021 20:47:26 +0500 Subject: [PATCH 23/28] DatafileprojectConfig line 248 added verification that redundant variations are getting removed --- .../Config/DatafileProjectConfig.php | 13 ++- .../ConfigTests/DatafileProjectConfigTest.php | 49 +++++++++- tests/OptimizelyTest.php | 54 +++++++--- tests/TestData.php | 98 +++++++++++++++++++ 4 files changed, 192 insertions(+), 22 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index bbd9f85c..2deb3aca 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -385,16 +385,21 @@ public function __construct($datafile, $logger, $errorHandler) } $this->_flagVariationsMap = array(); foreach ($this->_featureFlags as $flag) { - $variations = array(); + $flagVariations = array(); $flagRules = $this->getAllRulesForFlag($flag); foreach ($flagRules as $rule) { - $variations = array_merge($variations, array_filter(array_values($rule->getVariations()), function ($variation) use ($variations) { - return !in_array($variation->getId(), $variations); + $flagVariations = array_merge($flagVariations, array_filter(array_values($rule->getVariations()), function ($variation) use ($flagVariations) { + foreach ($flagVariations as $flagVariation) { + if ($flagVariation->getId() == $variation->getId()) { + return false; + } + } + return true; })); } - $this->_flagVariationsMap[$flag->getKey()] = $variations; + $this->_flagVariationsMap[$flag->getKey()] = $flagVariations; } // Add variations for rollout experiments to variationIdMap and variationKeyMap $this->_variationIdMap = $this->_variationIdMap + $rolloutVariationIdMap; diff --git a/tests/ConfigTests/DatafileProjectConfigTest.php b/tests/ConfigTests/DatafileProjectConfigTest.php index ccbefa50..bfd3d513 100644 --- a/tests/ConfigTests/DatafileProjectConfigTest.php +++ b/tests/ConfigTests/DatafileProjectConfigTest.php @@ -122,6 +122,10 @@ public function testInit() 'rollout_1_exp_3' => $this->config->getExperimentFromKey('rollout_1_exp_3'), 'rollout_2_exp_1' => $this->config->getExperimentFromKey('rollout_2_exp_1'), 'rollout_2_exp_2' => $this->config->getExperimentFromKey('rollout_2_exp_2'), + 'flag1_targeted_delivery' => $this->config->getExperimentFromKey('flag1_targeted_delivery'), + 'flag1_targeted_delivery2' => $this->config->getExperimentFromKey('flag1_targeted_delivery2'), + 'flag1_targeted_delivery3' => $this->config->getExperimentFromKey('flag1_targeted_delivery3'), + 'default-rollout-5969-20778120250' => $this->config->getExperimentFromKey('default-rollout-5969-20778120250'), ], $experimentKeyMap->getValue($this->config) ); @@ -147,6 +151,10 @@ public function testInit() '177774' => $this->config->getExperimentFromId('177774'), '177779' => $this->config->getExperimentFromId('177779'), '7716830083' => $this->config->getExperimentFromId('7716830083'), + '9300000018548' => $this->config->getExperimentFromId('9300000018548'), + '9300000018549' => $this->config->getExperimentFromId('9300000018549'), + '9300000018550' => $this->config->getExperimentFromId('9300000018550'), + 'default-rollout-5969-20778120250' => $this->config->getExperimentFromId('default-rollout-5969-20778120250'), ], $experimentIdMap->getValue($this->config) ); @@ -184,7 +192,10 @@ public function testInit() $this->assertEquals( [ '7718080042' => $this->config->getAudience('7718080042'), - '11155' => $this->config->getAudience('11155') + '11155' => $this->config->getAudience('11155'), + '20803170009' => $this->config->getAudience('20803170009'), + '20787080332' => $this->config->getAudience('20787080332'), + '20778250294' => $this->config->getAudience('20778250294') ], $audienceIdMap->getValue($this->config) ); @@ -233,7 +244,11 @@ public function testInit() $this->config->getVariationFromKey('group_experiment_2', 'group_exp_2_var_1'), $this->config->getVariationFromKey('group_experiment_2', 'group_exp_2_var_2') ], - 'empty_feature' => [] + 'empty_feature' => [], + 'same_variation_flag' => [ + $this->config->getVariationFromKey('flag1_targeted_delivery', 'new_variation'), + $this->config->getVariationFromKey('default-rollout-5969-20778120250', 'off') + ] ], $flagVariationsMap->getValue($this->config) ); @@ -299,6 +314,18 @@ public function testInit() ], 'test_experiment_json_feature' => [ 'json_variation' => $this->config->getVariationFromKey('test_experiment_json_feature', 'json_variation') + ], + 'flag1_targeted_delivery' => [ + 'new_variation' => $this->config->getVariationFromKey('flag1_targeted_delivery', 'new_variation') + ], + 'flag1_targeted_delivery2' => [ + 'new_variation' => $this->config->getVariationFromKey('flag1_targeted_delivery2', 'new_variation') + ], + 'flag1_targeted_delivery3' => [ + 'new_variation' => $this->config->getVariationFromKey('flag1_targeted_delivery3', 'new_variation') + ], + 'default-rollout-5969-20778120250' => [ + 'off' => $this->config->getVariationFromKey('default-rollout-5969-20778120250', 'off') ] ], $variationKeyMap->getValue($this->config) @@ -364,6 +391,18 @@ public function testInit() ], 'test_experiment_json_feature' => [ '122246' => $this->config->getVariationFromId('test_experiment_json_feature', '122246') + ], + 'flag1_targeted_delivery' => [ + '16421' => $this->config->getVariationFromId('flag1_targeted_delivery', '16421') + ], + 'flag1_targeted_delivery2' => [ + '16421' => $this->config->getVariationFromId('flag1_targeted_delivery2', '16421') + ], + 'flag1_targeted_delivery3' => [ + '16421' => $this->config->getVariationFromId('flag1_targeted_delivery3', '16421') + ], + 'default-rollout-5969-20778120250' => [ + '16419' => $this->config->getVariationFromId('default-rollout-5969-20778120250', '16419') ] ], $variationIdMap->getValue($this->config) @@ -383,7 +422,8 @@ public function testInit() 'multiple_variables_feature' => $this->config->getFeatureFlagFromKey('multiple_variables_feature'), 'multi_variate_feature' => $this->config->getFeatureFlagFromKey('multi_variate_feature'), 'mutex_group_feature' => $this->config->getFeatureFlagFromKey('mutex_group_feature'), - 'empty_feature' => $this->config->getFeatureFlagFromKey('empty_feature') + 'empty_feature' => $this->config->getFeatureFlagFromKey('empty_feature'), + 'same_variation_flag' => $this->config->getFeatureFlagFromKey('same_variation_flag') ], $featureFlagKeyMap->getValue($this->config) ); @@ -395,7 +435,8 @@ public function testInit() $this->assertEquals( [ '166660' => $this->config->getRolloutFromId('166660'), - '166661' => $this->config->getRolloutFromId('166661') + '166661' => $this->config->getRolloutFromId('166661'), + 'rollout-5969-20778120250' => $this->config->getRolloutFromId('rollout-5969-20778120250') ], $rolloutIdMap->getValue($this->config) ); diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index a394ea74..b8992e6b 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -2256,7 +2256,7 @@ public function testDecideAll() $userContext = $optimizelyMock->createUserContext('test_user', ['device_type' => 'iPhone']); - $this->notificationCenterMock->expects($this->exactly(9)) + $this->notificationCenterMock->expects($this->exactly(10)) ->method('sendNotifications'); //assert that sendImpressionEvent is called with expected params @@ -2287,7 +2287,7 @@ public function testDecideAll() $optimizelyDecisions = $optimizelyMock->decideAll($userContext); - $this->assertEquals(count($optimizelyDecisions), 9); + $this->assertEquals(count($optimizelyDecisions), 10); $this->compareOptimizelyDecisions($expectedOptimizelyDecision1, $optimizelyDecisions['double_single_variable_feature']); $this->compareOptimizelyDecisions($expectedOptimizelyDecision2, $optimizelyDecisions['boolean_feature']); @@ -2307,6 +2307,7 @@ public function testDecideAllCallsDecideForKeysAndReturnsItsResponse() 'boolean_single_variable_feature', 'string_single_variable_feature', 'multiple_variables_feature', + 'same_variation_flag', 'multi_variate_feature', 'mutex_group_feature', 'empty_feature' @@ -5098,7 +5099,7 @@ public function testGetEnabledFeaturesGivenNoFeatureIsEnabledForUser() ->getMock(); // Mock isFeatureEnabled to return false for all calls - $optimizelyMock->expects($this->exactly(9)) + $optimizelyMock->expects($this->exactly(10)) ->method('isFeatureEnabled') ->will($this->returnValue(false)); @@ -5124,7 +5125,7 @@ public function testGetEnabledFeaturesGivenFeaturesAreEnabledForUser() ]; // Mock isFeatureEnabled to return specific values - $optimizelyMock->expects($this->exactly(9)) + $optimizelyMock->expects($this->exactly(10)) ->method('isFeatureEnabled') ->will($this->returnValueMap($map)); @@ -5147,7 +5148,7 @@ public function testGetEnabledFeaturesGivenFeaturesAreEnabledForEmptyUserID() ]; // Mock isFeatureEnabled to return specific values - $optimizelyMock->expects($this->exactly(9)) + $optimizelyMock->expects($this->exactly(10)) ->method('isFeatureEnabled') ->will($this->returnValueMap($map)); @@ -5176,19 +5177,20 @@ public function testGetEnabledFeaturesWithUserAttributes() ['integer_single_variable_feature','user_id', $userAttributes, false], ['boolean_single_variable_feature','user_id', $userAttributes, false], ['string_single_variable_feature','user_id', $userAttributes, false], + ['same_variation_flag','user_id', $userAttributes, true], ['multi_variate_feature','user_id', $userAttributes, false], ['mutex_group_feature','user_id', $userAttributes, false], ['empty_feature','user_id', $userAttributes, true], ]; // Assert that isFeatureEnabled is called with the same attributes and mock to return value map - $optimizelyMock->expects($this->exactly(9)) + $optimizelyMock->expects($this->exactly(10)) ->method('isFeatureEnabled') ->with() ->will($this->returnValueMap($map)); $this->assertEquals( - ['empty_feature'], + ['same_variation_flag', 'empty_feature'], $optimizelyMock->getEnabledFeatures("user_id", $userAttributes) ); } @@ -5250,22 +5252,27 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() FeatureDecision::DECISION_SOURCE_FEATURE_TEST ); $decision7 = new FeatureDecision( + $experiment, + $enabledFeatureVariation, + FeatureDecision::DECISION_SOURCE_ROLLOUT + ); + $decision8 = new FeatureDecision( $disabledFeatureExperiment, $disabledFeatureVariation, FeatureDecision::DECISION_SOURCE_FEATURE_TEST ); - $decision8 = new FeatureDecision( + $decision9 = new FeatureDecision( $experiment, $enabledFeatureVariation, FeatureDecision::DECISION_SOURCE_ROLLOUT ); - $decision9 = new FeatureDecision( + $decision10 = new FeatureDecision( $disabledFeatureExperiment, $disabledFeatureVariation, FeatureDecision::DECISION_SOURCE_ROLLOUT ); - $decisionServiceMock->expects($this->exactly(9)) + $decisionServiceMock->expects($this->exactly(10)) ->method('getVariationForFeature') ->will($this->onConsecutiveCalls( $decision1, @@ -5276,7 +5283,9 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() $decision6, $decision7, $decision8, - $decision9 + $decision9, + $decision10 + )); $optimizelyMock->notificationCenter = $this->notificationCenterMock; @@ -5394,8 +5403,24 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() ) ) ); - $this->notificationCenterMock->expects($this->at(6)) + ->method('sendNotifications') + ->with( + NotificationType::DECISION, + array( + DecisionNotificationTypes::FEATURE, + 'user_id', + [], + (object) array( + 'featureKey'=>'same_variation_flag', + 'featureEnabled'=> true, + 'source'=> 'rollout', + 'sourceInfo'=> (object) array() + ) + ) + ); + + $this->notificationCenterMock->expects($this->at(7)) ->method('sendNotifications') ->with( NotificationType::DECISION, @@ -5415,7 +5440,7 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() ) ); - $this->notificationCenterMock->expects($this->at(7)) + $this->notificationCenterMock->expects($this->at(8)) ->method('sendNotifications') ->with( NotificationType::DECISION, @@ -5432,7 +5457,7 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() ) ); - $this->notificationCenterMock->expects($this->at(8)) + $this->notificationCenterMock->expects($this->at(9)) ->method('sendNotifications') ->with( NotificationType::DECISION, @@ -5455,6 +5480,7 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() 'integer_single_variable_feature', 'string_single_variable_feature', 'multiple_variables_feature', + 'same_variation_flag', 'mutex_group_feature' ], $optimizelyMock->getEnabledFeatures("user_id") diff --git a/tests/TestData.php b/tests/TestData.php index 10c9ed83..8eb4b09b 100644 --- a/tests/TestData.php +++ b/tests/TestData.php @@ -454,6 +454,21 @@ ], "version": "4", "audiences": [ + { + "id": "20803170009", + "conditions": "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"def\"}]]]", + "name": "aud2" + }, + { + "id": "20787080332", + "conditions": "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"abc\"}]]]", + "name": "aud1" + }, + { + "id": "20778250294", + "conditions": "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"ga\"}]]]", + "name": "aud" + }, { "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"iPhone\"}]], [\"or\", [\"or\", {\"name\": \"location\", \"type\": \"custom_attribute\", \"value\": \"San Francisco\"}]]]", "id": "7718080042", @@ -758,6 +773,13 @@ } ] }, + { + "experimentIds": [], + "rolloutId": "rollout-5969-20778120250", + "variables": [], + "id": "5969", + "key": "same_variation_flag" + }, { "id": "155559", "key": "multi_variate_feature", @@ -899,6 +921,82 @@ } ] }, + { + "experiments": [{ + "status": "Running", + "audienceConditions": ["or", "20778250294"], + "audienceIds": ["20778250294"], + "variations": [{ + "variables": [], + "id": "16421", + "key": "new_variation", + "featureEnabled": true + }], + "forcedVariations": {}, + "key": "flag1_targeted_delivery", + "layerId": "9300000018514", + "trafficAllocation": [{ + "entityId": "16421", + "endOfRange": 10000 + }], + "id": "9300000018548" + }, { + "status": "Running", + "audienceConditions": ["or", "20803170009"], + "audienceIds": ["20803170009"], + "variations": [{ + "variables": [], + "id": "16421", + "key": "new_variation", + "featureEnabled": true + }], + "forcedVariations": {}, + "key": "flag1_targeted_delivery2", + "layerId": "9300000018515", + "trafficAllocation": [{ + "entityId": "16421", + "endOfRange": 10000 + }], + "id": "9300000018549" + }, { + "status": "Running", + "audienceConditions": ["or", "20787080332"], + "audienceIds": ["20787080332"], + "variations": [{ + "variables": [], + "id": "16421", + "key": "new_variation", + "featureEnabled": true + }], + "forcedVariations": {}, + "key": "flag1_targeted_delivery3", + "layerId": "9300000018516", + "trafficAllocation": [{ + "entityId": "16421", + "endOfRange": 10000 + }], + "id": "9300000018550" + }, { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [{ + "variables": [], + "id": "16419", + "key": "off", + "featureEnabled": false + }], + "forcedVariations": {}, + "key": "default-rollout-5969-20778120250", + "layerId": "default-layer-rollout-5969-20778120250", + "trafficAllocation": [{ + "entityId": "16419", + "endOfRange": 10000 + }], + "id": "default-rollout-5969-20778120250" + }], + "id": "rollout-5969-20778120250" + }, { "id": "166661", "experiments": [ From 2bb89376b7c74727acfc30b73de44de18ec1428b Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 15 Oct 2021 19:16:11 +0500 Subject: [PATCH 24/28] fixed logging and unit tests --- src/Optimizely/OptimizelyUserContext.php | 4 ++-- tests/OptimizelyUserContextTest.php | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index d87937e4..bd6c5abe 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -99,9 +99,9 @@ public function findValidatedForcedDecision($flagKey, $ruleKey) $variation = $this->optimizelyClient->getFlagVariationByKey($flagKey, $variationKey); if ($variation) { array_push($decideReasons, 'Decided by forced decision.'); - array_push($decideReasons, sprintf('Variation "%s" is mapped to %s and user "%s" in the forced decision map.', $variationKey, $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId)); + array_push($decideReasons, sprintf('Variation (%s) is mapped to %s and user (%s) in the forced decision map.', $variationKey, $ruleKey? 'flag ('.$flagKey.'), rule ('.$ruleKey.')': 'flag ('.$flagKey.')', $this->userId)); } else { - array_push($decideReasons, sprintf('Invalid variation is mapped to %s and user "%s" in the forced decision map.', $ruleKey? 'flag "'.$flagKey.'", rule "'.$ruleKey.'"': 'flag "'.$flagKey.'"', $this->userId)); + array_push($decideReasons, sprintf('Invalid variation is mapped to %s and user (%s) in the forced decision map.', $ruleKey? 'flag ('.$flagKey.'), rule ('.$ruleKey.')': 'flag ('.$flagKey.')', $this->userId)); } } return [$variation, $decideReasons]; diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index fe2fd183..8fe22baa 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -327,7 +327,7 @@ public function testForcedDecisionFlagToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), [ 'Decided by forced decision.', - 'Variation "177773" is mapped to flag "boolean_single_variable_feature" and user "test_user" in the forced decision map.' + 'Variation (177773) is mapped to flag (boolean_single_variable_feature) and user (test_user) in the forced decision map.' ]); // Removing forced decision to test @@ -377,7 +377,7 @@ public function testForcedDecisionExperimentRuleToDecision() $this->assertEquals(count($decision->getUserContext()->getAttributes()), 1); $this->assertEquals($decision->getReasons(), [ 'Decided by forced decision.', - 'Variation "test_variation_1" is mapped to flag "boolean_feature", rule "test_experiment_2" and user "test_user" in the forced decision map.', + 'Variation (test_variation_1) is mapped to flag (boolean_feature), rule (test_experiment_2) and user (test_user) in the forced decision map.', "The user 'test_user' is bucketed into experiment 'test_experiment_2' of feature 'boolean_feature'." ]); @@ -427,7 +427,7 @@ public function testForcedDecisionRuleDeliveryRuleToDecision() 'Audiences for rule 2 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "2".', 'Decided by forced decision.', - 'Variation "177773" is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', + 'Variation (177773) is mapped to flag (boolean_single_variable_feature), rule (rollout_1_exp_3) and user (test_user) in the forced decision map.', "User 'test_user' is bucketed into rollout for feature flag 'boolean_single_variable_feature'." ]); @@ -480,7 +480,7 @@ public function testForcedDecisionInvalidDeliveryRuleToDecision() 'User "test_user" does not meet conditions for targeting rule "1".', 'Audiences for rule 2 collectively evaluated to FALSE.', 'User "test_user" does not meet conditions for targeting rule "2".', - 'Invalid variation is mapped to flag "boolean_single_variable_feature", rule "rollout_1_exp_3" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag (boolean_single_variable_feature), rule (rollout_1_exp_3) and user (test_user) in the forced decision map.', 'Audiences for rule Everyone Else collectively evaluated to TRUE.', 'User "test_user" meets condition for targeting rule "Everyone Else".', 'Assigned bucket 3041 to user "test_user" with bucketing ID "test_user".', @@ -508,7 +508,7 @@ public function testForcedDecisionInvalidExperimentRuleToDecision() $this->assertEquals($decision->getUserContext()->getUserId(), $userId); $this->assertEquals(1, count($decision->getUserContext()->getAttributes())); $this->assertEquals([ - 'Invalid variation is mapped to flag "boolean_feature" and user "test_user" in the forced decision map.', + 'Invalid variation is mapped to flag (boolean_feature) and user (test_user) in the forced decision map.', 'Audiences for experiment "test_experiment_2" collectively evaluated to TRUE.', 'Assigned bucket 9075 to user "test_user" with bucketing ID "test_user".', 'User "test_user" is in variation test_variation_2 of experiment test_experiment_2.', @@ -538,7 +538,7 @@ public function testForcedDecisionConflicts() $this->assertNull($decision->getRuleKey()); $this->assertEquals([ 'Decided by forced decision.', - 'Variation "test_variation_1" is mapped to flag "boolean_feature" and user "test_user" in the forced decision map.' + 'Variation (test_variation_1) is mapped to flag (boolean_feature) and user (test_user) in the forced decision map.' ], $decision->getReasons()); } From 0df97322bad12ecbef7e6512a1b7fda59831faef Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 20 Oct 2021 13:36:22 +0500 Subject: [PATCH 25/28] lint fix --- tests/OptimizelyTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index b8992e6b..f6161edf 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -5285,7 +5285,6 @@ public function testGetEnabledFeaturesCallsDecisionListenerForAllFeatures() $decision8, $decision9, $decision10 - )); $optimizelyMock->notificationCenter = $this->notificationCenterMock; From bc3bb03d89b0a03f4801d9008fd1d07d3daa8941 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Fri, 29 Oct 2021 20:48:02 +0500 Subject: [PATCH 26/28] OptimizelyDecisionContext and OptimizelyForcedDecision implemented --- .../DecisionService/DecisionService.php | 7 +- src/Optimizely/Optimizely.php | 4 +- src/Optimizely/OptimizelyUserContext.php | 95 ++++++++---- tests/OptimizelyTest.php | 15 +- tests/OptimizelyUserContextTest.php | 144 ++++++++++++------ 5 files changed, 183 insertions(+), 82 deletions(-) diff --git a/src/Optimizely/DecisionService/DecisionService.php b/src/Optimizely/DecisionService/DecisionService.php index 2b843649..c8c8a3e0 100644 --- a/src/Optimizely/DecisionService/DecisionService.php +++ b/src/Optimizely/DecisionService/DecisionService.php @@ -19,6 +19,7 @@ use Exception; use Monolog\Logger; use Optimizely\Bucketer; +use Optimizely\OptimizelyDecisionContext; use Optimizely\Config\ProjectConfigInterface; use Optimizely\Decide\OptimizelyDecideOption; use Optimizely\Entity\Experiment; @@ -375,7 +376,8 @@ private function getVariationFromExperimentRule(ProjectConfigInterface $projectC { $decideReasons = []; // check forced-decision first - list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey()); + $context = new OptimizelyDecisionContext($flagKey, $rule->getKey()); + list($decisionResponse, $reasons) = $user->findValidatedForcedDecision($context); $decideReasons = array_merge($decideReasons, $reasons); if ($decisionResponse) { return [$decisionResponse, $decideReasons]; @@ -407,7 +409,8 @@ public function getVariationFromDeliveryRule(ProjectConfigInterface $projectConf $skipToEveryoneElse = false; // check forced-decision first $rule = $rules[$ruleIndex]; - list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($flagKey, $rule->getKey()); + $context = new OptimizelyDecisionContext($flagKey, $rule->getKey()); + list($forcedDecisionResponse, $reasons) = $user->findValidatedForcedDecision($context); $decideReasons = array_merge($decideReasons, $reasons); if ($forcedDecisionResponse) { diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 6ff9fba4..84576319 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -28,6 +28,7 @@ use Optimizely\Decide\OptimizelyDecisionMessage; use Optimizely\DecisionService\DecisionService; use Optimizely\DecisionService\FeatureDecision; +use Optimizely\OptimizelyDecisionContext; use Optimizely\Entity\Experiment; use Optimizely\Entity\FeatureVariable; use Optimizely\Enums\DecisionNotificationTypes; @@ -353,7 +354,8 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp // get decision $decision = null; // check forced-decisions first - list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($flagKey, $ruleKey); + $context = new OptimizelyDecisionContext($flagKey, $ruleKey); + list($forcedDecisionResponse, $reasons) = $userContext->findValidatedForcedDecision($context); if ($forcedDecisionResponse) { $decision = new FeatureDecision(null, $forcedDecisionResponse, FeatureDecision::DECISION_SOURCE_FEATURE_TEST, $decideReasons); } else { diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index bd6c5abe..1e6dc8b3 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -35,43 +35,44 @@ public function __construct(Optimizely $optimizelyClient, $userId, array $attrib $this->forcedDecisions = $forcedDecisions; } - public function setForcedDecision($flagKey, $ruleKey, $variationKey) + public function setForcedDecision($context, $decision) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { return false; } - if (empty($flagKey)) { + $flagKey = $context->getFlagKey(); + if (!isset($flagKey)) { return false; } - $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); + $index = $this->findExistingRuleAndFlagKey($context); if ($index != -1) { - $this->forcedDecisions[$index]->setVariationKey($variationKey); + $this->forcedDecisions[$index]->setOptimizelyForcedDecision($decision); } else { if (!$this->forcedDecisions) { $this->forcedDecisions = array(); } - array_push($this->forcedDecisions, new ForcedDecision($flagKey, $ruleKey, $variationKey)); + array_push($this->forcedDecisions, new ForcedDecision($context, $decision)); } return true; } - public function getForcedDecision($flagKey, $ruleKey = null) + public function getForcedDecision($context) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { return null; } - return $this->findForcedDecision($flagKey, $ruleKey); + return $this->findForcedDecision($context); } - public function removeForcedDecision($flagKey, $ruleKey = null) + public function removeForcedDecision($context) { // check if SDK is ready if (!$this->optimizelyClient->isValid()) { return false; } - $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); + $index = $this->findExistingRuleAndFlagKey($context); if ($index != -1) { array_splice($this->forcedDecisions, $index, 1); return true; @@ -90,10 +91,12 @@ public function removeAllForcedDecisions() return true; } - public function findValidatedForcedDecision($flagKey, $ruleKey) + public function findValidatedForcedDecision($context) { $decideReasons = []; - $variationKey = $this->findForcedDecision($flagKey, $ruleKey); + $flagKey = $context->getFlagKey(); + $ruleKey = $context->getRuleKey(); + $variationKey = $this->findForcedDecision($context); $variation = null; if ($variationKey) { $variation = $this->optimizelyClient->getFlagVariationByKey($flagKey, $variationKey); @@ -107,11 +110,11 @@ public function findValidatedForcedDecision($flagKey, $ruleKey) return [$variation, $decideReasons]; } - private function findExistingRuleAndFlagKey($flagKey, $ruleKey) + private function findExistingRuleAndFlagKey($context) { if ($this->forcedDecisions) { for ($index = 0; $index < count($this->forcedDecisions); $index++) { - if ($this->forcedDecisions[$index]->getFlagKey() == $flagKey && $this->forcedDecisions[$index]->getRuleKey() == $ruleKey) { + if ($this->forcedDecisions[$index]->getOptimizelyDecisionContext()->getFlagKey() == $context->getFlagKey() && $this->forcedDecisions[$index]->getOptimizelyDecisionContext()->getRuleKey() == $context->getRuleKey()) { return $index; } } @@ -119,7 +122,7 @@ private function findExistingRuleAndFlagKey($flagKey, $ruleKey) return -1; } - public function findForcedDecision($flagKey, $ruleKey) + public function findForcedDecision($context) { $foundVariationKey = null; if (!$this->forcedDecisions) { @@ -128,9 +131,9 @@ public function findForcedDecision($flagKey, $ruleKey) if (count($this->forcedDecisions) == 0) { return null; } - $index = $this->findExistingRuleAndFlagKey($flagKey, $ruleKey); + $index = $this->findExistingRuleAndFlagKey($context); if ($index != -1) { - $foundVariationKey = $this->forcedDecisions[$index]->getVariationKey(); + $foundVariationKey = $this->forcedDecisions[$index]->getOptimizelyForcedDecision()->getVariationKey(); } return $foundVariationKey; } @@ -189,23 +192,47 @@ public function jsonSerialize() } } class ForcedDecision +{ + private $optimizelyDecisionContext; + private $optimizelyForcedDecision; + + public function __construct($optimizelyDecisionContext, $optimizelyForcedDecision) + { + $this->optimizelyDecisionContext = $optimizelyDecisionContext; + $this->setOptimizelyForcedDecision($optimizelyForcedDecision); + } + + /** + * @return mixed + */ + public function getOptimizelyDecisionContext() + { + return $this->optimizelyDecisionContext; + } + + /** + * @return mixed + */ + public function getOptimizelyForcedDecision() + { + return $this->optimizelyForcedDecision; + } + + public function setOptimizelyForcedDecision($optimizelyForcedDecision) + { + $this->optimizelyForcedDecision = $optimizelyForcedDecision; + } +} + +class OptimizelyDecisionContext { private $flagKey; private $ruleKey; - private $variationKey; - public function __construct($flagKey, $ruleKey, $variationKey) + public function __construct($flagKey, $ruleKey) { $this->flagKey = $flagKey; $this->ruleKey = $ruleKey; - $this->setVariationKey($variationKey); - } - - public function setVariationKey($variationKey) - { - if (isset($variationKey) && trim($variationKey) !== '') { - $this->variationKey = $variationKey; - } } /** @@ -223,6 +250,22 @@ public function getRuleKey() { return $this->ruleKey; } +} +class OptimizelyForcedDecision +{ + private $variationKey; + + public function __construct($variationKey) + { + $this->setVariationKey($variationKey); + } + + public function setVariationKey($variationKey) + { + if (isset($variationKey) && trim($variationKey) !== '') { + $this->variationKey = $variationKey; + } + } /** * @return mixed diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index f6161edf..c6b450c7 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -46,6 +46,8 @@ use Optimizely\Event\Builder\EventBuilder; use Optimizely\Logger\DefaultLogger; use Optimizely\Optimizely; +use Optimizely\OptimizelyDecisionContext; +use Optimizely\OptimizelyForcedDecision; use Optimizely\OptimizelyUserContext; use Optimizely\UserProfile\UserProfileServiceInterface; @@ -550,8 +552,9 @@ public function testSetForcedDecisionExperimentRuleToDecisionSendImpressionAndNo ); $optimizelyMock->notificationCenter = $this->notificationCenterMock; - - $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', 'test_experiment_double_feature', 'variation')); + $context = new OptimizelyDecisionContext('double_single_variable_feature', 'test_experiment_double_feature'); + $decision = new OptimizelyForcedDecision('variation'); + $this->assertTrue($userContext->setForcedDecision($context, $decision)); $optimizelyDecision = $optimizelyMock->decide($userContext, 'double_single_variable_feature'); $expectedOptimizelyDecision = new OptimizelyDecision( 'variation', @@ -642,7 +645,9 @@ public function testSetForcedDecisionDeliveryRuleToDecisionSendImpressionAndNoti $optimizelyMock->notificationCenter = $this->notificationCenterMock; - $this->assertTrue($userContext->setForcedDecision('boolean_single_variable_feature', 'rollout_1_exp_2', '177773')); + $context = new OptimizelyDecisionContext('boolean_single_variable_feature', 'rollout_1_exp_2'); + $decision = new OptimizelyForcedDecision('177773'); + $this->assertTrue($userContext->setForcedDecision($context, $decision)); $optimizelyDecision = $optimizelyMock->decide($userContext, 'boolean_single_variable_feature'); $expectedOptimizelyDecision = new OptimizelyDecision( '177773', @@ -722,7 +727,9 @@ public function testSetForcedDecisionFlagToDecisionSendImpressionAndNotification $optimizelyMock->notificationCenter = $this->notificationCenterMock; - $this->assertTrue($userContext->setForcedDecision('double_single_variable_feature', null, 'variation')); + $context = new OptimizelyDecisionContext('double_single_variable_feature', null); + $decision = new OptimizelyForcedDecision('variation'); + $this->assertTrue($userContext->setForcedDecision($context, $decision)); $optimizelyDecision = $optimizelyMock->decide($userContext, 'double_single_variable_feature'); $expectedOptimizelyDecision = new OptimizelyDecision( 'variation', diff --git a/tests/OptimizelyUserContextTest.php b/tests/OptimizelyUserContextTest.php index 8fe22baa..a0f406d5 100644 --- a/tests/OptimizelyUserContextTest.php +++ b/tests/OptimizelyUserContextTest.php @@ -23,6 +23,8 @@ use Optimizely\Logger\NoOpLogger; use Optimizely\Optimizely; use Optimizely\OptimizelyUserContext; +use Optimizely\OptimizelyDecisionContext; +use Optimizely\OptimizelyForcedDecision; class OptimizelyUserContextTest extends \PHPUnit_Framework_TestCase { @@ -270,13 +272,17 @@ public function testForcedDecisionInvalidDatafileReturnStatus() $invalidOptlyObject = new Optimizely("Invalid datafile"); $optUserContext = new OptimizelyUserContext($invalidOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("flag1", "targeted_delivery", "variation1"); + + $context = new OptimizelyDecisionContext("flag1", "targeted_delivery"); + $decision = new OptimizelyForcedDecision("variation1"); + + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertFalse($setForcedDecision); - $getForcedDecision = $optUserContext->getForcedDecision("flag1", "targeted_delivery"); + $getForcedDecision = $optUserContext->getForcedDecision($context); $this->assertNull($getForcedDecision); - $removeForcedDecision = $optUserContext->removeForcedDecision("flag1", "targeted_delivery"); + $removeForcedDecision = $optUserContext->removeForcedDecision($context); $this->assertFalse($removeForcedDecision); $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions(); @@ -291,13 +297,17 @@ public function testForcedDecisionValidDatafileReturnStatus() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("flag1", "targeted_delivery", "variation1"); + + $context = new OptimizelyDecisionContext("flag1", "targeted_delivery"); + $decision = new OptimizelyForcedDecision("variation1"); + + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision); - $getForcedDecision = $optUserContext->getForcedDecision("flag1", "targeted_delivery"); + $getForcedDecision = $optUserContext->getForcedDecision($context); $this->assertEquals($getForcedDecision, "variation1"); - $removeForcedDecision = $optUserContext->removeForcedDecision("flag1", "targeted_delivery"); + $removeForcedDecision = $optUserContext->removeForcedDecision($context); $this->assertTrue($removeForcedDecision); $removeAllForcedDecision = $optUserContext->removeAllForcedDecisions(); @@ -312,10 +322,14 @@ public function testForcedDecisionFlagToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", null, "177773"); + + $context = new OptimizelyDecisionContext("boolean_single_variable_feature", null); + $decision = new OptimizelyForcedDecision("177773"); + + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision); - $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature"); + $getForcedDecision = $optUserContext->getForcedDecision($context); $this->assertEquals($getForcedDecision, "177773"); $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -331,7 +345,7 @@ public function testForcedDecisionFlagToDecision() ]); // Removing forced decision to test - $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_single_variable_feature"); + $removeForcedDecision = $optUserContext->removeForcedDecision($context); $this->assertTrue($removeForcedDecision); $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -362,10 +376,14 @@ public function testForcedDecisionExperimentRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", "test_experiment_2", "test_variation_1"); + + $context = new OptimizelyDecisionContext("boolean_feature", 'test_experiment_2'); + $decision = new OptimizelyForcedDecision("test_variation_1"); + + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision); - $getForcedDecision = $optUserContext->getForcedDecision("boolean_feature", "test_experiment_2"); + $getForcedDecision = $optUserContext->getForcedDecision($context); $this->assertEquals($getForcedDecision, "test_variation_1"); $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -382,7 +400,7 @@ public function testForcedDecisionExperimentRuleToDecision() ]); // Removing forced decision to test - $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_feature", "test_experiment_2"); + $removeForcedDecision = $optUserContext->removeForcedDecision($context); $this->assertTrue($removeForcedDecision); $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -407,10 +425,14 @@ public function testForcedDecisionRuleDeliveryRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3", "177773"); + + $context = new OptimizelyDecisionContext("boolean_single_variable_feature", "rollout_1_exp_3"); + $decision = new OptimizelyForcedDecision("177773"); + + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision); - $getForcedDecision = $optUserContext->getForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3"); + $getForcedDecision = $optUserContext->getForcedDecision($context); $this->assertEquals($getForcedDecision, "177773"); $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -432,7 +454,7 @@ public function testForcedDecisionRuleDeliveryRuleToDecision() ]); // Removing forced decision to test - $removeForcedDecision = $optUserContext->removeForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3"); + $removeForcedDecision = $optUserContext->removeForcedDecision($context); $this->assertTrue($removeForcedDecision); $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -464,7 +486,9 @@ public function testForcedDecisionInvalidDeliveryRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_single_variable_feature", "rollout_1_exp_3", "invalid"); + $context = new OptimizelyDecisionContext("boolean_single_variable_feature", "rollout_1_exp_3"); + $decision = new OptimizelyForcedDecision("invalid"); + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision); $decision = $optUserContext->decide('boolean_single_variable_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -497,7 +521,9 @@ public function testForcedDecisionInvalidExperimentRuleToDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision = $optUserContext->setForcedDecision("boolean_feature", null, "invalid"); + $context = new OptimizelyDecisionContext("boolean_feature", null); + $decision = new OptimizelyForcedDecision("invalid"); + $setForcedDecision = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision); $decision = $optUserContext->decide('boolean_feature', [OptimizelyDecideOption::INCLUDE_REASONS]); @@ -525,10 +551,14 @@ public function testForcedDecisionConflicts() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $setForcedDecision1 = $optUserContext->setForcedDecision($featureKey, null, "test_variation_1"); + $context = new OptimizelyDecisionContext($featureKey, null); + $decision = new OptimizelyForcedDecision("test_variation_1"); + $setForcedDecision1 = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision1); - $setForcedDecision2 = $optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2"); + $context = new OptimizelyDecisionContext($featureKey, "test_experiment_2"); + $decision = new OptimizelyForcedDecision("test_variation_2"); + $setForcedDecision2 = $optUserContext->setForcedDecision($context, $decision); $this->assertTrue($setForcedDecision2); // flag-to-decision is the 1st priority @@ -551,19 +581,27 @@ public function testGetForcedDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_1")); - $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); - - $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_2")); - $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey)); - - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_1")); - $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); - - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2")); - $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); - - $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey)); + $contextWithNullRuleKey = new OptimizelyDecisionContext($featureKey, null); + $decision = new OptimizelyForcedDecision("test_variation_1"); + $this->assertTrue($optUserContext->setForcedDecision($contextWithNullRuleKey, $decision)); + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($contextWithNullRuleKey)); + + $context = new OptimizelyDecisionContext($featureKey, null); + $decision = new OptimizelyForcedDecision("test_variation_2"); + $this->assertTrue($optUserContext->setForcedDecision($context, $decision)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($context)); + + $context = new OptimizelyDecisionContext($featureKey, "test_experiment_2"); + $decision = new OptimizelyForcedDecision("test_variation_1"); + $this->assertTrue($optUserContext->setForcedDecision($context, $decision)); + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($context)); + + $context = new OptimizelyDecisionContext($featureKey, "test_experiment_2"); + $decision = new OptimizelyForcedDecision("test_variation_2"); + $this->assertTrue($optUserContext->setForcedDecision($context, $decision)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($context)); + + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($contextWithNullRuleKey)); } public function testRemoveForcedDecision() @@ -575,21 +613,25 @@ public function testRemoveForcedDecision() $validOptlyObject = new Optimizely($this->datafile); $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_1")); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2")); + $contextWithFlag = new OptimizelyDecisionContext($featureKey, null); + $decisionForFlag = new OptimizelyForcedDecision("test_variation_1"); + $contextWithRule = new OptimizelyDecisionContext($featureKey, "test_experiment_2"); + $decisionForRule = new OptimizelyForcedDecision("test_variation_2"); + $this->assertTrue($optUserContext->setForcedDecision($contextWithFlag, $decisionForFlag)); + $this->assertTrue($optUserContext->setForcedDecision($contextWithRule, $decisionForRule)); - $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); - $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($contextWithFlag)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($contextWithRule)); - $this->assertTrue($optUserContext->removeForcedDecision($featureKey)); - $this->assertNull($optUserContext->getForcedDecision($featureKey)); - $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + $this->assertTrue($optUserContext->removeForcedDecision($contextWithFlag)); + $this->assertNull($optUserContext->getForcedDecision($contextWithFlag)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($contextWithRule)); - $this->assertTrue($optUserContext->removeForcedDecision($featureKey, "test_experiment_2")); - $this->assertNull($optUserContext->getForcedDecision($featureKey, "test_experiment_2")); - $this->assertNull($optUserContext->getForcedDecision($featureKey)); + $this->assertTrue($optUserContext->removeForcedDecision($contextWithRule)); + $this->assertNull($optUserContext->getForcedDecision($contextWithRule)); + $this->assertNull($optUserContext->getForcedDecision($contextWithFlag)); - $this->assertFalse($optUserContext->removeForcedDecision($featureKey)); // no more saved decisions + $this->assertFalse($optUserContext->removeForcedDecision($contextWithFlag)); // no more saved decisions } public function testRemoveAllForcedDecisions() @@ -603,15 +645,19 @@ public function testRemoveAllForcedDecisions() $optUserContext = new OptimizelyUserContext($validOptlyObject, $userId, $attributes); $this->assertTrue($optUserContext->removeAllForcedDecisions()); // no saved decisions - $this->assertTrue($optUserContext->setForcedDecision($featureKey, null, "test_variation_1")); - $this->assertTrue($optUserContext->setForcedDecision($featureKey, "test_experiment_2", "test_variation_2")); + $contextWithFlag = new OptimizelyDecisionContext($featureKey, null); + $decisionForFlag = new OptimizelyForcedDecision("test_variation_1"); + $contextWithRule = new OptimizelyDecisionContext($featureKey, "test_experiment_2"); + $decisionForRule = new OptimizelyForcedDecision("test_variation_2"); + $this->assertTrue($optUserContext->setForcedDecision($contextWithFlag, $decisionForFlag)); + $this->assertTrue($optUserContext->setForcedDecision($contextWithRule, $decisionForRule)); - $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($featureKey)); - $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($featureKey, "test_experiment_2")); + $this->assertEquals("test_variation_1", $optUserContext->getForcedDecision($contextWithFlag)); + $this->assertEquals("test_variation_2", $optUserContext->getForcedDecision($contextWithRule)); $this->assertTrue($optUserContext->removeAllForcedDecisions()); - $this->assertNull($optUserContext->getForcedDecision($featureKey, "test_experiment_2")); - $this->assertNull($optUserContext->getForcedDecision($featureKey)); + $this->assertNull($optUserContext->getForcedDecision($contextWithRule)); + $this->assertNull($optUserContext->getForcedDecision($contextWithFlag)); $this->assertTrue($optUserContext->removeAllForcedDecisions()); // no more saved decisions } From 7566d551be975b8e365031e18c2d0ed67c3c2624 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar <54209343+ozayr-zaviar@users.noreply.github.com> Date: Thu, 4 Nov 2021 00:17:30 +0500 Subject: [PATCH 27/28] impression corrected (#234) --- src/Optimizely/Event/Builder/EventBuilder.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Optimizely/Event/Builder/EventBuilder.php b/src/Optimizely/Event/Builder/EventBuilder.php index 39ef68ab..744d2cf1 100644 --- a/src/Optimizely/Event/Builder/EventBuilder.php +++ b/src/Optimizely/Event/Builder/EventBuilder.php @@ -150,19 +150,26 @@ private function getCommonParams($config, $userId, $attributes) */ private function getImpressionParams(Experiment $experiment, $variation, $flagKey, $ruleKey, $ruleType, $enabled) { - $variationKey = $variation->getKey() ? $variation->getKey() : ''; + $variationKey = ''; + $variationId = null; + if ($variation) { + $variationKey = $variation->getKey() ? $variation->getKey() : ''; + $variationId = $variation->getId(); + } $experimentID = ''; $campaignID = ''; - if ($experiment->getId()) { - $experimentID = $experiment->getId(); - $campaignID = $experiment->getLayerId(); + if ($experiment) { + if ($experiment->getId()) { + $experimentID = $experiment->getId(); + $campaignID = $experiment->getLayerId(); + } } $impressionParams = [ DECISIONS => [ [ CAMPAIGN_ID => $campaignID, EXPERIMENT_ID => $experimentID, - VARIATION_ID => $variation->getId(), + VARIATION_ID => $variationId, METADATA => [ FLAG_KEY => $flagKey, RULE_KEY => $ruleKey, From 32122b7185e5bc43d88c5d1c050a55c3110fd81f Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 4 Nov 2021 13:29:27 +0500 Subject: [PATCH 28/28] comments addressed --- src/Optimizely/Config/DatafileProjectConfig.php | 14 ++++++++++---- src/Optimizely/Optimizely.php | 2 +- src/Optimizely/OptimizelyUserContext.php | 2 +- src/Optimizely/Utils/Errors.php | 1 + 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Optimizely/Config/DatafileProjectConfig.php b/src/Optimizely/Config/DatafileProjectConfig.php index 2deb3aca..9283f79c 100644 --- a/src/Optimizely/Config/DatafileProjectConfig.php +++ b/src/Optimizely/Config/DatafileProjectConfig.php @@ -389,14 +389,20 @@ public function __construct($datafile, $logger, $errorHandler) $flagRules = $this->getAllRulesForFlag($flag); foreach ($flagRules as $rule) { - $flagVariations = array_merge($flagVariations, array_filter(array_values($rule->getVariations()), function ($variation) use ($flagVariations) { + $filtered_variations = []; + foreach (array_values($rule->getVariations()) as $variation) { + $exist = false; foreach ($flagVariations as $flagVariation) { if ($flagVariation->getId() == $variation->getId()) { - return false; + $exist = true; + break; } } - return true; - })); + if (!$exist) { + array_push($filtered_variations, $variation); + } + } + $flagVariations = array_merge($flagVariations, $filtered_variations); } $this->_flagVariationsMap[$flag->getKey()] = $flagVariations; diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 84576319..179360de 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -1267,7 +1267,7 @@ public function isValid() if (!$this->getConfig()) { $this->_logger->log( Logger::ERROR, - "Optimizely SDK not configured properly yet." + Errors::NO_CONFIG ); return false; } diff --git a/src/Optimizely/OptimizelyUserContext.php b/src/Optimizely/OptimizelyUserContext.php index 1e6dc8b3..41cc7288 100644 --- a/src/Optimizely/OptimizelyUserContext.php +++ b/src/Optimizely/OptimizelyUserContext.php @@ -125,7 +125,7 @@ private function findExistingRuleAndFlagKey($context) public function findForcedDecision($context) { $foundVariationKey = null; - if (!$this->forcedDecisions) { + if (!isset($this->forcedDecisions)) { return null; } if (count($this->forcedDecisions) == 0) { diff --git a/src/Optimizely/Utils/Errors.php b/src/Optimizely/Utils/Errors.php index f883412e..196b6729 100644 --- a/src/Optimizely/Utils/Errors.php +++ b/src/Optimizely/Utils/Errors.php @@ -20,4 +20,5 @@ class Errors { const INVALID_FORMAT = 'Provided %s is in an invalid format.'; const INVALID_DATAFILE = 'Datafile has invalid format. Failing "%s".'; + const NO_CONFIG = 'Optimizely SDK not configured properly yet.'; }