From de619315922a10309045f161d793ce8a77b5b830 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 2 Jul 2025 05:48:26 +0600 Subject: [PATCH 01/15] update: Extend LRUCache with remove method and corresponding tests --- lib/optimizely/odp/lru_cache.rb | 15 ++++++- spec/odp/lru_cache_spec.rb | 80 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/lib/optimizely/odp/lru_cache.rb b/lib/optimizely/odp/lru_cache.rb index 8ce61549..23bf4e67 100644 --- a/lib/optimizely/odp/lru_cache.rb +++ b/lib/optimizely/odp/lru_cache.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2022, Optimizely and contributors +# Copyright 2022-2025, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -91,6 +91,19 @@ def peek(key) @cache_mutex.synchronize { @map[key]&.value } end + + # Remove the element associated with the provided key from the cache + # + # @param key - The key to remove + + def remove(key) + return if @capacity <= 0 + + @cache_mutex.synchronize do + @map.delete(key) + end + nil + end end class CacheElement diff --git a/spec/odp/lru_cache_spec.rb b/spec/odp/lru_cache_spec.rb index 46363c8b..deafeecc 100644 --- a/spec/odp/lru_cache_spec.rb +++ b/spec/odp/lru_cache_spec.rb @@ -149,4 +149,84 @@ cache.save('cow', 'crate') expect(cache.lookup('cow')).to eq 'crate' end + + it 'should remove existing key' do + cache = Optimizely::LRUCache.new(3, 1000) + + cache.save('1', 100) + cache.save('2', 200) + cache.save('3', 300) + + expect(cache.lookup('1')).to eq 100 + expect(cache.lookup('2')).to eq 200 + expect(cache.lookup('3')).to eq 300 + + cache.remove('2') + + expect(cache.lookup('1')).to eq 100 + expect(cache.lookup('2')).to be_nil + expect(cache.lookup('3')).to eq 300 + end + + it 'should handle removing non-existent key' do + cache = Optimizely::LRUCache.new(3, 1000) + cache.save('1', 100) + cache.save('2', 200) + + cache.remove('3') # Doesn't exist + + expect(cache.lookup('1')).to eq 100 + expect(cache.lookup('2')).to eq 200 + end + + it 'should handle removing from zero sized cache' do + cache = Optimizely::LRUCache.new(0, 1000) + cache.save('1', 100) + cache.remove('1') + + expect(cache.lookup('1')).to be_nil + end + + it 'should handle removing and adding back a key' do + cache = Optimizely::LRUCache.new(3, 1000) + cache.save('1', 100) + cache.save('2', 200) + cache.save('3', 300) + + cache.remove('2') + cache.save('2', 201) + + expect(cache.lookup('1')).to eq 100 + expect(cache.lookup('2')).to eq 201 + expect(cache.lookup('3')).to eq 300 + end + + it 'should handle thread safety' do + max_size = 100 + cache = Optimizely::LRUCache.new(max_size, 1000) + + (1..max_size).each do |i| + cache.save(i.to_s, i * 100) + end + + threads = [] + (1..(max_size / 2)).each do |i| + thread = Thread.new do + cache.remove(i.to_s) + end + threads << thread + end + + threads.each(&:join) + + (1..max_size).each do |i| + if i <= max_size / 2 + expect(cache.lookup(i.to_s)).to be_nil + else + expect(cache.lookup(i.to_s)).to eq(i * 100) + end + end + + expect(cache.instance_variable_get('@map').size).to eq(max_size / 2) + end end From 01e3a9f8545afd2b9700be5975a327bb57b7355a Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 2 Jul 2025 05:52:03 +0600 Subject: [PATCH 02/15] update: Clean up whitespace in LRUCache implementation and tests --- lib/optimizely/odp/lru_cache.rb | 2 +- spec/odp/lru_cache_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/odp/lru_cache.rb b/lib/optimizely/odp/lru_cache.rb index 23bf4e67..6d4c9af3 100644 --- a/lib/optimizely/odp/lru_cache.rb +++ b/lib/optimizely/odp/lru_cache.rb @@ -91,7 +91,7 @@ def peek(key) @cache_mutex.synchronize { @map[key]&.value } end - + # Remove the element associated with the provided key from the cache # # @param key - The key to remove diff --git a/spec/odp/lru_cache_spec.rb b/spec/odp/lru_cache_spec.rb index deafeecc..8c65d909 100644 --- a/spec/odp/lru_cache_spec.rb +++ b/spec/odp/lru_cache_spec.rb @@ -173,7 +173,7 @@ cache.save('1', 100) cache.save('2', 200) - cache.remove('3') # Doesn't exist + cache.remove('3') # Doesn't exist expect(cache.lookup('1')).to eq 100 expect(cache.lookup('2')).to eq 200 From 40d0571ca4cf01f12d497b7851a335995b092114 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 2 Jul 2025 05:52:38 +0600 Subject: [PATCH 03/15] update: Extend copyright notice to include 2025 --- spec/odp/lru_cache_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/odp/lru_cache_spec.rb b/spec/odp/lru_cache_spec.rb index 8c65d909..32db021f 100644 --- a/spec/odp/lru_cache_spec.rb +++ b/spec/odp/lru_cache_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2022, Optimizely and contributors +# Copyright 2022-2025, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From 2282eb12e4bc46df7772d2b55acdfe816cd9c49a Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 9 Jul 2025 17:47:31 +0600 Subject: [PATCH 04/15] update: Implement Default CMAB Service --- lib/optimizely/cmab/cmab_service.rb | 153 ++++++++++++ .../config/datafile_project_config.rb | 4 +- .../decide/optimizely_decide_option.rb | 3 + spec/{ => cmab}/cmab_client_spec.rb | 0 spec/cmab/cmab_service_spec.rb | 233 ++++++++++++++++++ 5 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 lib/optimizely/cmab/cmab_service.rb rename spec/{ => cmab}/cmab_client_spec.rb (100%) create mode 100644 spec/cmab/cmab_service_spec.rb diff --git a/lib/optimizely/cmab/cmab_service.rb b/lib/optimizely/cmab/cmab_service.rb new file mode 100644 index 00000000..134b9fb8 --- /dev/null +++ b/lib/optimizely/cmab/cmab_service.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +# +# Copyright 2025 Optimizely and contributors +# +# 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 +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +require 'optimizely/odp/lru_cache' +require 'optimizely/decide/optimizely_decide_option' +require 'digest' +require 'json' +require 'securerandom' + +module Optimizely + CmabDecision = Struct.new(:variation_id, :cmab_uuid) + CmabCacheValue = Struct.new(:attributes_hash, :variation_id, :cmab_uuid) + + # Default CMAB service implementation + class DefaultCmabService + # Initializes a new instance of the CmabService. + # + # @param cmab_cache [LRUCache] The cache object used for storing CMAB data. Must be an instance of LRUCache. + # @param cmab_client [DefaultCmabClient] The client used to interact with the CMAB service. Must be an instance of DefaultCmabClient. + # @param logger [Logger, nil] Optional logger for logging messages. Defaults to nil. + # + # @raise [ArgumentError] If cmab_cache is not an instance of LRUCache. + # @raise [ArgumentError] If cmab_client is not an instance of DefaultCmabClient. + def initialize(cmab_cache, cmab_client, logger = nil) + @cmab_cache = cmab_cache + @cmab_client = cmab_client + @logger = logger + end + + def get_decision(project_config, user_context, rule_id, options) + # Retrieves a decision for a given user and rule, utilizing a cache for efficiency. + # + # This method filters user attributes, checks for various cache-related options, + # and either fetches a fresh decision or returns a cached one if appropriate. + # It supports options to ignore the cache, reset the cache, or invalidate a specific user's cache entry. + # + # @param project_config [Object] The project configuration object. + # @param user_context [Object] The user context containing user_id and attributes. + # @param rule_id [String] The identifier for the decision rule. + # @param options [Array, nil] Optional flags to control cache behavior. Supported options: + # - OptimizelyDecideOption::IGNORE_CMAB_CACHE: Bypass cache and fetch a new decision. + # - OptimizelyDecideOption::RESET_CMAB_CACHE: Reset the entire cache. + # - OptimizelyDecideOption::INVALIDATE_USER_CMAB_CACHE: Invalidate cache for the specific user and rule. + # + # @return [CmabDecision] The decision object containing variation_id and cmab_uuid. + + filtered_attributes = filter_attributes(project_config, user_context, rule_id) + + return fetch_decision(rule_id, user_context.user_id, filtered_attributes) if options&.include?(Decide::OptimizelyDecideOption::IGNORE_CMAB_CACHE) + + @cmab_cache.reset if options&.include?(Decide::OptimizelyDecideOption::RESET_CMAB_CACHE) + + cache_key = get_cache_key(user_context.user_id, rule_id) + + @cmab_cache.remove(cache_key) if options&.include?(Decide::OptimizelyDecideOption::INVALIDATE_USER_CMAB_CACHE) + cached_value = @cmab_cache.lookup(cache_key) + attributes_hash = hash_attributes(filtered_attributes) + + if cached_value + return CmabDecision.new(variation_id: cached_value.variation_id, cmab_uuid: cached_value.cmab_uuid) if cached_value.attributes_hash == attributes_hash + + @cmab_cache.remove(cache_key) + end + cmab_decision = fetch_decision(rule_id, user_context.user_id, filtered_attributes) + @cmab_cache.save(cache_key, + CmabCacheValue.new( + attributes_hash: attributes_hash, + variation_id: cmab_decision.variation_id, + cmab_uuid: cmab_decision.cmab_uuid + )) + cmab_decision + end + + private + + def fetch_decision(rule_id, user_id, attributes) + # Fetches a decision for a given rule and user, along with user attributes. + # + # Generates a unique UUID for the decision request, then delegates to the CMAB client + # to fetch the variation ID. Returns a CmabDecision object containing the variation ID + # and the generated UUID. + # + # @param rule_id [String] The identifier for the rule to evaluate. + # @param user_id [String] The identifier for the user. + # @param attributes [Hash] A hash of user attributes to be used in decision making. + # @return [CmabDecision] The decision object containing the variation ID and UUID. + cmab_uuid = SecureRandom.uuid + variation_id = @cmab_client.fetch_decision(rule_id, user_id, attributes, cmab_uuid) + CmabDecision.new(variation_id: variation_id, cmab_uuid: cmab_uuid) + end + + def filter_attributes(project_config, user_context, rule_id) + # Filters the user attributes based on the CMAB attribute IDs defined in the experiment. + # + # @param project_config [Object] The project configuration object containing experiment and attribute mappings. + # @param user_context [Object] The user context object containing user attributes. + # @param rule_id [String] The ID of the experiment (rule) to filter attributes for. + # @return [Hash] A hash of filtered user attributes whose keys match the CMAB attribute IDs for the given experiment. + user_attributes = user_context.user_attributes + filtered_user_attributes = {} + + experiment = project_config.experiment_id_map[rule_id] + return filtered_user_attributes if experiment.nil? || experiment['cmab'].nil? + + cmab_attribute_ids = experiment['cmab']['attributeIds'] + cmab_attribute_ids.each do |attribute_id| + attribute = project_config.attribute_id_map[attribute_id] + filtered_user_attributes[attribute.key] = user_attributes[attribute.key] if attribute && user_attributes.key?(attribute.key) + end + + filtered_user_attributes + end + + def get_cache_key(user_id, rule_id) + # Generates a cache key string based on the provided user ID and rule ID. + # + # The cache key is constructed in the format: "--", + # where is the length of the user_id string. + # + # @param user_id [String] The unique identifier for the user. + # @param rule_id [String] The unique identifier for the rule. + # @return [String] The generated cache key. + "#{user_id.length}-#{user_id}-#{rule_id}" + end + + def hash_attributes(attributes) + # Generates an MD5 hash for a given attributes hash. + # + # The method sorts the attributes by key, serializes them to a JSON string, + # and then computes the MD5 hash of the resulting string. This ensures that + # the hash is consistent regardless of the original key order in the input hash. + # + # @param attributes [Hash] The attributes to be hashed. + # @return [String] The MD5 hash of the sorted and serialized attributes. + sorted_attrs = JSON.generate(attributes.sort.to_h) + Digest::MD5.hexdigest(sorted_attrs) + end + end +end diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index 1f03171d..51673e23 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/lib/optimizely/config/datafile_project_config.rb @@ -27,7 +27,8 @@ class DatafileProjectConfig < ProjectConfig attr_reader :datafile, :account_id, :attributes, :audiences, :typed_audiences, :events, :experiments, :feature_flags, :groups, :project_id, :bot_filtering, :revision, :sdk_key, :environment_key, :rollouts, :version, :send_flag_decisions, - :attribute_key_map, :attribute_id_to_key_map, :audience_id_map, :event_key_map, :experiment_feature_map, + :attribute_key_map, :attribute_id_to_key_map, :attribute_id_map, + :audience_id_map, :event_key_map, :experiment_feature_map, :experiment_id_map, :experiment_key_map, :feature_flag_key_map, :feature_variable_key_map, :group_id_map, :rollout_id_map, :rollout_experiment_id_map, :variation_id_map, :variation_id_to_variable_usage_map, :variation_key_map, :variation_id_map_by_experiment_id, @@ -82,6 +83,7 @@ def initialize(datafile, logger, error_handler) # Utility maps for quick lookup @attribute_key_map = generate_key_map(@attributes, 'key') + @attribute_id_map = generate_key_map(@attributes, 'id') @attribute_id_to_key_map = {} @attributes.each do |attribute| @attribute_id_to_key_map[attribute['id']] = attribute['key'] diff --git a/lib/optimizely/decide/optimizely_decide_option.rb b/lib/optimizely/decide/optimizely_decide_option.rb index f89dcd51..1b6781c2 100644 --- a/lib/optimizely/decide/optimizely_decide_option.rb +++ b/lib/optimizely/decide/optimizely_decide_option.rb @@ -23,6 +23,9 @@ module OptimizelyDecideOption IGNORE_USER_PROFILE_SERVICE = 'IGNORE_USER_PROFILE_SERVICE' INCLUDE_REASONS = 'INCLUDE_REASONS' EXCLUDE_VARIABLES = 'EXCLUDE_VARIABLES' + IGNORE_CMAB_CACHE = 'IGNORE_CMAB_CACHE' + RESET_CMAB_CACHE = 'RESET_CMAB_CACHE' + INVALIDATE_USER_CMAB_CACHE = 'INVALIDATE_USER_CMAB_CACHE' end end end diff --git a/spec/cmab_client_spec.rb b/spec/cmab/cmab_client_spec.rb similarity index 100% rename from spec/cmab_client_spec.rb rename to spec/cmab/cmab_client_spec.rb diff --git a/spec/cmab/cmab_service_spec.rb b/spec/cmab/cmab_service_spec.rb new file mode 100644 index 00000000..6c3c0011 --- /dev/null +++ b/spec/cmab/cmab_service_spec.rb @@ -0,0 +1,233 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'optimizely/cmab/cmab_service' +require 'optimizely/odp/lru_cache' +require 'optimizely/cmab/cmab_client' +require 'optimizely/decide/optimizely_decide_option' + +describe Optimizely::DefaultCmabService do + let(:mock_cmab_cache) { instance_double(Optimizely::LRUCache) } + let(:mock_cmab_client) { instance_double(Optimizely::DefaultCmabClient) } + let(:mock_logger) { double('logger') } + let(:cmab_service) { described_class.new(mock_cmab_cache, mock_cmab_client, mock_logger) } + + let(:mock_project_config) { double('project_config') } + let(:mock_user_context) { double('user_context') } + let(:user_id) { 'user123' } + let(:rule_id) { 'exp1' } + let(:user_attributes) { {'age' => 25, 'location' => 'USA'} } + + let(:mock_experiment) { {'cmab' => {'attributeIds' => %w[66 77]}} } + let(:mock_attr1) { double('attribute', key: 'age') } + let(:mock_attr2) { double('attribute', key: 'location') } + + before do + allow(mock_user_context).to receive(:user_id).and_return(user_id) + allow(mock_user_context).to receive(:user_attributes).and_return(user_attributes) + + allow(mock_project_config).to receive(:experiment_id_map).and_return({rule_id => mock_experiment}) + allow(mock_project_config).to receive(:attribute_id_map).and_return({ + '66' => mock_attr1, + '77' => mock_attr2 + }) + end + + describe '#get_decision' do + it 'returns decision from cache when valid' do + expected_key = cmab_service.send(:get_cache_key, user_id, rule_id) + expected_attributes = {'age' => 25, 'location' => 'USA'} + expected_hash = cmab_service.send(:hash_attributes, expected_attributes) + + cached_value = Optimizely::CmabCacheValue.new( + attributes_hash: expected_hash, + variation_id: 'varA', + cmab_uuid: 'uuid-123' + ) + + allow(mock_cmab_cache).to receive(:lookup).with(expected_key).and_return(cached_value) + + decision = cmab_service.get_decision(mock_project_config, mock_user_context, rule_id, []) + + expect(mock_cmab_cache).to have_received(:lookup).with(expected_key) + expect(decision.variation_id).to eq('varA') + expect(decision.cmab_uuid).to eq('uuid-123') + end + + it 'ignores cache when option given' do + allow(mock_cmab_client).to receive(:fetch_decision).and_return('varB') + expected_attributes = {'age' => 25, 'location' => 'USA'} + + decision = cmab_service.get_decision( + mock_project_config, + mock_user_context, + rule_id, + [Optimizely::Decide::OptimizelyDecideOption::IGNORE_CMAB_CACHE] + ) + + expect(decision.variation_id).to eq('varB') + expect(decision.cmab_uuid).to be_a(String) + expect(mock_cmab_client).to have_received(:fetch_decision).with( + rule_id, + user_id, + expected_attributes, + decision.cmab_uuid + ) + end + + it 'invalidates user cache when option given' do + allow(mock_cmab_client).to receive(:fetch_decision).and_return('varC') + allow(mock_cmab_cache).to receive(:lookup).and_return(nil) + allow(mock_cmab_cache).to receive(:remove) + allow(mock_cmab_cache).to receive(:save) + + cmab_service.get_decision( + mock_project_config, + mock_user_context, + rule_id, + [Optimizely::Decide::OptimizelyDecideOption::INVALIDATE_USER_CMAB_CACHE] + ) + + key = cmab_service.send(:get_cache_key, user_id, rule_id) + expect(mock_cmab_cache).to have_received(:remove).with(key) + end + + it 'resets cache when option given' do + allow(mock_cmab_client).to receive(:fetch_decision).and_return('varD') + allow(mock_cmab_cache).to receive(:reset) + allow(mock_cmab_cache).to receive(:lookup).and_return(nil) + allow(mock_cmab_cache).to receive(:save) + + decision = cmab_service.get_decision( + mock_project_config, + mock_user_context, + rule_id, + [Optimizely::Decide::OptimizelyDecideOption::RESET_CMAB_CACHE] + ) + + expect(mock_cmab_cache).to have_received(:reset) + expect(decision.variation_id).to eq('varD') + expect(decision.cmab_uuid).to be_a(String) + end + + it 'fetches new decision when hash changes' do + old_cached_value = Optimizely::CmabCacheValue.new( + attributes_hash: 'old_hash', + variation_id: 'varA', + cmab_uuid: 'uuid-123' + ) + + allow(mock_cmab_cache).to receive(:lookup).and_return(old_cached_value) + allow(mock_cmab_cache).to receive(:remove) + allow(mock_cmab_cache).to receive(:save) + allow(mock_cmab_client).to receive(:fetch_decision).and_return('varE') + + expected_attributes = {'age' => 25, 'location' => 'USA'} + cmab_service.send(:hash_attributes, expected_attributes) + expected_key = cmab_service.send(:get_cache_key, user_id, rule_id) + + decision = cmab_service.get_decision(mock_project_config, mock_user_context, rule_id, []) + + expect(mock_cmab_cache).to have_received(:remove).with(expected_key) + expect(mock_cmab_cache).to have_received(:save).with( + expected_key, + an_instance_of(Optimizely::CmabCacheValue) + ) + expect(decision.variation_id).to eq('varE') + expect(mock_cmab_client).to have_received(:fetch_decision).with( + rule_id, + user_id, + expected_attributes, + decision.cmab_uuid + ) + end + + it 'only passes cmab attributes to client' do + allow(mock_user_context).to receive(:user_attributes).and_return({ + 'age' => 25, + 'location' => 'USA', + 'extra_attr' => 'value', + 'another_extra' => 123 + }) + allow(mock_cmab_client).to receive(:fetch_decision).and_return('varF') + + decision = cmab_service.get_decision( + mock_project_config, + mock_user_context, + rule_id, + [Optimizely::Decide::OptimizelyDecideOption::IGNORE_CMAB_CACHE] + ) + + # Verify only age and location are passed + expect(mock_cmab_client).to have_received(:fetch_decision).with( + rule_id, + user_id, + {'age' => 25, 'location' => 'USA'}, + decision.cmab_uuid + ) + end + end + + describe '#filter_attributes' do + it 'returns correct subset of attributes' do + filtered = cmab_service.send(:filter_attributes, mock_project_config, mock_user_context, rule_id) + + expect(filtered['age']).to eq(25) + expect(filtered['location']).to eq('USA') + end + + it 'returns empty hash when no cmab config' do + allow(mock_project_config).to receive(:experiment_id_map).and_return({rule_id => {'cmab' => nil}}) + + filtered = cmab_service.send(:filter_attributes, mock_project_config, mock_user_context, rule_id) + + expect(filtered).to eq({}) + end + + it 'returns empty hash when experiment not found' do + allow(mock_project_config).to receive(:experiment_id_map).and_return({}) + + filtered = cmab_service.send(:filter_attributes, mock_project_config, mock_user_context, rule_id) + + expect(filtered).to eq({}) + end + end + + describe '#hash_attributes' do + it 'produces stable output regardless of key order' do + attrs1 = {'b' => 2, 'a' => 1} + attrs2 = {'a' => 1, 'b' => 2} + + hash1 = cmab_service.send(:hash_attributes, attrs1) + hash2 = cmab_service.send(:hash_attributes, attrs2) + + expect(hash1).to eq(hash2) + end + end + + describe '#get_cache_key' do + it 'generates correct cache key format' do + key = cmab_service.send(:get_cache_key, 'user123', 'exp1') + + expect(key).to eq('7-user123-exp1') + end + end + + describe '#fetch_decision' do + it 'generates uuid and calls client' do + allow(mock_cmab_client).to receive(:fetch_decision).and_return('varX') + attributes = {'age' => 25} + + decision = cmab_service.send(:fetch_decision, rule_id, user_id, attributes) + + expect(decision.variation_id).to eq('varX') + expect(decision.cmab_uuid).to match(/\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/) + expect(mock_cmab_client).to have_received(:fetch_decision).with( + rule_id, + user_id, + attributes, + decision.cmab_uuid + ) + end + end +end From 8ca3aee221e83104335c7ca754aab40b6f7f5596 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 9 Jul 2025 21:23:18 +0600 Subject: [PATCH 05/15] update: Enable keyword initialization for CmabDecision and CmabCacheValue structs (otherwise breaks in ruby version change) --- lib/optimizely/cmab/cmab_service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/cmab/cmab_service.rb b/lib/optimizely/cmab/cmab_service.rb index 134b9fb8..b56a785b 100644 --- a/lib/optimizely/cmab/cmab_service.rb +++ b/lib/optimizely/cmab/cmab_service.rb @@ -22,8 +22,8 @@ require 'securerandom' module Optimizely - CmabDecision = Struct.new(:variation_id, :cmab_uuid) - CmabCacheValue = Struct.new(:attributes_hash, :variation_id, :cmab_uuid) + CmabDecision = Struct.new(:variation_id, :cmab_uuid, keyword_init: true) + CmabCacheValue = Struct.new(:attributes_hash, :variation_id, :cmab_uuid, keyword_init: true) # Default CMAB service implementation class DefaultCmabService From 761bc4390da936e6f9dcfebf952050d092c03b92 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 14 Jul 2025 14:51:15 +0600 Subject: [PATCH 06/15] update: Refactor bucketing logic to handle empty traffic ranges and improve logging --- lib/optimizely/bucketer.rb | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/optimizely/bucketer.rb b/lib/optimizely/bucketer.rb index 15f711cb..3bff9c45 100644 --- a/lib/optimizely/bucketer.rb +++ b/lib/optimizely/bucketer.rb @@ -44,6 +44,25 @@ def bucket(project_config, experiment, bucketing_id, user_id) # user_id - String ID for user. # # Returns variation in which visitor with ID user_id has been placed. Nil if no variation. + + variation_id, decide_reasons = bucket_to_entity_id(project_config, experiment, bucketing_id, user_id) + if variation_id && variation_id != '' + experiment_id = experiment['id'] + variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) + return variation, decide_reasons + end + + # Handle the case when the traffic range is empty due to sticky bucketing + if variation_id == '' + message = 'Bucketed into an empty traffic range. Returning nil.' + @logger.log(Logger::DEBUG, message) + decide_reasons.push(message) + end + + [nil, decide_reasons] + end + + def bucket_to_entity_id(project_config, experiment, bucketing_id, user_id) return nil, [] if experiment.nil? decide_reasons = [] @@ -87,19 +106,7 @@ def bucket(project_config, experiment, bucketing_id, user_id) variation_id, find_bucket_reasons = find_bucket(bucketing_id, user_id, experiment_id, traffic_allocations) decide_reasons.push(*find_bucket_reasons) - if variation_id && variation_id != '' - variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) - return variation, decide_reasons - end - - # Handle the case when the traffic range is empty due to sticky bucketing - if variation_id == '' - message = 'Bucketed into an empty traffic range. Returning nil.' - @logger.log(Logger::DEBUG, message) - decide_reasons.push(message) - end - - [nil, decide_reasons] + [variation_id, decide_reasons] end def find_bucket(bucketing_id, user_id, parent_id, traffic_allocations) From ebb1b7dbbcc5f42f821a5191e75b918d7fd3c93f Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 14 Jul 2025 15:03:24 +0600 Subject: [PATCH 07/15] update: Add support for CMAB traffic allocation in bucketing logic --- lib/optimizely/bucketer.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/optimizely/bucketer.rb b/lib/optimizely/bucketer.rb index 3bff9c45..daf435f9 100644 --- a/lib/optimizely/bucketer.rb +++ b/lib/optimizely/bucketer.rb @@ -103,6 +103,14 @@ def bucket_to_entity_id(project_config, experiment, bucketing_id, user_id) end traffic_allocations = experiment['trafficAllocation'] + if experiment['cmab'] + traffic_allocations = [ + { + entityId: '$', + endOfRange: experiment['cmab']['trafficAllocation'] + } + ] + end variation_id, find_bucket_reasons = find_bucket(bucketing_id, user_id, experiment_id, traffic_allocations) decide_reasons.push(*find_bucket_reasons) From d6dd3aae3ed0fecdd84e7f57afda474383bc318e Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 14 Jul 2025 19:52:39 +0600 Subject: [PATCH 08/15] update: Enhance DecisionService to support CMAB traffic allocation and decision retrieval --- lib/optimizely/decision_service.rb | 52 ++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 3303907d..80739ded 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -37,7 +37,10 @@ class DecisionService # This contains all the forced variations set by the user by calling setForcedVariation. attr_reader :forced_variation_map - Decision = Struct.new(:experiment, :variation, :source) + Decision = Struct.new(:experiment, :variation, :source, :cmab_uuid) + CmabDecisionResult = Struct.new(:error, :result, :reasons) + VariationResult = Struct.new(:cmab_uuid, :error, :reasons, :variation) + DecisionResult = Struct.new(:decision, :error, :reasons) DECISION_SOURCES = { 'EXPERIMENT' => 'experiment', @@ -45,11 +48,12 @@ class DecisionService 'ROLLOUT' => 'rollout' }.freeze - def initialize(logger, user_profile_service = nil) + def initialize(logger, cmab_service, user_profile_service = nil) @logger = logger @user_profile_service = user_profile_service @bucketer = Bucketer.new(logger) @forced_variation_map = {} + @cmab_service = cmab_service end def get_variation(project_config, experiment_id, user_context, user_profile_tracker = nil, decide_options = [], reasons = []) @@ -467,6 +471,50 @@ def validated_forced_decision(project_config, context, user_context) private + def get_decision_for_cmab_experiment(project_config, experiment, user_context, bucketing_id, decide_options = []) + # Determines the CMAB (Contextual Multi-Armed Bandit) decision for a given experiment and user context. + # + # This method first checks if the user is bucketed into the CMAB experiment based on traffic allocation. + # If the user is not bucketed, it returns a CmabDecisionResult indicating exclusion. + # If the user is bucketed, it attempts to fetch a CMAB decision using the CMAB service. + # In case of errors during CMAB decision retrieval, it logs the error and returns a result indicating failure. + # + # @param project_config [ProjectConfig] The current project configuration. + # @param experiment [Hash] The experiment configuration hash. + # @param user_context [OptimizelyUserContext] The user context object containing user information. + # @param bucketing_id [String] The bucketing ID used for traffic allocation. + # @param decide_options [Array] Optional array of decision options. + # + # @return [CmabDecisionResult] The result of the CMAB decision process, including decision error status, decision data, and reasons. + decide_reasons = [] + user_id = user_context.user_id + + # Check if user is in CMAB traffic allocation + bucketed_entity_id, bucket_reasons = @bucketer.bucket_to_entity_id( + project_config, experiment, user_id, bucketing_id + ) + decide_reasons.extend(bucket_reasons) + unless bucketed_entity_id + message = "User \"#{user_context.user_id}\" not in CMAB experiment \"#{experiment['key']}\" due to traffic allocation." + @logger.log(Logger::INFO, message) + decide_reasons.push(message) + CmabDecisionResult.new(false, nil, decide_reasons) + end + + # User is in CMAB allocation, proceed to CMAB decision + begin + cmab_decision = @cmab_service.get_decision( + project_config, user_context, experiment['id'], decide_options + ) + CmabDecisionResult.new(false, cmab_decision, decide_reasons) + rescue StandardError => e + error_message = "Failed to fetch CMAB decision for experiment '#{experiment['key']}'" + decide_reasons.push(error_message) + @logger&.log(Logger::ERROR, "#{error_message} #{e}") + CmabDecisionResult.new(true, nil, decide_reasons) + end + end + def get_whitelisted_variation_id(project_config, experiment_id, user_id) # Determine if a user is whitelisted into a variation for the given experiment and return the ID of that variation # From 75ee816ed7a36afdf817ae491e37d4ec52046002 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 16 Jul 2025 19:24:12 +0600 Subject: [PATCH 09/15] update: Integrate CMAB decision logic into DecisionService and update related tests --- lib/optimizely/decision_service.rb | 81 ++++++----- spec/decision_service_spec.rb | 213 +++++++++++++++-------------- 2 files changed, 157 insertions(+), 137 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 80739ded..6f32a962 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -29,7 +29,8 @@ class DecisionService # 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 + # 6. Check cmab service + # 7. Use Murmurhash3 to bucket the user attr_reader :bucketer @@ -39,7 +40,7 @@ class DecisionService Decision = Struct.new(:experiment, :variation, :source, :cmab_uuid) CmabDecisionResult = Struct.new(:error, :result, :reasons) - VariationResult = Struct.new(:cmab_uuid, :error, :reasons, :variation) + VariationResult = Struct.new(:cmab_uuid, :error, :reasons, :variation_id) DecisionResult = Struct.new(:decision, :error, :reasons) DECISION_SOURCES = { @@ -77,25 +78,25 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac decide_reasons.push(*bucketing_id_reasons) # Check to make sure experiment is active experiment = project_config.get_experiment_from_id(experiment_id) - return nil, decide_reasons if experiment.nil? + return VariationResult.new(nil, false, decide_reasons, nil) if experiment.nil? experiment_key = experiment['key'] unless project_config.experiment_running?(experiment) message = "Experiment '#{experiment_key}' is not running." @logger.log(Logger::INFO, message) decide_reasons.push(message) - return nil, decide_reasons + return VariationResult.new(nil, false, decide_reasons, nil) end # Check if a forced variation is set for the user forced_variation, reasons_received = get_forced_variation(project_config, experiment['key'], user_id) decide_reasons.push(*reasons_received) - return forced_variation['id'], decide_reasons if forced_variation + return VariationResult.new(nil, false, decide_reasons, forced_variation['id']) if forced_variation # Check if user is in a white-listed variation whitelisted_variation_id, reasons_received = get_whitelisted_variation_id(project_config, experiment_id, user_id) decide_reasons.push(*reasons_received) - return whitelisted_variation_id, decide_reasons if whitelisted_variation_id + return VariationResult.new(nil, false, decide_reasons, whitelisted_variation_id) if whitelisted_variation_id should_ignore_user_profile_service = decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE # Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService @@ -106,7 +107,7 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac message = "Returning previously activated variation ID #{saved_variation_id} of experiment '#{experiment_key}' for user '#{user_id}' from user profile." @logger.log(Logger::INFO, message) decide_reasons.push(message) - return saved_variation_id, decide_reasons + return VariationResult.new(nil, false, decide_reasons, saved_variation_id) end end @@ -117,27 +118,43 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac message = "User '#{user_id}' does not meet the conditions to be in experiment '#{experiment_key}'." @logger.log(Logger::INFO, message) decide_reasons.push(message) - return nil, decide_reasons + return VariationResult.new(nil, false, decide_reasons, nil) end - # Bucket normally - variation, bucket_reasons = @bucketer.bucket(project_config, experiment, bucketing_id, user_id) - decide_reasons.push(*bucket_reasons) - variation_id = variation ? variation['id'] : nil + # Check if this is a CMAB experiment + # If so, handle CMAB-specific traffic allocation and decision logic. + # Otherwise, proceed with standard bucketing logic for non-CMAB experiments. + if experiment.key?('cmab') + cmab_decision_result = get_decision_for_cmab_experiment(project_config, experiment, user_context, bucketing_id, decide_options) + decide_reasons.push(*cmab_decision_result.reasons) + if cmab_decision_result.error + # CMAB decision failed, return error + return VariationResult.new(nil, true, decide_reasons, nil) + end - message = '' - if variation_id - variation_key = variation['key'] - message = "User '#{user_id}' is in variation '#{variation_key}' of experiment '#{experiment_id}'." + cmab_decision = cmab_decision_result.result + variation_id = cmab_decision&.variation_id + cmab_uuid = cmab_decision&.cmab_uuid else - message = "User '#{user_id}' is in no variation." + # Bucket normally + variation, bucket_reasons = @bucketer.bucket(project_config, experiment, bucketing_id, user_id) + decide_reasons.push(*bucket_reasons) + variation_id = variation ? variation['id'] : nil + cmab_uuid = nil + message = '' + if variation_id + variation_key = variation['key'] + message = "User '#{user_id}' is in variation '#{variation_key}' of experiment '#{experiment_id}'." + else + message = "User '#{user_id}' is in no variation." + end + @logger.log(Logger::INFO, message) + decide_reasons.push(message) end - @logger.log(Logger::INFO, message) - decide_reasons.push(message) # Persist bucketing decision user_profile_tracker.update_user_profile(experiment_id, variation_id) unless should_ignore_user_profile_service && user_profile_tracker - [variation_id, decide_reasons] + VariationResult.new(cmab_uuid, false, decide_reasons, variation_id) end def get_variation_for_feature(project_config, feature_flag, user_context, decide_options = []) @@ -203,7 +220,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont message = "The feature flag '#{feature_flag_key}' is not used in any experiments." @logger.log(Logger::DEBUG, message) decide_reasons.push(message) - return nil, decide_reasons + return DecisionResult.new(nil, false, decide_reasons) end # Evaluate each experiment and return the first bucketed experiment variation @@ -213,11 +230,15 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont message = "Feature flag experiment with ID '#{experiment_id}' is not in the datafile." @logger.log(Logger::DEBUG, message) decide_reasons.push(message) - return nil, decide_reasons + return DecisionResult.new(nil, false, decide_reasons) end experiment_id = experiment['id'] - variation_id, reasons_received = get_variation_from_experiment_rule(project_config, feature_flag_key, experiment, user_context, user_profile_tracker, decide_options) + variation_result = get_variation_from_experiment_rule(project_config, feature_flag_key, experiment, user_context, user_profile_tracker, decide_options) + error = variation_result.error + reasons_received = variation_result.reasons + variation_id = variation_result.variation_id + cmab_uuid = variation_result.cmab_uuid decide_reasons.push(*reasons_received) next unless variation_id @@ -225,14 +246,15 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) variation = project_config.get_variation_from_flag(feature_flag['key'], variation_id, 'id') if variation.nil? - return Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST']), decide_reasons + decision = Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST'], cmab_uuid) + return DecisionResult.new(decision, error, decide_reasons) end message = "The user '#{user_id}' is not bucketed into any of the experiments on the feature '#{feature_flag_key}'." @logger.log(Logger::INFO, message) decide_reasons.push(message) - [nil, decide_reasons] + DecisionResult.new(nil, false, decide_reasons) end def get_variation_for_feature_rollout(project_config, feature_flag, user_context) @@ -298,12 +320,9 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use variation, forced_reasons = validated_forced_decision(project_config, context, user) reasons.push(*forced_reasons) - return [variation['id'], reasons] if variation - - variation_id, response_reasons = get_variation(project_config, rule['id'], user, user_profile_tracker, options) - reasons.push(*response_reasons) + return VariationResult.new(nil, false, reasons, variation['id']) if variation - [variation_id, reasons] + get_variation(project_config, rule['id'], user, user_profile_tracker, options) end def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index, user_context) @@ -493,7 +512,7 @@ def get_decision_for_cmab_experiment(project_config, experiment, user_context, b bucketed_entity_id, bucket_reasons = @bucketer.bucket_to_entity_id( project_config, experiment, user_id, bucketing_id ) - decide_reasons.extend(bucket_reasons) + decide_reasons.push(*bucket_reasons) unless bucketed_entity_id message = "User \"#{user_context.user_id}\" not in CMAB experiment \"#{experiment['key']}\" due to traffic allocation." @logger.log(Logger::INFO, message) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index af22b18b..e8c01729 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -26,8 +26,9 @@ let(:error_handler) { Optimizely::NoOpErrorHandler.new } let(:spy_logger) { spy('logger') } let(:spy_user_profile_service) { spy('user_profile_service') } + let(:spy_cmab_service) { spy('cmab_service') } let(:config) { Optimizely::DatafileProjectConfig.new(config_body_JSON, spy_logger, error_handler) } - let(:decision_service) { Optimizely::DecisionService.new(spy_logger, spy_user_profile_service) } + let(:decision_service) { Optimizely::DecisionService.new(spy_logger, spy_cmab_service, spy_user_profile_service) } let(:project_instance) { Optimizely::Project.new(datafile: config_body_JSON, logger: spy_logger, error_handler: error_handler) } let(:user_context) { project_instance.create_user_context('some-user', {}) } after(:example) { project_instance.close } @@ -46,9 +47,9 @@ it 'should return the correct variation ID for a given user for whom a variation has been forced' do decision_service.set_forced_variation(config, 'test_experiment', 'test_user', 'variation') user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - expect(variation_received).to eq('111129') - expect(reasons).to eq(["Variation 'variation' is mapped to experiment '111127' and user 'test_user' in the forced variation map"]) + variation_result = decision_service.get_variation(config, '111127', user_context) + expect(variation_result.variation_id).to eq('111129') + expect(variation_result.reasons).to eq(["Variation 'variation' is mapped to experiment '111127' and user 'test_user' in the forced variation map"]) # 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) @@ -62,9 +63,9 @@ } decision_service.set_forced_variation(config, 'test_experiment_with_audience', 'test_user', 'control_with_audience') user_context = project_instance.create_user_context('test_user', user_attributes) - variation_received, reasons = decision_service.get_variation(config, '122227', user_context) - expect(variation_received).to eq('122228') - expect(reasons).to eq(["Variation 'control_with_audience' is mapped to experiment '122227' and user 'test_user' in the forced variation map"]) + variation_result = decision_service.get_variation(config, '122227', user_context) + expect(variation_result.variation_id).to eq('122228') + expect(variation_result.reasons).to eq(["Variation 'control_with_audience' is mapped to experiment '122227' and user 'test_user' in the forced variation map"]) # 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) @@ -74,13 +75,13 @@ it 'should return the correct variation ID for a given user ID and key of a running experiment' do user_context = project_instance.create_user_context('test_user') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq('111128') + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq('111128') - expect(reasons).to eq([ - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in variation 'control' of experiment '111127'." - ]) + expect(variation_result.reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'test_user' is in variation 'control' of experiment '111127'.") @@ -92,12 +93,12 @@ allow(decision_service.bucketer).to receive(:bucket).and_return(nil) user_context = project_instance.create_user_context('test_user') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq(nil) - expect(reasons).to eq([ - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in no variation." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq(nil) + expect(variation_result.reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in no variation." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'test_user' is in no variation.") @@ -105,20 +106,20 @@ it 'should return correct variation ID if user ID is in whitelisted Variations and variation is valid' do user_context = project_instance.create_user_context('forced_user1') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "User 'forced_user1' is whitelisted into variation 'control' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "User 'forced_user1' is whitelisted into variation 'control' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user1' is whitelisted into variation 'control' of experiment '111127'.") user_context = project_instance.create_user_context('forced_user2') - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - expect(variation_received).to eq('111129') - expect(reasons).to eq([ - "User 'forced_user2' is whitelisted into variation 'variation' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context) + expect(variation_result.variation_id).to eq('111129') + expect(variation_result.reasons).to eq([ + "User 'forced_user2' is whitelisted into variation 'variation' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user2' is whitelisted into variation 'variation' of experiment '111127'.") @@ -135,20 +136,20 @@ } user_context = project_instance.create_user_context('forced_user1', user_attributes) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "User 'forced_user1' is whitelisted into variation 'control' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "User 'forced_user1' is whitelisted into variation 'control' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user1' is whitelisted into variation 'control' of experiment '111127'.") user_context = project_instance.create_user_context('forced_user2', user_attributes) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context) - expect(variation_received).to eq('111129') - expect(reasons).to eq([ - "User 'forced_user2' is whitelisted into variation 'variation' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context) + expect(variation_result.variation_id).to eq('111129') + expect(variation_result.reasons).to eq([ + "User 'forced_user2' is whitelisted into variation 'variation' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_user2' is whitelisted into variation 'variation' of experiment '111127'.") @@ -161,11 +162,11 @@ 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'} user_context = project_instance.create_user_context('forced_audience_user', user_attributes) - variation_received, reasons = decision_service.get_variation(config, '122227', user_context) - expect(variation_received).to eq('122229') - expect(reasons).to eq([ - "User 'forced_audience_user' is whitelisted into variation 'variation_with_audience' of experiment '122227'." - ]) + variation_result = decision_service.get_variation(config, '122227', user_context) + expect(variation_result.variation_id).to eq('122229') + expect(variation_result.reasons).to eq([ + "User 'forced_audience_user' is whitelisted into variation 'variation_with_audience' of experiment '122227'." + ]) expect(spy_logger).to have_received(:log) .once.with( Logger::INFO, @@ -180,9 +181,9 @@ it 'should return nil if the experiment key is invalid' do user_context = project_instance.create_user_context('test_user', {}) - variation_received, reasons = decision_service.get_variation(config, 'totally_invalid_experiment', user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq([]) + variation_result = decision_service.get_variation(config, 'totally_invalid_experiment', user_context) + expect(variation_result.variation_id).to eq(nil) + expect(variation_result.reasons).to eq([]) expect(spy_logger).to have_received(:log) .once.with(Logger::ERROR, "Experiment id 'totally_invalid_experiment' is not in datafile.") @@ -192,14 +193,14 @@ user_attributes = {'browser_type' => 'chrome'} user_context = project_instance.create_user_context('test_user', user_attributes) user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation(config, '122227', user_context, user_profile_tracker) - expect(variation_received).to eq(nil) - expect(reasons).to eq([ - "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", - "Audience '11154' evaluated to FALSE.", - "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE.", - "User 'test_user' does not meet the conditions to be in experiment 'test_experiment_with_audience'." - ]) + variation_result = decision_service.get_variation(config, '122227', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq(nil) + expect(variation_result.reasons).to eq([ + "Starting to evaluate audience '11154' with conditions: [\"and\", [\"or\", [\"or\", {\"name\": \"browser_type\", \"type\": \"custom_attribute\", \"value\": \"firefox\"}]]].", + "Audience '11154' evaluated to FALSE.", + "Audiences for experiment 'test_experiment_with_audience' collectively evaluated to FALSE.", + "User 'test_user' does not meet the conditions to be in experiment 'test_experiment_with_audience'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'test_user' does not meet the conditions to be in experiment 'test_experiment_with_audience'.") @@ -211,9 +212,9 @@ it 'should return nil if the given experiment is not running' do user_context = project_instance.create_user_context('test_user') - variation_received, reasons = decision_service.get_variation(config, '100027', user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["Experiment 'test_experiment_not_started' is not running."]) + variation_result = decision_service.get_variation(config, '100027', user_context) + expect(variation_result.variation_id).to eq(nil) + expect(variation_result.reasons).to eq(["Experiment 'test_experiment_not_started' is not running."]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "Experiment 'test_experiment_not_started' is not running.") @@ -227,11 +228,11 @@ it 'should respect forced variations within mutually exclusive grouped experiments' do user_context = project_instance.create_user_context('forced_group_user1') - variation_received, reasons = decision_service.get_variation(config, '133332', user_context) - expect(variation_received).to eq('130004') - expect(reasons).to eq([ - "User 'forced_group_user1' is whitelisted into variation 'g1_e2_v2' of experiment '133332'." - ]) + variation_result = decision_service.get_variation(config, '133332', user_context) + expect(variation_result.variation_id).to eq('130004') + expect(variation_result.reasons).to eq([ + "User 'forced_group_user1' is whitelisted into variation 'g1_e2_v2' of experiment '133332'." + ]) expect(spy_logger).to have_received(:log) .once.with(Logger::INFO, "User 'forced_group_user1' is whitelisted into variation 'g1_e2_v2' of experiment '133332'.") @@ -244,13 +245,13 @@ it 'should bucket normally if user is whitelisted into a forced variation that is not in the datafile' do user_context = project_instance.create_user_context('forced_user_with_invalid_variation') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "User 'forced_user_with_invalid_variation' is whitelisted into variation 'invalid_variation', which is not in the datafile.", - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'forced_user_with_invalid_variation' is in variation 'control' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "User 'forced_user_with_invalid_variation' is whitelisted into variation 'invalid_variation', which is not in the datafile.", + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'forced_user_with_invalid_variation' is in variation 'control' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log) .once.with( Logger::INFO, @@ -270,12 +271,12 @@ } user_context = project_instance.create_user_context('test_user', user_attributes) user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq('111129') - expect(reasons).to eq([ - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in variation 'variation' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq('111129') + expect(variation_result.reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'variation' of experiment '111127'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -296,11 +297,11 @@ user_context = project_instance.create_user_context('test_user') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) user_profile_tracker.load_user_profile - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq('111129') - expect(reasons).to eq([ - "Returning previously activated variation ID 111129 of experiment 'test_experiment' for user 'test_user' from user profile." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq('111129') + expect(variation_result.reasons).to eq([ + "Returning previously activated variation ID 111129 of experiment 'test_experiment' for user 'test_user' from user profile." + ]) expect(spy_logger).to have_received(:log).once .with(Logger::INFO, "Returning previously activated variation ID 111129 of experiment 'test_experiment' for user 'test_user' from user profile.") @@ -328,12 +329,12 @@ user_context = project_instance.create_user_context('test_user') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) user_profile_tracker.load_user_profile - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in variation 'control' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment '111127'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -355,13 +356,13 @@ user_context = project_instance.create_user_context('test_user') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) user_profile_tracker.load_user_profile - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "User 'test_user' was previously bucketed into variation ID '111111' for experiment '111127', but no matching variation was found. Re-bucketing user.", - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in variation 'control' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "User 'test_user' was previously bucketed into variation ID '111111' for experiment '111127', but no matching variation was found. Re-bucketing user.", + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment '111127'." + ]) # bucketing should have occurred expect(decision_service.bucketer).to have_received(:bucket).once @@ -373,13 +374,13 @@ user_context = project_instance.create_user_context('test_user') user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) user_profile_tracker.load_user_profile - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker) user_profile_tracker.save_user_profile - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in variation 'control' of experiment '111127'." - ]) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment '111127'." + ]) expect(spy_logger).to have_received(:log).once .with(Logger::ERROR, "Error while looking up user profile for user ID 'test_user': uncaught throw :LookupError.") @@ -395,12 +396,12 @@ user_context = project_instance.create_user_context('test_user', nil) user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, spy_user_profile_service, spy_logger) user_profile_tracker.load_user_profile - variation_received, reasons = decision_service.get_variation(config, '111127', user_context, user_profile_tracker, [Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE]) - expect(variation_received).to eq('111128') - expect(reasons).to eq([ - "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", - "User 'test_user' is in variation 'control' of experiment '111127'." - ]) + variation_result = decision_service.get_variation(config, '111127', user_context, user_profile_tracker, [Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE]) + expect(variation_result.variation_id).to eq('111128') + expect(variation_result.reasons).to eq([ + "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.", + "User 'test_user' is in variation 'control' of experiment '111127'." + ]) expect(decision_service.bucketer).to have_received(:bucket) expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?) From f48dbc231ed13e7fa6071f3bea2f3e7f216f31cd Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 18 Jul 2025 19:53:20 +0600 Subject: [PATCH 10/15] update: Refactor DecisionService to return DecisionResult struct instead of Decision struct --- lib/optimizely/decision_service.rb | 37 +++---- spec/decision_service_spec.rb | 157 +++++++++++++++-------------- 2 files changed, 96 insertions(+), 98 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 6f32a962..56fa922e 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -164,7 +164,7 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide # feature_flag - The feature flag the user wants to access # user_context - Optimizely user context instance # - # Returns Decision struct (nil if the user is not bucketed into any of the experiments on the feature) + # Returns DecisionResult struct. get_variations_for_feature_list(project_config, [feature_flag], user_context, decide_options).first end @@ -178,7 +178,7 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, # decide_options: Decide options. # # Returns: - # Array of Decision struct. + # Array of DecisionResult struct. ignore_ups = decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE user_profile_tracker = nil unless ignore_ups && @user_profile_service @@ -187,18 +187,10 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, end decisions = [] feature_flags.each do |feature_flag| - decide_reasons = [] # check if the feature is being experiment on and whether the user is bucketed into the experiment - decision, reasons_received = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options) - decide_reasons.push(*reasons_received) - if decision - decisions << [decision, decide_reasons] - else - # Proceed to rollout if the decision is nil - rollout_decision, reasons_received = get_variation_for_feature_rollout(project_config, feature_flag, user_context) - decide_reasons.push(*reasons_received) - decisions << [rollout_decision, decide_reasons] - end + decision_result = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options) + decision_result = get_variation_for_feature_rollout(project_config, feature_flag, user_context) unless decision_result.decision + decisions << decision_result end user_profile_tracker&.save_user_profile decisions @@ -211,8 +203,8 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont # feature_flag - The feature flag the user wants to access # user_context - Optimizely user context instance # - # Returns Decision struct (nil if the user is not bucketed into any of the experiments on the feature) - # or nil if the user is not bucketed into any of the experiments on the feature + # Returns a DecisionResult containing the decision (or nil if not bucketed), + # an error flag, and an array of decision reasons. decide_reasons = [] user_id = user_context.user_id feature_flag_key = feature_flag['key'] @@ -265,7 +257,8 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context # feature_flag - The feature flag the user wants to access # user_context - Optimizely user context instance # - # Returns the Decision struct or nil if not bucketed into any of the targeting rules + # Returns a DecisionResult containing the decision (or nil if not bucketed), + # an error flag, and an array of decision reasons. decide_reasons = [] rollout_id = feature_flag['rolloutId'] @@ -274,7 +267,7 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context message = "Feature flag '#{feature_flag_key}' is not used in a rollout." @logger.log(Logger::DEBUG, message) decide_reasons.push(message) - return nil, decide_reasons + return DecisionResult.new(nil, false, decide_reasons) end rollout = project_config.get_rollout_from_id(rollout_id) @@ -282,10 +275,10 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context message = "Rollout with ID '#{rollout_id}' is not in the datafile '#{feature_flag['key']}'" @logger.log(Logger::DEBUG, message) decide_reasons.push(message) - return nil, decide_reasons + return DecisionResult.new(nil, false, decide_reasons) end - return nil, decide_reasons if rollout['experiments'].empty? + return DecisionResult.new(nil, false, decide_reasons) if rollout['experiments'].empty? index = 0 rollout_rules = rollout['experiments'] @@ -294,14 +287,14 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context decide_reasons.push(*reasons_received) if variation rule = rollout_rules[index] - feature_decision = Decision.new(rule, variation, DECISION_SOURCES['ROLLOUT']) - return [feature_decision, decide_reasons] + feature_decision = Decision.new(rule, variation, DECISION_SOURCES['ROLLOUT'], nil) + return DecisionResult.new(feature_decision, false, decide_reasons) end index = skip_to_everyone_else ? (rollout_rules.length - 1) : (index + 1) end - [nil, decide_reasons] + DecisionResult.new(nil, false, decide_reasons) end def get_variation_from_experiment_rule(project_config, flag_key, rule, user, user_profile_tracker, options = []) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index e8c01729..e6bd7d58 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -418,9 +418,9 @@ describe 'when the feature flag\'s experiment ids array is empty' do it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['empty_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["The feature flag 'empty_feature' is not used in any experiments."]) + decision_result = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq(["The feature flag 'empty_feature' is not used in any experiments."]) expect(spy_logger).to have_received(:log).once .with(Logger::DEBUG, "The feature flag 'empty_feature' is not used in any experiments.") @@ -433,9 +433,9 @@ # any string that is not an experiment id in the data file feature_flag['experimentIds'] = ['1333333337'] user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["Feature flag experiment with ID '1333333337' is not in the datafile."]) + decision_result = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq(["Feature flag experiment with ID '1333333337' is not in the datafile."]) expect(spy_logger).to have_received(:log).once .with(Logger::DEBUG, "Feature flag experiment with ID '1333333337' is not in the datafile.") end @@ -449,14 +449,14 @@ # make sure the user is not bucketed into the feature experiment allow(decision_service).to receive(:get_variation) .with(config, multivariate_experiment['id'], user_context, user_profile_tracker, []) - .and_return([nil, nil]) + .and_return(Optimizely::DecisionService::VariationResult.new(nil, false, [], nil)) end it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['multi_variate_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker, []) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["The user 'user_1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'."]) + decision_result = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker, []) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq(["The user 'user_1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'."]) expect(spy_logger).to have_received(:log).once .with(Logger::INFO, "The user 'user_1' is not bucketed into any of the experiments on the feature 'multi_variate_feature'.") @@ -466,7 +466,7 @@ describe 'and the user is bucketed into a variation for the experiment on the feature flag' do before(:each) do # mock and return the first variation of the `test_experiment_multivariate` experiment, which is attached to the `multi_variate_feature` - allow(decision_service).to receive(:get_variation).and_return('122231') + allow(decision_service).to receive(:get_variation).and_return(Optimizely::DecisionService::VariationResult.new(nil, false, [], '122231')) end it 'should return the variation' do @@ -476,10 +476,15 @@ config.variation_id_map['test_experiment_multivariate']['122231'], Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + expected_decision_result = Optimizely::DecisionService::DecisionResult.new( + expected_decision, + false, + [] + ) user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) - expect(variation_received).to eq(expected_decision) - expect(reasons).to eq([]) + decision_result = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) + expect(decision_result).to eq(expected_decision_result) + expect(decision_result.reasons).to eq([]) end end end @@ -497,15 +502,15 @@ Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) allow(decision_service).to receive(:get_variation) - .and_return(variation['id']) + .and_return(Optimizely::DecisionService::VariationResult.new(nil, false, [], variation['id'])) end it 'should return the variation the user is bucketed into' do feature_flag = config.feature_flag_key_map['mutex_group_feature'] user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id) - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) - expect(variation_received).to eq(expected_decision) - expect(reasons).to eq([]) + decision_result = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) + expect(decision_result.decision).to eq(expected_decision) + expect(decision_result.reasons).to eq([]) end end @@ -516,17 +521,17 @@ mutex_exp2 = config.experiment_key_map['group1_exp2'] allow(decision_service).to receive(:get_variation) .with(config, mutex_exp['id'], user_context, user_profile_tracker, []) - .and_return([nil, nil]) + .and_return(Optimizely::DecisionService::VariationResult.new(nil, false, [], nil)) allow(decision_service).to receive(:get_variation) .with(config, mutex_exp2['id'], user_context, user_profile_tracker, []) - .and_return([nil, nil]) + .and_return(Optimizely::DecisionService::VariationResult.new(nil, false, [], nil)) end it 'should return nil and log a message' do feature_flag = config.feature_flag_key_map['mutex_group_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["The user 'user_1' is not bucketed into any of the experiments on the feature 'mutex_group_feature'."]) + decision_result = decision_service.get_variation_for_feature_experiment(config, feature_flag, user_context, user_profile_tracker) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq(["The user 'user_1' is not bucketed into any of the experiments on the feature 'mutex_group_feature'."]) expect(spy_logger).to have_received(:log).once .with(Logger::INFO, "The user 'user_1' is not bucketed into any of the experiments on the feature 'mutex_group_feature'.") @@ -544,9 +549,9 @@ describe 'when the feature flag is not associated with a rollout' do it 'should log a message and return nil' do feature_flag = config.feature_flag_key_map['boolean_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["Feature flag '#{feature_flag['key']}' is not used in a rollout."]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq(["Feature flag '#{feature_flag['key']}' is not used in a rollout."]) expect(spy_logger).to have_received(:log).once .with(Logger::DEBUG, "Feature flag '#{feature_flag['key']}' is not used in a rollout.") end @@ -556,9 +561,9 @@ it 'should log a message and return nil' do feature_flag = config.feature_flag_key_map['boolean_feature'].dup feature_flag['rolloutId'] = 'invalid_rollout_id' - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq(["Rollout with ID 'invalid_rollout_id' is not in the datafile 'boolean_feature'"]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq(["Rollout with ID 'invalid_rollout_id' is not in the datafile 'boolean_feature'"]) expect(spy_logger).to have_received(:log).once .with(Logger::ERROR, "Rollout with ID 'invalid_rollout_id' is not in the datafile.") @@ -571,9 +576,9 @@ experimentless_rollout['experiments'] = [] allow(config).to receive(:get_rollout_from_id).and_return(experimentless_rollout) feature_flag = config.feature_flag_key_map['boolean_single_variable_feature'] - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq([]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq([]) end end @@ -588,10 +593,10 @@ allow(decision_service.bucketer).to receive(:bucket) .with(config, rollout_experiment, user_id, user_id) .and_return(variation) - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(expected_decision) - expect(reasons).to eq(["User 'user_1' meets the audience conditions for targeting rule '1'.", - "User 'user_1' is in the traffic group of targeting rule '1'."]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(expected_decision) + expect(decision_result.reasons).to eq(["User 'user_1' meets the audience conditions for targeting rule '1'.", + "User 'user_1' is in the traffic group of targeting rule '1'."]) end end @@ -610,13 +615,13 @@ .with(config, everyone_else_experiment, user_id, user_id) .and_return(nil) - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq([ - "User 'user_1' meets the audience conditions for targeting rule '1'.", - "User 'user_1' is not in the traffic group for targeting rule '1'.", - "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'." - ]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq([ + "User 'user_1' meets the audience conditions for targeting rule '1'.", + "User 'user_1' is not in the traffic group for targeting rule '1'.", + "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'." + ]) # make sure we only checked the audience for the first rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -641,14 +646,14 @@ .with(config, everyone_else_experiment, user_id, user_id) .and_return(variation) - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(expected_decision) - expect(reasons).to eq([ - "User 'user_1' meets the audience conditions for targeting rule '1'.", - "User 'user_1' is not in the traffic group for targeting rule '1'.", - "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'.", - "User 'user_1' is in the traffic group of targeting rule 'Everyone Else'." - ]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(expected_decision) + expect(decision_result.reasons).to eq([ + "User 'user_1' meets the audience conditions for targeting rule '1'.", + "User 'user_1' is not in the traffic group for targeting rule '1'.", + "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'.", + "User 'user_1' is in the traffic group of targeting rule 'Everyone Else'." + ]) # make sure we only checked the audience for the first rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -676,14 +681,14 @@ .with(config, everyone_else_experiment, user_id, user_id) .and_return(variation) - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(expected_decision) - expect(reasons).to eq([ - "User 'user_1' does not meet the conditions for targeting rule '1'.", - "User 'user_1' does not meet the conditions for targeting rule '2'.", - "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'.", - "User 'user_1' is in the traffic group of targeting rule 'Everyone Else'." - ]) + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(expected_decision) + expect(decision_result.reasons).to eq([ + "User 'user_1' does not meet the conditions for targeting rule '1'.", + "User 'user_1' does not meet the conditions for targeting rule '2'.", + "User 'user_1' meets the audience conditions for targeting rule 'Everyone Else'.", + "User 'user_1' is in the traffic group of targeting rule 'Everyone Else'." + ]) # verify we tried to bucket in all targeting rules and the everyone else rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).exactly(3).times @@ -706,9 +711,9 @@ expect(decision_service.bucketer).not_to receive(:bucket) .with(config, everyone_else_experiment, user_id, user_id) - variation_received, reasons = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) - expect(variation_received).to eq(nil) - expect(reasons).to eq([ + decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq([ "User 'user_1' does not meet the conditions for targeting rule '1'.", "User 'user_1' does not meet the conditions for targeting rule '2'.", "User 'user_1' does not meet the conditions for targeting rule 'Everyone Else'." @@ -746,11 +751,11 @@ 'experiment' => expected_experiment, 'variation' => expected_variation } - allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return([expected_decision, nil]) + allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return(Optimizely::DecisionService::DecisionResult.new(expected_decision, false, [])) - decision_received, reasons = decision_service.get_variation_for_feature(config, feature_flag, user_context) - expect(decision_received).to eq(expected_decision) - expect(reasons).to eq([]) + decision_result = decision_service.get_variation_for_feature(config, feature_flag, user_context) + expect(decision_result.decision).to eq(expected_decision) + expect(decision_result.reasons).to eq([]) end end @@ -765,24 +770,24 @@ variation, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return([nil, nil]) - allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return([expected_decision, nil]) + allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return(Optimizely::DecisionService::DecisionResult.new(expected_decision, false, [])) - decision_received, reasons = decision_service.get_variation_for_feature(config, feature_flag, user_context) - expect(decision_received).to eq(expected_decision) - expect(reasons).to eq([]) + decision_result = decision_service.get_variation_for_feature(config, feature_flag, user_context) + expect(decision_result.decision).to eq(expected_decision) + expect(decision_result.reasons).to eq([]) end end describe 'and the user is not bucketed into the feature rollout' do it 'should log a message and return nil' do feature_flag = config.feature_flag_key_map['string_single_variable_feature'] - allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return([nil, nil]) - allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return([nil, nil]) + allow(decision_service).to receive(:get_variation_for_feature_experiment).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + allow(decision_service).to receive(:get_variation_for_feature_rollout).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) - decision_received, reasons = decision_service.get_variation_for_feature(config, feature_flag, user_context) - expect(decision_received).to eq(nil) - expect(reasons).to eq([]) + decision_result = decision_service.get_variation_for_feature(config, feature_flag, user_context) + expect(decision_result.decision).to eq(nil) + expect(decision_result.reasons).to eq([]) end end end From 0e9e4f8ff6657def8aceb43fed68d7c209caca3b Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 23 Jul 2025 14:26:14 +0600 Subject: [PATCH 11/15] update: Integrate CMAB components into Project class and enhance decision handling --- lib/optimizely.rb | 45 ++++- lib/optimizely/decide/optimizely_decision.rb | 19 ++ lib/optimizely/decision_service.rb | 18 +- spec/optimizely_user_context_spec.rb | 1 + spec/project_spec.rb | 188 ++++++++++++------- 5 files changed, 188 insertions(+), 83 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index f2e1dd82..6eb5bf86 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -43,11 +43,17 @@ require_relative 'optimizely/odp/odp_manager' require_relative 'optimizely/helpers/sdk_settings' require_relative 'optimizely/user_profile_tracker' +require_relative 'optimizely/cmab/cmab_client' +require_relative 'optimizely/cmab/cmab_service' module Optimizely class Project include Optimizely::Decide + # CMAB Constants + DEFAULT_CMAB_CACHE_TIMEOUT = (30 * 60 * 1000) + DEFAULT_CMAB_CACHE_SIZE = 1000 + attr_reader :notification_center # @api no-doc attr_reader :config_manager, :decision_service, :error_handler, :event_dispatcher, @@ -131,7 +137,19 @@ def initialize( setup_odp!(@config_manager.sdk_key) - @decision_service = DecisionService.new(@logger, @user_profile_service) + # Initialize CMAB components + @cmab_client = DefaultCmabClient.new( + retry_config: CmabRetryConfig.new, + logger: @logger + ) + @cmab_cache = LRUCache.new(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT) + @cmab_service = DefaultCmabService.new( + @cmab_cache, + @cmab_client, + @logger + ) + + @decision_service = DecisionService.new(@logger, @cmab_service, @user_profile_service) @event_processor = if event_processor.respond_to?(:process) event_processor @@ -358,9 +376,17 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti decision_list = @decision_service.get_variations_for_feature_list(config, flags_without_forced_decision, user_context, decide_options) flags_without_forced_decision.each_with_index do |flag, i| - decision = decision_list[i][0] - reasons = decision_list[i][1] + decision = decision_list[i].decision + reasons = decision_list[i].reasons + error = decision_list[i].error flag_key = flag['key'] + # store error decision against key and remove key from valid keys + if error + optimizely_decision = OptimizelyDecision.new_error_decision(flag_key, user_context, reasons) + decisions[flag_key] = optimizely_decision + valid_keys.delete(flag_key) if valid_keys.include?(flag_key) + next + end flag_decisions[flag_key] = decision decision_reasons_dict[flag_key] ||= [] decision_reasons_dict[flag_key].push(*reasons) @@ -599,8 +625,8 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil) end user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false) - decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context) - + decision_result = @decision_service.get_variation_for_feature(config, feature_flag, user_context) + decision = decision_result.decision feature_enabled = false source_string = Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] if decision.is_a?(Optimizely::DecisionService::Decision) @@ -839,7 +865,8 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil) end user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false) - decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context) + decision_result = @decision_service.get_variation_for_feature(config, feature_flag, user_context) + decision = decision_result.decision variation = decision ? decision['variation'] : nil feature_enabled = variation ? variation['featureEnabled'] : false all_variables = {} @@ -1029,7 +1056,8 @@ def get_variation_with_config(experiment_key, user_id, attributes, config) user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false) user_profile_tracker = UserProfileTracker.new(user_id, @user_profile_service, @logger) user_profile_tracker.load_user_profile - variation_id, = @decision_service.get_variation(config, experiment_id, user_context, user_profile_tracker) + variation_result = @decision_service.get_variation(config, experiment_id, user_context, user_profile_tracker) + variation_id = variation_result.variation_id user_profile_tracker.save_user_profile variation = config.get_variation_from_id(experiment_key, variation_id) unless variation_id.nil? variation_key = variation['key'] if variation @@ -1097,7 +1125,8 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type, end user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false) - decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context) + decision_result = @decision_service.get_variation_for_feature(config, feature_flag, user_context) + decision = decision_result.decision variation = decision ? decision['variation'] : nil feature_enabled = variation ? variation['featureEnabled'] : false diff --git a/lib/optimizely/decide/optimizely_decision.rb b/lib/optimizely/decide/optimizely_decision.rb index 06b109b3..ea1964d3 100644 --- a/lib/optimizely/decide/optimizely_decision.rb +++ b/lib/optimizely/decide/optimizely_decision.rb @@ -55,6 +55,25 @@ def as_json def to_json(*args) as_json.to_json(*args) end + + # Create a new OptimizelyDecision representing an error state. + # + # @param key [String] The flag key + # @param user [OptimizelyUserContext] The user context + # @param reasons [Array] List of reasons explaining the error + # + # @return [OptimizelyDecision] OptimizelyDecision with error state values + def self.new_error_decision(key, user, reasons = []) + new( + variation_key: nil, + enabled: false, + variables: {}, + rule_key: nil, + flag_key: key, + user_context: user, + reasons: reasons + ) + end end end end diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 56fa922e..55bc73c0 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -66,8 +66,7 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac # user_profile_tracker: Tracker for reading and updating user profile of the user. # reasons: Decision reasons. # - # Returns variation ID where visitor will be bucketed - # (nil if experiment is inactive or user does not meet audience conditions) + # Returns VariationResult struct user_profile_tracker = UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) unless user_profile_tracker.is_a?(Optimizely::UserProfileTracker) decide_reasons = [] decide_reasons.push(*reasons) @@ -189,7 +188,12 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, feature_flags.each do |feature_flag| # check if the feature is being experiment on and whether the user is bucketed into the experiment decision_result = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options) - decision_result = get_variation_for_feature_rollout(project_config, feature_flag, user_context) unless decision_result.decision + # Only process rollout if no experiment decision was found and no error + if decision_result.decision.nil? && !decision_result.error + decision_result_rollout = get_variation_for_feature_rollout(project_config, feature_flag, user_context) unless decision_result.decision + decision_result.decision = decision_result_rollout.decision + decision_result.reasons.push(*decision_result_rollout.reasons) + end decisions << decision_result end user_profile_tracker&.save_user_profile @@ -232,7 +236,8 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont variation_id = variation_result.variation_id cmab_uuid = variation_result.cmab_uuid decide_reasons.push(*reasons_received) - + puts 'final reasons' + puts decide_reasons next unless variation_id variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) @@ -312,10 +317,11 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(flag_key, rule['key']) variation, forced_reasons = validated_forced_decision(project_config, context, user) reasons.push(*forced_reasons) - return VariationResult.new(nil, false, reasons, variation['id']) if variation - get_variation(project_config, rule['id'], user, user_profile_tracker, options) + variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options) + variation_result.reasons = reasons + variation_result.reasons + variation_result end def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index, user_context) diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb index 515068c0..42d71065 100644 --- a/spec/optimizely_user_context_spec.rb +++ b/spec/optimizely_user_context_spec.rb @@ -556,6 +556,7 @@ decision = user_context_obj.decide(feature_key, [Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS]) expect(decision.variation_key).to eq('18257766532') expect(decision.rule_key).to eq('18322080788') + # puts decision.reasons expect(decision.reasons).to include('Invalid variation is mapped to flag (feature_1), rule (exp_with_audience) and user (tester) in the forced decision map.') # delivery-rule-to-decision diff --git a/spec/project_spec.rb b/spec/project_spec.rb index f857a5ce..feb205a3 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -1681,7 +1681,7 @@ def callback(_args); end it 'should return false and send an impression when the user is not bucketed into any variation' do allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.is_feature_enabled('multi_variate_feature', 'test_user')).to be(false) @@ -1703,7 +1703,7 @@ def callback(_args); end ) allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) expect(project_instance.is_feature_enabled('boolean_single_variable_feature', 'test_user')).to be true @@ -1723,7 +1723,7 @@ def callback(_args); end Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) expect(variation_to_return['featureEnabled']).to be false expect(project_instance.is_feature_enabled('boolean_single_variable_feature', 'test_user')).to be false @@ -1744,7 +1744,7 @@ def callback(_args); end Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) expect(variation_to_return['featureEnabled']).to be true expect(project_instance.is_feature_enabled('boolean_single_variable_feature', 'test_user')).to be true @@ -1841,7 +1841,7 @@ def callback(_args); end Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], any_args ).ordered - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) expect(project_instance.is_feature_enabled('multi_variate_feature', 'test_user')).to be true @@ -1862,7 +1862,7 @@ def callback(_args); end Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) expect(variation_to_return['featureEnabled']).to be false - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) expect(project_instance.is_feature_enabled('multi_variate_feature', 'test_user')).to be false @@ -1888,7 +1888,7 @@ def callback(_args); end Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) # Activate listener expect(project_instance.notification_center).to receive(:send_notifications).once.with( @@ -1925,7 +1925,7 @@ def callback(_args); end Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args @@ -1959,7 +1959,7 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) # DECISION listener called when the user is in rollout with variation feature true. expect(variation_to_return['featureEnabled']).to be true @@ -1983,8 +1983,21 @@ def callback(_args); end end it 'should call decision listener when user is bucketed into rollout with featureEnabled property is false' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::Decision) - + experiment_to_return = config_body['rollouts'][0]['experiments'][1] + variation_to_return = experiment_to_return['variations'][0] + decision_to_return = Optimizely::DecisionService::Decision.new( + experiment_to_return, + variation_to_return, + Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + ) + # Ensure featureEnabled is false for this test + expect(variation_to_return['featureEnabled']).to be false + + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) + + expect(project_instance.notification_center).to receive(:send_notifications).once.with( + Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args + ).ordered # DECISION listener called when the user is in rollout with variation feature off. expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -1999,7 +2012,7 @@ def callback(_args); end end it 'call decision listener when the user is not bucketed into any experiment or rollout' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args ).ordered @@ -2113,26 +2126,42 @@ def callback(_args); end rollout_to_return = config_body['rollouts'][0]['experiments'][0] allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return( - Optimizely::DecisionService::Decision.new( - experiment_to_return, - experiment_to_return['variations'][0], - Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + experiment_to_return, + experiment_to_return['variations'][0], + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ), false, [] ), - nil, - Optimizely::DecisionService::Decision.new( - rollout_to_return, - rollout_to_return['variations'][0], - Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + Optimizely::DecisionService::DecisionResult.new( + nil, false, [] ), - Optimizely::DecisionService::Decision.new( - experiment_to_return, - experiment_to_return['variations'][1], - Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + rollout_to_return, + rollout_to_return['variations'][0], + Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] + ), false, [] + ), + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + experiment_to_return, + experiment_to_return['variations'][1], + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] + ), false, [] + ), + Optimizely::DecisionService::DecisionResult.new( + nil, false, [] + ), + Optimizely::DecisionService::DecisionResult.new( + nil, false, [] + ), + Optimizely::DecisionService::DecisionResult.new( + nil, false, [] ), - nil, - nil, - nil, - nil + Optimizely::DecisionService::DecisionResult.new( + nil, false, [] + ) ) expect(project_instance.notification_center).to receive(:send_notifications).exactly(10).times.with( @@ -2274,7 +2303,8 @@ def callback(_args); end 'experiment' => nil, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_string('string_single_variable_feature', 'string_variable', user_id, user_attributes)) .to eq('wingardium leviosa') @@ -2294,7 +2324,8 @@ def callback(_args); end 'experiment' => nil, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_string('boolean_single_variable_feature', 'boolean_variable', user_id, user_attributes)) .to eq(nil) @@ -2315,7 +2346,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_string('integer_single_variable_feature', 'integer_variable', user_id, user_attributes)) .to eq(nil) @@ -2334,7 +2366,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_string('string_single_variable_feature', 'string_variable', user_id, user_attributes)) .to eq('cta_1') @@ -2351,7 +2384,7 @@ def callback(_args); end describe 'when the feature flag is not enabled for the user' do it 'should return the default variable value' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.get_feature_variable_string('string_single_variable_feature', 'string_variable', user_id, user_attributes)) .to eq('wingardium leviosa') @@ -2424,7 +2457,8 @@ def callback(_args); end 'experiment' => nil, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -2496,7 +2530,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -2525,7 +2560,7 @@ def callback(_args); end describe 'when the feature flag is not enabled for the user' do it 'should return the default variable value' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -2608,7 +2643,8 @@ def callback(_args); end 'experiment' => nil, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_boolean('boolean_single_variable_feature', 'boolean_variable', user_id, user_attributes)) .to eq(true) @@ -2652,8 +2688,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_double('double_single_variable_feature', 'double_variable', user_id, user_attributes)) .to eq(42.42) @@ -2698,8 +2734,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable_integer('integer_single_variable_feature', 'integer_variable', user_id, user_attributes)) .to eq(42) @@ -2741,7 +2777,8 @@ def callback(_args); end Decision = Struct.new(:experiment, :variation, :source) # rubocop:disable Lint/ConstantDefinitionInBlock variation_to_return = project_config.rollout_id_map['166661']['experiments'][0]['variations'][0] decision_to_return = Decision.new({'key' => 'test-exp'}, variation_to_return, 'feature-test') - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decisiont_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decisiont_result_to_return) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -2814,7 +2851,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decisiont_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decisiont_result_to_return) allow(project_config).to receive(:variation_id_to_variable_usage_map).and_return(variation_id_to_variable_usage_map) expect(project_instance.notification_center).to receive(:send_notifications).once.with( @@ -2873,7 +2911,7 @@ def callback(_args); end describe 'when the feature flag is not enabled for the user' do it 'should return the default variable value' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -2976,7 +3014,8 @@ def callback(_args); end 'experiment' => nil, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable('string_single_variable_feature', 'string_variable', user_id, user_attributes)) .to eq('wingardium leviosa') @@ -2996,7 +3035,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable('string_single_variable_feature', 'string_variable', user_id, user_attributes)) .to eq('cta_1') @@ -3017,7 +3057,8 @@ def callback(_args); end 'experiment' => nil, 'variation' => variation_to_return } - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable('boolean_single_variable_feature', 'boolean_variable', user_id, user_attributes)) .to eq(true) @@ -3038,8 +3079,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable('double_single_variable_feature', 'double_variable', user_id, user_attributes)) .to eq(42.42) @@ -3060,8 +3101,8 @@ def callback(_args); end 'experiment' => experiment_to_return, 'variation' => variation_to_return } - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) expect(project_instance.get_feature_variable('integer_single_variable_feature', 'integer_variable', user_id, user_attributes)) .to eq(42) @@ -3078,7 +3119,7 @@ def callback(_args); end describe 'when the feature flag is not enabled for the user' do it 'should return the default variable value' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.get_feature_variable('string_single_variable_feature', 'string_variable', user_id, user_attributes)) .to eq('wingardium leviosa') @@ -3243,8 +3284,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) # DECISION listener called when the user is in experiment with variation feature off. expect(project_instance.notification_center).to receive(:send_notifications).once.with( @@ -3287,8 +3328,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) # DECISION listener called when the user is in experiment with variation feature on. expect(project_instance.notification_center).to receive(:send_notifications).once.with( @@ -3325,8 +3366,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) # DECISION listener called when the user is in rollout with variation feature on. expect(variation_to_return['featureEnabled']).to be true @@ -3360,7 +3401,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_result_to_return) # DECISION listener called when the user is in rollout with variation feature on. expect(variation_to_return['featureEnabled']).to be false @@ -3392,7 +3434,7 @@ def callback(_args); end end it 'should call listener with default variable type and value, when user neither in experiment nor in rollout' do - allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil) + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:DECISION], @@ -3768,8 +3810,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) decision_list_to_be_returned = [] - decision_list_to_be_returned << [decision_to_return, []] + decision_list_to_be_returned << decision_result_to_return allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_be_returned) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') @@ -3813,8 +3856,9 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) decision_list_to_be_returned = [] - decision_list_to_be_returned << [decision_to_return, []] + decision_list_to_be_returned << decision_result_to_return allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_be_returned) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') @@ -3898,7 +3942,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['ROLLOUT'] ) - decision_list_to_return = [[decision_to_return, []]] + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + decision_list_to_return = [decision_result_to_return] allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = project_instance.create_user_context('user1') decision = project_instance.decide(user_context, 'multi_variate_feature') @@ -4070,7 +4115,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - decision_list_to_be_returned = [[decision_to_return, []]] + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + decision_list_to_be_returned = [decision_result_to_return] allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_be_returned) user_context = project_instance.create_user_context('user1') @@ -4094,7 +4140,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - decision_list_to_return = [[decision_to_return, []]] + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + decision_list_to_return = [decision_result_to_return] allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = project_instance.create_user_context('user1') @@ -4201,7 +4248,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - decision_list_to_return = [[decision_to_return, []]] + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + decision_list_to_return = [decision_result_to_return] allow(project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) allow(project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = project_instance.create_user_context('user1') @@ -4421,7 +4469,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - decision_list_to_return = [[decision_to_return, []]] + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + decision_list_to_return = [decision_result_to_return] allow(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) allow(custom_project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = custom_project_instance.create_user_context('user1') @@ -4450,7 +4499,8 @@ def callback(_args); end variation_to_return, Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] ) - decision_list_to_return = [[decision_to_return, []]] + decision_result_to_return = Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, []) + decision_list_to_return = [decision_result_to_return] allow(custom_project_instance.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event)) allow(custom_project_instance.decision_service).to receive(:get_variations_for_feature_list).and_return(decision_list_to_return) user_context = custom_project_instance.create_user_context('user1') From 35cea8425dcfdf7d2244cbf1a1dfa20be7420841 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 23 Jul 2025 22:02:36 +0600 Subject: [PATCH 12/15] update: Refactor CMAB traffic allocation handling and enhance decision service error logging --- lib/optimizely/bucketer.rb | 4 +- lib/optimizely/decision_service.rb | 24 +-- spec/decision_service_spec.rb | 238 ++++++++++++++++++++++++++++- spec/project_spec.rb | 31 +++- 4 files changed, 277 insertions(+), 20 deletions(-) diff --git a/lib/optimizely/bucketer.rb b/lib/optimizely/bucketer.rb index daf435f9..d62e088d 100644 --- a/lib/optimizely/bucketer.rb +++ b/lib/optimizely/bucketer.rb @@ -106,8 +106,8 @@ def bucket_to_entity_id(project_config, experiment, bucketing_id, user_id) if experiment['cmab'] traffic_allocations = [ { - entityId: '$', - endOfRange: experiment['cmab']['trafficAllocation'] + 'entityId' => '$', + 'endOfRange' => experiment['cmab']['trafficAllocation'] } ] end diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 55bc73c0..976c742f 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -134,23 +134,25 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac cmab_decision = cmab_decision_result.result variation_id = cmab_decision&.variation_id cmab_uuid = cmab_decision&.cmab_uuid + variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) else # Bucket normally variation, bucket_reasons = @bucketer.bucket(project_config, experiment, bucketing_id, user_id) decide_reasons.push(*bucket_reasons) variation_id = variation ? variation['id'] : nil cmab_uuid = nil - message = '' - if variation_id - variation_key = variation['key'] - message = "User '#{user_id}' is in variation '#{variation_key}' of experiment '#{experiment_id}'." - else - message = "User '#{user_id}' is in no variation." - end - @logger.log(Logger::INFO, message) - decide_reasons.push(message) end + variation_key = variation['key'] if variation + message = if variation_id + "User '#{user_id}' is in variation '#{variation_key}' of experiment '#{experiment_id}'." + else + "User '#{user_id}' is in no variation." + end + + @logger.log(Logger::INFO, message) + decide_reasons.push(message) if message + # Persist bucketing decision user_profile_tracker.update_user_profile(experiment_id, variation_id) unless should_ignore_user_profile_service && user_profile_tracker VariationResult.new(cmab_uuid, false, decide_reasons, variation_id) @@ -236,8 +238,6 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont variation_id = variation_result.variation_id cmab_uuid = variation_result.cmab_uuid decide_reasons.push(*reasons_received) - puts 'final reasons' - puts decide_reasons next unless variation_id variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) @@ -516,7 +516,7 @@ def get_decision_for_cmab_experiment(project_config, experiment, user_context, b message = "User \"#{user_context.user_id}\" not in CMAB experiment \"#{experiment['key']}\" due to traffic allocation." @logger.log(Logger::INFO, message) decide_reasons.push(message) - CmabDecisionResult.new(false, nil, decide_reasons) + return CmabDecisionResult.new(false, nil, decide_reasons) end # User is in CMAB allocation, proceed to CMAB decision diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index e6bd7d58..e524203e 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -714,10 +714,10 @@ decision_result = decision_service.get_variation_for_feature_rollout(config, feature_flag, user_context) expect(decision_result.decision).to eq(nil) expect(decision_result.reasons).to eq([ - "User 'user_1' does not meet the conditions for targeting rule '1'.", - "User 'user_1' does not meet the conditions for targeting rule '2'.", - "User 'user_1' does not meet the conditions for targeting rule 'Everyone Else'." - ]) + "User 'user_1' does not meet the conditions for targeting rule '1'.", + "User 'user_1' does not meet the conditions for targeting rule '2'.", + "User 'user_1' does not meet the conditions for targeting rule 'Everyone Else'." + ]) # verify we tried to bucket in all targeting rules and the everyone else rule expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?).once @@ -937,4 +937,234 @@ expect(reasons).to eq(["Variation 'control' is mapped to experiment '111127' and user 'test_user_2' in the forced variation map"]) end end + describe 'CMAB experiments' do + describe 'when user is in traffic allocation' do + it 'should return correct variation and CMAB UUID from CMAB service' do + # Create a CMAB experiment configuration + cmab_experiment = { + 'id' => '111150', + 'key' => 'cmab_experiment', + 'status' => 'Running', + 'layerId' => '111150', + 'audienceIds' => [], + 'forcedVariations' => {}, + 'variations' => [ + {'id' => '111151', 'key' => 'variation_1'}, + {'id' => '111152', 'key' => 'variation_2'} + ], + 'trafficAllocation' => [ + {'entityId' => '111151', 'endOfRange' => 5000}, + {'entityId' => '111152', 'endOfRange' => 10_000} + ], + 'cmab' => {'trafficAllocation' => 5000} + } + user_context = project_instance.create_user_context('test_user', {}) + + # Mock experiment lookup to return our CMAB experiment + allow(config).to receive(:get_experiment_from_id).with('111150').and_return(cmab_experiment) + allow(config).to receive(:experiment_running?).with(cmab_experiment).and_return(true) + + # Mock audience evaluation to pass + allow(Optimizely::Audience).to receive(:user_meets_audience_conditions?).and_return([true, []]) + + # Mock bucketer to return a valid entity ID (user is in traffic allocation) + allow(decision_service.bucketer).to receive(:bucket_to_entity_id) + .with(config, cmab_experiment, 'test_user', 'test_user') + .and_return(['$', []]) + + # Mock CMAB service to return a decision + allow(spy_cmab_service).to receive(:get_decision) + .with(config, user_context, '111150', []) + .and_return(Optimizely::CmabDecision.new(variation_id: '111151', cmab_uuid: 'test-cmab-uuid-123')) + + # Mock variation lookup + allow(config).to receive(:get_variation_from_id_by_experiment_id) + .with('111150', '111151') + .and_return({'id' => '111151', 'key' => 'variation_1'}) + + variation_result = decision_service.get_variation(config, '111150', user_context) + + expect(variation_result.variation_id).to eq('111151') + expect(variation_result.cmab_uuid).to eq('test-cmab-uuid-123') + expect(variation_result.error).to eq(false) + expect(variation_result.reasons).to include( + "User 'test_user' is in variation 'variation_1' of experiment '111150'." + ) + + # Verify CMAB service was called + expect(spy_cmab_service).to have_received(:get_decision).once + end + end + + describe 'when user is not in traffic allocation' do + it 'should return nil variation and log traffic allocation message' do + cmab_experiment = { + 'id' => '111150', + 'key' => 'cmab_experiment', + 'status' => 'Running', + 'layerId' => '111150', + 'audienceIds' => [], + 'forcedVariations' => {}, + 'variations' => [ + {'id' => '111151', 'key' => 'variation_1'} + ], + 'trafficAllocation' => [ + {'entityId' => '111151', 'endOfRange' => 10_000} + ], + 'cmab' => {'trafficAllocation' => 1000} + } + user_context = project_instance.create_user_context('test_user', {}) + + # Mock experiment lookup to return our CMAB experiment + allow(config).to receive(:get_experiment_from_id).with('111150').and_return(cmab_experiment) + allow(config).to receive(:experiment_running?).with(cmab_experiment).and_return(true) + + # Mock audience evaluation to pass + allow(Optimizely::Audience).to receive(:user_meets_audience_conditions?).and_return([true, []]) + + variation_result = decision_service.get_variation(config, '111150', user_context) + + expect(variation_result.variation_id).to eq(nil) + expect(variation_result.cmab_uuid).to eq(nil) + expect(variation_result.error).to eq(false) + expect(variation_result.reasons).to include( + 'User "test_user" not in CMAB experiment "cmab_experiment" due to traffic allocation.' + ) + + # Verify CMAB service was not called since user is not in traffic allocation + expect(spy_cmab_service).not_to have_received(:get_decision) + end + end + + describe 'when CMAB service returns an error' do + it 'should return nil variation and include error in reasons' do + cmab_experiment = { + 'id' => '111150', + 'key' => 'cmab_experiment', + 'status' => 'Running', + 'layerId' => '111150', + 'audienceIds' => [], + 'forcedVariations' => {}, + 'variations' => [ + {'id' => '111151', 'key' => 'variation_1'} + ], + 'trafficAllocation' => [ + {'entityId' => '111151', 'endOfRange' => 10_000} + ], + 'cmab' => {'trafficAllocation' => 5000} + } + user_context = project_instance.create_user_context('test_user', {}) + + # Mock experiment lookup to return our CMAB experiment + allow(config).to receive(:get_experiment_from_id).with('111150').and_return(cmab_experiment) + allow(config).to receive(:experiment_running?).with(cmab_experiment).and_return(true) + + # Mock audience evaluation to pass + allow(Optimizely::Audience).to receive(:user_meets_audience_conditions?).and_return([true, []]) + + # Mock bucketer to return a valid entity ID (user is in traffic allocation) + allow(decision_service.bucketer).to receive(:bucket_to_entity_id) + .with(config, cmab_experiment, 'test_user', 'test_user') + .and_return(['$', []]) + + # Mock CMAB service to return an error + allow(spy_cmab_service).to receive(:get_decision) + .with(config, user_context, '111150', []) + .and_raise(StandardError.new('CMAB service error')) + + variation_result = decision_service.get_variation(config, '111150', user_context) + + expect(variation_result.variation_id).to be_nil + expect(variation_result.cmab_uuid).to be_nil + expect(variation_result.error).to eq(true) + expect(variation_result.reasons).to include( + "Failed to fetch CMAB decision for experiment 'cmab_experiment'" + ) + + # Verify CMAB service was called but errored + expect(spy_cmab_service).to have_received(:get_decision).once + end + end + + describe 'when user has forced variation' do + it 'should return forced variation and skip CMAB service call' do + # Use a real experiment from the datafile and modify it to be a CMAB experiment + real_experiment = config.get_experiment_from_key('test_experiment') + cmab_experiment = real_experiment.dup + cmab_experiment['cmab'] = {'trafficAllocation' => 5000} + + user_context = project_instance.create_user_context('test_user', {}) + + # Set up forced variation first (using real experiment that exists in datafile) + decision_service.set_forced_variation(config, 'test_experiment', 'test_user', 'variation') + + # Mock the experiment to be a CMAB experiment after setting forced variation + allow(config).to receive(:get_experiment_from_id).with('111127').and_return(cmab_experiment) + allow(config).to receive(:experiment_running?).with(cmab_experiment).and_return(true) + + # Add spy for bucket_to_entity_id method + allow(decision_service.bucketer).to receive(:bucket_to_entity_id).and_call_original + + variation_result = decision_service.get_variation(config, '111127', user_context) + + expect(variation_result.variation_id).to eq('111129') + expect(variation_result.cmab_uuid).to be_nil + expect(variation_result.error).to eq(false) + expect(variation_result.reasons).to include( + "Variation 'variation' is mapped to experiment '111127' and user 'test_user' in the forced variation map" + ) + + # Verify CMAB service was not called since user has forced variation + expect(spy_cmab_service).not_to have_received(:get_decision) + # Verify bucketer was not called since forced variations short-circuit bucketing + expect(decision_service.bucketer).not_to have_received(:bucket_to_entity_id) + end + end + + describe 'when user has whitelisted variation' do + it 'should return whitelisted variation and skip CMAB service call' do + # Create a CMAB experiment with whitelisted users + cmab_experiment = { + 'id' => '111150', + 'key' => 'cmab_experiment', + 'status' => 'Running', + 'layerId' => '111150', + 'audienceIds' => [], + 'forcedVariations' => { + 'whitelisted_user' => '111151' # User is whitelisted to variation_1 + }, + 'variations' => [ + {'id' => '111151', 'key' => 'variation_1'}, + {'id' => '111152', 'key' => 'variation_2'} + ], + 'trafficAllocation' => [ + {'entityId' => '111151', 'endOfRange' => 5000}, + {'entityId' => '111152', 'endOfRange' => 10_000} + ], + 'cmab' => {'trafficAllocation' => 5000} + } + user_context = project_instance.create_user_context('whitelisted_user', {}) + + # Mock experiment lookup to return our CMAB experiment + allow(config).to receive(:get_experiment_from_id).with('111150').and_return(cmab_experiment) + allow(config).to receive(:experiment_running?).with(cmab_experiment).and_return(true) + + # Mock the get_whitelisted_variation_id method directly + allow(decision_service).to receive(:get_whitelisted_variation_id) + .with(config, '111150', 'whitelisted_user') + .and_return(['111151', "User 'whitelisted_user' is whitelisted into variation 'variation_1' of experiment '111150'."]) + + variation_result = decision_service.get_variation(config, '111150', user_context) + + expect(variation_result.variation_id).to eq('111151') + expect(variation_result.cmab_uuid).to be_nil + expect(variation_result.error).to eq(false) + expect(variation_result.reasons).to include( + "User 'whitelisted_user' is whitelisted into variation 'variation_1' of experiment '111150'." + ) + # Verify CMAB service was not called since user is whitelisted + expect(spy_cmab_service).not_to have_received(:get_decision) + end + end + end end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index feb205a3..28437a16 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -1992,9 +1992,9 @@ def callback(_args); end ) # Ensure featureEnabled is false for this test expect(variation_to_return['featureEnabled']).to be false - + allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(Optimizely::DecisionService::DecisionResult.new(decision_to_return, false, [])) - + expect(project_instance.notification_center).to receive(:send_notifications).once.with( Optimizely::NotificationCenter::NOTIFICATION_TYPES[:LOG_EVENT], any_args ).ordered @@ -4281,6 +4281,33 @@ def callback(_args); end ]) end end + describe 'when decision service fails with CMAB error' do + it 'should return error decision when CMAB decision service fails' do + # Add the HTTP stub to prevent real requests + stub_request(:post, 'https://logx.optimizely.com/v1/events') + .to_return(status: 200, body: '', headers: {}) + + feature_flag_key = 'boolean_single_variable_feature' + + # Mock the decision service to return an error result + error_decision_result = double('DecisionResult') + allow(error_decision_result).to receive(:decision).and_return(nil) + allow(error_decision_result).to receive(:error).and_return(true) + allow(error_decision_result).to receive(:reasons).and_return(['CMAB service failed to fetch decision']) + + # Mock get_variations_for_feature_list instead of get_variation_for_feature + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list) + .and_return([error_decision_result]) + + user_context = project_instance.create_user_context('test_user') + decision = user_context.decide(feature_flag_key) + + expect(decision.enabled).to eq(false) + expect(decision.variation_key).to be_nil + expect(decision.flag_key).to eq(feature_flag_key) + expect(decision.reasons).to include('CMAB service failed to fetch decision') + end + end end describe '#decide_all' do From 9c31bb830e6b03f1924d11001a8e7c4000cf688a Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 23 Jul 2025 22:29:17 +0600 Subject: [PATCH 13/15] update: Refactor OptimizelyDecision instantiation to use keyword arguments for clarity --- lib/optimizely.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 6eb5bf86..4c4beafa 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -355,7 +355,7 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti # If the feature flag is nil, create a default OptimizelyDecision and move to the next key if feature_flag.nil? - decisions[key] = OptimizelyDecision.new(nil, false, nil, nil, key, user_context, []) + decisions[key] = OptimizelyDecision.new(variation_key: nil, enabled: false, variables: nil, rule_key: nil, flag_key: key, user_context: user_context, reasons: []) next end valid_keys.push(key) From 56bd5249b92dd575d8f0e10921f3404203bcf7d7 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 24 Jul 2025 21:46:46 +0600 Subject: [PATCH 14/15] update: Remove commented debug output from Optimizely user context spec --- spec/optimizely_user_context_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb index 42d71065..515068c0 100644 --- a/spec/optimizely_user_context_spec.rb +++ b/spec/optimizely_user_context_spec.rb @@ -556,7 +556,6 @@ decision = user_context_obj.decide(feature_key, [Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS]) expect(decision.variation_key).to eq('18257766532') expect(decision.rule_key).to eq('18322080788') - # puts decision.reasons expect(decision.reasons).to include('Invalid variation is mapped to flag (feature_1), rule (exp_with_audience) and user (tester) in the forced decision map.') # delivery-rule-to-decision From 600bb7963806dfc2d23f9bb083148b3088508e68 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 25 Jul 2025 22:41:48 +0600 Subject: [PATCH 15/15] Trigger CI build