Refactoring and passing ProjectConfig from Optimizely itself. (#176)

Author: aliabbasrizvi

optimizely/bucketer.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2017, Optimizely
+# Copyright 2016-2017, 2019 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
@@ -29,15 +29,10 @@
 class Bucketer(object):
   """ Optimizely bucketing algorithm that evenly distributes visitors. """
 
-  def __init__(self, project_config):
-    """ Bucketer init method to set bucketing seed and project config data.
-
-    Args:
-      project_config: Project config data to be used in making bucketing decisions.
-    """
+  def __init__(self):
+    """ Bucketer init method to set bucketing seed and logger instance. """
 
     self.bucket_seed = HASH_SEED
-    self.config = project_config
 
   def _generate_unsigned_hash_code_32_bit(self, bucketing_id):
     """ Helper method to retrieve hash code.
@@ -65,10 +60,11 @@ def _generate_bucket_value(self, bucketing_id):
     ratio = float(self._generate_unsigned_hash_code_32_bit(bucketing_id)) / MAX_HASH_VALUE
     return math.floor(ratio * MAX_TRAFFIC_VALUE)
 
-  def find_bucket(self, bucketing_id, parent_id, traffic_allocations):
+  def find_bucket(self, project_config, bucketing_id, parent_id, traffic_allocations):
     """ Determine entity based on bucket value and traffic allocations.
 
     Args:
+      project_config: Instance of ProjectConfig.
       bucketing_id: ID to be used for bucketing the user.
       parent_id: ID representing group or experiment.
       traffic_allocations: Traffic allocations representing traffic allotted to experiments or variations.
@@ -79,7 +75,7 @@ def find_bucket(self, bucketing_id, parent_id, traffic_allocations):
 
     bucketing_key = BUCKETING_ID_TEMPLATE.format(bucketing_id=bucketing_id, parent_id=parent_id)
     bucketing_number = self._generate_bucket_value(bucketing_key)
-    self.config.logger.debug('Assigned bucket %s to user with bucketing ID "%s".' % (
+    project_config.logger.debug('Assigned bucket %s to user with bucketing ID "%s".' % (
       bucketing_number,
       bucketing_id
     ))
@@ -91,10 +87,11 @@ def find_bucket(self, bucketing_id, parent_id, traffic_allocations):
 
     return None
 
-  def bucket(self, experiment, user_id, bucketing_id):
+  def bucket(self, project_config, experiment, user_id, bucketing_id):
     """ For a given experiment and bucketing ID determines variation to be shown to user.
 
     Args:
+      project_config: Instance of ProjectConfig.
       experiment: Object representing the experiment for which user is to be bucketed.
       user_id: ID for user.
       bucketing_id: ID to be used for bucketing the user.
@@ -108,40 +105,40 @@ def bucket(self, experiment, user_id, bucketing_id):
 
     # Determine if experiment is in a mutually exclusive group
     if experiment.groupPolicy in GROUP_POLICIES:
-      group = self.config.get_group(experiment.groupId)
+      group = project_config.get_group(experiment.groupId)
 
       if not group:
         return None
 
-      user_experiment_id = self.find_bucket(bucketing_id, experiment.groupId, group.trafficAllocation)
+      user_experiment_id = self.find_bucket(project_config, bucketing_id, experiment.groupId, group.trafficAllocation)
       if not user_experiment_id:
-        self.config.logger.info('User "%s" is in no experiment.' % user_id)
+        project_config.logger.info('User "%s" is in no experiment.' % user_id)
         return None
 
       if user_experiment_id != experiment.id:
-        self.config.logger.info('User "%s" is not in experiment "%s" of group %s.' % (
+        project_config.logger.info('User "%s" is not in experiment "%s" of group %s.' % (
           user_id,
           experiment.key,
           experiment.groupId
         ))
         return None
 
-      self.config.logger.info('User "%s" is in experiment %s of group %s.' % (
+      project_config.logger.info('User "%s" is in experiment %s of group %s.' % (
         user_id,
         experiment.key,
         experiment.groupId
       ))
 
     # Bucket user if not in white-list and in group (if any)
-    variation_id = self.find_bucket(bucketing_id, experiment.id, experiment.trafficAllocation)
+    variation_id = self.find_bucket(project_config, bucketing_id, experiment.id, experiment.trafficAllocation)
     if variation_id:
-      variation = self.config.get_variation_from_id(experiment.key, variation_id)
-      self.config.logger.info('User "%s" is in variation "%s" of experiment %s.' % (
+      variation = project_config.get_variation_from_id(experiment.key, variation_id)
+      project_config.logger.info('User "%s" is in variation "%s" of experiment %s.' % (
         user_id,
         variation.key,
         experiment.key
       ))
       return variation
 
-    self.config.logger.info('User "%s" is in no variation.' % user_id)
+    project_config.logger.info('User "%s" is in no variation.' % user_id)
     return None

optimizely/decision_service.py

@@ -27,11 +27,10 @@
 class DecisionService(object):
   """ Class encapsulating all decision related capabilities. """
 
-  def __init__(self, config, user_profile_service):
-    self.bucketer = bucketer.Bucketer(config)
+  def __init__(self, logger, user_profile_service):
+    self.bucketer = bucketer.Bucketer()
+    self.logger = logger
     self.user_profile_service = user_profile_service
-    self.config = config
-    self.logger = config.logger
 
   def _get_bucketing_id(self, user_id, attributes):
     """ Helper method to determine bucketing ID for the user.
@@ -55,10 +54,11 @@ def _get_bucketing_id(self, user_id, attributes):
 
     return user_id
 
-  def get_forced_variation(self, experiment, user_id):
+  def get_forced_variation(self, project_config, experiment, user_id):
     """ Determine if a user is forced into a variation for the given experiment and return that variation.
 
     Args:
+      project_config: Instance of ProjectConfig.
       experiment: Object representing the experiment for which user is to be bucketed.
       user_id: ID for the user.
 
@@ -69,17 +69,18 @@ def get_forced_variation(self, experiment, user_id):
     forced_variations = experiment.forcedVariations
     if forced_variations and user_id in forced_variations:
       variation_key = forced_variations.get(user_id)
-      variation = self.config.get_variation_from_key(experiment.key, variation_key)
+      variation = project_config.get_variation_from_key(experiment.key, variation_key)
       if variation:
         self.logger.info('User "%s" is forced in variation "%s".' % (user_id, variation_key))
       return variation
 
     return None
 
-  def get_stored_variation(self, experiment, user_profile):
+  def get_stored_variation(self, project_config, experiment, user_profile):
     """ Determine if the user has a stored variation available for the given experiment and return that.
 
     Args:
+      project_config: Instance of ProjectConfig.
       experiment: Object representing the experiment for which user is to be bucketed.
       user_profile: UserProfile object representing the user's profile.
 
@@ -91,7 +92,7 @@ def get_stored_variation(self, experiment, user_profile):
     variation_id = user_profile.get_variation_for_experiment(experiment.id)
 
     if variation_id:
-      variation = self.config.get_variation_from_id(experiment.key, variation_id)
+      variation = project_config.get_variation_from_id(experiment.key, variation_id)
       if variation:
         self.logger.info('Found a stored decision. User "%s" is in variation "%s" of experiment "%s".' % (
           user_id,
@@ -102,7 +103,7 @@ def get_stored_variation(self, experiment, user_profile):
 
     return None
 
-  def get_variation(self, experiment, user_id, attributes, ignore_user_profile=False):
+  def get_variation(self, project_config, experiment, user_id, attributes, ignore_user_profile=False):
     """ Top-level function to help determine variation user should be put in.
 
     First, check if experiment is running.
@@ -112,6 +113,7 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
     Fifth, bucket the user and return the variation.
 
     Args:
+      project_config: Instance of ProjectConfig.
       experiment: Experiment for which user variation needs to be determined.
       user_id: ID for user.
       attributes: Dict representing user attributes.
@@ -127,12 +129,12 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
       return None
 
     # Check if the user is forced into a variation
-    variation = self.config.get_forced_variation(experiment.key, user_id)
+    variation = project_config.get_forced_variation(experiment.key, user_id)
     if variation:
       return variation
 
     # Check to see if user is white-listed for a certain variation
-    variation = self.get_forced_variation(experiment, user_id)
+    variation = self.get_forced_variation(project_config, experiment, user_id)
     if variation:
       return variation
 
@@ -147,14 +149,14 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
 
       if validator.is_user_profile_valid(retrieved_profile):
         user_profile = UserProfile(**retrieved_profile)
-        variation = self.get_stored_variation(experiment, user_profile)
+        variation = self.get_stored_variation(project_config, experiment, user_profile)
         if variation:
           return variation
       else:
         self.logger.warning('User profile has invalid format.')
 
     # Bucket user and store the new decision
-    if not audience_helper.is_user_in_experiment(self.config, experiment, attributes, self.logger):
+    if not audience_helper.is_user_in_experiment(project_config, experiment, attributes, self.logger):
       self.logger.info('User "%s" does not meet conditions to be in experiment "%s".' % (
         user_id,
         experiment.key
@@ -163,7 +165,7 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
 
     # Determine bucketing ID to be used
     bucketing_id = self._get_bucketing_id(user_id, attributes)
-    variation = self.bucketer.bucket(experiment, user_id, bucketing_id)
+    variation = self.bucketer.bucket(project_config, experiment, user_id, bucketing_id)
 
     if variation:
       # Store this new decision and return the variation for the user
@@ -177,11 +179,12 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
 
     return None
 
-  def get_variation_for_rollout(self, rollout, user_id, attributes=None):
+  def get_variation_for_rollout(self, project_config, rollout, user_id, attributes=None):
     """ Determine which experiment/variation the user is in for a given rollout.
     Returns the variation of the first experiment the user qualifies for.
 
     Args:
+      project_config: Instance of ProjectConfig.
       rollout: Rollout for which we are getting the variation.
       user_id: ID for user.
       attributes: Dict representing user attributes.
@@ -193,10 +196,10 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
     # Go through each experiment in order and try to get the variation for the user
     if rollout and len(rollout.experiments) > 0:
       for idx in range(len(rollout.experiments) - 1):
-        experiment = self.config.get_experiment_from_key(rollout.experiments[idx].get('key'))
+        experiment = project_config.get_experiment_from_key(rollout.experiments[idx].get('key'))
 
         # Check if user meets audience conditions for targeting rule
-        if not audience_helper.is_user_in_experiment(self.config, experiment, attributes, self.logger):
+        if not audience_helper.is_user_in_experiment(project_config, experiment, attributes, self.logger):
           self.logger.debug('User "%s" does not meet conditions for targeting rule %s.' % (
             user_id,
             idx + 1
@@ -206,7 +209,7 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
         self.logger.debug('User "%s" meets conditions for targeting rule %s.' % (user_id, idx + 1))
         # Determine bucketing ID to be used
         bucketing_id = self._get_bucketing_id(user_id, attributes)
-        variation = self.bucketer.bucket(experiment, user_id, bucketing_id)
+        variation = self.bucketer.bucket(project_config, experiment, user_id, bucketing_id)
         if variation:
           self.logger.debug('User "%s" is in variation %s of experiment %s.' % (
             user_id,
@@ -221,34 +224,36 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
           break
 
       # Evaluate last rule i.e. "Everyone Else" rule
-      everyone_else_experiment = self.config.get_experiment_from_key(rollout.experiments[-1].get('key'))
-      if audience_helper.is_user_in_experiment(self.config,
-                                               self.config.get_experiment_from_key(rollout.experiments[-1].get('key')),
-                                               attributes,
-                                               self.logger):
+      everyone_else_experiment = project_config.get_experiment_from_key(rollout.experiments[-1].get('key'))
+      if audience_helper.is_user_in_experiment(
+        project_config,
+        project_config.get_experiment_from_key(rollout.experiments[-1].get('key')),
+        attributes,
+        self.logger):
         # Determine bucketing ID to be used
         bucketing_id = self._get_bucketing_id(user_id, attributes)
-        variation = self.bucketer.bucket(everyone_else_experiment, user_id, bucketing_id)
+        variation = self.bucketer.bucket(project_config, everyone_else_experiment, user_id, bucketing_id)
         if variation:
           self.logger.debug('User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
           return Decision(everyone_else_experiment, variation, enums.DecisionSources.ROLLOUT)
 
     return Decision(None, None, enums.DecisionSources.ROLLOUT)
 
-  def get_experiment_in_group(self, group, bucketing_id):
+  def get_experiment_in_group(self, project_config, group, bucketing_id):
     """ Determine which experiment in the group the user is bucketed into.
 
     Args:
+      project_config: Instance of ProjectConfig.
       group: The group to bucket the user into.
       bucketing_id: ID to be used for bucketing the user.
 
     Returns:
       Experiment if the user is bucketed into an experiment in the specified group. None otherwise.
     """
 
-    experiment_id = self.bucketer.find_bucket(bucketing_id, group.id, group.trafficAllocation)
+    experiment_id = self.bucketer.find_bucket(project_config, bucketing_id, group.id, group.trafficAllocation)
     if experiment_id:
-      experiment = self.config.get_experiment_from_id(experiment_id)
+      experiment = project_config.get_experiment_from_id(experiment_id)
       if experiment:
         self.logger.info('User with bucketing ID "%s" is in experiment %s of group %s.' % (
           bucketing_id,
@@ -264,10 +269,11 @@ def get_experiment_in_group(self, group, bucketing_id):
 
     return None
 
-  def get_variation_for_feature(self, feature, user_id, attributes=None):
+  def get_variation_for_feature(self, project_config, feature, user_id, attributes=None):
     """ Returns the experiment/variation the user is bucketed in for the given feature.
 
     Args:
+      project_config: Instance of ProjectConfig.
       feature: Feature for which we are determining if it is enabled or not for the given user.
       user_id: ID for user.
       attributes: Dict representing user attributes.
@@ -276,17 +282,15 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
       Decision namedtuple consisting of experiment and variation for the user.
     """
 
-    experiment = None
-    variation = None
     bucketing_id = self._get_bucketing_id(user_id, attributes)
 
     # First check if the feature is in a mutex group
     if feature.groupId:
-      group = self.config.get_group(feature.groupId)
+      group = project_config.get_group(feature.groupId)
       if group:
-        experiment = self.get_experiment_in_group(group, bucketing_id)
+        experiment = self.get_experiment_in_group(project_config, group, bucketing_id)
         if experiment and experiment.id in feature.experimentIds:
-          variation = self.get_variation(experiment, user_id, attributes)
+          variation = self.get_variation(project_config, experiment, user_id, attributes)
 
           if variation:
             self.logger.debug('User "%s" is in variation %s of experiment %s.' % (
@@ -301,9 +305,9 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
     # Next check if the feature is being experimented on
     elif feature.experimentIds:
       # If an experiment is not in a group, then the feature can only be associated with one experiment
-      experiment = self.config.get_experiment_from_id(feature.experimentIds[0])
+      experiment = project_config.get_experiment_from_id(feature.experimentIds[0])
       if experiment:
-        variation = self.get_variation(experiment, user_id, attributes)
+        variation = self.get_variation(project_config, experiment, user_id, attributes)
 
         if variation:
           self.logger.debug('User "%s" is in variation %s of experiment %s.' % (
@@ -315,7 +319,7 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
 
     # Next check if user is part of a rollout
     if feature.rolloutId:
-      rollout = self.config.get_rollout_from_id(feature.rolloutId)
-      return self.get_variation_for_rollout(rollout, user_id, attributes)
+      rollout = project_config.get_rollout_from_id(feature.rolloutId)
+      return self.get_variation_for_rollout(project_config, rollout, user_id, attributes)
     else:
       return Decision(None, None, enums.DecisionSources.ROLLOUT)