lib/optimizely/decision_service.rb -> Removed user profile tracker instantiation.

lib/optimizely/user_profile_tracker.rb -> Improved error logging message.
spec/decision_service_spec.rb -> Refactored user profile tracking in tests.
spec/project_spec.rb -> Updated decision service method stubs.
spec/user_profile_tracker.rb -> Updated lookup, update and save tests for user_profile_tracker

Author: FarhanAnjum-opti

lib/optimizely/decision_service.rb

@@ -94,7 +94,6 @@ def get_variation(project_config, experiment_id, user_context, user_profile_trac
       return whitelisted_variation_id, decide_reasons if whitelisted_variation_id
 
       should_ignore_user_profile_service = decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE
-      # user_profile_tracker = Optimizely::UserProfileTracker.new(user_context.user_id, @user_profile_service, @logger) if user_profile_tracker.nil?
       # Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService
       unless should_ignore_user_profile_service && user_profile_tracker
         saved_variation_id, reasons_received = get_saved_variation_id(project_config, experiment_id, user_profile_tracker.user_profile)

lib/optimizely/user_profile_tracker.rb

@@ -23,8 +23,8 @@ def load_user_profile(reasons = [], error_handler = nil)
       begin
         @user_profile = @user_profile_service.lookup(@user_id) if @user_profile_service
       rescue => e
-        message = "Error while loading user profile in user profile tracker for user ID '#{@user_id}': #{e}."
-        reasons << e.message
+        message = "Error while looking up user profile for user ID '#{@user_id}': #{e}."
+        reasons << message
         @logger.log(Logger::ERROR, message)
         error_handler&.handle_error(e)
       end

spec/decision_service_spec.rb

@@ -263,50 +263,14 @@
     end
 
     describe 'when a UserProfile service is provided' do
-      it 'should look up the UserProfile, bucket normally, and save the result if no saved profile is found' do
-        expected_user_profile = {
-          user_id: 'test_user',
-          experiment_bucket_map: {
-            '111127' => {
-              variation_id: '111128'
-            }
-          }
-        }
-        expect(spy_user_profile_service).to receive(:lookup).once.and_return(nil)
-
-        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('111128')
-        expect(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
-        # bucketing decision should have been saved
-        expect(spy_user_profile_service).to have_received(:save).once.with(expected_user_profile)
-        expect(spy_logger).to have_received(:log).once
-                                                 .with(Logger::INFO, "Saved variation ID 111128 of experiment ID 111127 for user 'test_user'.")
-      end
-
-      it 'should look up the UserProfile, bucket normally (using Bucketing ID attribute), and save the result if no saved profile is found' do
-        expected_user_profile = {
-          user_id: 'test_user',
-          experiment_bucket_map: {
-            '111127' => {
-              variation_id: '111129'
-            }
-          }
-        }
+      it 'bucket normally (using Bucketing ID attribute)' do
         user_attributes = {
           'browser_type' => 'firefox',
           Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 'pid'
         }
-        expect(spy_user_profile_service).to receive(:lookup).once.and_return(nil)
-
         user_context = project_instance.create_user_context('test_user', user_attributes)
-        variation_received, reasons = decision_service.get_variation(config, '111127', user_context)
+        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.",
@@ -315,13 +279,9 @@
 
         # bucketing should have occurred
         expect(decision_service.bucketer).to have_received(:bucket).once
-        # bucketing decision should have been saved
-        expect(spy_user_profile_service).to have_received(:save).once.with(expected_user_profile)
-        expect(spy_logger).to have_received(:log).once
-                                                 .with(Logger::INFO, "Saved variation ID 111129 of experiment ID 111127 for user 'test_user'.")
       end
 
-      it 'should look up the user profile and skip normal bucketing if a profile with a saved decision is found' do
+      it 'skip normal bucketing if a profile with a saved decision is found' do
         saved_user_profile = {
           user_id: 'test_user',
           experiment_bucket_map: {
@@ -334,7 +294,9 @@
           .with('test_user').once.and_return(saved_user_profile)
 
         user_context = project_instance.create_user_context('test_user')
-        variation_received, reasons = decision_service.get_variation(config, '111127', user_context)
+        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."
@@ -350,7 +312,7 @@
         expect(spy_user_profile_service).not_to have_received(:save)
       end
 
-      it 'should look up the user profile and bucket normally if a profile without a saved decision is found' do
+      it 'bucket normally if a profile without a saved decision is found' do
         saved_user_profile = {
           user_id: 'test_user',
           experiment_bucket_map: {
@@ -364,7 +326,9 @@
           .once.with('test_user').and_return(saved_user_profile)
 
         user_context = project_instance.create_user_context('test_user')
-        variation_received, reasons = decision_service.get_variation(config, '111127', user_context)
+        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.",
@@ -373,20 +337,6 @@
 
         # bucketing should have occurred
         expect(decision_service.bucketer).to have_received(:bucket).once
-
-        # user profile should have been updated with bucketing decision
-        expected_user_profile = {
-          user_id: 'test_user',
-          experiment_bucket_map: {
-            '111127' => {
-              variation_id: '111128'
-            },
-            '122227' => {
-              variation_id: '122228'
-            }
-          }
-        }
-        expect(spy_user_profile_service).to have_received(:save).once.with(expected_user_profile)
       end
 
       it 'should bucket normally if the user profile contains a variation ID not in the datafile' do
@@ -403,7 +353,9 @@
           .once.with('test_user').and_return(saved_user_profile)
 
         user_context = project_instance.create_user_context('test_user')
-        variation_received, reasons = decision_service.get_variation(config, '111127', user_context)
+        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.",
@@ -413,27 +365,18 @@
 
         # bucketing should have occurred
         expect(decision_service.bucketer).to have_received(:bucket).once
-
-        # user profile should have been updated with bucketing decision
-        expected_user_profile = {
-          user_id: 'test_user',
-          experiment_bucket_map: {
-            '111127' => {
-              variation_id: '111128'
-            }
-          }
-        }
-        expect(spy_user_profile_service).to have_received(:save).with(expected_user_profile)
       end
 
-      it 'should bucket normally if the user profile service throws an error during lookup' do
+      it 'should bucket normally if the user profile tracker throws an error during lookup' do
         expect(spy_user_profile_service).to receive(:lookup).once.with('test_user').and_throw(:LookupError)
 
         user_context = project_instance.create_user_context('test_user')
-        variation_received, reasons = decision_service.get_variation(config, '111127', user_context)
+        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)
+        user_profile_tracker.save_user_profile
         expect(variation_received).to eq('111128')
         expect(reasons).to eq([
-                                "Error while looking up user profile for user ID 'test_user': uncaught throw :LookupError.",
                                 "Audiences for experiment 'test_experiment' collectively evaluated to TRUE.",
                                 "User 'test_user' is in variation 'control' of experiment '111127'."
                               ])
@@ -444,46 +387,15 @@
         expect(decision_service.bucketer).to have_received(:bucket).once
       end
 
-      it 'should log an error if the user profile service throws an error during save' do
-        expect(spy_user_profile_service).to receive(:save).once.and_throw(:SaveError)
-
-        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('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(spy_logger).to have_received(:log).once
-                                                 .with(Logger::ERROR, "Error while saving user profile for user ID 'test_user': uncaught throw :SaveError.")
-      end
-
       describe 'IGNORE_USER_PROFILE_SERVICE decide option' do
         it 'should ignore user profile service if this option is set' do
           allow(spy_user_profile_service).to receive(:lookup)
             .with('test_user').once.and_return(nil)
 
           user_context = project_instance.create_user_context('test_user', nil)
-          variation_received, reasons = decision_service.get_variation(config, '111127', user_context, [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'."
-                                ])
-
-          expect(decision_service.bucketer).to have_received(:bucket)
-          expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?)
-          expect(spy_user_profile_service).not_to have_received(:lookup)
-          expect(spy_user_profile_service).not_to have_received(:save)
-        end
-
-        it 'should not ignore user profile service if this option is not set' do
-          allow(spy_user_profile_service).to receive(:lookup)
-            .with('test_user').once.and_return(nil)
-
-          user_context = project_instance.create_user_context('test_user')
-          variation_received, reasons = decision_service.get_variation(config, '111127', user_context)
+          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.",
@@ -492,8 +404,6 @@
 
           expect(decision_service.bucketer).to have_received(:bucket)
           expect(Optimizely::Audience).to have_received(:user_meets_audience_conditions?)
-          expect(spy_user_profile_service).to have_received(:lookup)
-          expect(spy_user_profile_service).to have_received(:save)
         end
       end
     end

spec/project_spec.rb

@@ -4407,8 +4407,9 @@ def callback(_args); end
           variation_to_return,
           Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']
         )
+        decision_list_to_return = [[decision_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_variation_for_feature).and_return(decision_to_return)
+        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')
         decision = custom_project_instance.decide(user_context, 'multi_variate_feature')
         expect(decision.as_json).to include(
@@ -4435,8 +4436,9 @@ def callback(_args); end
           variation_to_return,
           Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']
         )
+        decision_list_to_return = [[decision_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_variation_for_feature).and_return(decision_to_return)
+        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')
         decision = custom_project_instance.decide(user_context, 'multi_variate_feature')
         expect(decision.as_json).to include(

spec/user_profile_tracker_spec.rb

@@ -0,0 +1,102 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rspec'
+
+RSpec.describe Optimizely::UserProfileTracker do
+  let(:user_id) { 'test_user' }
+  let(:mock_user_profile_service) { instance_double('UserProfileService') }
+  let(:mock_logger) { instance_double('Logger') }
+  let(:user_profile_tracker) { described_class.new(user_id, mock_user_profile_service, mock_logger) }
+
+  describe '#initialize' do
+    it 'initializes with a user ID and default values' do
+      tracker = described_class.new(user_id)
+      expect(tracker.user_profile[:user_id]).to eq(user_id)
+      expect(tracker.user_profile[:experiment_bucket_map]).to eq({})
+    end
+
+    it 'accepts a user profile service and logger' do
+      expect(user_profile_tracker.instance_variable_get(:@user_profile_service)).to eq(mock_user_profile_service)
+      expect(user_profile_tracker.instance_variable_get(:@logger)).to eq(mock_logger)
+    end
+  end
+
+  describe '#load_user_profile' do
+    it 'loads the user profile from the service if provided' do
+      expected_profile = {
+        user_id: user_id,
+        experiment_bucket_map: { '111127' => { variation_id: '111128' } }
+      }
+      allow(mock_user_profile_service).to receive(:lookup).with(user_id).and_return(expected_profile)
+      user_profile_tracker.load_user_profile
+      expect(user_profile_tracker.user_profile).to eq(expected_profile)
+    end
+
+    it 'handles errors during lookup and logs them' do
+      allow(mock_user_profile_service).to receive(:lookup).with(user_id).and_raise(StandardError.new('lookup error'))
+      allow(mock_logger).to receive(:log)
+
+      reasons = []
+      user_profile_tracker.load_user_profile(reasons)
+
+      expect(reasons).to include('lookup error')
+      expect(mock_logger).to have_received(:log).with(Logger::ERROR, "Error while loading user profile in user profile tracker for user ID 'test_user': lookup error.")
+    end
+
+    it 'does nothing if reasons array is nil' do
+      expect(mock_user_profile_service).not_to receive(:lookup)
+      user_profile_tracker.load_user_profile(nil)
+    end
+  end
+
+  describe '#update_user_profile' do
+    let(:experiment_id) { '111127' }
+    let(:variation_id) { '111128' }
+
+    before do
+      allow(mock_logger).to receive(:log)
+    end
+
+    it 'updates the experiment bucket map with the given experiment and variation IDs' do
+      user_profile_tracker.update_user_profile(experiment_id, variation_id)
+
+      # Verify the experiment and variation were added
+      expect(user_profile_tracker.user_profile[:experiment_bucket_map][experiment_id][:variation_id]).to eq(variation_id)
+      # Verify the profile_updated flag was set
+      expect(user_profile_tracker.instance_variable_get(:@profile_updated)).to eq(true)
+      # Verify a log message was recorded
+      expect(mock_logger).to have_received(:log).with(Logger::INFO, "Updated variation ID #{variation_id} of experiment ID #{experiment_id} for user 'test_user'.")
+    end
+  end
+
+  describe '#save_user_profile' do
+    it 'saves the user profile if updates were made and service is available' do
+      allow(mock_user_profile_service).to receive(:save)
+      allow(mock_logger).to receive(:log)
+
+      user_profile_tracker.update_user_profile('111127', '111128')
+      user_profile_tracker.save_user_profile
+
+      expect(mock_user_profile_service).to have_received(:save).with(user_profile_tracker.user_profile)
+      expect(mock_logger).to have_received(:log).with(Logger::INFO, "Saved user profile for user 'test_user'.")
+    end
+
+    it 'does not save the user profile if no updates were made' do
+      allow(mock_user_profile_service).to receive(:save)
+
+      user_profile_tracker.save_user_profile
+      expect(mock_user_profile_service).not_to have_received(:save)
+    end
+
+    it 'handles errors during save and logs them' do
+      allow(mock_user_profile_service).to receive(:save).and_raise(StandardError.new('save error'))
+      allow(mock_logger).to receive(:log)
+
+      user_profile_tracker.update_user_profile('111127', '111128')
+      user_profile_tracker.save_user_profile
+
+      expect(mock_logger).to have_received(:log).with(Logger::ERROR, "Failed to save user profile for user 'test_user': save error.")
+    end
+  end
+end