From ec4dcfca52aaff01da1f08ecf008e481d32b53b1 Mon Sep 17 00:00:00 2001 From: rashidsp Date: Thu, 16 May 2019 18:26:37 +0500 Subject: [PATCH 1/2] Fix: Rollout experiment mapping issue --- lib/optimizely/project_config.rb | 7 +++++-- spec/project_config_spec.rb | 32 ++++++++++++++++++++++++++++++-- spec/spec_params.rb | 16 ++++++++++++++++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/optimizely/project_config.rb b/lib/optimizely/project_config.rb index 742d71df..745bb347 100644 --- a/lib/optimizely/project_config.rb +++ b/lib/optimizely/project_config.rb @@ -55,6 +55,7 @@ class ProjectConfig attr_reader :group_key_map attr_reader :rollout_id_map attr_reader :rollout_experiment_id_map + attr_reader :rollout_experiment_key_map attr_reader :variation_id_map attr_reader :variation_id_to_variable_usage_map attr_reader :variation_key_map @@ -120,13 +121,15 @@ def initialize(datafile, logger, error_handler) end end @rollout_id_map = generate_key_map(@rollouts, 'id') - # split out the experiment id map for rollouts + # split out the experiment id and key map for rollouts @rollout_experiment_id_map = {} + @rollout_experiment_key_map = {} @rollout_id_map.each_value do |rollout| exps = rollout.fetch('experiments') @rollout_experiment_id_map = @rollout_experiment_id_map.merge(generate_key_map(exps, 'id')) + @rollout_experiment_key_map = @rollout_experiment_key_map.merge(generate_key_map(exps, 'key')) end - @all_experiments = @experiment_key_map.merge(@rollout_experiment_id_map) + @all_experiments = @experiment_key_map.merge(@rollout_experiment_key_map) @all_experiments.each do |key, exp| variations = exp.fetch('variations') variations.each do |variation| diff --git a/spec/project_config_spec.rb b/spec/project_config_spec.rb index fc4d3769..b4446802 100644 --- a/spec/project_config_spec.rb +++ b/spec/project_config_spec.rb @@ -264,6 +264,14 @@ feature_enabled => true, 'variables' => [] } + }, + 'rollout_exp_with_diff_id_and_key' => { + '177781' => { + 'id' => '177781', + 'key' => 'rollout_var_with_diff_id_and_key', + feature_enabled => true, + 'variables' => [] + } } } @@ -448,6 +456,14 @@ feature_enabled => true, 'variables' => [] } + }, + 'rollout_exp_with_diff_id_and_key' => { + 'rollout_var_with_diff_id_and_key' => { + 'id' => '177781', + 'key' => 'rollout_var_with_diff_id_and_key', + feature_enabled => true, + 'variables' => [] + } } } @@ -641,7 +657,8 @@ 'value' => 'false' } }, - '177780' => {} + '177780' => {}, + '177781' => {} } expected_rollout_id_map = { @@ -654,7 +671,17 @@ '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] + '177779' => config_body['rollouts'][1]['experiments'][1], + '177780' => config_body['rollouts'][1]['experiments'][2] + } + + expected_rollout_experiment_key_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] } expect(project_config.attribute_key_map).to eq(expected_attribute_key_map) @@ -669,6 +696,7 @@ 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_id_map).to eq(expected_rollout_experiment_id_map) + expect(project_config.rollout_experiment_key_map).to eq(expected_rollout_experiment_key_map) end it 'should initialize properties correctly upon creating project with typed audience dict' do diff --git a/spec/spec_params.rb b/spec/spec_params.rb index c7957ac0..a97671bd 100644 --- a/spec/spec_params.rb +++ b/spec/spec_params.rb @@ -659,6 +659,22 @@ module OptimizelySpec 'entityId' => '177780', 'endOfRange' => 1500 }] + }, { + 'id' => '177780', + 'key' => 'rollout_exp_with_diff_id_and_key', + 'status' => 'Running', + 'layerId' => '166661', + 'audienceIds' => [], + 'variations' => [{ + 'id' => '177781', + 'key' => 'rollout_var_with_diff_id_and_key', + 'featureEnabled' => true, + 'variables' => [] + }], + 'trafficAllocation' => [{ + 'entityId' => '177781', + 'endOfRange' => 1500 + }] }] }] }.freeze From 8f25a7c31e83c6284c7b96b105e7fc3c75c1a142 Mon Sep 17 00:00:00 2001 From: rashidsp Date: Fri, 17 May 2019 12:31:48 +0500 Subject: [PATCH 2/2] Addressing comments --- lib/optimizely/project_config.rb | 5 +---- spec/project_config_spec.rb | 10 ---------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/lib/optimizely/project_config.rb b/lib/optimizely/project_config.rb index 745bb347..edbbbec0 100644 --- a/lib/optimizely/project_config.rb +++ b/lib/optimizely/project_config.rb @@ -54,7 +54,6 @@ class ProjectConfig attr_reader :feature_variable_key_map attr_reader :group_key_map attr_reader :rollout_id_map - attr_reader :rollout_experiment_id_map attr_reader :rollout_experiment_key_map attr_reader :variation_id_map attr_reader :variation_id_to_variable_usage_map @@ -121,12 +120,10 @@ def initialize(datafile, logger, error_handler) end end @rollout_id_map = generate_key_map(@rollouts, 'id') - # split out the experiment id and key map for rollouts - @rollout_experiment_id_map = {} + # split out the experiment key map for rollouts @rollout_experiment_key_map = {} @rollout_id_map.each_value do |rollout| exps = rollout.fetch('experiments') - @rollout_experiment_id_map = @rollout_experiment_id_map.merge(generate_key_map(exps, 'id')) @rollout_experiment_key_map = @rollout_experiment_key_map.merge(generate_key_map(exps, 'key')) end @all_experiments = @experiment_key_map.merge(@rollout_experiment_key_map) diff --git a/spec/project_config_spec.rb b/spec/project_config_spec.rb index b4446802..3927b956 100644 --- a/spec/project_config_spec.rb +++ b/spec/project_config_spec.rb @@ -666,15 +666,6 @@ '166661' => config_body['rollouts'][1] } - 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], - '177780' => config_body['rollouts'][1]['experiments'][2] - } - expected_rollout_experiment_key_map = { '177770' => config_body['rollouts'][0]['experiments'][0], '177772' => config_body['rollouts'][0]['experiments'][1], @@ -695,7 +686,6 @@ 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_id_map).to eq(expected_rollout_experiment_id_map) expect(project_config.rollout_experiment_key_map).to eq(expected_rollout_experiment_key_map) end