diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 4c4beafa..aa50ce4e 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -139,8 +139,9 @@ def initialize( # Initialize CMAB components @cmab_client = DefaultCmabClient.new( - retry_config: CmabRetryConfig.new, - logger: @logger + nil, + CmabRetryConfig.new, + @logger ) @cmab_cache = LRUCache.new(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT) @cmab_service = DefaultCmabService.new( @@ -211,7 +212,7 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide experiment = decision.experiment rule_key = experiment ? experiment['key'] : nil experiment_id = experiment ? experiment['id'] : nil - variation = decision['variation'] + variation = decision.variation variation_key = variation ? variation['key'] : nil variation_id = variation ? variation['id'] : nil feature_enabled = variation ? variation['featureEnabled'] : false @@ -219,7 +220,7 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide end if !decide_options.include?(OptimizelyDecideOption::DISABLE_DECISION_EVENT) && (decision_source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || config.send_flag_decisions) - send_impression(config, experiment, variation_key || '', flag_key, rule_key || '', feature_enabled, decision_source, user_id, attributes) + send_impression(config, experiment, variation_key || '', flag_key, rule_key || '', feature_enabled, decision_source, user_id, attributes, decision&.cmab_uuid) decision_event_dispatched = true end @@ -1244,7 +1245,7 @@ def validate_instantiation_options raise InvalidInputError, 'event_dispatcher' end - def send_impression(config, experiment, variation_key, flag_key, rule_key, enabled, rule_type, user_id, attributes = nil) + def send_impression(config, experiment, variation_key, flag_key, rule_key, enabled, rule_type, user_id, attributes = nil, cmab_uuid = nil) if experiment.nil? experiment = { 'id' => '', @@ -1276,6 +1277,7 @@ def send_impression(config, experiment, variation_key, flag_key, rule_key, enabl variation_key: variation_key, enabled: enabled } + metadata[:cmab_uuid] = cmab_uuid unless cmab_uuid.nil? user_event = UserEventFactory.create_impression_event(config, experiment, variation_id, metadata, user_id, attributes) @event_processor.process(user_event) diff --git a/lib/optimizely/audience.rb b/lib/optimizely/audience.rb index 3e919ad8..6ea523e4 100644 --- a/lib/optimizely/audience.rb +++ b/lib/optimizely/audience.rb @@ -72,6 +72,20 @@ def user_meets_audience_conditions?(config, experiment, user_context, logger, lo decide_reasons.push(message) audience_conditions = JSON.parse(audience_conditions) if audience_conditions.is_a?(String) + # Convert all symbol keys to string keys in the parsed conditions + stringify_keys = lambda do |obj| + case obj + when Hash + obj.transform_keys(&:to_s).transform_values { |v| stringify_keys.call(v) } + when Array + obj.map { |item| stringify_keys.call(item) } + else + obj + end + end + + audience_conditions = stringify_keys.call(audience_conditions) + result = ConditionTreeEvaluator.evaluate(audience_conditions, evaluate_user_conditions) result_str = result.nil? ? 'UNKNOWN' : result.to_s.upcase message = format(logs_hash['AUDIENCE_EVALUATION_RESULT'], audience_id, result_str) diff --git a/lib/optimizely/cmab/cmab_client.rb b/lib/optimizely/cmab/cmab_client.rb index 113f1d4a..f9a21cff 100644 --- a/lib/optimizely/cmab/cmab_client.rb +++ b/lib/optimizely/cmab/cmab_client.rb @@ -122,7 +122,7 @@ def _do_fetch(url, request_body, timeout) raise CmabInvalidResponseError, error_message end - body['predictions'][0]['variationId'] + body['predictions'][0]['variation_id'] end def validate_response(body) @@ -137,7 +137,7 @@ def validate_response(body) body['predictions'].is_a?(Array) && !body['predictions'].empty? && body['predictions'][0].is_a?(Hash) && - body['predictions'][0].key?('variationId') + body['predictions'][0].key?('variation_id') end def _do_fetch_with_retry(url, request_body, retry_config, timeout) diff --git a/lib/optimizely/cmab/cmab_service.rb b/lib/optimizely/cmab/cmab_service.rb index b56a785b..ceed3066 100644 --- a/lib/optimizely/cmab/cmab_service.rb +++ b/lib/optimizely/cmab/cmab_service.rb @@ -119,7 +119,10 @@ def filter_attributes(project_config, user_context, rule_id) 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) + next unless attribute + + attribute_key = attribute['key'] + filtered_user_attributes[attribute_key] = user_attributes[attribute_key] if user_attributes.key?(attribute_key) end filtered_user_attributes diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 976c742f..a97bf4d6 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -134,7 +134,7 @@ 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) + variation = variation_id ? project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) : nil else # Bucket normally variation, bucket_reasons = @bucketer.bucket(project_config, experiment, bucketing_id, user_id) @@ -238,6 +238,10 @@ 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) + + # If there's an error, return immediately instead of falling back to next experiment + return DecisionResult.new(nil, error, decide_reasons) if error + next unless variation_id variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) @@ -509,7 +513,7 @@ def get_decision_for_cmab_experiment(project_config, experiment, user_context, b # 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 + project_config, experiment, bucketing_id, user_id ) decide_reasons.push(*bucket_reasons) unless bucketed_entity_id @@ -526,7 +530,7 @@ def get_decision_for_cmab_experiment(project_config, experiment, user_context, b ) CmabDecisionResult.new(false, cmab_decision, decide_reasons) rescue StandardError => e - error_message = "Failed to fetch CMAB decision for experiment '#{experiment['key']}'" + error_message = "Failed to fetch CMAB data for experiment #{experiment['key']}." decide_reasons.push(error_message) @logger&.log(Logger::ERROR, "#{error_message} #{e}") CmabDecisionResult.new(true, nil, decide_reasons) diff --git a/spec/cmab/cmab_client_spec.rb b/spec/cmab/cmab_client_spec.rb index f25c78fa..daa9ccd4 100644 --- a/spec/cmab/cmab_client_spec.rb +++ b/spec/cmab/cmab_client_spec.rb @@ -60,7 +60,7 @@ it 'should return the variation id on success' do WebMock.stub_request(:post, expected_url) .with(body: expected_body_for_webmock, headers: expected_headers) - .to_return(status: 200, body: {'predictions' => [{'variationId' => 'abc123'}]}.to_json, headers: {'Content-Type' => 'application/json'}) + .to_return(status: 200, body: {'predictions' => [{'variation_id' => 'abc123'}]}.to_json, headers: {'Content-Type' => 'application/json'}) result = client.fetch_decision(rule_id, user_id, attributes, cmab_uuid) @@ -137,7 +137,7 @@ it 'should return the variation id on first try' do WebMock.stub_request(:post, expected_url) .with(body: expected_body_for_webmock, headers: expected_headers) - .to_return(status: 200, body: {'predictions' => [{'variationId' => 'abc123'}]}.to_json, headers: {'Content-Type' => 'application/json'}) + .to_return(status: 200, body: {'predictions' => [{'variation_id' => 'abc123'}]}.to_json, headers: {'Content-Type' => 'application/json'}) result = client_with_retry.fetch_decision(rule_id, user_id, attributes, cmab_uuid) @@ -152,7 +152,7 @@ .with(body: expected_body_for_webmock, headers: expected_headers) .to_return({status: 500}, {status: 500}, - {status: 200, body: {'predictions' => [{'variationId' => 'xyz456'}]}.to_json, headers: {'Content-Type' => 'application/json'}}) + {status: 200, body: {'predictions' => [{'variation_id' => 'xyz456'}]}.to_json, headers: {'Content-Type' => 'application/json'}}) result = client_with_retry.fetch_decision(rule_id, user_id, attributes, cmab_uuid) diff --git a/spec/cmab/cmab_service_spec.rb b/spec/cmab/cmab_service_spec.rb index 6c3c0011..de94d39b 100644 --- a/spec/cmab/cmab_service_spec.rb +++ b/spec/cmab/cmab_service_spec.rb @@ -19,8 +19,8 @@ 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') } + let(:mock_attr1) { {'key' => 'age'} } + let(:mock_attr2) { {'key' => 'location'} } before do allow(mock_user_context).to receive(:user_id).and_return(user_id) diff --git a/spec/decision_service_spec.rb b/spec/decision_service_spec.rb index e524203e..fe2cc881 100644 --- a/spec/decision_service_spec.rb +++ b/spec/decision_service_spec.rb @@ -1078,7 +1078,7 @@ 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'" + 'Failed to fetch CMAB data for experiment cmab_experiment.' ) # Verify CMAB service was called but errored 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 28437a16..9abbc39f 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -4308,6 +4308,103 @@ def callback(_args); end expect(decision.reasons).to include('CMAB service failed to fetch decision') end end + describe 'CMAB experiments' do + it 'should include CMAB UUID in dispatched event when decision service returns CMAB result' do + # Use an existing feature flag from the test config + feature_flag_key = 'boolean_single_variable_feature' + + # Get an existing experiment that actually exists in the datafile + # Looking at the test config, let's use experiment ID '122230' which exists + existing_experiment = project_config.get_experiment_from_id('122230') + + # Modify the existing experiment to be a CMAB experiment + cmab_experiment = existing_experiment.dup + cmab_experiment['trafficAllocation'] = [] # Empty for CMAB + cmab_experiment['cmab'] = {'attributeIds' => %w[808797688 808797689], 'trafficAllocation' => 4000} + + # Mock the config to return our modified CMAB experiment + allow(project_instance.config_manager.config).to receive(:get_experiment_from_id) + .with('122230') + .and_return(cmab_experiment) + + allow(project_instance.config_manager.config).to receive(:experiment_running?) + .with(cmab_experiment) + .and_return(true) + + # Get the feature flag and update it to reference our CMAB experiment + feature_flag = project_instance.config_manager.config.get_feature_flag_from_key(feature_flag_key) + feature_flag['experimentIds'] = ['122230'] + + # Use existing variations from the original experiment + variation_to_use = existing_experiment['variations'][0] + + # Create a decision with CMAB UUID + expected_cmab_uuid = 'uuid-cmab' + decision_with_cmab = Optimizely::DecisionService::Decision.new( + cmab_experiment, + variation_to_use, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'], + expected_cmab_uuid + ) + + decision_result_with_cmab = Optimizely::DecisionService::DecisionResult.new( + decision_with_cmab, + false, + [] + ) + + # Mock get_variations_for_feature_list to return CMAB result + allow(project_instance.decision_service).to receive(:get_variations_for_feature_list) + .and_return([decision_result_with_cmab]) + + # Set up time and UUID mocks for consistent event data + allow(Time).to receive(:now).and_return(time_now) + allow(SecureRandom).to receive(:uuid).and_return('a68cf1ad-0393-4e18-af87-efe8f01a7c9c') + + # Create array to capture dispatched events + dispatched_events = [] + allow(project_instance.event_dispatcher).to receive(:dispatch_event) do |event| + dispatched_events << event + end + + user_context = project_instance.create_user_context('test_user') + decision = user_context.decide(feature_flag_key) + + # Wait for batch processing thread to send event + sleep 0.1 until project_instance.event_processor.event_queue.empty? + + # Verify the decision contains expected information + expect(decision.enabled).to eq(true) + expect(decision.variation_key).to eq(variation_to_use['key']) + expect(decision.rule_key).to eq(existing_experiment['key']) + expect(decision.flag_key).to eq(feature_flag_key) + + # Verify an event was dispatched + expect(dispatched_events.length).to eq(1) + + dispatched_event = dispatched_events[0] + + # Remove the puts statement and verify the event structure and CMAB UUID + expect(dispatched_event.params).to have_key(:visitors) + expect(dispatched_event.params[:visitors].length).to be > 0 + expect(dispatched_event.params[:visitors][0]).to have_key(:snapshots) + expect(dispatched_event.params[:visitors][0][:snapshots].length).to be > 0 + expect(dispatched_event.params[:visitors][0][:snapshots][0]).to have_key(:decisions) + expect(dispatched_event.params[:visitors][0][:snapshots][0][:decisions].length).to be > 0 + + # Get the metadata and assert CMAB UUID + metadata = dispatched_event.params[:visitors][0][:snapshots][0][:decisions][0][:metadata] + expect(metadata).to have_key(:cmab_uuid) + expect(metadata[:cmab_uuid]).to eq(expected_cmab_uuid) + + # Also verify other expected metadata fields + expect(metadata[:flag_key]).to eq(feature_flag_key) + expect(metadata[:rule_key]).to eq('test_experiment_multivariate') + expect(metadata[:rule_type]).to eq('feature-test') + expect(metadata[:variation_key]).to eq('Fred') + expect(metadata[:enabled]).to eq(true) + end + end end describe '#decide_all' do