[FSSDK-9780] Return Latest Experiment When Duplicate Keys in Config (#430)

* firt run to add guard againsts duplicate key

* cleanup

* fix logger

* cleanup comments

* linting

* fix logger

Author: Mat001

optimizely/config_manager.py

@@ -159,7 +159,7 @@ def _set_config(self, datafile: Optional[str | bytes]) -> None:
 
         self._config = config
         self._sdk_key = self._sdk_key or config.sdk_key
-        self.optimizely_config = OptimizelyConfigService(config).get_config()
+        self.optimizely_config = OptimizelyConfigService(config, self.logger).get_config()
         self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE)
 
         internal_notification_center = _NotificationCenterRegistry.get_notification_center(

optimizely/optimizely.py

@@ -1039,7 +1039,7 @@ def get_optimizely_config(self) -> Optional[OptimizelyConfig]:
         if hasattr(self.config_manager, 'optimizely_config'):
             return self.config_manager.optimizely_config
 
-        return OptimizelyConfigService(project_config).get_config()
+        return OptimizelyConfigService(project_config, self.logger).get_config()
 
     def create_user_context(
         self, user_id: str, attributes: Optional[UserAttributes] = None

optimizely/optimizely_config.py

@@ -19,6 +19,8 @@
 from .helpers.types import VariationDict, ExperimentDict, RolloutDict, AttributeDict, EventDict
 from .project_config import ProjectConfig
 
+from .logger import Logger
+
 
 class OptimizelyConfig:
     def __init__(
@@ -126,11 +128,12 @@ def __init__(self, id: Optional[str], name: Optional[str], conditions: Optional[
 class OptimizelyConfigService:
     """ Class encapsulating methods to be used in creating instance of OptimizelyConfig. """
 
-    def __init__(self, project_config: ProjectConfig):
+    def __init__(self, project_config: ProjectConfig, logger: Logger):
         """
         Args:
             project_config ProjectConfig
         """
+        self.logger = logger
         self.is_valid = True
 
         if not isinstance(project_config, ProjectConfig):
@@ -411,7 +414,12 @@ def _get_experiments_maps(self) -> tuple[dict[str, OptimizelyExperiment], dict[s
                 audiences_map[audience_id] = audience_name if audience_name is not None else ''
 
         all_experiments = self._get_all_experiments()
+
         for exp in all_experiments:
+            # check if experiment key already exists
+            if exp["key"] in experiments_key_map:
+                self.logger.warning(f"Duplicate experiment keys found in datafile: {exp['key']}")
+
             optly_exp = OptimizelyExperiment(
                 exp['id'], exp['key'], self._get_variations_map(exp)
             )

tests/test_optimizely_config.py

@@ -4,17 +4,18 @@
 # You may obtain a copy of the License at
 #
 # http://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,
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
 import json
+from unittest.mock import patch
 
 from optimizely import optimizely, project_config
 from optimizely import optimizely_config
+from optimizely import logger
 from . import base
 
 
@@ -23,7 +24,8 @@ def setUp(self):
         base.BaseTest.setUp(self)
         opt_instance = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
         self.project_config = opt_instance.config_manager.get_config()
-        self.opt_config_service = optimizely_config.OptimizelyConfigService(self.project_config)
+        self.opt_config_service = optimizely_config.OptimizelyConfigService(self.project_config,
+                                                                            logger=logger.SimpleLogger())
 
         self.expected_config = {
             'sdk_key': 'features-test',
@@ -1452,7 +1454,7 @@ def test__get_config(self):
     def test__get_config__invalid_project_config(self):
         """ Test that get_config returns None when invalid project config supplied. """
 
-        opt_service = optimizely_config.OptimizelyConfigService({"key": "invalid"})
+        opt_service = optimizely_config.OptimizelyConfigService({"key": "invalid"}, None)
         self.assertIsNone(opt_service.get_config())
 
     def test__get_experiments_maps(self):
@@ -1473,6 +1475,81 @@ def test__get_experiments_maps(self):
 
         self.assertEqual(expected_id_map, self.to_dict(actual_id_map))
 
+    def test__duplicate_experiment_keys(self):
+        """ Test that multiple features don't have the same experiment key. """
+
+        # update the test datafile with an additional feature flag with the same experiment rule key
+        new_experiment = {
+            'key': 'test_experiment',    # added duplicate "test_experiment"
+            'status': 'Running',
+            'layerId': '8',
+            "audienceConditions": [
+                "or",
+                "11160"
+            ],
+            'audienceIds': ['11160'],
+            'id': '111137',
+            'forcedVariations': {},
+            'trafficAllocation': [
+                {'entityId': '222242', 'endOfRange': 8000},
+                {'entityId': '', 'endOfRange': 10000}
+            ],
+            'variations': [
+                {
+                    'id': '222242',
+                    'key': 'control',
+                    'variables': [],
+                }
+            ],
+        }
+
+        new_feature = {
+            'id': '91117',
+            'key': 'new_feature',
+            'experimentIds': ['111137'],
+            'rolloutId': '',
+            'variables': [
+                {'id': '127', 'key': 'is_working', 'defaultValue': 'true', 'type': 'boolean'},
+                {'id': '128', 'key': 'environment', 'defaultValue': 'devel', 'type': 'string'},
+                {'id': '129', 'key': 'cost', 'defaultValue': '10.99', 'type': 'double'},
+                {'id': '130', 'key': 'count', 'defaultValue': '999', 'type': 'integer'},
+                {'id': '131', 'key': 'variable_without_usage', 'defaultValue': '45', 'type': 'integer'},
+                {'id': '132', 'key': 'object', 'defaultValue': '{"test": 12}', 'type': 'string',
+                 'subType': 'json'},
+                {'id': '133', 'key': 'true_object', 'defaultValue': '{"true_test": 23.54}', 'type': 'json'},
+            ],
+        }
+
+        # add new experiment rule with the same key and a new feature with the same rule key
+        self.config_dict_with_features['experiments'].append(new_experiment)
+        self.config_dict_with_features['featureFlags'].append(new_feature)
+
+        config_with_duplicate_key = self.config_dict_with_features
+        opt_instance = optimizely.Optimizely(json.dumps(config_with_duplicate_key))
+        self.project_config = opt_instance.config_manager.get_config()
+
+        with patch('optimizely.logger.SimpleLogger.warning') as mock_logger:
+            self.opt_config_service = optimizely_config.OptimizelyConfigService(self.project_config,
+                                                                                logger=logger.SimpleLogger())
+
+            actual_key_map, actual_id_map = self.opt_config_service._get_experiments_maps()
+
+            self.assertIsInstance(actual_key_map, dict)
+            for exp in actual_key_map.values():
+                self.assertIsInstance(exp, optimizely_config.OptimizelyExperiment)
+
+            # Assert that the warning method of the mock logger was called with the expected message
+            expected_warning_message = f"Duplicate experiment keys found in datafile: {new_experiment['key']}"
+            mock_logger.assert_called_with(expected_warning_message)
+
+        # assert we get ID of the duplicated experiment
+        assert actual_key_map.get('test_experiment').id == "111137"
+
+        # assert we get one duplicated experiment
+        keys_list = list(actual_key_map.keys())
+        assert "test_experiment" in keys_list, "Key 'test_experiment' not found in actual key map"
+        assert keys_list.count("test_experiment") == 1, "Key 'test_experiment' found more than once in actual key map"
+
     def test__get_features_map(self):
         """ Test that get_features_map returns expected features map. """
 
@@ -1674,7 +1751,7 @@ def test_get_audiences(self):
             error_handler=None
         )
 
-        config_service = optimizely_config.OptimizelyConfigService(proj_conf)
+        config_service = optimizely_config.OptimizelyConfigService(proj_conf, logger=logger.SimpleLogger())
 
         for audience in config_service.audiences:
             self.assertIsInstance(audience, optimizely_config.OptimizelyAudience)
@@ -1742,7 +1819,7 @@ def test_stringify_audience_conditions_all_cases(self):
             '("us" OR ("female" AND "adult")) AND ("fr" AND ("male" OR "adult"))'
         ]
 
-        config_service = optimizely_config.OptimizelyConfigService(config)
+        config_service = optimizely_config.OptimizelyConfigService(config, None)
 
         for i in range(len(audiences_input)):
             result = config_service.stringify_conditions(audiences_input[i], audiences_map)
@@ -1760,7 +1837,7 @@ def test_optimizely_audience_conversion(self):
             error_handler=None
         )
 
-        config_service = optimizely_config.OptimizelyConfigService(proj_conf)
+        config_service = optimizely_config.OptimizelyConfigService(proj_conf, None)
 
         for audience in config_service.audiences:
             self.assertIsInstance(audience, optimizely_config.OptimizelyAudience)
@@ -1776,7 +1853,7 @@ def test_get_variations_from_experiments_map(self):
             error_handler=None
         )
 
-        config_service = optimizely_config.OptimizelyConfigService(proj_conf)
+        config_service = optimizely_config.OptimizelyConfigService(proj_conf, None)
 
         experiments_key_map, experiments_id_map = config_service._get_experiments_maps()