Skip to content

Commit 122e6ab

Browse files
ozayr-zaviarozayr-zaviar
authored
feat: Duplicate experiment key issue with multi feature flag (#226)
1 parent 39cfc1c commit 122e6ab

File tree

10 files changed

+222
-66
lines changed

10 files changed

+222
-66
lines changed

‎src/Optimizely/Bucketer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public function bucket(ProjectConfigInterface $config, Experiment $experiment, $
197197
list($variationId, $reasons) = $this->findBucket($bucketingId, $userId, $experiment->getId(), $experiment->getTrafficAllocation());
198198
$decideReasons = array_merge($decideReasons, $reasons);
199199
if (!empty($variationId)) {
200-
$variation = $config->getVariationFromId($experiment->getKey(), $variationId);
200+
$variation = $config->getVariationFromIdByExperimentId($experiment->getId(), $variationId);
201201

202202
return [ $variation, $decideReasons ];
203203
}

‎src/Optimizely/Config/DatafileProjectConfig.php‎

Lines changed: 86 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Copyright 2019-2020, Optimizely
3+
* Copyright 2019-2021, Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -123,6 +123,16 @@ class DatafileProjectConfig implements ProjectConfigInterface
123123
*/
124124
private $_variationIdMap;
125125

126+
/**
127+
* @var array Associative array of experiment id to associative array of variation ID to variations.
128+
*/
129+
private $_variationIdMapByExperimentId;
130+
131+
/**
132+
* @var array Associative array of experiment id to associative array of variation key to variations.
133+
*/
134+
private $_variationKeyMapByExperimentId;
135+
126136
/**
127137
* @var array Associative array of event key to Event(s) in the datafile.
128138
*/
@@ -247,7 +257,7 @@ public function __construct($datafile, $logger, $errorHandler)
247257
}
248258

249259
$this->_groupIdMap = ConfigParser::generateMap($groups, 'id', Group::class);
250-
$this->_experimentKeyMap = ConfigParser::generateMap($experiments, 'key', Experiment::class);
260+
$this->_experimentIdMap = ConfigParser::generateMap($experiments, 'id', Experiment::class);
251261
$this->_eventKeyMap = ConfigParser::generateMap($events, 'key', Event::class);
252262
$this->_attributeKeyMap = ConfigParser::generateMap($attributes, 'key', Attribute::class);
253263
$typedAudienceIdMap = ConfigParser::generateMap($typedAudiences, 'id', Audience::class);
@@ -256,32 +266,38 @@ public function __construct($datafile, $logger, $errorHandler)
256266
$this->_featureFlags = ConfigParser::generateMap($featureFlags, null, FeatureFlag::class);
257267

258268
foreach (array_values($this->_groupIdMap) as $group) {
259-
$experimentsInGroup = ConfigParser::generateMap($group->getExperiments(), 'key', Experiment::class);
269+
$experimentsInGroup = ConfigParser::generateMap($group->getExperiments(), 'id', Experiment::class);
260270
foreach (array_values($experimentsInGroup) as $experiment) {
261271
$experiment->setGroupId($group->getId());
262272
$experiment->setGroupPolicy($group->getPolicy());
263273
}
264-
$this->_experimentKeyMap = $this->_experimentKeyMap + $experimentsInGroup;
274+
$this->_experimentIdMap = $this->_experimentIdMap + $experimentsInGroup;
265275
}
266276

267277
foreach ($this->_rollouts as $rollout) {
268278
foreach ($rollout->getExperiments() as $experiment) {
269-
$this->_experimentKeyMap[$experiment->getKey()] = $experiment;
279+
$this->_experimentIdMap[$experiment->getId()] = $experiment;
270280
}
271281
}
272282

273283
$this->_variationKeyMap = [];
274284
$this->_variationIdMap = [];
275-
$this->_experimentIdMap = [];
285+
$this->_variationKeyMapByExperimentId = [];
286+
$this->_variationIdMapByExperimentId = [];
287+
$this->_experimentKeyMap = [];
276288

277-
foreach (array_values($this->_experimentKeyMap) as $experiment) {
289+
foreach (array_values($this->_experimentIdMap) as $experiment) {
278290
$this->_variationKeyMap[$experiment->getKey()] = [];
279291
$this->_variationIdMap[$experiment->getKey()] = [];
280-
$this->_experimentIdMap[$experiment->getId()] = $experiment;
292+
$this->_variationIdMapByExperimentId[$experiment->getId()] = [];
293+
$this->_variationKeyMapByExperimentId[$experiment->getId()] = [];
294+
$this->_experimentKeyMap[$experiment->getKey()] = $experiment;
281295

282296
foreach ($experiment->getVariations() as $variation) {
283297
$this->_variationKeyMap[$experiment->getKey()][$variation->getKey()] = $variation;
284298
$this->_variationIdMap[$experiment->getKey()][$variation->getId()] = $variation;
299+
$this->_variationKeyMapByExperimentId[$experiment->getId()][$variation->getKey()] = $variation;
300+
$this->_variationIdMapByExperimentId[$experiment->getId()][$variation->getId()] = $variation;
285301
}
286302
}
287303

@@ -300,24 +316,32 @@ public function __construct($datafile, $logger, $errorHandler)
300316

301317
$rolloutVariationIdMap = [];
302318
$rolloutVariationKeyMap = [];
319+
$rolloutVariationIdMapByExperimentId = [];
320+
$rolloutVariationKeyMapByExperimentId = [];
303321
foreach ($this->_rollouts as $rollout) {
304322
$this->_rolloutIdMap[$rollout->getId()] = $rollout;
305323

306324
foreach ($rollout->getExperiments() as $rule) {
307325
$rolloutVariationIdMap[$rule->getKey()] = [];
308326
$rolloutVariationKeyMap[$rule->getKey()] = [];
327+
$rolloutVariationIdMapByExperimentId[$rule->getId()] = [];
328+
$rolloutVariationKeyMapByExperimentId[$rule->getId()] = [];
309329

310330
$variations = $rule->getVariations();
311331
foreach ($variations as $variation) {
312332
$rolloutVariationIdMap[$rule->getKey()][$variation->getId()] = $variation;
313333
$rolloutVariationKeyMap[$rule->getKey()][$variation->getKey()] = $variation;
334+
$rolloutVariationIdMapByExperimentId[$rule->getId()][$variation->getId()] = $variation;
335+
$rolloutVariationKeyMapByExperimentId[$rule->getId()][$variation->getKey()] = $variation;
314336
}
315337
}
316338
}
317339

318340
// Add variations for rollout experiments to variationIdMap and variationKeyMap
319341
$this->_variationIdMap = $this->_variationIdMap + $rolloutVariationIdMap;
320342
$this->_variationKeyMap = $this->_variationKeyMap + $rolloutVariationKeyMap;
343+
$this->_variationIdMapByExperimentId = $this->_variationIdMapByExperimentId + $rolloutVariationIdMapByExperimentId;
344+
$this->_variationKeyMapByExperimentId = $this->_variationKeyMapByExperimentId + $rolloutVariationKeyMapByExperimentId;
321345

322346
foreach (array_values($this->_featureFlags) as $featureFlag) {
323347
$this->_featureKeyMap[$featureFlag->getKey()] = $featureFlag;
@@ -655,6 +679,60 @@ public function getVariationFromId($experimentKey, $variationId)
655679
return new Variation();
656680
}
657681

682+
/**
683+
* @param $experimentId string ID for experiment.
684+
* @param $variationId string ID for variation.
685+
*
686+
* @return Variation Entity corresponding to the provided experiment ID and variation ID.
687+
* Dummy entity is returned if key or ID is invalid.
688+
*/
689+
public function getVariationFromIdByExperimentId($experimentId, $variationId)
690+
{
691+
if (isset($this->_variationIdMapByExperimentId[$experimentId])
692+
&& isset($this->_variationIdMapByExperimentId[$experimentId][$variationId])
693+
) {
694+
return $this->_variationIdMapByExperimentId[$experimentId][$variationId];
695+
}
696+
697+
$this->_logger->log(
698+
Logger::ERROR,
699+
sprintf(
700+
'No variation ID "%s" defined in datafile for experiment "%s".',
701+
$variationId,
702+
$experimentId
703+
)
704+
);
705+
$this->_errorHandler->handleError(new InvalidVariationException('Provided variation is not in datafile.'));
706+
return new Variation();
707+
}
708+
709+
/**
710+
* @param $experimentId string ID for experiment.
711+
* @param $variationKey string Key for variation.
712+
*
713+
* @return Variation Entity corresponding to the provided experiment ID and variation Key.
714+
* Dummy entity is returned if key or ID is invalid.
715+
*/
716+
public function getVariationFromKeyByExperimentId($experimentId, $variationKey)
717+
{
718+
if (isset($this->_variationKeyMapByExperimentId[$experimentId])
719+
&& isset($this->_variationKeyMapByExperimentId[$experimentId][$variationKey])
720+
) {
721+
return $this->_variationKeyMapByExperimentId[$experimentId][$variationKey];
722+
}
723+
724+
$this->_logger->log(
725+
Logger::ERROR,
726+
sprintf(
727+
'No variation Key "%s" defined in datafile for experiment "%s".',
728+
$variationKey,
729+
$experimentId
730+
)
731+
);
732+
$this->_errorHandler->handleError(new InvalidVariationException('Provided variation is not in datafile.'));
733+
return new Variation();
734+
}
735+
658736
/**
659737
* Gets the feature variable instance given feature flag key and variable key
660738
*

‎src/Optimizely/Config/ProjectConfigInterface.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Copyright 2016, 2018-2020 Optimizely
3+
* Copyright 2016, 2018-2021 Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -141,6 +141,24 @@ public function getVariationFromKey($experimentKey, $variationKey);
141141
*/
142142
public function getVariationFromId($experimentKey, $variationId);
143143

144+
/**
145+
* @param $experimentId string ID for experiment.
146+
* @param $variationId string ID for variation.
147+
*
148+
* @return Variation Entity corresponding to the provided experiment ID and variation ID.
149+
* Dummy entity is returned if key or ID is invalid.
150+
*/
151+
public function getVariationFromIdByExperimentId($experimentId, $variationId);
152+
153+
/**
154+
* @param $experimentId string ID for experiment.
155+
* @param $variationKey string Key for variation.
156+
*
157+
* @return Variation Entity corresponding to the provided experiment ID and variation Key.
158+
* Dummy entity is returned if key or ID is invalid.
159+
*/
160+
public function getVariationFromKeyByExperimentId($experimentId, $variationKey);
161+
144162
/**
145163
* Gets the feature variable instance given feature flag key and variable key
146164
*

‎src/Optimizely/DecisionService/DecisionService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ private function getWhitelistedVariation(ProjectConfigInterface $projectConfig,
518518
$forcedVariations = $experiment->getForcedVariations();
519519
if (!is_null($forcedVariations) && isset($forcedVariations[$userId])) {
520520
$variationKey = $forcedVariations[$userId];
521-
$variation = $projectConfig->getVariationFromKey($experiment->getKey(), $variationKey);
521+
$variation = $projectConfig->getVariationFromKeyByExperimentId($experiment->getId(), $variationKey);
522522
if ($variationKey && !empty($variation->getKey())) {
523523
$message = sprintf('User "%s" is forced in variation "%s" of experiment "%s".', $userId, $variationKey, $experiment->getKey());
524524

‎src/Optimizely/Event/Builder/EventBuilder.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Copyright 2016-2020, Optimizely
3+
* Copyright 2016-2021, Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -222,19 +222,19 @@ private function getConversionParams($eventEntity, $eventTags)
222222
* Create impression event to be sent to the logging endpoint.
223223
*
224224
* @param $config ProjectConfigInterface Configuration for the project.
225-
* @param $experimentKey Experiment Experiment being activated.
225+
* @param $experimentId Experiment Experiment being activated.
226226
* @param $variationKey string Variation user
227227
* @param $userId string ID of user.
228228
* @param $attributes array Attributes of the user.
229229
*
230230
* @return LogEvent Event object to be sent to dispatcher.
231231
*/
232-
public function createImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
232+
public function createImpressionEvent($config, $experimentId, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
233233
{
234234
$eventParams = $this->getCommonParams($config, $userId, $attributes);
235235

236-
$experiment = $config->getExperimentFromKey($experimentKey);
237-
$variation = $config->getVariationFromKey($experimentKey, $variationKey);
236+
$experiment = $config->getExperimentFromId($experimentId);
237+
$variation = $config->getVariationFromKeyByExperimentId($experimentId, $variationKey);
238238
$impressionParams = $this->getImpressionParams($experiment, $variation, $flagKey, $ruleKey, $ruleType, $enabled);
239239

240240
$eventParams[VISITORS][0][SNAPSHOTS][] = $impressionParams;

‎src/Optimizely/Optimizely.php‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,17 @@ private function validateUserInputs($attributes, $eventTags = null)
201201
}
202202

203203
/**
204-
* @param string Experiment key
204+
* @param string Experiment ID
205205
* @param string Variation key
206206
* @param string User ID
207207
* @param array Associative array of user attributes
208208
* @param DatafileProjectConfig DatafileProjectConfig instance
209209
*/
210-
protected function sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
210+
protected function sendImpressionEvent($config, $experimentId, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
211211
{
212+
$experimentKey = $config->getExperimentFromId($experimentId)->getKey();
212213
$impressionEvent = $this->_eventBuilder
213-
->createImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes);
214+
->createImpressionEvent($config, $experimentId, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes);
214215
$this->_logger->log(Logger::INFO, sprintf('Activating user "%s" in experiment "%s".', $userId, $experimentKey));
215216
$this->_logger->log(
216217
Logger::DEBUG,
@@ -244,10 +245,10 @@ protected function sendImpressionEvent($config, $experimentKey, $variationKey, $
244245
$this->notificationCenter->sendNotifications(
245246
NotificationType::ACTIVATE,
246247
array(
247-
$config->getExperimentFromKey($experimentKey),
248+
$config->getExperimentFromId($experimentId),
248249
$userId,
249250
$attributes,
250-
$config->getVariationFromKey($experimentKey, $variationKey),
251+
$config->getVariationFromKeyByExperimentId($experimentId, $variationKey),
251252
$impressionEvent
252253
)
253254
);
@@ -340,6 +341,7 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp
340341
$variationKey = null;
341342
$featureEnabled = false;
342343
$ruleKey = null;
344+
$experimentId = null;
343345
$flagKey = $key;
344346
$allVariables = [];
345347
$decisionEventDispatched = false;
@@ -360,9 +362,11 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp
360362
$variationKey = $variation->getKey();
361363
$featureEnabled = $variation->getFeatureEnabled();
362364
$ruleKey = $decision->getExperiment()->getKey();
365+
$experimentId = $decision->getExperiment()->getId();
363366
} else {
364367
$variationKey = null;
365368
$ruleKey = null;
369+
$experimentId = null;
366370
}
367371

368372
// send impression only if decide options do not contain DISABLE_DECISION_EVENT
@@ -374,7 +378,7 @@ public function decide(OptimizelyUserContext $userContext, $key, array $decideOp
374378
if ($source == FeatureDecision::DECISION_SOURCE_FEATURE_TEST || $sendFlagDecisions) {
375379
$this->sendImpressionEvent(
376380
$config,
377-
$ruleKey,
381+
$experimentId,
378382
$variationKey === null ? '' : $variationKey,
379383
$flagKey,
380384
$ruleKey === null ? '' : $ruleKey,
@@ -531,8 +535,9 @@ public function activate($experimentKey, $userId, $attributes = null)
531535
$this->_logger->log(Logger::INFO, sprintf('Not activating user "%s".', $userId));
532536
return null;
533537
}
538+
$experimentId = $config->getExperimentFromKey($experimentKey)->getId();
534539

535-
$this->sendImpressionEvent($config, $experimentKey, $variationKey, '', $experimentKey, FeatureDecision::DECISION_SOURCE_EXPERIMENT, true, $userId, $attributes);
540+
$this->sendImpressionEvent($config, $experimentId, $variationKey, '', $experimentKey, FeatureDecision::DECISION_SOURCE_EXPERIMENT, true, $userId, $attributes);
536541

537542
return $variationKey;
538543
}
@@ -818,19 +823,21 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
818823
$featureEnabled = $variation->getFeatureEnabled();
819824
}
820825
$ruleKey = $decision->getExperiment() ? $decision->getExperiment()->getKey() : '';
821-
$this->sendImpressionEvent($config, $ruleKey, $variation ? $variation->getKey() : '', $featureFlagKey, $ruleKey, $decision->getSource(), $featureEnabled, $userId, $attributes);
826+
$experimentId = $decision->getExperiment() ? $decision->getExperiment()->getId() : '';
827+
$this->sendImpressionEvent($config, $experimentId, $variation ? $variation->getKey() : '', $featureFlagKey, $ruleKey, $decision->getSource(), $featureEnabled, $userId, $attributes);
822828
}
823829

824830
if ($variation) {
825831
$experimentKey = $decision->getExperiment()->getKey();
832+
$experimentId = $decision->getExperiment()->getId();
826833
$featureEnabled = $variation->getFeatureEnabled();
827834
if ($decision->getSource() == FeatureDecision::DECISION_SOURCE_FEATURE_TEST) {
828835
$sourceInfo = (object) array(
829836
'experimentKey'=> $experimentKey,
830837
'variationKey'=> $variation->getKey()
831838
);
832839

833-
$this->sendImpressionEvent($config, $experimentKey, $variation->getKey(), $featureFlagKey, $experimentKey, $decision->getSource(), $featureEnabled, $userId, $attributes);
840+
$this->sendImpressionEvent($config, $experimentId, $variation->getKey(), $featureFlagKey, $experimentKey, $decision->getSource(), $featureEnabled, $userId, $attributes);
834841
} else {
835842
$this->_logger->log(Logger::INFO, "The user '{$userId}' is not being experimented on Feature Flag '{$featureFlagKey}'.");
836843
}

0 commit comments

Comments
 (0)