Forced Bucketing - Implementation (#63)

Author: oakbani

lib/optimizely.rb

@@ -138,6 +138,11 @@ def get_variation(experiment_key, user_id, attributes = nil)
         return nil
       end
 
+      unless user_id.is_a? String
+        @logger.log(Logger::ERROR, "User id: #{user_id} is not a string")
+        return nil
+      end
+
       unless user_inputs_valid?(attributes)
         @logger.log(Logger::INFO, "Not activating user '#{user_id}.")
         return nil
@@ -151,6 +156,36 @@ def get_variation(experiment_key, user_id, attributes = nil)
       nil
     end
 
+    def set_forced_variation(experiment_key, user_id, variation_key)
+      # Force a user into a variation for a given experiment.
+      #
+      # experiment_key - String - key identifying the experiment.
+      # user_id - String - The user ID to be used for bucketing.
+      # variation_key - The variation key specifies the variation which the user will 
+      #   be forced into. If nil, then clear the existing experiment-to-variation mapping.
+      #
+      # Returns - Boolean - indicates if the set completed successfully.
+
+      @config.set_forced_variation(experiment_key, user_id, variation_key);
+    end
+
+    def get_forced_variation(experiment_key, user_id)
+      # Gets the forced variation for a given user and experiment.
+      #
+      # experiment_key - String - Key identifying the experiment.
+      # user_id - String - The user ID to be used for bucketing.
+      #
+      # Returns String|nil The forced variation key.
+
+      forced_variation_key = nil
+      forced_variation = @config.get_forced_variation(experiment_key, user_id);
+      if forced_variation
+        forced_variation_key = forced_variation['key']
+      end
+
+      forced_variation_key
+    end
+
     def track(event_key, user_id, attributes = nil, event_tags = nil)
       # Send conversion event to Optimizely.
       #

lib/optimizely/decision_service.rb

@@ -22,11 +22,12 @@ class DecisionService
     # The decision service contains all logic relating to how a user bucketing decisions is made.
     # This includes all of the following (in order):
     #
-    # 1. Checking experiment status
-    # 2. Checking whitelisting
-    # 3. Checking user profile service for past bucketing decisions (sticky bucketing)
-    # 3. Checking audience targeting
-    # 4. Using Murmurhash3 to bucket the user
+    # 1. Check experiment status
+    # 2. Check forced bucketing
+    # 3. Check whitelisting
+    # 4. Check user profile service for past bucketing decisions (sticky bucketing)
+    # 5. Check audience targeting
+    # 6. Use Murmurhash3 to bucket the user
 
     attr_reader :bucketer
     attr_reader :config
@@ -58,9 +59,13 @@ def get_variation(experiment_key, user_id, attributes = nil)
         return nil
       end
 
-      # Check if user is in a forced variation
-      forced_variation_id = get_forced_variation_id(experiment_key, user_id)
-      return forced_variation_id if forced_variation_id
+      # Check if a forced variation is set for the user
+      forced_variation = @config.get_forced_variation(experiment_key, user_id)
+      return forced_variation['id'] if forced_variation
+
+      # Check if user is in a white-listed variation
+      whitelisted_variation_id = get_whitelisted_variation_id(experiment_key, user_id)
+      return whitelisted_variation_id if whitelisted_variation_id
 
       # Check for saved bucketing decisions
       user_profile = get_user_profile(user_id)
@@ -91,38 +96,38 @@ def get_variation(experiment_key, user_id, attributes = nil)
     end
 
     private
-    
-    def get_forced_variation_id(experiment_key, user_id)
-      # Determine if a user is forced into a variation for the given experiment and return the ID of that variation
+
+    def get_whitelisted_variation_id(experiment_key, user_id)
+      # Determine if a user is whitelisted into a variation for the given experiment and return the ID of that variation
       #
       # experiment_key - Key representing the experiment for which user is to be bucketed
       # user_id - ID for the user
       #
-      # Returns variation ID into which user_id is forced (nil if no variation)
+      # Returns variation ID into which user_id is whitelisted (nil if no variation)
 
-      forced_variations = @config.get_forced_variations(experiment_key)
+      whitelisted_variations = @config.get_whitelisted_variations(experiment_key)
 
-      return nil unless forced_variations
+      return nil unless whitelisted_variations
 
-      forced_variation_key = forced_variations[user_id]
+      whitelisted_variation_key = whitelisted_variations[user_id]
 
-      return nil unless forced_variation_key
+      return nil unless whitelisted_variation_key
 
-      forced_variation_id = @config.get_variation_id_from_key(experiment_key, forced_variation_key)
+      whitelisted_variation_id = @config.get_variation_id_from_key(experiment_key, whitelisted_variation_key)
 
-      unless forced_variation_id
+      unless whitelisted_variation_id
         @config.logger.log(
           Logger::INFO,
-          "User '#{user_id}' is whitelisted into variation '#{forced_variation_key}', which is not in the datafile."
+          "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}', which is not in the datafile."
         )
         return nil
       end
 
       @config.logger.log(
         Logger::INFO,
-        "User '#{user_id}' is whitelisted into variation '#{forced_variation_key}' of experiment '#{experiment_key}'."
+        "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}' of experiment '#{experiment_key}'."
       )
-      forced_variation_id
+      whitelisted_variation_id
     end
 
     def get_saved_variation_id(experiment_id, user_profile)

lib/optimizely/project_config.rb

@@ -52,6 +52,12 @@ class ProjectConfig
     attr_reader :variation_id_map
     attr_reader :variation_key_map
 
+    # Hash of user IDs to a Hash
+    # of experiments to variations. This contains all the forced variations
+    # set by the user by calling setForcedVariation (it is not the same as the
+    # whitelisting forcedVariations data structure in the Experiments class).
+    attr_reader :forced_variation_map
+
     def initialize(datafile, logger, error_handler)
       # ProjectConfig init method to fetch and set project config data
       #
@@ -92,6 +98,9 @@ def initialize(datafile, logger, error_handler)
       @audience_id_map = generate_key_map(@audiences, 'id')
       @variation_id_map = {}
       @variation_key_map = {}
+      @forced_variation_map = {}
+      @variation_id_to_variable_usage_map = {}
+      @variation_id_to_experiment_map = {}
       @experiment_key_map.each do |key, exp|
         variations = exp.fetch('variations')
         @variation_id_map[key] = generate_key_map(variations, 'id')
@@ -209,19 +218,113 @@ def get_variation_id_from_key(experiment_key, variation_key)
       nil
     end
 
-    def get_forced_variations(experiment_key)
-      # Retrieves forced variations for a given experiment Key
+    def get_whitelisted_variations(experiment_key)
+      # Retrieves whitelisted variations for a given experiment Key
       #
       # experiment_key - String Key representing the experiment
       #
-      # Returns forced variations for the experiment or nil
+      # Returns whitelisted variations for the experiment or nil
 
       experiment = @experiment_key_map[experiment_key]
       return experiment['forcedVariations'] if experiment
       @logger.log Logger::ERROR, "Experiment key '#{experiment_key}' is not in datafile."
       @error_handler.handle_error InvalidExperimentError
     end
 
+    def get_forced_variation(experiment_key, user_id)
+      # Gets the forced variation for the given user and experiment.
+      #
+      # experiment_key - String Key for experiment.
+      # user_id - String ID for user
+      #
+      # Returns Variation The variation which the given user and experiment should be forced into.
+
+      # check for nil and empty string user ID
+      if user_id.nil? or user_id.empty?
+        @logger.log(Logger::DEBUG, "User ID is invalid")
+        return nil
+      end
+
+      unless @forced_variation_map.has_key? (user_id)
+        @logger.log(Logger::DEBUG, "User '#{user_id}' is not in the forced variation map.")
+        return nil
+      end
+
+      experimentToVariationMap = @forced_variation_map[user_id]
+      experiment = get_experiment_from_key(experiment_key)
+      experiment_id = experiment["id"] if experiment
+      # check for nil and empty string experiment ID
+      if experiment_id.nil? or experiment_id.empty?
+        # this case is logged in get_experiment_from_key
+        return nil
+      end
+
+      unless experimentToVariationMap.has_key? (experiment_id)
+        @logger.log(Logger::DEBUG, "No experiment '#{experiment_key}' mapped to user '#{user_id}' in the forced variation map.")
+        return nil
+      end
+
+      variation_id = experimentToVariationMap[experiment_id]
+      variation_key = ""
+      variation = get_variation_from_id(experiment_key,variation_id)
+      variation_key = variation["key"] if variation
+
+      # check if the variation exists in the datafile
+      if variation_key.empty?
+        # this case is logged in get_variation_from_id
+        return nil
+      end
+
+      @logger.log(Logger::DEBUG, "Variation '#{variation_key}' is mapped to experiment '#{experiment_key}' and user '#{user_id}' in the forced variation map")
+      
+      variation
+    end
+
+    def set_forced_variation(experiment_key, user_id, variation_key)
+      # Sets a Hash of user IDs to a Hash of experiments to forced variations.
+      #
+      # experiment_key - String Key for experiment.
+      # user_id - String ID for user.
+      # variation_key - String Key for variation. If null, then clear the existing experiment-to-variation mapping.
+      #
+      # Returns a boolean value that indicates if the set completed successfully.
+
+      #  check for null and empty string user ID
+      if user_id.nil? or user_id.empty?
+        @logger.log(Logger::DEBUG, "User ID is invalid")
+        return false
+      end
+
+      experiment = get_experiment_from_key(experiment_key)
+      experiment_id = experiment["id"] if experiment
+      #  check if the experiment exists in the datafile
+      if experiment_id.nil? or experiment_id.empty?
+        return false
+      end
+
+      #  clear the forced variation if the variation key is null
+      if variation_key.nil? or variation_key.empty?
+        @forced_variation_map[user_id].delete(experiment_id) if @forced_variation_map.has_key? (user_id)
+        @logger.log(Logger::DEBUG, "Variation mapped to experiment '#{experiment_key}' has been removed for user '#{user_id}'.")
+        return true
+      end
+
+      variation_id = get_variation_id_from_key(experiment_key, variation_key)
+
+      #  check if the variation exists in the datafile
+      unless variation_id
+        #  this case is logged in get_variation_id_from_key
+        return false
+      end
+
+      unless @forced_variation_map.has_key? user_id 
+        @forced_variation_map[user_id] = {}
+      end
+      @forced_variation_map[user_id][experiment_id] = variation_id
+      @logger.log(Logger::DEBUG, "Set variation '#{variation_id}' for experiment '#{experiment_id}' and user '#{user_id}' in the forced variation map.")
+      return true
+    end
+
     def get_attribute_id(attribute_key)
       attribute = @attribute_key_map[attribute_key]
       return attribute['id'] if attribute
@@ -253,7 +356,6 @@ def variation_id_exists?(experiment_id, variation_id)
         return true if variation
         @logger.log Logger::ERROR, "Variation ID '#{variation_id}' is not in datafile."
         @error_handler.handle_error InvalidVariationError
-        return false
       end
 
       false

spec/decision_service_spec.rb

@@ -30,23 +30,32 @@
     before(:example) do
       # stub out bucketer and audience evaluator so we can make sure they are / aren't called
       allow(decision_service.bucketer).to receive(:bucket).and_call_original
-      allow(decision_service).to receive(:get_forced_variation_id).and_call_original
+      allow(decision_service).to receive(:get_whitelisted_variation_id).and_call_original
       allow(Optimizely::Audience).to receive(:user_in_experiment?).and_call_original
 
       # by default, spy user profile service should no-op. we override this behavior in specific tests
       allow(spy_user_profile_service).to receive(:lookup).and_return(nil)
     end
 
+    it 'should return the correct variation ID for a given user for whom a variation has been forced' do
+      config.set_forced_variation('test_experiment','test_user', 'variation')
+      expect(decision_service.get_variation('test_experiment', 'test_user')).to eq('111129')
+      # Setting forced variation should short circuit whitelist check, bucketing and audience evaluation
+      expect(decision_service).not_to have_received(:get_whitelisted_variation_id)
+      expect(decision_service.bucketer).not_to have_received(:bucket)
+      expect(Optimizely::Audience).not_to have_received(:user_in_experiment?)
+    end
+
     it 'should return the correct variation ID for a given user ID and key of a running experiment' do
       expect(decision_service.get_variation('test_experiment', 'test_user')).to eq('111128')
 
       expect(spy_logger).to have_received(:log)
                             .once.with(Logger::INFO,"User 'test_user' is in variation 'control' of experiment 'test_experiment'.")
-      expect(decision_service).to have_received(:get_forced_variation_id).once
+      expect(decision_service).to have_received(:get_whitelisted_variation_id).once
       expect(decision_service.bucketer).to have_received(:bucket).once
     end
 
-    it 'should return correct variation ID if user ID is in forcedVariations and variation is valid' do
+    it 'should return correct variation ID if user ID is in whitelisted Variations and variation is valid' do
       expect(decision_service.get_variation('test_experiment', 'forced_user1')).to eq('111128')
       expect(spy_logger).to have_received(:log)
                             .once.with(Logger::INFO, "User 'forced_user1' is whitelisted into variation 'control' of experiment 'test_experiment'.")
@@ -55,13 +64,13 @@
       expect(spy_logger).to have_received(:log)
                             .once.with(Logger::INFO, "User 'forced_user2' is whitelisted into variation 'variation' of experiment 'test_experiment'.")
 
-      # forced variations should short circuit bucketing
+      # whitelisted variations should short circuit bucketing
       expect(decision_service.bucketer).not_to have_received(:bucket)
-      # forced variations should short circuit audience evaluation
+      # whitelisted variations should short circuit audience evaluation
       expect(Optimizely::Audience).not_to have_received(:user_in_experiment?)
     end
 
-    it 'should return the correct variation ID for a user in a forced variation (even when audience conditions do not match)' do
+    it 'should return the correct variation ID for a user in a whitelisted variation (even when audience conditions do not match)' do
       user_attributes = {'browser_type' => 'wrong_browser'}
       expect(decision_service.get_variation('test_experiment_with_audience', 'forced_audience_user', user_attributes)).to eq('122229')
       expect(spy_logger).to have_received(:log)
@@ -90,7 +99,7 @@
                             .once.with(Logger::INFO,"User 'test_user' does not meet the conditions to be in experiment 'test_experiment_with_audience'.")
 
       # should have checked forced variations
-      expect(decision_service).to have_received(:get_forced_variation_id).once
+      expect(decision_service).to have_received(:get_whitelisted_variation_id).once
       # wrong audience conditions should short circuit bucketing
       expect(decision_service.bucketer).not_to have_received(:bucket)
     end
@@ -101,7 +110,7 @@
                             .once.with(Logger::INFO,"Experiment 'test_experiment_not_started' is not running.")
 
       # non-running experiments should short circuit whitelisting
-      expect(decision_service).not_to have_received(:get_forced_variation_id)
+      expect(decision_service).not_to have_received(:get_whitelisted_variation_id)
       # non-running experiments should short circuit audience evaluation
       expect(Optimizely::Audience).not_to have_received(:user_in_experiment?)
       # non-running experiments should short circuit bucketing