Skip to content

Commit de31675

Browse files
aliabbasrizvialiabbasrizvi
authored
Merge pull request #4 from optimizely/aliabbasrizvi/validate_preconditions
Validate preconditions before calling activate and track
2 parents 3133189 + 4b45965 commit de31675

File tree

6 files changed

+179
-7
lines changed

6 files changed

+179
-7
lines changed

‎src/Optimizely/Bucketer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public function bucket(ProjectConfig $config, Experiment $experiment, $userId)
113113

114114
// Check if user is whitelisted for a variation.
115115
$forcedVariations = $experiment->getForcedVariations();
116-
if (!is_null($forcedVariations) and isset($forcedVariations[$userId])) {
116+
if (!is_null($forcedVariations) && isset($forcedVariations[$userId])) {
117117
$variationKey = $forcedVariations[$userId];
118118
$variation = $config->getVariationFromKey($experiment->getKey(), $variationKey);
119119
return $variation;

‎src/Optimizely/Entity/Experiment.php

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@
2121

2222
class Experiment
2323
{
24+
/**
25+
* @const string String denoting running state of experiment.
26+
*/
27+
const STATUS_RUNNING = 'Running';
28+
29+
/**
30+
* @const string String denoting policy of mutually exclusive group.
31+
*/
32+
const MUTEX_GROUP_POLICY = 'random';
33+
2434
/**
2535
* @var string Experiment ID.
2636
*/
@@ -262,6 +272,28 @@ public function setTrafficAllocation($trafficAllocation)
262272
*/
263273
public function isInMutexGroup()
264274
{
265-
return !is_null($this->_groupPolicy) && $this->_groupPolicy == 'random';
275+
return !is_null($this->_groupPolicy) && $this->_groupPolicy == self::MUTEX_GROUP_POLICY;
276+
}
277+
278+
/**
279+
* Determine if experiment is running or not.
280+
*
281+
* @return boolean True if experiment has status "Running". False otherwise.
282+
*/
283+
public function isExperimentRunning()
284+
{
285+
return !is_null($this->_status) && $this->_status == self::STATUS_RUNNING;
286+
}
287+
288+
/**
289+
* Determine if user is in forced variation of experiment.
290+
*
291+
* @param $userId string ID of the user.
292+
* @return boolean True if user is in forced variation of experiment. False otherwise.
293+
*/
294+
public function isUserInForcedVariation($userId)
295+
{
296+
$forcedVariations = $this->getForcedVariations();
297+
return !is_null($forcedVariations) && isset($forcedVariations[$userId]);
266298
}
267299
}

‎src/Optimizely/Optimizely.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,18 @@ private function validateInputs($datafile, $skipJsonValidation)
119119
*/
120120
private function validatePreconditions($experiment, $userId, $attributes)
121121
{
122+
//@TODO(ali): Insert attributes validation
123+
124+
if (!$experiment->isExperimentRunning()) {
125+
return false;
126+
}
127+
128+
if ($experiment->isUserInForcedVariation($userId)) {
129+
return true;
130+
}
131+
132+
//@TODO(ali): Insert audience check
133+
122134
return true;
123135
}
124136

@@ -214,7 +226,7 @@ public function getVariation($experimentKey, $userId, $attributes = null)
214226
{
215227
$experiment = $this->_config->getExperimentFromKey($experimentKey);
216228

217-
if (!is_null($experiment->getKey())) {
229+
if (is_null($experiment->getKey())) {
218230
return null;
219231
}
220232

‎tests/OptimizelyTest.php

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,14 @@ class OptimizelyTest extends \PHPUnit_Framework_TestCase
3131
{
3232
private $datafile;
3333
private $eventBuilderMock;
34+
private $optimizelyObject;
3435
private $projectConfig;
3536

3637
public function setUp()
3738
{
3839
$this->datafile = DATAFILE;
3940
$this->projectConfig = new ProjectConfig($this->datafile);
41+
$this->optimizelyObject = new Optimizely($this->datafile);
4042

4143
// Mock EventBuilder
4244
$this->eventBuilderMock = $this->getMockBuilder(EventBuilder::class)
@@ -143,6 +145,92 @@ public function testValidateInputsInvalidFileJsonValidationSkipped()
143145
);
144146
}
145147

148+
public function testValidatePreconditionsExperimentNotRunning()
149+
{
150+
$validatePreconditions = new \ReflectionMethod('Optimizely\Optimizely', 'validatePreconditions');
151+
$validatePreconditions->setAccessible(true);
152+
153+
$this->assertFalse(
154+
$validatePreconditions->invoke(
155+
$this->optimizelyObject,
156+
$this->projectConfig->getExperimentFromKey('paused_experiment'),
157+
'test_user',
158+
[])
159+
);
160+
}
161+
162+
public function testValidatePreconditionsExperimentRunning()
163+
{
164+
$validatePreconditions = new \ReflectionMethod('Optimizely\Optimizely', 'validatePreconditions');
165+
$validatePreconditions->setAccessible(true);
166+
167+
$this->assertTrue(
168+
$validatePreconditions->invoke(
169+
$this->optimizelyObject,
170+
$this->projectConfig->getExperimentFromKey('test_experiment'),
171+
'test_user',
172+
[])
173+
);
174+
}
175+
176+
public function testValidatePreconditionsUserInForcedVariationNotInExperiment()
177+
{
178+
$validatePreconditions = new \ReflectionMethod('Optimizely\Optimizely', 'validatePreconditions');
179+
$validatePreconditions->setAccessible(true);
180+
181+
$this->assertTrue(
182+
$validatePreconditions->invoke(
183+
$this->optimizelyObject,
184+
$this->projectConfig->getExperimentFromKey('test_experiment'),
185+
'user_1',
186+
[])
187+
);
188+
}
189+
190+
public function testValidatePreconditionsUserInForcedVariationInExperiment()
191+
{
192+
$validatePreconditions = new \ReflectionMethod('Optimizely\Optimizely', 'validatePreconditions');
193+
$validatePreconditions->setAccessible(true);
194+
195+
$this->assertTrue(
196+
$validatePreconditions->invoke(
197+
$this->optimizelyObject,
198+
$this->projectConfig->getExperimentFromKey('test_experiment'),
199+
'user1',
200+
[])
201+
);
202+
}
203+
204+
public function testValidatePreconditionsUserNotInForcedVariationNotInExperiment()
205+
{
206+
$validatePreconditions = new \ReflectionMethod('Optimizely\Optimizely', 'validatePreconditions');
207+
$validatePreconditions->setAccessible(true);
208+
209+
// Will get updated when we have audience evaluation and user does not meet conditions
210+
$this->assertTrue(
211+
$validatePreconditions->invoke(
212+
$this->optimizelyObject,
213+
$this->projectConfig->getExperimentFromKey('test_experiment'),
214+
'test_user',
215+
[])
216+
);
217+
}
218+
219+
public function testValidatePreconditionsUserNotInForcedVariationInExperiment()
220+
{
221+
$validatePreconditions = new \ReflectionMethod('Optimizely\Optimizely', 'validatePreconditions');
222+
$validatePreconditions->setAccessible(true);
223+
224+
// Will get updated when we have audience evaluation and user does meets conditions
225+
$this->assertTrue(
226+
$validatePreconditions->invoke(
227+
$this->optimizelyObject,
228+
$this->projectConfig->getExperimentFromKey('test_experiment'),
229+
'test_user',
230+
[])
231+
);
232+
}
233+
146234
public function testActivateNoAttributes()
147235
{
148236
$this->eventBuilderMock->expects($this->once())
@@ -190,6 +278,32 @@ public function testActivateWithAttributes()
190278
$this->assertEquals('control', $optlyObject->activate('test_experiment', 'test_user', $userAttributes));
191279
}
192280

281+
public function testActivateExperimentNotRunning()
282+
{
283+
$this->eventBuilderMock->expects($this->never())
284+
->method('createImpressionEvent');
285+
286+
$optlyObject = new Optimizely($this->datafile, new ValidEventDispatcher());
287+
288+
$eventBuilder = new \ReflectionProperty(Optimizely::class, '_eventBuilder');
289+
$eventBuilder->setAccessible(true);
290+
$eventBuilder->setValue($optlyObject, $this->eventBuilderMock);
291+
292+
// Call activate
293+
$this->assertNull($optlyObject->activate('paused_experiment', 'test_user', null));
294+
}
295+
296+
297+
public function testGetVariation()
298+
{
299+
$this->assertEquals('control', $this->optimizelyObject->getVariation('test_experiment', 'test_user'));
300+
}
301+
302+
public function testGetVariationExperimentNotRunning()
303+
{
304+
$this->assertNull($this->optimizelyObject->getVariation('paused_experiment', 'test_user'));
305+
}
306+
193307
public function testTrackNoAttributesNoEventValue()
194308
{
195309
$this->eventBuilderMock->expects($this->once())

‎tests/ProjectConfigTest.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public function testInit()
6969
$experimentKeyMap->setAccessible(true);
7070
$this->assertEquals([
7171
'test_experiment' => $this->config->getExperimentFromKey('test_experiment'),
72+
'paused_experiment' => $this->config->getExperimentFromKey('paused_experiment'),
7273
'group_experiment_1' => $this->config->getExperimentFromKey('group_experiment_1'),
7374
'group_experiment_2' => $this->config->getExperimentFromKey('group_experiment_2')
7475
], $experimentKeyMap->getValue($this->config));
@@ -79,7 +80,8 @@ public function testInit()
7980
$this->assertEquals([
8081
'7716830082' => $this->config->getExperimentFromId('7716830082'),
8182
'7723330021' => $this->config->getExperimentFromId('7723330021'),
82-
'7718750065' => $this->config->getExperimentFromId('7718750065')
83+
'7718750065' => $this->config->getExperimentFromId('7718750065'),
84+
'7716830585' => $this->config->getExperimentFromId('7716830585')
8385
], $experimentIdMap->getValue($this->config));
8486

8587
// Check event key map
@@ -112,6 +114,10 @@ public function testInit()
112114
'control' => $this->config->getVariationFromKey('test_experiment', 'control'),
113115
'variation' => $this->config->getVariationFromKey('test_experiment', 'variation')
114116
],
117+
'paused_experiment' => [
118+
'control' => $this->config->getVariationFromKey('paused_experiment', 'control'),
119+
'variation' => $this->config->getVariationFromKey('paused_experiment', 'variation')
120+
],
115121
'group_experiment_1' => [
116122
'group_exp_1_var_1' => $this->config->getVariationFromKey('group_experiment_1', 'group_exp_1_var_1'),
117123
'group_exp_1_var_2' => $this->config->getVariationFromKey('group_experiment_1', 'group_exp_1_var_2')
@@ -130,6 +136,10 @@ public function testInit()
130136
'7722370027' => $this->config->getVariationFromId('test_experiment', '7722370027'),
131137
'7721010009' => $this->config->getVariationFromId('test_experiment', '7721010009')
132138
],
139+
'paused_experiment' => [
140+
'7722370427' => $this->config->getVariationFromId('paused_experiment', '7722370427'),
141+
'7721010509' => $this->config->getVariationFromId('paused_experiment', '7721010509')
142+
],
133143
'group_experiment_1' => [
134144
'7722260071' => $this->config->getVariationFromId('group_experiment_1', '7722260071'),
135145
'7722360022' => $this->config->getVariationFromId('group_experiment_1', '7722360022')
@@ -192,7 +202,7 @@ public function testGetEventValidKey()
192202
$event = $this->config->getEvent('purchase');
193203
$this->assertEquals('purchase', $event->getKey());
194204
$this->assertEquals('7718020063', $event->getId());
195-
$this->assertEquals(['7716830082', '7723330021', '7718750065'], $event->getExperimentIds());
205+
$this->assertEquals(['7716830082', '7723330021', '7718750065', '7716830585'], $event->getExperimentIds());
196206
}
197207

198208
public function testGetEventInvalidKey()

‎tests/TestData.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,17 @@
2626
"trafficAllocation": [{"entityId": "7722370027", "endOfRange": 4000},
2727
{"entityId": "7721010009", "endOfRange": 8000}], "audienceIds": ["7718080042"],
2828
"variations": [{"id": "7722370027", "key": "control"}, {"id": "7721010009", "key": "variation"}],
29-
"forcedVariations": {"user1": "control"}, "id": "7716830082"}], "version": "2",
29+
"forcedVariations": {"user1": "control"}, "id": "7716830082"}, {"status": "Paused", "key": "paused_experiment", "layerId": "7719779139",
30+
"trafficAllocation": [{"entityId": "7722370427", "endOfRange": 5000},
31+
{"entityId": "7721010509", "endOfRange": 8000}], "audienceIds": [],
32+
"variations": [{"id": "7722370427", "key": "control"}, {"id": "7721010509", "key": "variation"}],
33+
"forcedVariations": {}, "id": "7716830585"}], "version": "2",
3034
"audiences": [{"conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"device_type\", \"type\": \"custom_attribute\", \"value\": \"iPhone\"}]], [\"or\", [\"or\", {\"name\": \"location\", \"type\": \"custom_attribute\", \"value\": \"San Francisco\"}]]]", "id": "7718080042", "name": "iPhone users in San Francisco"}],
3135
"groups": [{"policy": "random", "trafficAllocation": [{"entityId": "7723330021", "endOfRange": 2000}, {"entityId": "7718750065", "endOfRange": 6000}], "experiments": [{"status": "Running", "key": "group_experiment_1", "layerId": "7721010011", "trafficAllocation": [{"entityId": "7722260071", "endOfRange": 5000}, {"entityId": "7722360022", "endOfRange": 10000}], "audienceIds": [], "variations": [{"id": "7722260071", "key": "group_exp_1_var_1"}, {"id": "7722360022", "key": "group_exp_1_var_2"}], "forcedVariations": {"user1": "group_exp_1_var_1"}, "id": "7723330021"}, {"status": "Running", "key": "group_experiment_2", "layerId": "7721020020", "trafficAllocation": [{"entityId": "7713030086", "endOfRange": 5000}, {"entityId": "7725250007", "endOfRange": 10000}], "audienceIds": [],
3236
"variations": [{"id": "7713030086", "key": "group_exp_2_var_1"}, {"id": "7725250007", "key": "group_exp_2_var_2"}], "forcedVariations": {}, "id": "7718750065"}], "id": "7722400015"}],
3337
"attributes": [{"id": "7723280020", "key": "device_type"}, {"id": "7723340004", "key": "location"}],
3438
"projectId": "7720880029", "accountId": "1592310167",
35-
"events": [{"experimentIds": ["7716830082", "7723330021", "7718750065"], "id": "7718020063", "key": "purchase"}],
39+
"events": [{"experimentIds": ["7716830082", "7723330021", "7718750065", "7716830585"], "id": "7718020063", "key": "purchase"}],
3640
"revision": "15"}');
3741

3842
/**

0 commit comments

Comments
 (0)