From 5fd61638c2eb234a9d09b8b2d2eccec9ad2adb86 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 30 Nov 2023 16:44:58 -0500 Subject: [PATCH 1/2] feat: log duplicate experiment keys includes some linting --- .../OptimizelyConfigService.php | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) 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, From ea0697523159efd891a9cba4112ba43891b6c56d Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 30 Nov 2023 16:45:49 -0500 Subject: [PATCH 2/2] test: updated existing dupe exp keys test --- .../OptimizelyConfigServiceTest.php | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) 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()