Skip to content

feat(ForcedDecisions): add forced-decisions APIs to OptimizelyUserContext #233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8607ff2
Work in progress
NomanShoaib Sep 21, 2021
e013fea
fix
NomanShoaib Sep 21, 2021
0b1f517
implemented forcedDecision
NomanShoaib Sep 22, 2021
cd4176c
fixed issues
NomanShoaib Sep 24, 2021
5c753aa
fixed all errors
NomanShoaib Sep 27, 2021
0fd717b
fixed old tests and logs errors
NomanShoaib Sep 27, 2021
7e2400f
fix
NomanShoaib Sep 28, 2021
9cb1d03
fix
NomanShoaib Sep 28, 2021
ae48a61
added remove forcedecision removeall force decision apis
NomanShoaib Sep 28, 2021
6a9a5f0
Unit tests added and fixed some bugs
NomanShoaib Oct 1, 2021
60ae438
Added unit tests of forceddecision
NomanShoaib Oct 4, 2021
df1c82f
Comments fixed
NomanShoaib Oct 6, 2021
80e5026
return true in removeallforcedecision even if forceddecision list is …
NomanShoaib Oct 6, 2021
979a0e7
Added depreciation warning in
NomanShoaib Oct 6, 2021
d22ad11
Fixed experimentId null or empty error in impression event
NomanShoaib Oct 6, 2021
a5843c2
fixed logging and !=null comments
NomanShoaib Oct 6, 2021
f387f89
lint fix
NomanShoaib Oct 6, 2021
3557793
removed depreciation warning
NomanShoaib Oct 7, 2021
8797195
made rule key compulsory instead of optional and moved it to 2nd arg …
NomanShoaib Oct 11, 2021
83ef326
- moved the two methods of remove and remove all forceddecision after…
NomanShoaib Oct 11, 2021
b7b980d
Added Decided by forceddecision reason
NomanShoaib Oct 12, 2021
280371c
Added reasons in test case
NomanShoaib Oct 13, 2021
9168430
DatafileprojectConfig line 248 added verification that redundant vari…
NomanShoaib Oct 14, 2021
2bb8937
fixed logging and unit tests
NomanShoaib Oct 15, 2021
0df9732
lint fix
NomanShoaib Oct 20, 2021
bc3bb03
OptimizelyDecisionContext and OptimizelyForcedDecision implemented
ozayr-zaviar Oct 29, 2021
7566d55
impression corrected (#234)
ozayr-zaviar Nov 3, 2021
32122b7
comments addressed
ozayr-zaviar Nov 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions src/Optimizely/Config/DatafileProjectConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +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.
*
Expand Down Expand Up @@ -376,7 +383,24 @@ public function __construct($datafile, $logger, $errorHandler)
}
}
}
$this->_flagVariationsMap = array();
foreach ($this->_featureFlags as $flag) {
$flagVariations = array();
$flagRules = $this->getAllRulesForFlag($flag);

foreach ($flagRules as $rule) {
$flagVariations = array_merge($flagVariations, array_filter(array_values($rule->getVariations()), function ($variation) use ($flagVariations) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of closures in PHP suggests this is copying the value of $flagVariations and not referencing it. Not sure if this could provide an edge case where the $flagVariations array is modified outside of this set of functions.

foreach ($flagVariations as $flagVariation) {
if ($flagVariation->getId() == $variation->getId()) {
return false;
}
}
return true;
}));
}

$this->_flagVariationsMap[$flag->getKey()] = $flagVariations;
}
// Add variations for rollout experiments to variationIdMap and variationKeyMap
$this->_variationIdMap = $this->_variationIdMap + $rolloutVariationIdMap;
$this->_variationKeyMap = $this->_variationKeyMap + $rolloutVariationKeyMap;
Expand Down Expand Up @@ -404,6 +428,18 @@ public function __construct($datafile, $logger, $errorHandler)
}
}

private function getAllRulesForFlag(FeatureFlag $flag)
{
$rules = array();
foreach ($flag->getExperimentIds() as $experimentId) {
array_push($rules, $this->_experimentIdMap[$experimentId]);
}
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.
*
Expand Down Expand Up @@ -614,6 +650,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
*
Expand Down Expand Up @@ -868,6 +924,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
*
Expand Down
18 changes: 17 additions & 1 deletion src/Optimizely/Config/ProjectConfigInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment on lines +211 to +218
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implication of these new interface functions to existing clients with custom ProjectConfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will simply return flag variations map do i have to add something other then comment above?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I think it's good as is.

/**
* Determines if given experiment is a feature test.
*
Expand Down
171 changes: 111 additions & 60 deletions src/Optimizely/DecisionService/DecisionService.php

Large diffs are not rendered by default.

26 changes: 21 additions & 5 deletions src/Optimizely/Event/Builder/EventBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Optimizely\Utils\EventTagUtils;
use Optimizely\Utils\GeneratorUtils;
use Optimizely\Utils\Validator;
use phpDocumentor\Reflection\Types\This;

class EventBuilder
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
73 changes: 55 additions & 18 deletions src/Optimizely/Optimizely.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -201,11 +202,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)
{
Expand Down Expand Up @@ -347,22 +352,32 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp
$decisionEventDispatched = false;

// get decision
$decision = $this->_decisionService->getVariationForFeature(
$config,
$featureFlag,
$userId,
$userAttributes,
$decideOptions
);

$decideReasons = $decision->getReasons();
$decision = null;
// check forced-decisions first
$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 {
// regular decision
$decision = $this->_decisionService->getVariationForFeature(
$config,
$featureFlag,
$userContext,
$decideOptions
);
}
$decideReasons = array_merge($decideReasons, $reasons);
$decideReasons = array_merge($decideReasons, $decision->getReasons());
$variation = $decision->getVariation();

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;
Expand Down Expand Up @@ -687,7 +702,8 @@ public function getVariation($experimentKey, $userId, $attributes = null)
return null;
}

list($variation, $reasons) = $this->_decisionService->getVariation($config, $experiment, $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())) {
Expand Down Expand Up @@ -815,7 +831,8 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
}

$featureEnabled = false;
$decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $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)) {
Expand Down Expand Up @@ -948,8 +965,8 @@ public function getFeatureVariableValueForType(
// Error logged in DatafileProjectConfig - getFeatureFlagFromKey
return null;
}

$decision = $this->_decisionService->getVariationForFeature($config, $featureFlag, $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;
Expand Down Expand Up @@ -1124,7 +1141,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;
Expand Down Expand Up @@ -1247,7 +1264,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."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a static message for this that can be referenced?

);
return false;
}
return true;
}

/**
Expand Down Expand Up @@ -1281,4 +1305,17 @@ 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)
{
return $this->getConfig()->getFlagVariationByKey($flagKey, $variationKey);
}
}
Loading