checks audience for every one else (#94)

Author: rashidsp

lib/optimizely/decision_service.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 #
-#    Copyright 2017, Optimizely and contributors
+#    Copyright 2017-2018, 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.
@@ -235,33 +235,27 @@ def get_variation_for_feature_rollout(feature_flag, user_id, attributes = nil)
           next
         end
 
-        # User failed traffic allocation, jump to Everyone Else rule
-        @config.logger.log(
-          Logger::DEBUG,
-          "Attempting to bucket user '#{user_id}' into rollout rule for audience '#{audience_name}'."
-        )
-
         # Evaluate if user satisfies the traffic allocation for this rollout rule
         variation = @bucketer.bucket(rollout_rule, bucketing_id, user_id)
         return Decision.new(rollout_rule, variation, DECISION_SOURCE_ROLLOUT) unless variation.nil?
-        # User failed traffic allocation, jump to Everyone Else rule
-        @config.logger.log(
-          Logger::DEBUG,
-          "User '#{user_id}' was excluded due to traffic allocation. Checking 'Everyone Else' rule now."
-        )
-
         break
       end
 
       # get last rule which is the everyone else rule
       everyone_else_experiment = rollout_rules[number_of_rules]
+      # Check that user meets audience conditions for last rule
+      unless Audience.user_in_experiment?(@config, everyone_else_experiment, attributes)
+        audience_id = everyone_else_experiment['audienceIds'][0]
+        audience = @config.get_audience_from_id(audience_id)
+        audience_name = audience['name']
+        @config.logger.log(
+          Logger::DEBUG,
+          "User '#{user_id}' does not meet the conditions to be in rollout rule for audience '#{audience_name}'."
+        )
+        return nil
+      end
       variation = @bucketer.bucket(everyone_else_experiment, bucketing_id, user_id)
       return Decision.new(everyone_else_experiment, variation, DECISION_SOURCE_ROLLOUT) unless variation.nil?
-
-      @config.logger.log(
-        Logger::DEBUG,
-        "User '#{user_id}' was excluded from the 'Everyone Else' rule for feature flag"
-      )
       nil
     end
 

spec/decision_service_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 #
-#    Copyright 2017, Optimizely and contributors
+#    Copyright 2017-2018, 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.
@@ -487,17 +487,12 @@
           feature_flag = config.feature_flag_key_map['boolean_single_variable_feature']
           rollout_experiment = config.rollout_id_map[feature_flag['rolloutId']]['experiments'][0]
           variation = rollout_experiment['variations'][0]
-          audience_id = rollout_experiment['audienceIds'][0]
-          audience_name = config.get_audience_from_id(audience_id)['name']
           expected_decision = Optimizely::DecisionService::Decision.new(rollout_experiment, variation, Optimizely::DecisionService::DECISION_SOURCE_ROLLOUT)
           allow(Optimizely::Audience).to receive(:user_in_experiment?).and_return(true)
           allow(decision_service.bucketer).to receive(:bucket)
             .with(rollout_experiment, user_id, user_id)
             .and_return(variation)
           expect(decision_service.get_variation_for_feature_rollout(feature_flag, user_id, user_attributes)).to eq(expected_decision)
-          expect(spy_logger).to have_received(:log).once
-                                                   .with(Logger::DEBUG, "Attempting to bucket user '#{user_id}' into rollout rule "\
-                  "for audience '#{audience_name}'.")
         end
       end
 
@@ -523,19 +518,6 @@
                                                                                .with(config, rollout['experiments'][0], user_attributes)
             expect(Optimizely::Audience).not_to have_received(:user_in_experiment?)
               .with(config, rollout['experiments'][1], user_attributes)
-
-            # verify log messages
-            experiment = rollout['experiments'][0]
-            audience_id = experiment['audienceIds'][0]
-            audience_name = config.get_audience_from_id(audience_id)['name']
-
-            expect(spy_logger).to have_received(:log).once
-                                                     .with(Logger::DEBUG, "Attempting to bucket user '#{user_id}' into rollout rule "\
-                    "for audience '#{audience_name}'.")
-            expect(spy_logger).to have_received(:log).once
-                                                     .with(Logger::DEBUG, "User '#{user_id}' was excluded due to traffic allocation. Checking 'Everyone Else' rule now.")
-            expect(spy_logger).to have_received(:log).once
-                                                     .with(Logger::DEBUG, "User '#{user_id}' was excluded from the 'Everyone Else' rule for feature flag")
           end
         end
 
@@ -561,16 +543,6 @@
                                                                                .with(config, rollout['experiments'][0], user_attributes)
             expect(Optimizely::Audience).not_to have_received(:user_in_experiment?)
               .with(config, rollout['experiments'][1], user_attributes)
-
-            # verify log messages
-            experiment = rollout['experiments'][0]
-            audience_id = experiment['audienceIds'][0]
-            audience_name = config.get_audience_from_id(audience_id)['name']
-
-            expect(spy_logger).to have_received(:log).once
-                                                     .with(Logger::DEBUG, "Attempting to bucket user '#{user_id}' into rollout rule for audience '#{audience_name}'.")
-            expect(spy_logger).to have_received(:log).once
-                                                     .with(Logger::DEBUG, "User '#{user_id}' was excluded due to traffic allocation. Checking 'Everyone Else' rule now.")
           end
         end
       end
@@ -584,18 +556,22 @@
         variation = everyone_else_experiment['variations'][0]
         expected_decision = Optimizely::DecisionService::Decision.new(everyone_else_experiment, variation, Optimizely::DecisionService::DECISION_SOURCE_ROLLOUT)
         allow(Optimizely::Audience).to receive(:user_in_experiment?).and_return(false)
+
+        allow(Optimizely::Audience).to receive(:user_in_experiment?)
+          .with(config, everyone_else_experiment, user_attributes)
+          .and_return(true)
         allow(decision_service.bucketer).to receive(:bucket)
           .with(everyone_else_experiment, user_id, user_id)
           .and_return(variation)
 
         expect(decision_service.get_variation_for_feature_rollout(feature_flag, user_id, user_attributes)).to eq(expected_decision)
 
-        # verify we tried to bucket in all targeting rules except for the everyone else rule
+        # verify we tried to bucket in all targeting rules and the everyone else rule
         expect(Optimizely::Audience).to have_received(:user_in_experiment?).once
                                                                            .with(config, rollout['experiments'][0], user_attributes)
         expect(Optimizely::Audience).to have_received(:user_in_experiment?)
           .with(config, rollout['experiments'][1], user_attributes)
-        expect(Optimizely::Audience).not_to have_received(:user_in_experiment?)
+        expect(Optimizely::Audience).to have_received(:user_in_experiment?)
           .with(config, rollout['experiments'][2], user_attributes)
 
         # verify log messages
@@ -611,6 +587,40 @@
         expect(spy_logger).to have_received(:log).once
                                                  .with(Logger::DEBUG, "User '#{user_id}' does not meet the conditions to be in rollout rule for audience '#{audience_name}'.")
       end
+
+      it 'should not bucket the user into the "Everyone Else" rule when audience mismatch' do
+        feature_flag = config.feature_flag_key_map['boolean_single_variable_feature']
+        rollout = config.rollout_id_map[feature_flag['rolloutId']]
+        everyone_else_experiment = rollout['experiments'][2]
+        everyone_else_experiment['audienceIds'] = ['11155']
+        allow(Optimizely::Audience).to receive(:user_in_experiment?).and_return(false)
+
+        expect(decision_service.bucketer).not_to receive(:bucket)
+          .with(everyone_else_experiment, user_id, user_id)
+
+        expect(decision_service.get_variation_for_feature_rollout(feature_flag, user_id, user_attributes)).to eq(nil)
+
+        # verify we tried to bucket in all targeting rules and the everyone else rule
+        expect(Optimizely::Audience).to have_received(:user_in_experiment?).once
+                                                                           .with(config, rollout['experiments'][0], user_attributes)
+        expect(Optimizely::Audience).to have_received(:user_in_experiment?)
+          .with(config, rollout['experiments'][1], user_attributes)
+        expect(Optimizely::Audience).to have_received(:user_in_experiment?)
+          .with(config, rollout['experiments'][2], user_attributes)
+
+        # verify log messages
+        experiment = rollout['experiments'][0]
+        audience_id = experiment['audienceIds'][0]
+        audience_name = config.get_audience_from_id(audience_id)['name']
+        expect(spy_logger).to have_received(:log).once
+                                                 .with(Logger::DEBUG, "User '#{user_id}' does not meet the conditions to be in rollout rule for audience '#{audience_name}'.")
+
+        experiment = rollout['experiments'][1]
+        audience_id = experiment['audienceIds'][0]
+        audience_name = config.get_audience_from_id(audience_id)['name']
+        expect(spy_logger).to have_received(:log).twice
+                                                 .with(Logger::DEBUG, "User '#{user_id}' does not meet the conditions to be in rollout rule for audience '#{audience_name}'.")
+      end
     end
   end