Add comments and unit tests

optimizely · jaeopt · Jan 6, 2020 · Dec 5, 2019 · Dec 6, 2019 · Dec 6, 2019
commit 6f2a79fad5fabae24b5d513465f044f3fc0eeea9
diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py
@@ -25,7 +25,7 @@
 from .event_dispatcher import EventDispatcher as default_event_dispatcher
 from .helpers import enums, validator
 from .notification_center import NotificationCenter
-from .optimizely_config import OptimizelyConfigBuilder
+from .optimizely_config import OptimizelyConfigService
 
 
 class Optimizely(object):
@@ -736,6 +736,12 @@ def get_forced_variation(self, experiment_key, user_id):
         return forced_variation.key if forced_variation else None
 
     def get_optimizely_config(self):
+        """ Gets OptimizelyConfig instance for the current project config.
+
+        Returns:
+            OptimizelyConfig instance. None if the optimizely instance is invalid or
+            project config isn't available.
+        """
         if not self.is_valid:
             self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('get_optimizely_config'))
             return None
@@ -745,4 +751,4 @@ def get_optimizely_config(self):
             self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_optimizely_config'))
             return None
 
-        return OptimizelyConfigBuilder(project_config).build()
+        return OptimizelyConfigService(project_config).get_optimizely_config()
diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py
@@ -52,15 +52,25 @@ def __init__(self, id, key, type, value):
         self.value = value
 
 
-class OptimizelyConfigBuilder(object):
+class OptimizelyConfigService(object):
+    """ Class encapsulating methods to be used in creating instance of OptimizelyConfig. """
 
     def __init__(self, project_config):
+        """
+        Arguments:
+            project_config ProjectConfig
+        """
         self.experiments = project_config.experiments
         self.feature_flags = project_config.feature_flags
         self.groups = project_config.groups
         self.revision = project_config.revision
 
-    def build(self):
+    def get_optimizely_config(self):
+        """ Returns instance of OptimizelyConfig
+
+        Returns:
+            Optimizely Config instance.
+        """
         self._create_lookup_maps()
 
         experiments_key_map, experiments_id_map = self._get_experiments_maps()
@@ -69,6 +79,8 @@ def build(self):
         return OptimizelyConfig(self.revision, experiments_key_map, features_map)
 
     def _create_lookup_maps(self):
+        """ Creates lookup maps to avoid redundant iteration of config objects.  """
+
         self.exp_id_to_feature_map = {}
         for feature in self.feature_flags:
             for id in feature['experimentIds']:
@@ -90,6 +102,15 @@ def _create_lookup_maps(self):
             self.feature_key_variable_id_to_variable_map[feature['key']] = variables_id_map
 
     def _get_variables_map(self, variation, experiment):
+        """ Gets variables map for given variation and experiment.
+
+        Arguments:
+            variation dict
+            experiment dict
+
+        Returns:
+            dict - Map of variable key to OptimizelyVariable for the given variation.
+        """
         feature_flag = self.exp_id_to_feature_map.get(experiment['id'], None)
         if feature_flag is None:
             return {}
@@ -99,14 +120,22 @@ def _get_variables_map(self, variation, experiment):
         variables_map = copy.deepcopy(self.feature_key_variable_key_to_variable_map[feature_flag['key']])
 
         # set variation specific variable value if any
-        if variation.get('featureEnabled', None):
+        if variation.get('featureEnabled'):
             for variable in variation.get('variables', []):
                 feature_variable = self.feature_key_variable_id_to_variable_map[feature_flag['key']][variable['id']]
                 variables_map[feature_variable.key].value = variable['value']
 
         return variables_map
 
     def _get_variations_map(self, experiment):
+        """ Gets variation map for the given experiment.
+
+        Arguments:
+            experiment dict
+
+        Returns:
+            dict -- Map of variation key to OptimizelyVariation.
+        """
         variations_map = {}
 
         for variation in experiment.get('variations', []):
@@ -122,18 +151,30 @@ def _get_variations_map(self, experiment):
         return variations_map
 
     def _get_all_experiments(self):
+        """ Gets all experiments in the project config.
+
+        Returns:
+            list -- List of dicts of experiments.
+        """
         experiments = self.experiments
 
         for group in self.groups:
-            experiments = experiments + group.experiments
+            experiments = experiments + group['experiments']
 
         return experiments
 
     def _get_experiments_maps(self):
+        """ Gets maps for all the experiments in the project config.
+
+        Returns:
+            dict, dict -- experiment key/id to OptimizelyExperiment maps.
+        """
+        # Key map is required for the OptimizelyConfig response.
         experiments_key_map = {}
+        # Id map comes in handy to figure out feature experiment.
         experiments_id_map = {}
-        all_experiments = self._get_all_experiments()
 
+        all_experiments = self._get_all_experiments()
         for exp in all_experiments:
             optly_exp = OptimizelyExperiment(
                 exp['id'], exp['key'], self._get_variations_map(exp)
@@ -145,6 +186,14 @@ def _get_experiments_maps(self):
         return experiments_key_map, experiments_id_map
 
     def _get_features_map(self, experiments_id_map):
+        """ Gets features map for the project config.
+
+        Arguments:
+            experiments_id_map dict -- experiment id to OptimizelyExperiment map
+
+        Returns:
+            dict -- feaure key to OptimizelyFeature map
+        """
         features_map = {}
 
         for feature in self.feature_flags:

diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py
@@ -23,6 +23,7 @@
 from optimizely import exceptions
 from optimizely import logger
 from optimizely import optimizely
+from optimizely import optimizely_config
 from optimizely import project_config
 from optimizely import version
 from optimizely.event.event_factory import EventFactory
@@ -3911,6 +3912,39 @@ def test_get_feature_variable_returns__default_value__complex_audience_match(sel
         self.assertEqual(10, opt_obj.get_feature_variable_integer('feat2_with_var', 'z', 'user1', {}))
         self.assertEqual(10, opt_obj.get_feature_variable('feat2_with_var', 'z', 'user1', {}))
 
+    def test_get_optimizely_config__invalid_object(self):
+        """ Test that get_optimizely_config logs error if Optimizely instance is invalid. """
+
+        class InvalidConfigManager(object):
+            pass
+
+        opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager())
+
+        with mock.patch.object(opt_obj, 'logger') as mock_client_logging:
+            self.assertIsNone(opt_obj.get_optimizely_config())
+
+        mock_client_logging.error.assert_called_once_with(
+            'Optimizely instance is not valid. Failing "get_optimizely_config".')
+
+    def test_get_optimizely_config__invalid_config(self):
+        """ Test that get_optimizely_config logs error if config is invalid. """
+
+        opt_obj = optimizely.Optimizely('invalid_datafile')
+
+        with mock.patch.object(opt_obj, 'logger') as mock_client_logging:
+            self.assertIsNone(opt_obj.get_optimizely_config())
+
+        mock_client_logging.error.assert_called_once_with(
+            'Invalid config. Optimizely instance is not valid. ' 'Failing "get_optimizely_config".'
+        )
+
+    def test_get_optimizely_config_returns_instance_of_optimizely_config(self):
+        """ Test that get_optimizely_config returns an instance of OptimizelyConfig. """
+
+        opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+        opt_config = opt_obj.get_optimizely_config()
+        self.assertIsInstance(opt_config, optimizely_config.OptimizelyConfig)
+
 
 class OptimizelyWithExceptionTest(base.BaseTest):
     def setUp(self):