Skip to content

Commit 79788ee

Browse files
oakbanioakbani
committed
🖊️ Decision Object introduced
🖊️ Logic slightly changed based on Decision 🖊️ nits addressed
1 parent 1253af6 commit 79788ee

File tree

3 files changed

+118
-49
lines changed

3 files changed

+118
-49
lines changed

‎src/Optimizely/DecisionService/Decision.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
/**
3+
* Copyright 2017, Optimizely Inc and Contributors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
namespace Optimizely\DecisionService;
18+
19+
class Decision
20+
{
21+
const DECISION_SOURCE_EXPERIMENT = 'experiment';
22+
const DECISION_SOURCE_ROLLOUT = 'rollout';
23+
24+
/**
25+
* @var string The ID experiment in this decision.
26+
*/
27+
private $_experimentId;
28+
29+
/**
30+
* @var string The ID variation in this decision.
31+
*/
32+
private $_variationId;
33+
34+
/**
35+
* The source of the decision. Either DECISION_SOURCE_EXPERIMENT or DECISION_SOURCE_ROLLOUT
36+
* @var string
37+
*/
38+
private $_source;
39+
40+
/**
41+
* Decision constructor.
42+
*
43+
* @param $experimentId
44+
* @param $variationId
45+
* @param $source
46+
*/
47+
public function __construct($experimentId, $variationId, $source)
48+
{
49+
$this->_experimentId = $experimentId;
50+
$this->_variationId = $variationId;
51+
$this->_source = $source;
52+
}
53+
54+
public function getExperimentId()
55+
{
56+
return $this->_experimentId;
57+
}
58+
59+
public function getVariationId()
60+
{
61+
return $this->_variationId;
62+
}
63+
64+
public function getSource()
65+
{
66+
return $this->_source;
67+
}
68+
}

‎src/Optimizely/DecisionService/DecisionService.php‎

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@
2020
use Monolog\Logger;
2121
use Optimizely\Bucketer;
2222
use Optimizely\Entity\Experiment;
23+
use Optimizely\Entity\FeatureFlag;
24+
use Optimizely\Entity\Rollout;
2325
use Optimizely\Entity\Variation;
2426
use Optimizely\Logger\LoggerInterface;
2527
use Optimizely\ProjectConfig;
26-
use Optimizely\UserProfile\Decision;
2728
use Optimizely\UserProfile\UserProfileServiceInterface;
2829
use Optimizely\UserProfile\UserProfile;
2930
use Optimizely\UserProfile\UserProfileUtils;
3031
use Optimizely\Utils\Validator;
31-
use Optimizely\Entity\FeatureFlag;
32-
use Optimizely\Entity\Rollout;
3332

3433
// This value was decided between App Backend, Audience, and Oasis teams, but may possibly change.
3534
// We decided to prefix the reserved keyword with '$' because it is a symbol that is not
@@ -42,10 +41,11 @@
4241
*
4342
* The decision service contains all logic around how a user decision is made. This includes all of the following (in order):
4443
* 1. Checking experiment status.
45-
* 2. Checking whitelisting.
46-
* 3. Check sticky bucketing.
47-
* 4. Checking audience targeting.
48-
* 5. Using Murmurhash3 to bucket the user.
44+
* 2. Checking force bucketing
45+
* 3. Checking whitelisting.
46+
* 4. Check sticky bucketing.
47+
* 5. Checking audience targeting.
48+
* 6. Using Murmurhash3 to bucket the user.
4949
*
5050
* @package Optimizely
5151
*/
@@ -144,13 +144,14 @@ public function getVariation(Experiment $experiment, $userId, $attributes = null
144144
}
145145

146146
/**
147-
* Gets the Bucketing Id for Bucketing
147+
* Gets the Bucketing ID for Bucketing
148148
* @param string $userId
149-
* @param array $userAttributes
149+
* @param array $userAttributes
150+
*
150151
* @return string
151152
*/
152153
private function getBucketingId($userId, $userAttributes){
153-
// by default, the bucketing ID should be the user ID
154+
// By default, the bucketing ID should be the user ID
154155
$bucketingId = $userId;
155156

156157
// If the bucketing ID key is defined in userAttributes, then use that in place of the userID for the murmur hash key
@@ -167,28 +168,26 @@ private function getBucketingId($userId, $userAttributes){
167168
* @param FeatureFlag $featureFlag The feature flag the user wants to access
168169
* @param string $userId user id
169170
* @param array $userAttributes user attributes
170-
* @return array/null {"experiment" : Experiment, "variation": Variation } / null
171+
* @return Decision / null
171172
*/
172173
public function getVariationForFeature(FeatureFlag $featureFlag, $userId, $userAttributes){
173174
//Evaluate in this order:
174-
//1. Attempt to bucket user into all experiments in the feature flag.
175-
//2. Attempt to bucket user into rollout in the feature flag.
175+
//1. Attempt to bucket user into experiment using feature flag.
176+
//2. Attempt to bucket user into rollout using the feature flag.
176177

177178
// Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments
178-
$result = $this->getVariationForFeatureExperiment($featureFlag, $userId, $userAttributes);
179-
if($result)
180-
return $result;
179+
$decision = $this->getVariationForFeatureExperiment($featureFlag, $userId, $userAttributes);
180+
if($decision)
181+
return $decision;
181182

182183
// Check if the feature flag has rollout and the user is bucketed into one of it's rules
183-
$variation = $this->getVariationForFeatureRollout($featureFlag, $userId, $userAttributes);
184-
if($variation){
184+
$decision = $this->getVariationForFeatureRollout($featureFlag, $userId, $userAttributes);
185+
if($decision){
185186
$this->_logger->log(Logger::INFO,
186187
"User '{$userId}' is bucketed into a rollout for feature flag '{$featureFlag->getKey()}'."
187188
);
188189

189-
return array(
190-
"experiment" => null,
191-
"variation" => $variation);
190+
return $decision;
192191

193192
} else{
194193
$this->_logger->log(Logger::INFO,
@@ -204,7 +203,7 @@ public function getVariationForFeature(FeatureFlag $featureFlag, $userId, $userA
204203
* @param FeatureFlag $featureFlag The feature flag the user wants to access
205204
* @param string $userId user id
206205
* @param array $userAttributes user userAttributes
207-
* @return array/null {"experiment" : Experiment, "variation": Variation } / null
206+
* @return Decision / null
208207
*/
209208
public function getVariationForFeatureExperiment(FeatureFlag $featureFlag, $userId, $userAttributes){
210209
$feature_flag_key = $featureFlag->getKey();
@@ -229,10 +228,8 @@ public function getVariationForFeatureExperiment(FeatureFlag $featureFlag, $user
229228
if($variation instanceof Variation && $variation != new Variation){
230229
$this->_logger->log(Logger::INFO,
231230
"The user '{$userId}' is bucketed into experiment '{$experiment->getKey()}' of feature '{$feature_flag_key}'.");
232-
return array(
233-
"experiment"=> $experiment,
234-
"variation" => $variation
235-
);
231+
232+
return new Decision($experiment->getId(), $variation->getId(), DECISION::DECISION_SOURCE_EXPERIMENT);
236233
}
237234
}
238235

@@ -249,7 +246,7 @@ public function getVariationForFeatureExperiment(FeatureFlag $featureFlag, $user
249246
* @param FeatureFlag $featureFlag The feature flag the user wants to access
250247
* @param string $userId user id
251248
* @param array $userAttributes user userAttributes
252-
* @return Variation/null
249+
* @return Decision/ null
253250
*/
254251
public function getVariationForFeatureRollout(FeatureFlag $featureFlag, $userId, $userAttributes){
255252
$bucketing_id = $this->getBucketingId($userId, $userAttributes);
@@ -290,7 +287,7 @@ public function getVariationForFeatureRollout(FeatureFlag $featureFlag, $userId
290287
// Evaluate if user satisfies the traffic allocation for this rollout rule
291288
$variation = $this->_bucketer->bucket($this->_projectConfig, $experiment, $bucketing_id, $userId);
292289
if($variation && $variation != new Variation()){
293-
return $variation;
290+
return new Decision($experiment->getId(), $variation->getId(), DECISION::DECISION_SOURCE_ROLLOUT);
294291
} else {
295292
$this->_logger->log(Logger::DEBUG,
296293
"User '{$userId}' was excluded due to traffic allocation. Checking 'Eveyrone Else' rule now.");
@@ -302,7 +299,7 @@ public function getVariationForFeatureRollout(FeatureFlag $featureFlag, $userId
302299
$experiment = $rolloutRules[sizeof($rolloutRules)-1];
303300
$variation = $this->_bucketer->bucket($this->_projectConfig, $experiment, $bucketing_id, $userId);
304301
if($variation && $variation != new Variation()){
305-
return $variation;
302+
return new Decision($experiment->getId(), $variation->getId(), DECISION::DECISION_SOURCE_ROLLOUT);
306303
} else {
307304
$this->_logger->log(Logger::DEBUG,
308305
"User '{$userId}' was excluded from the 'Everyone Else' rule for feature flag");
@@ -431,7 +428,7 @@ private function saveVariation(Experiment $experiment, Variation $variation, Use
431428
$decision = $userProfile->getDecisionForExperiment($experimentId);
432429
$variationId = $variation->getId();
433430
if (is_null($decision)) {
434-
$decision = new Decision($variationId);
431+
$decision = new \Optimizely\UserProfile\Decision($variationId);
435432
} else {
436433
$decision->setVariationId($variationId);
437434
}

‎tests/DecisionServiceTests/DecisionServiceTest.php‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Exception;
2020
use Monolog\Logger;
2121
use Optimizely\Bucketer;
22+
use Optimizely\DecisionService\Decision;
2223
use Optimizely\DecisionService\DecisionService;
2324
use Optimizely\Entity\Experiment;
2425
use Optimizely\Entity\Variation;
@@ -667,16 +668,14 @@ public function testGetVariationForFeatureExperimentGivenNonMutexGroupAndUserNot
667668
// should return the variation when the user is bucketed into a variation for the experiment on the feature flag
668669
public function testGetVariationForFeatureExperimentGivenNonMutexGroupAndUserIsBucketed(){
669670
// return the first variation of the `test_experiment_multivariate` experiment, which is attached to the `multi_variate_feature`
671+
$experiment = $this->config->getExperimentFromKey('test_experiment_multivariate');
670672
$variation = $this->config->getVariationFromId('test_experiment_multivariate','122231');
671673
$this->decisionServiceMock->expects($this->at(0))
672674
->method('getVariation')
673675
->will($this->returnValue($variation));
674676

675677
$feature_flag = $this->config->getFeatureFlagFromKey('multi_variate_feature');
676-
$expected_decision = [
677-
'experiment' => $this->config->getExperimentFromKey('test_experiment_multivariate'),
678-
'variation' => $this->config->getVariationFromId('test_experiment_multivariate','122231')
679-
];
678+
$expected_decision = new Decision($experiment->getId(), $variation->getId(), Decision::DECISION_SOURCE_EXPERIMENT);
680679

681680
$this->loggerMock->expects($this->at(0))
682681
->method('log')
@@ -699,10 +698,8 @@ public function testGetVariationForFeatureExperimentGivenMutexGroupAndUserIsBuck
699698

700699
$mutex_exp = $this->config->getExperimentFromKey('group_experiment_1');
701700
$variation = $mutex_exp->getVariations()[0];
702-
$expected_decision = [
703-
'experiment' => $mutex_exp,
704-
'variation' => $variation
705-
];
701+
$expected_decision = new Decision($mutex_exp->getId(), $variation->getId(), Decision::DECISION_SOURCE_EXPERIMENT);
702+
706703
$feature_flag = $this->config->getFeatureFlagFromKey('boolean_feature');
707704
$this->loggerMock->expects($this->at(0))
708705
->method('log')
@@ -775,18 +772,16 @@ public function testGetVariationForFeatureWhenBucketedToFeatureRollout(){
775772
$rollout = $this->config->getRolloutFromId($rollout_id);
776773
$experiment = $rollout->getExperiments()[0];
777774
$expected_variation = $experiment->getVariations()[0];
778-
$expected_decision = [
779-
'experiment' => null,
780-
'variation' => $expected_variation
781-
];
775+
$expected_decision = new Decision(
776+
$experiment->getId(), $expected_variation->getId(), Decision::DECISION_SOURCE_ROLLOUT);
782777

783778
$decisionServiceMock
784779
->method('getVariationForFeatureExperiment')
785780
->will($this->returnValue(null));
786781

787782
$decisionServiceMock
788783
->method('getVariationForFeatureRollout')
789-
->will($this->returnValue($expected_variation));
784+
->will($this->returnValue($expected_decision));
790785

791786
$this->loggerMock->expects($this->at(0))
792787
->method('log')
@@ -895,7 +890,10 @@ public function testGetVariationForFeatureRolloutWhenUserIsBucketedInTheTargetin
895890
$rollout_id = $feature_flag->getRolloutId();
896891
$rollout = $this->config->getRolloutFromId($rollout_id);
897892
$experiment = $rollout->getExperiments()[0];
898-
$expected_variation = $experiment->getVariations()[0];
893+
$expected_variation = $experiment->getVariations()[0];
894+
895+
$expected_decision = new Decision(
896+
$experiment->getId(), $expected_variation->getId(), Decision::DECISION_SOURCE_ROLLOUT);
899897
// Provide attributes such that user qualifies for audience
900898
$user_attributes = ["browser_type" => "chrome"];
901899

@@ -914,7 +912,7 @@ public function testGetVariationForFeatureRolloutWhenUserIsBucketedInTheTargetin
914912

915913
$this->assertEquals(
916914
$this->decisionService->getVariationForFeatureRollout($feature_flag, 'user_1', $user_attributes),
917-
$expected_variation
915+
$expected_decision
918916
);
919917
}
920918

@@ -927,7 +925,10 @@ public function testGetVariationForFeatureRolloutWhenUserIsNotBucketedInTheTarge
927925
$experiment0 = $rollout->getExperiments()[0];
928926
// Everyone Else Rule
929927
$experiment2 = $rollout->getExperiments()[2];
930-
$expected_variation = $experiment2->getVariations()[0];
928+
$expected_variation = $experiment2->getVariations()[0];
929+
930+
$expected_decision = new Decision(
931+
$experiment2->getId(), $expected_variation->getId(), Decision::DECISION_SOURCE_ROLLOUT);
931932

932933
// Provide attributes such that user qualifies for audience
933934
$user_attributes = ["browser_type" => "chrome"];
@@ -956,7 +957,7 @@ public function testGetVariationForFeatureRolloutWhenUserIsNotBucketedInTheTarge
956957

957958
$this->assertEquals(
958959
$this->decisionService->getVariationForFeatureRollout($feature_flag, 'user_1', $user_attributes),
959-
$expected_variation
960+
$expected_decision
960961
);
961962
}
962963

@@ -1018,7 +1019,10 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar
10181019
$experiment1 = $rollout->getExperiments()[1];
10191020
// Everyone Else Rule
10201021
$experiment2 = $rollout->getExperiments()[2];
1021-
$expected_variation = $experiment2->getVariations()[0];
1022+
$expected_variation = $experiment2->getVariations()[0];
1023+
1024+
$expected_decision = new Decision(
1025+
$experiment2->getId(), $expected_variation->getId(), Decision::DECISION_SOURCE_ROLLOUT);
10221026

10231027
// Provide null attributes so that user does not qualify for audience
10241028
$user_attributes = [];
@@ -1045,7 +1049,7 @@ public function testGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTar
10451049

10461050
$this->assertEquals(
10471051
$this->decisionService->getVariationForFeatureRollout($feature_flag, 'user_1', $user_attributes),
1048-
$expected_variation
1052+
$expected_decision
10491053
);
10501054
}
10511055
}

0 commit comments

Comments
 (0)