[FSSDK-9778] return last experiment when duplicate key in config

lib/core/optimizely_config/index.tests.js

@@ -15,14 +15,16 @@
  */
 import { assert } from 'chai';
 import { cloneDeep } from 'lodash';
+import sinon from 'sinon';
 
 import { createOptimizelyConfig, OptimizelyConfig } from './';
 import { createProjectConfig } from '../project_config';
 import {
   getTestProjectConfigWithFeatures,
   getTypedAudiencesConfig,
   getSimilarRuleKeyConfig,
-  getSimilarExperimentKeyConfig
+  getSimilarExperimentKeyConfig,
+  getDuplicateExperimentKeyConfig,
 } from '../../tests/test_data';
 
 var datafile = getTestProjectConfigWithFeatures();
@@ -53,6 +55,7 @@ describe('lib/core/optimizely_config', function() {
     var projectSimilarRuleKeyConfigObject;
     var optimizelySimilarExperimentkeyConfigObject;
     var projectSimilarExperimentKeyConfigObject;
+
     beforeEach(function() {
       projectConfigObject = createProjectConfig(cloneDeep(datafile));
       optimizelyConfigObject = createOptimizelyConfig(projectConfigObject, JSON.stringify(datafile));
@@ -85,6 +88,24 @@ describe('lib/core/optimizely_config', function() {
       });
     });
 
+    it('should keep the last experiment in case of duplicate key and log a warning', function() {
+      const datafile = getDuplicateExperimentKeyConfig();
+      const configObj = createProjectConfig(datafile, JSON.stringify(datafile));
+
+      const logger = {
+        warn: sinon.spy(),
+      }
+
+      const optimizelyConfig = createOptimizelyConfig(configObj, JSON.stringify(datafile), logger);
+      const experimentsMap = optimizelyConfig.experimentsMap;
+
+      const duplicateKey = 'experiment_rule';
+      const lastExperiment = datafile.experiments[datafile.experiments.length - 1];
+
+      assert.equal(experimentsMap['experiment_rule'].id, lastExperiment.id);
+      assert.isTrue(logger.warn.calledWithExactly(`Duplicate experiment keys found in datafile: ${duplicateKey}`));
+    });
+
     it('should return all the feature flags', function() {
       var featureFlagsCount = Object.keys(optimizelyConfigObject.featuresMap).length;
       assert.equal(featureFlagsCount, 9);

lib/core/optimizely_config/index.ts

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import { LoggerFacade, getLogger } from '../../modules/logging';
 import { ProjectConfig } from '../project_config';
 import { DEFAULT_OPERATOR_TYPES } from '../condition_tree_evaluator';
 import {
@@ -61,7 +62,8 @@ export class OptimizelyConfig {
   public events: OptimizelyEvent[];
   private datafile: string;
 
-  constructor(configObj: ProjectConfig, datafile: string) {
+
+  constructor(configObj: ProjectConfig, datafile: string, logger?: LoggerFacade) {
     this.sdkKey = configObj.sdkKey ?? '';
     this.environmentKey = configObj.environmentKey ?? '';
     this.attributes = configObj.attributes;
@@ -76,10 +78,12 @@ export class OptimizelyConfig {
 
     const variableIdMap = OptimizelyConfig.getVariableIdMap(configObj);
 
-    const experimentsMapById = OptimizelyConfig.getExperimentsMapById(
-      configObj, featureIdVariablesMap, variableIdMap
+    const { experimentsMapById, experimentsMapByKey } = OptimizelyConfig.getExperimentsMap(
+      configObj, featureIdVariablesMap, variableIdMap, logger,
     );
-    this.experimentsMap = OptimizelyConfig.getExperimentsKeyMap(experimentsMapById);
+
+    this.experimentsMap = experimentsMapByKey;
+
     this.featuresMap = OptimizelyConfig.getFeaturesMap(
       configObj, featureIdVariablesMap, experimentsMapById, variableIdMap
     );
@@ -347,39 +351,52 @@ export class OptimizelyConfig {
    * @param       {ProjectConfig}                           configObj
    * @param       {FeatureVariablesMap}                     featureIdVariableMap
    * @param       {{[id: string]: FeatureVariable}}         variableIdMap
-   * @returns     {[id: string]: OptimizelyExperiment}      Experiments mapped by id
+   * @returns     { experimentsMapById: { [id: string]: OptimizelyExperiment }, experimentsMapByKey: OptimizelyExperimentsMap }      Experiments mapped by id and key
    */
-  static getExperimentsMapById(
+  static getExperimentsMap(
     configObj: ProjectConfig,
     featureIdVariableMap: FeatureVariablesMap,
-    variableIdMap: {[id: string]: FeatureVariable}
-  ): { [id: string]: OptimizelyExperiment } {
+    variableIdMap: {[id: string]: FeatureVariable},
+    logger?: LoggerFacade,
+  ) : { experimentsMapById: { [id: string]: OptimizelyExperiment }, experimentsMapByKey: OptimizelyExperimentsMap } {
     const rolloutExperimentIds = this.getRolloutExperimentIds(configObj.rollouts);
 
-    const experiments = configObj.experiments;
+    const experimentsMapById: { [id : string]: OptimizelyExperiment } = {};
+    const experimentsMapByKey: OptimizelyExperimentsMap = {};
 
-    return (experiments || []).reduce((experimentsMap: { [id: string]: OptimizelyExperiment }, experiment) => {
-      if (rolloutExperimentIds.indexOf(experiment.id) === -1) {
-        const featureIds = configObj.experimentFeatureMap[experiment.id];
-        let featureId = '';
-        if (featureIds && featureIds.length > 0) {
-          featureId = featureIds[0];
-        }
-        const variationsMap = OptimizelyConfig.getVariationsMap(
-          experiment.variations,
-          featureIdVariableMap,
-          variableIdMap,
-          featureId.toString()
-        );
-        experimentsMap[experiment.id] = {
-          id: experiment.id,
-          key: experiment.key,
-          audiences: OptimizelyConfig.getExperimentAudiences(experiment, configObj),
-          variationsMap: variationsMap,
-        };
+    const experiments = configObj.experiments || [];
+    experiments.forEach((experiment) => {
+      if (rolloutExperimentIds.indexOf(experiment.id) !== -1) {
+        return;
       }
-      return experimentsMap;
-    }, {});
+
+      const featureIds = configObj.experimentFeatureMap[experiment.id];
+      let featureId = '';
+      if (featureIds && featureIds.length > 0) {
+        featureId = featureIds[0];
+      }
+      const variationsMap = OptimizelyConfig.getVariationsMap(
+        experiment.variations,
+        featureIdVariableMap,
+        variableIdMap,
+        featureId.toString()
+      );
+
+      const optimizelyExperiment: OptimizelyExperiment = {
+        id: experiment.id,
+        key: experiment.key,
+        audiences: OptimizelyConfig.getExperimentAudiences(experiment, configObj),
+        variationsMap: variationsMap,
+      };
+
+      experimentsMapById[experiment.id] = optimizelyExperiment;
+      if (experimentsMapByKey[experiment.key] && logger) {
+        logger.warn(`Duplicate experiment keys found in datafile: ${experiment.key}`);
+      }
+      experimentsMapByKey[experiment.key] = optimizelyExperiment;
+    });
+
+    return { experimentsMapById, experimentsMapByKey };
   }
 
   /**
@@ -461,6 +478,6 @@ export class OptimizelyConfig {
  * @param   {string}                    datafile
  * @returns {OptimizelyConfig}          An instance of OptimizelyConfig
  */
-export function createOptimizelyConfig(configObj: ProjectConfig, datafile: string): OptimizelyConfig {
-  return new OptimizelyConfig(configObj, datafile);
+export function createOptimizelyConfig(configObj: ProjectConfig, datafile: string, logger?: LoggerFacade): OptimizelyConfig {
+  return new OptimizelyConfig(configObj, datafile, logger);
 }

lib/core/project_config/project_config_manager.ts

@@ -210,7 +210,7 @@ export class ProjectConfigManager {
    */
   getOptimizelyConfig(): OptimizelyConfig | null {
     if (!this.optimizelyConfigObj && this.configObj) {
-      this.optimizelyConfigObj = createOptimizelyConfig(this.configObj, toDatafile(this.configObj));
+      this.optimizelyConfigObj = createOptimizelyConfig(this.configObj, toDatafile(this.configObj), logger);
     }
     return this.optimizelyConfigObj;
   }

lib/tests/test_data.js

@@ -3951,6 +3951,208 @@ export var getSimilarExperimentKeyConfig = function() {
   return cloneDeep(similarExperimentKeysConfig);
 };
 
+const duplicateExperimentKeyConfig = {
+  "accountId": "23793010390",
+  "projectId": "24812320344",
+  "revision": "24",
+  "attributes": [
+    {
+      "id": "24778491463",
+      "key": "country"
+    },
+    {
+      "id": "24802951640",
+      "key": "likes_donuts"
+    }
+  ],
+  "audiences": [
+    {
+      "name": "mutext_feat",
+      "conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]",
+      "id": "24837020039"
+    },
+    {
+      "id": "$opt_dummy_audience",
+      "name": "Optimizely-Generated Audience for Backwards Compatibility",
+      "conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]"
+    }
+  ],
+  "version": "4",
+  "events": [],
+  "integrations": [],
+  "anonymizeIP": true,
+  "botFiltering": false,
+  "typedAudiences": [
+    {
+      "name": "mutext_feat",
+      "conditions": [
+        "and",
+        [
+          "or",
+          [
+            "or",
+            {
+              "match": "exact",
+              "name": "country",
+              "type": "custom_attribute",
+              "value": "US"
+            }
+          ],
+          [
+            "or",
+            {
+              "match": "exact",
+              "name": "likes_donuts",
+              "type": "custom_attribute",
+              "value": true
+            }
+          ]
+        ]
+      ],
+      "id": "24837020039"
+    }
+  ],
+  "variables": [],
+  "environmentKey": "production",
+  "sdkKey": "BBhivmjEBF1KLK8HkMrvj",
+  "featureFlags": [
+    {
+      "id": "101043",
+      "key": "mutext_feat2",
+      "rolloutId": "rollout-101043-24783691394",
+      "experimentIds": [
+        "9300000361925"
+      ],
+      "variables": []
+    },
+    {
+      "id": "101044",
+      "key": "mutex_feat",
+      "rolloutId": "rollout-101044-24783691394",
+      "experimentIds": [
+        "9300000365056"
+      ],
+      "variables": []
+    }
+  ],
+  "rollouts": [
+    {
+      "id": "rollout-101043-24783691394",
+      "experiments": [
+        {
+          "id": "default-rollout-101043-24783691394",
+          "key": "default-rollout-101043-24783691394",
+          "status": "Running",
+          "layerId": "rollout-101043-24783691394",
+          "variations": [
+            {
+              "id": "321340",
+              "key": "off",
+              "featureEnabled": false,
+              "variables": []
+            }
+          ],
+          "trafficAllocation": [
+            {
+              "entityId": "321340",
+              "endOfRange": 10000
+            }
+          ],
+          "forcedVariations": {},
+          "audienceIds": [],
+          "audienceConditions": []
+        }
+      ]
+    },
+    {
+      "id": "rollout-101044-24783691394",
+      "experiments": [
+        {
+          "id": "default-rollout-101044-24783691394",
+          "key": "default-rollout-101044-24783691394",
+          "status": "Running",
+          "layerId": "rollout-101044-24783691394",
+          "variations": [
+            {
+              "id": "321343",
+              "key": "off",
+              "featureEnabled": false,
+              "variables": []
+            }
+          ],
+          "trafficAllocation": [
+            {
+              "entityId": "321343",
+              "endOfRange": 10000
+            }
+          ],
+          "forcedVariations": {},
+          "audienceIds": [],
+          "audienceConditions": []
+        }
+      ]
+    }
+  ],
+  "experiments": [
+    {
+      "id": "9300000361925",
+      "key": "experiment_rule",
+      "status": "Running",
+      "layerId": "9300000284731",
+      "variations": [
+        {
+          "id": "321342",
+          "key": "variation_1",
+          "featureEnabled": true,
+          "variables": []
+        }
+      ],
+      "trafficAllocation": [
+        {
+          "entityId": "321342",
+          "endOfRange": 10000
+        }
+      ],
+      "forcedVariations": {},
+      "audienceIds": [
+        "24837020039"
+      ],
+      "audienceConditions": [
+        "or",
+        "24837020039"
+      ]
+    },
+    {
+      "id": "9300000365056",
+      "key": "experiment_rule",
+      "status": "Running",
+      "layerId": "9300000287826",
+      "variations": [
+        {
+          "id": "321345",
+          "key": "variation_1",
+          "featureEnabled": true,
+          "variables": []
+        }
+      ],
+      "trafficAllocation": [
+        {
+          "entityId": "321345",
+          "endOfRange": 10000
+        }
+      ],
+      "forcedVariations": {},
+      "audienceIds": [],
+      "audienceConditions": []
+    }
+  ],
+  "groups": []
+};
+
+export const getDuplicateExperimentKeyConfig = function() {
+  return cloneDeep(duplicateExperimentKeyConfig);
+};
+
 export default {
   getTestProjectConfig: getTestProjectConfig,
   getTestDecideProjectConfig: getTestDecideProjectConfig,
@@ -3966,4 +4168,5 @@ export default {
   getMutexFeatureTestsConfig: getMutexFeatureTestsConfig,
   getSimilarRuleKeyConfig: getSimilarRuleKeyConfig,
   getSimilarExperimentKeyConfig: getSimilarExperimentKeyConfig,
+  getDuplicateExperimentKeyConfig: getDuplicateExperimentKeyConfig,
 };