[FSSDK-9784] Return Latest Experiment When Duplicate Keys in Config (#…

…280) * feat: log duplicate experiment keys includes some linting * test: updated existing dupe exp keys test
optimizely · Dec 4, 2023 · 4963820 · 4963820
1 parent 3fd73a2
commit 4963820
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 25 deletions.
diff --git a/src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php b/src/Optimizely/OptimizelyConfig/OptimizelyConfigService.php
@@ -1,12 +1,12 @@
 <?php
 /**
- * Copyright 2020-2021, Optimizely Inc and Contributors
+ * Copyright 2020-2021, 2023 Optimizely Inc and Contributors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ * https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -16,9 +16,12 @@
  */
 namespace Optimizely\OptimizelyConfig;
 
+use Monolog\Logger;
 use Optimizely\Config\ProjectConfigInterface;
 use Optimizely\Entity\Experiment;
 use Optimizely\Entity\Variation;
+use Optimizely\Logger\DefaultLogger;
+use Optimizely\Logger\LoggerInterface;
 
 class OptimizelyConfigService
 {
@@ -73,7 +76,14 @@ class OptimizelyConfigService
      */
     private $featKeyOptlyVariableIdVariableMap;
 
-    public function __construct(ProjectConfigInterface $projectConfig)
+    /**
+     * Provided or default logger for logging.
+     *
+     * @var LoggerInterface $logger
+     */
+    private readonly LoggerInterface $logger;
+
+    public function __construct(ProjectConfigInterface $projectConfig, LoggerInterface $logger = null)
     {
         $this->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
@@ -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()