diff --git a/src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php b/src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php index d2dadb7c..2d29698a 100644 --- a/src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php +++ b/src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php @@ -1,12 +1,12 @@ experiments = $projectConfig->getAllExperiments(); $this->featureFlags = $projectConfig->getFeatureFlags(); @@ -82,7 +92,8 @@ public function __construct(ProjectConfigInterface $projectConfig) $this->environmentKey = $projectConfig->getEnvironmentKey(); $this->sdkKey = $projectConfig->getSdkKey(); $this->projectConfig = $projectConfig; - + $this->logger = $logger ?: new DefaultLogger(); + $this->createLookupMaps(); } @@ -258,7 +269,7 @@ protected function getVariablesMap(Experiment $experiment, Variation $variation) // Set default variables for variation. $variablesMap = $this->featKeyOptlyVariableKeyVariableMap[$featureKey]; - + // Return default variable values if feature is not enabled. if (!$variation->getFeatureEnabled()) { return $variablesMap; @@ -267,13 +278,13 @@ protected function getVariablesMap(Experiment $experiment, Variation $variation) // Set variation specific value if any. foreach ($variation->getVariables() as $variableUsage) { $id = $variableUsage->getId(); - + $optVariable = $this->featKeyOptlyVariableIdVariableMap[$featureKey][$id]; - + $key = $optVariable->getKey(); $value = $variableUsage->getValue(); $type = $optVariable->getType(); - + $modifiedOptVariable = new OptimizelyVariable( $id, $key, @@ -287,7 +298,7 @@ protected function getVariablesMap(Experiment $experiment, Variation $variation) return $variablesMap; } - + /** * Generates Variations map for the given Experiment. * @@ -301,7 +312,7 @@ protected function getVariationsMap(Experiment $experiment) foreach ($experiment->getVariations() as $variation) { $variablesMap = $this->getVariablesMap($experiment, $variation); - + $variationKey = $variation->getKey(); $optVariation = new OptimizelyVariation( $variation->getId(), @@ -401,11 +412,17 @@ protected function getExperimentsMaps() foreach ($this->experiments as $exp) { $expId = $exp->getId(); $expKey = $exp->getKey(); + $audiences = ''; if ($exp->getAudienceConditions() != null) { $audienceConditions = $exp->getAudienceConditions(); $audiences = $this->getSerializedAudiences($audienceConditions); } + + if (array_key_exists($expKey, $experimentsKeyMap)) { + $this->logger->log(Logger::WARNING, sprintf('Duplicate experiment keys found in datafile: %s', $expKey)); + } + $optExp = new OptimizelyExperiment( $expId, $expKey, diff --git a/tests/OptimizelyConfigTests/OptimizelyConfigServiceTest.php b/tests/OptimizelyConfigTests/OptimizelyConfigServiceTest.php index 730ebff4..2a5cf5cf 100644 --- a/tests/OptimizelyConfigTests/OptimizelyConfigServiceTest.php +++ b/tests/OptimizelyConfigTests/OptimizelyConfigServiceTest.php @@ -16,9 +16,10 @@ */ namespace Optimizely\Tests; -use Exception; +use Monolog\Logger; use Optimizely\Config\DatafileProjectConfig; use Optimizely\ErrorHandler\NoOpErrorHandler; +use Optimizely\Logger\DefaultLogger; use Optimizely\Logger\NoOpLogger; use Optimizely\OptimizelyConfig\OptimizelyAttribute; use Optimizely\OptimizelyConfig\OptimizelyAudience; @@ -29,10 +30,12 @@ use Optimizely\OptimizelyConfig\OptimizelyFeature; use Optimizely\OptimizelyConfig\OptimizelyVariable; use Optimizely\OptimizelyConfig\OptimizelyVariation; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class OptimizelyConfigServiceTest extends TestCase { + private MockObject $loggerMock; protected function setUp() : void { @@ -149,6 +152,9 @@ protected function setUp() : void $this->expectedExpIdMap['17301270474'] = $abExperiment; $this->expectedExpIdMap['17258450439'] = $groupExperiment; $this->expectedExpIdMap['17279300791'] = $featExperiment; + + // Mock Logger + $this->loggerMock = $this->getMockBuilder(DefaultLogger::class)->getMock(); } protected static function getMethod($name) @@ -203,28 +209,26 @@ public function testGetVariationsMap() public function testGetOptimizelyConfigWithDuplicateExperimentKeys() { + $duplicatedExperimentKey = 'targeted_delivery'; + $secondDuplicatedExperimentId = '9300000007573'; + $this->loggerMock->expects($this->once()) + ->method('log') + ->with( + Logger::WARNING, + sprintf('Duplicate experiment keys found in datafile: %s', $duplicatedExperimentKey) + ); + $this->datafile = DATAFILE_FOR_DUPLICATE_EXP_KEYS; $this->projectConfig = new DatafileProjectConfig( $this->datafile, new NoOpLogger(), new NoOpErrorHandler() ); - $this->optConfigService = new OptimizelyConfigService($this->projectConfig); + $this->optConfigService = new OptimizelyConfigService($this->projectConfig, $this->loggerMock); $optimizelyConfig = $this->optConfigService->getConfig(); - $this->assertEquals(Count($optimizelyConfig->getExperimentsMap()), 1); - $experimentRulesFlag1 = $optimizelyConfig->getFeaturesMap()['flag1']->getExperimentRules(); // 9300000007569 - $experimentRulesFlag2 = $optimizelyConfig->getFeaturesMap()['flag2']->getExperimentRules(); // 9300000007573 - foreach ($experimentRulesFlag1 as $experimentRule) { - if ($experimentRule->getKey() == 'targeted_delivery') { - $this->assertEquals($experimentRule->getId(), '9300000007569'); - } - } - foreach ($experimentRulesFlag2 as $experimentRule) { - if ($experimentRule->getKey() == 'targeted_delivery') { - $this->assertEquals($experimentRule->getId(), '9300000007573'); - } - } + $this->assertEquals(1, Count($optimizelyConfig->getExperimentsMap())); + $this->assertEquals($optimizelyConfig->getExperimentsMap()[$duplicatedExperimentKey]->getId(), $secondDuplicatedExperimentId); } public function testGetOptimizelyConfigWithDuplicateRuleKeys()