[FSSDK-9783] Return Latest Experiment When Duplicate Keys in Config (#370)

* feat: log duplicate experiment keys

Also refactored private fields to readonly & standard names

* test: updated existing dupe exp keys test

Also refactored private fields to readonly & names

* fix: remove `readonly` from fields that aren't mine

Author: mikechu-optimizely

OptimizelySDK.Tests/OptimizelyConfigTests/OptimizelyConfigTest.cs

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022, Optimizely
+ * Copyright 2020-2023, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,28 +29,30 @@ namespace OptimizelySDK.Tests.OptimizelyConfigTests
     [TestFixture]
     public class OptimizelyConfigTest
     {
-        private Mock<ILogger> LoggerMock;
+        private Mock<ILogger> _loggerMock;
 
         [SetUp]
         public void Setup()
         {
-            LoggerMock = new Mock<ILogger>();
-            LoggerMock.Setup(l => l.Log(It.IsAny<LogLevel>(), It.IsAny<string>()));
+            _loggerMock = new Mock<ILogger>();
+            _loggerMock.Setup(l => l.Log(It.IsAny<LogLevel>(), It.IsAny<string>()));
         }
 
         #region Test OptimizelyConfigService
 
-        private static Type[] ParameterTypes =
+        private static readonly Type[] parameterTypes =
         {
             typeof(ProjectConfig),
+            typeof(ILogger),
         };
 
         private PrivateObject CreatePrivateOptimizelyConfigService(ProjectConfig projectConfig)
         {
-            return new PrivateObject(typeof(OptimizelyConfigService), ParameterTypes,
+            return new PrivateObject(typeof(OptimizelyConfigService), parameterTypes,
                 new object[]
                 {
                     projectConfig,
+                    new NoOpLogger(),
                 });
         }
 
@@ -144,22 +146,26 @@ public void TestGetOptimizelyConfigServiceNullConfig()
         }
 
         [Test]
-        [Obsolete]
         public void TestGetOptimizelyConfigWithDuplicateExperimentKeys()
         {
+            // OptimizelySDK.Tests/similar_exp_keys.json has DUPLICATED_EXPERIMENT_KEY
+            const string DUPLICATED_EXPERIMENT_KEY = "targeted_delivery";
+            const string SECOND_DUPLICATED_EXPERIMENT_ID = "9300000007573";
             var datafileProjectConfig = DatafileProjectConfig.Create(
-                TestData.DuplicateExpKeysDatafile, new NoOpLogger(),
+                TestData.DuplicateExpKeysDatafile,
+                new NoOpLogger(),
                 new ErrorHandler.NoOpErrorHandler());
-            var optimizelyConfigService = new OptimizelyConfigService(datafileProjectConfig);
+            var optimizelyConfigService =
+                new OptimizelyConfigService(datafileProjectConfig, _loggerMock.Object);
+
             var optimizelyConfig = optimizelyConfigService.GetOptimizelyConfig();
-            Assert.AreEqual(optimizelyConfig.ExperimentsMap.Count, 1);
 
-            var experimentMapFlag1 =
-                optimizelyConfig.FeaturesMap["flag1"].ExperimentsMap; //9300000007569
-            var experimentMapFlag2 =
-                optimizelyConfig.FeaturesMap["flag2"].ExperimentsMap; // 9300000007573
-            Assert.AreEqual(experimentMapFlag1["targeted_delivery"].Id, "9300000007569");
-            Assert.AreEqual(experimentMapFlag2["targeted_delivery"].Id, "9300000007573");
+            Assert.AreEqual(optimizelyConfig.ExperimentsMap.Count, 1);
+            Assert.AreEqual(optimizelyConfig.ExperimentsMap[DUPLICATED_EXPERIMENT_KEY].Id,
+                SECOND_DUPLICATED_EXPERIMENT_ID);
+            _loggerMock.Verify(
+                l => l.Log(LogLevel.WARN,
+                    "Duplicate experiment keys found in datafile: targeted_delivery"), Times.Once);
         }
 
         [Test]

OptimizelySDK/OptlyConfig/OptimizelyConfigService.cs

@@ -1,11 +1,11 @@
 /*
- * Copyright 2019-2021, Optimizely
+ * Copyright 2019-2021, 2023, Optimizely
  *
  * 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,
@@ -15,30 +15,34 @@
  */
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using OptimizelySDK.Entity;
+using OptimizelySDK.Logger;
 
 namespace OptimizelySDK.OptlyConfig
 {
     public class OptimizelyConfigService
     {
-        private OptimizelyConfig OptimizelyConfig;
+        private OptimizelyConfig _optimizelyConfig;
 
-        private IDictionary<string, List<FeatureVariable>> featureIdVariablesMap;
+        private IDictionary<string, List<FeatureVariable>> _featureIdVariablesMap;
 
-        public OptimizelyConfigService(ProjectConfig projectConfig)
+        private readonly ILogger _logger;
+
+        public OptimizelyConfigService(ProjectConfig projectConfig, ILogger logger = null)
         {
             if (projectConfig == null)
             {
                 return;
             }
 
-            featureIdVariablesMap = GetFeatureVariablesByIdMap(projectConfig);
+            _logger = logger ?? new DefaultLogger();
+
+            _featureIdVariablesMap = GetFeatureVariablesByIdMap(projectConfig);
             var attributes = GetAttributes(projectConfig);
             var audiences = GetAudiences(projectConfig);
             var experimentsMapById = GetExperimentsMapById(projectConfig);
@@ -47,7 +51,7 @@ public OptimizelyConfigService(ProjectConfig projectConfig)
             var featureMap = GetFeaturesMap(projectConfig, experimentsMapById);
             var events = GetEvents(projectConfig);
 
-            OptimizelyConfig = new OptimizelyConfig(projectConfig.Revision,
+            _optimizelyConfig = new OptimizelyConfig(projectConfig.Revision,
                 projectConfig.SDKKey,
                 projectConfig.EnvironmentKey,
                 attributes,
@@ -118,6 +122,12 @@ IDictionary<string, OptimizelyExperiment> experimentsMapById
 
             foreach (var experiment in experimentsMapById.Values)
             {
+                if (experimentKeyMaps.ContainsKey(experiment.Key))
+                {
+                    _logger.Log(LogLevel.WARN,
+                        $"Duplicate experiment keys found in datafile: {experiment.Key}");
+                }
+
                 experimentKeyMaps[experiment.Key] = experiment;
             }
 
@@ -210,7 +220,7 @@ bool isFeatureEnabled
 
             if (!string.IsNullOrEmpty(featureId))
             {
-                variablesMap = featureIdVariablesMap[featureId]?.
+                variablesMap = _featureIdVariablesMap[featureId]?.
                     Select(f => new OptimizelyVariable(f.Id,
                         f.Key,
                         f.Type.ToString().ToLower(),
@@ -451,7 +461,7 @@ private IDictionary<string, FeatureVariable> GetVariableIdMap(ProjectConfig proj
 
         public OptimizelyConfig GetOptimizelyConfig()
         {
-            return OptimizelyConfig;
+            return _optimizelyConfig;
         }
     }
 }