feat: Duplicate experiment key issue with multi feature flag

lib/optimizely/bucketer.rb

@@ -88,7 +88,7 @@ def bucket(project_config, experiment, bucketing_id, user_id)
       decide_reasons.push(*find_bucket_reasons)
 
       if variation_id && variation_id != ''
-        variation = project_config.get_variation_from_id(experiment_key, variation_id)
+        variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)
         return variation, decide_reasons
       end
 

lib/optimizely/config/datafile_project_config.rb

@@ -54,10 +54,12 @@ class DatafileProjectConfig < ProjectConfig
     attr_reader :feature_variable_key_map
     attr_reader :group_id_map
     attr_reader :rollout_id_map
-    attr_reader :rollout_experiment_key_map
+    attr_reader :rollout_experiment_id_map
     attr_reader :variation_id_map
     attr_reader :variation_id_to_variable_usage_map
     attr_reader :variation_key_map
+    attr_reader :variation_id_map_by_experiment_id
+    attr_reader :variation_key_map_by_experiment_id
 
     def initialize(datafile, logger, error_handler)
       # ProjectConfig init method to fetch and set project config data
@@ -117,9 +119,11 @@ def initialize(datafile, logger, error_handler)
       @audience_id_map = @audience_id_map.merge(generate_key_map(@typed_audiences, 'id')) unless @typed_audiences.empty?
       @variation_id_map = {}
       @variation_key_map = {}
+      @variation_id_map_by_experiment_id = {}
+      @variation_key_map_by_experiment_id = {}
       @variation_id_to_variable_usage_map = {}
       @variation_id_to_experiment_map = {}
-      @experiment_key_map.each_value do |exp|
+      @experiment_id_map.each_value do |exp|
         # Excludes experiments from rollouts
         variations = exp.fetch('variations')
         variations.each do |variation|
@@ -129,12 +133,12 @@ def initialize(datafile, logger, error_handler)
       end
       @rollout_id_map = generate_key_map(@rollouts, 'id')
       # split out the experiment key map for rollouts
-      @rollout_experiment_key_map = {}
+      @rollout_experiment_id_map = {}
       @rollout_id_map.each_value do |rollout|
         exps = rollout.fetch('experiments')
-        @rollout_experiment_key_map = @rollout_experiment_key_map.merge(generate_key_map(exps, 'key'))
+        @rollout_experiment_id_map = @rollout_experiment_id_map.merge(generate_key_map(exps, 'id'))
       end
-      @all_experiments = @experiment_key_map.merge(@rollout_experiment_key_map)
+      @all_experiments = @experiment_id_map.merge(@rollout_experiment_id_map)
       @all_experiments.each do |key, exp|
         variations = exp.fetch('variations')
         variations.each do |variation|
@@ -145,8 +149,10 @@ def initialize(datafile, logger, error_handler)
 
           @variation_id_to_variable_usage_map[variation_id] = generate_key_map(variation_variables, 'id')
         end
-        @variation_id_map[key] = generate_key_map(variations, 'id')
-        @variation_key_map[key] = generate_key_map(variations, 'key')
+        @variation_id_map[exp['key']] = generate_key_map(variations, 'id')
+        @variation_key_map[exp['key']] = generate_key_map(variations, 'key')
+        @variation_id_map_by_experiment_id[key] = generate_key_map(variations, 'id')
+        @variation_key_map_by_experiment_id[key] = generate_key_map(variations, 'key')
       end
       @feature_flag_key_map = generate_key_map(@feature_flags, 'key')
       @experiment_feature_map = {}
@@ -281,6 +287,52 @@ def get_variation_from_id(experiment_key, variation_id)
       nil
     end
 
+    def get_variation_from_id_by_experiment_id(experiment_id, variation_id)
+      # Get variation given experiment ID and variation ID
+      #
+      # experiment_id - ID representing parent experiment of variation
+      # variation_id - ID of the variation
+      #
+      # Returns the variation or nil if not found
+
+      variation_id_map_by_experiment_id = @variation_id_map_by_experiment_id[experiment_id]
+      if variation_id_map_by_experiment_id
+        variation = variation_id_map_by_experiment_id[variation_id]
+        return variation if variation
+
+        @logger.log Logger::ERROR, "Variation id '#{variation_id}' is not in datafile."
+        @error_handler.handle_error InvalidVariationError
+        return nil
+      end
+
+      @logger.log Logger::ERROR, "Experiment id '#{experiment_id}' is not in datafile."
+      @error_handler.handle_error InvalidExperimentError
+      nil
+    end
+
+    def get_variation_from_key_by_experiment_id(experiment_id, variation_key)
+      # Get variation given experiment ID and variation key
+      #
+      # experiment_id - ID representing parent experiment of variation
+      # variation_key - Key of the variation
+      #
+      # Returns the variation or nil if not found
+
+      variation_key_map_by_experiment_id = @variation_key_map_by_experiment_id[experiment_id]
+      if variation_key_map_by_experiment_id
+        variation = variation_key_map_by_experiment_id[variation_key]
+        return variation if variation
+
+        @logger.log Logger::ERROR, "Variation key '#{variation_key}' is not in datafile."
+        @error_handler.handle_error InvalidVariationError
+        return nil
+      end
+
+      @logger.log Logger::ERROR, "Experiment id '#{experiment_id}' is not in datafile."
+      @error_handler.handle_error InvalidExperimentError
+      nil
+    end
+
     def get_variation_id_from_key(experiment_key, variation_key)
       # Get variation ID given experiment key and variation key
       #

lib/optimizely/project_config.rb

@@ -66,6 +66,10 @@ def get_audience_from_id(audience_id); end
 
     def get_variation_from_id(experiment_key, variation_id); end
 
+    def get_variation_from_id_by_experiment_id(experiment_id, variation_id); end
+
+    def get_variation_from_key_by_experiment_id(experiment_id, variation_key); end
+
     def get_variation_id_from_key(experiment_key, variation_key); end
 
     def get_whitelisted_variations(experiment_key); end

spec/config/datafile_project_config_spec.rb

@@ -730,13 +730,13 @@
         '166661' => config_body['rollouts'][1]
       }
 
-      expected_rollout_experiment_key_map = {
+      expected_rollout_experiment_id_map = {
         '177770' => config_body['rollouts'][0]['experiments'][0],
         '177772' => config_body['rollouts'][0]['experiments'][1],
         '177776' => config_body['rollouts'][0]['experiments'][2],
         '177774' => config_body['rollouts'][1]['experiments'][0],
         '177779' => config_body['rollouts'][1]['experiments'][1],
-        'rollout_exp_with_diff_id_and_key' => config_body['rollouts'][1]['experiments'][2]
+        '177780' => config_body['rollouts'][1]['experiments'][2]
       }
 
       expect(project_config.attribute_key_map).to eq(expected_attribute_key_map)
@@ -750,7 +750,7 @@
       expect(project_config.variation_key_map).to eq(expected_variation_key_map)
       expect(project_config.variation_id_to_variable_usage_map).to eq(expected_variation_id_to_variable_usage_map)
       expect(project_config.rollout_id_map).to eq(expected_rollout_id_map)
-      expect(project_config.rollout_experiment_key_map).to eq(expected_rollout_experiment_key_map)
+      expect(project_config.rollout_experiment_id_map).to eq(expected_rollout_experiment_id_map)
     end
 
     it 'should initialize properties correctly upon creating project with typed audience dict' do
@@ -842,6 +842,58 @@
       end
     end
 
+    describe 'get_variation_from_id_by_experiment_id' do
+      it 'should log a message when provided experiment id is invalid' do
+        config.get_variation_from_id_by_experiment_id('invalid_id', 'some_variation')
+        expect(spy_logger).to have_received(:log).with(Logger::ERROR,
+                                                       "Experiment id 'invalid_id' is not in datafile.")
+      end
+      it 'should return nil when provided variation id is invalid' do
+        expect(config.get_variation_from_id_by_experiment_id('111127', 'invalid_variation')).to eq(nil)
+        expect(spy_logger).to have_received(:log).with(Logger::ERROR,
+                                                       "Variation id 'invalid_variation' is not in datafile.")
+      end
+
+      it 'should return variation having featureEnabled false when not provided in the datafile' do
+        config_body = OptimizelySpec::VALID_CONFIG_BODY
+        experiment_id = config_body['experiments'][1]['id']
+        variation_id = config_body['experiments'][1]['variations'][1]['id']
+
+        config_body['experiments'][1]['variations'][1][feature_enabled] = nil
+
+        config_body_json = JSON.dump(config_body)
+        project_config = Optimizely::DatafileProjectConfig.new(config_body_json, logger, error_handler)
+
+        expect(project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)[feature_enabled]).to eq(false)
+      end
+    end
+
+    describe 'get_variation_from_key_by_experiment_id' do
+      it 'should log a message when provided experiment id is invalid' do
+        config.get_variation_from_key_by_experiment_id('invalid_id', 'some_variation')
+        expect(spy_logger).to have_received(:log).with(Logger::ERROR,
+                                                       "Experiment id 'invalid_id' is not in datafile.")
+      end
+      it 'should return nil when provided variation key is invalid' do
+        expect(config.get_variation_from_key_by_experiment_id('111127', 'invalid_variation')).to eq(nil)
+        expect(spy_logger).to have_received(:log).with(Logger::ERROR,
+                                                       "Variation key 'invalid_variation' is not in datafile.")
+      end
+
+      it 'should return variation having featureEnabled false when not provided in the datafile' do
+        config_body = OptimizelySpec::VALID_CONFIG_BODY
+        experiment_id = config_body['experiments'][1]['id']
+        variation_key = config_body['experiments'][1]['variations'][1]['key']
+
+        config_body['experiments'][1]['variations'][1][feature_enabled] = nil
+
+        config_body_json = JSON.dump(config_body)
+        project_config = Optimizely::DatafileProjectConfig.new(config_body_json, logger, error_handler)
+
+        expect(project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key)[feature_enabled]).to eq(false)
+      end
+    end
+
     describe 'get_variation_id_from_key' do
       it 'should log a message when there is no variation key map for the experiment' do
         config.get_variation_id_from_key('invalid_key', 'invalid_variation')