feat(api): Accepting all types for attributes values (#125)

Author: rashidsp

lib/optimizely.rb

@@ -258,6 +258,8 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
         }, @logger, Logger::ERROR
       )
 
+      return false unless user_inputs_valid?(attributes)
+
       feature_flag = @config.get_feature_flag_from_key(feature_flag_key)
       unless feature_flag
         @logger.log(Logger::ERROR, "No feature flag was found for key '#{feature_flag_key}'.")
@@ -307,6 +309,8 @@ def get_enabled_features(user_id, attributes = nil)
 
       return enabled_features unless Optimizely::Helpers::Validator.inputs_valid?({user_id: user_id}, @logger, Logger::ERROR)
 
+      return enabled_features unless user_inputs_valid?(attributes)
+
       @config.feature_flags.each do |feature|
         enabled_features.push(feature['key']) if is_feature_enabled(
           feature['key'],
@@ -450,6 +454,8 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type,
         @logger, Logger::ERROR
       )
 
+      return nil unless user_inputs_valid?(attributes)
+
       feature_flag = @config.get_feature_flag_from_key(feature_flag_key)
       unless feature_flag
         @logger.log(Logger::INFO, "No feature flag was found for key '#{feature_flag_key}'.")

lib/optimizely/decision_service.rb

@@ -366,17 +366,18 @@ def get_bucketing_id(user_id, attributes)
       #
       # user_id - String user ID
       # attributes - Hash user attributes
-      # By default, the bucketing ID should be the user ID
-      bucketing_id = user_id
+      # Returns String representing bucketing ID if it is a String type in attributes else return user ID
 
-      # If the bucketing ID key is defined in attributes, then use that in place of the userID
-      if attributes && attributes[Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID']].is_a?(String)
-        unless attributes[Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID']].empty?
-          bucketing_id = attributes[Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID']]
-          @config.logger.log(Logger::DEBUG, "Setting the bucketing ID '#{bucketing_id}'")
-        end
+      return user_id unless attributes
+
+      bucketing_id = attributes[Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID']]
+
+      if bucketing_id
+        return bucketing_id if bucketing_id.is_a?(String)
+
+        @config.logger.log(Logger::WARN, 'Bucketing ID attribute is not a string. Defaulted to user ID.')
       end
-      bucketing_id
+      user_id
     end
   end
 end

lib/optimizely/event_builder.rb

@@ -15,10 +15,12 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 #
-require_relative './audience'
-require_relative './params'
-require_relative './version'
-require_relative '../optimizely/helpers/event_tag_utils'
+require_relative 'audience'
+require_relative 'helpers/constants'
+require_relative 'helpers/event_tag_utils'
+require_relative 'params'
+require_relative 'version'
+
 require 'securerandom'
 
 module Optimizely
@@ -74,9 +76,9 @@ def get_common_params(user_id, attributes)
       visitor_attributes = []
 
       attributes&.keys&.each do |attribute_key|
-        # Omit null attribute values
+        # Omit attribute values that are not supported by the log endpoint.
         attribute_value = attributes[attribute_key]
-        unless attribute_value.nil?
+        if Helpers::Validator.attribute_valid?(attribute_key, attribute_value)
           attribute_id = @config.get_attribute_id attribute_key
           if attribute_id
             visitor_attributes.push(

lib/optimizely/helpers/constants.rb

@@ -327,6 +327,8 @@ module Constants
         'v3' => '3',
         'v4' => '4'
       }.freeze
+
+      ATTRIBUTE_VALID_TYPES = [FalseClass, Float, Integer, String, TrueClass].freeze
     end
   end
 end

lib/optimizely/helpers/validator.rb

@@ -34,6 +34,19 @@ def attributes_valid?(attributes)
         attributes.is_a?(Hash)
       end
 
+      def attribute_valid?(attribute_key, attribute_value)
+        # Determines if provided attribute_key and attribute_value are valid.
+        #
+        # attribute_key - Variable which needs to be validated.
+        # attribute_value - Variable which needs to be validated.
+        #
+        # Returns boolean depending on validity of attribute_key and attribute_value.
+
+        return false unless attribute_key.is_a?(String) || attribute_key.is_a?(Symbol)
+
+        Helpers::Constants::ATTRIBUTE_VALID_TYPES.any? { |type| attribute_value.is_a?(type) }
+      end
+
       def event_tags_valid?(event_tags)
         # Determines if provided event tags are valid.
         #

spec/condition_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 #
-#    Copyright 2016-2017, Optimizely and contributors
+#    Copyright 2016-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.
@@ -145,4 +145,19 @@
     condition = JSON.parse(condition)
     expect(@condition_evaluator.evaluate(condition)).to be true
   end
+
+  it 'should evaluate to true when user attributes match the audience conditions' do
+    user_attributes = {
+      'device_type' => 'iPhone',
+      'is_firefox' => false,
+      'num_users' => 15,
+      'pi_value' => 3.14
+    }
+    condition_evaluator = Optimizely::ConditionEvaluator.new(user_attributes)
+    condition = '["and", ["or", ["or", {"name": "device_type", "type": "custom_attribute", "value": "iPhone"}]],
+    ["or", ["or", {"name": "is_firefox", "type": "custom_attribute", "value": false}]], ["or", ["or", {"name": "num_users",
+      "type": "custom_attribute", "value": 15}]], ["or", ["or", {"name": "pi_value", "type": "custom_attribute", "value": 3.14}]]]'
+    condition = JSON.parse(condition)
+    expect(condition_evaluator.evaluate(condition)).to be true
+  end
 end

spec/decision_service_spec.rb

@@ -676,4 +676,41 @@
       end
     end
   end
+  describe '#get_bucketing_id' do
+    it 'should log a message and return user ID when bucketing ID is not a String' do
+      user_attributes = {
+        'browser_type' => 'firefox',
+        Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 5
+      }
+      expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('test_user')
+      expect(spy_logger).to have_received(:log).once.with(Logger::WARN, 'Bucketing ID attribute is not a string. Defaulted to user ID.')
+    end
+
+    it 'should not log any message and return user ID when bucketing ID is nil' do
+      user_attributes = {
+        'browser_type' => 'firefox',
+        Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => nil
+      }
+      expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('test_user')
+      expect(spy_logger).not_to have_received(:log)
+    end
+
+    it 'should not log any message and return given bucketing ID when bucketing ID is a String' do
+      user_attributes = {
+        'browser_type' => 'firefox',
+        Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => 'i_am_bucketing_id'
+      }
+      expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('i_am_bucketing_id')
+      expect(spy_logger).not_to have_received(:log)
+    end
+
+    it 'should not log any message and return empty String when bucketing ID is empty String' do
+      user_attributes = {
+        'browser_type' => 'firefox',
+        Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID'] => ''
+      }
+      expect(decision_service.send(:get_bucketing_id, 'test_user', user_attributes)).to eq('')
+      expect(spy_logger).not_to have_received(:log)
+    end
+  end
 end

spec/event_builder_spec.rb

@@ -125,6 +125,81 @@
     expect(impression_event.http_verb).to eq(:post)
   end
 
+  it 'should create a valid Event when create_impression_event is called with attributes of different valid types' do
+    @expected_impression_params[:visitors][0][:attributes] = [
+      {
+        entity_id: '111094',
+        key: 'browser_type',
+        type: 'custom',
+        value: 'firefox'
+      }, {
+        entity_id: '111095',
+        key: 'boolean_key',
+        type: 'custom',
+        value: true
+      }, {
+        entity_id: '111096',
+        key: 'integer_key',
+        type: 'custom',
+        value: 5
+      }, {
+        entity_id: '111097',
+        key: 'double_key',
+        type: 'custom',
+        value: 5.5
+      },
+      entity_id: Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BOT_FILTERING'],
+      key: Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BOT_FILTERING'],
+      type: 'custom',
+      value: true
+    ]
+
+    experiment = config.get_experiment_from_key('test_experiment')
+    attributes = {
+      'browser_type' => 'firefox',
+      'boolean_key' => true,
+      'integer_key' => 5,
+      'double_key' => 5.5
+    }
+    impression_event = @event_builder.create_impression_event(experiment, '111128', 'test_user', attributes)
+    expect(impression_event.params).to eq(@expected_impression_params)
+    expect(impression_event.url).to eq(@expected_endpoint)
+    expect(impression_event.http_verb).to eq(:post)
+  end
+
+  it 'should create a valid Event and exclude attributes of invalid types' do
+    @expected_impression_params[:visitors][0][:attributes] = [
+      {
+        entity_id: '111094',
+        key: 'browser_type',
+        type: 'custom',
+        value: 'firefox'
+      },
+      {
+        entity_id: '111096',
+        key: 'integer_key',
+        type: 'custom',
+        value: 5
+      },
+      entity_id: Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BOT_FILTERING'],
+      key: Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BOT_FILTERING'],
+      type: 'custom',
+      value: true
+    ]
+
+    experiment = config.get_experiment_from_key('test_experiment')
+    attributes = {
+      'browser_type' => 'firefox',
+      'boolean_key' => nil,
+      'integer_key' => 5,
+      'double_key' => {}
+    }
+    impression_event = @event_builder.create_impression_event(experiment, '111128', 'test_user', attributes)
+    expect(impression_event.params).to eq(@expected_impression_params)
+    expect(impression_event.url).to eq(@expected_endpoint)
+    expect(impression_event.http_verb).to eq(:post)
+  end
+
   it 'should create a valid Event when create_impression_event is called with attributes as a false value' do
     @expected_impression_params[:visitors][0][:attributes].unshift(
       entity_id: '111094',

spec/project_config_spec.rb

@@ -42,7 +42,10 @@
       expect(project_config.revision).to eq(config_body['revision'])
 
       expected_attribute_key_map = {
-        'browser_type' => config_body['attributes'][0]
+        'browser_type' => config_body['attributes'][0],
+        'boolean_key' => config_body['attributes'][1],
+        'integer_key' => config_body['attributes'][2],
+        'double_key' => config_body['attributes'][3]
       }
       expected_audience_id_map = {
         '11154' => config_body['audiences'][0],