Add logger to Event tag util (#90)

Author: oakbani

lib/optimizely.rb

@@ -85,7 +85,7 @@ def initialize(datafile, event_dispatcher = nil, logger = nil, error_handler = n
       end
 
       @decision_service = DecisionService.new(@config, @user_profile_service)
-      @event_builder = EventBuilder.new(@config)
+      @event_builder = EventBuilder.new(@config, logger)
       @notification_center = NotificationCenter.new(@logger, @error_handler)
     end
 

lib/optimizely/event_builder.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.
@@ -49,9 +49,11 @@ class BaseEventBuilder
     CUSTOM_ATTRIBUTE_FEATURE_TYPE = 'custom'
 
     attr_reader :config
+    attr_reader :logger
 
-    def initialize(config)
+    def initialize(config, logger)
       @config = config
+      @logger = logger
     end
 
     private
@@ -215,10 +217,10 @@ def get_conversion_params(event_key, event_tags, experiment_variation_map)
         }
 
         if event_tags
-          revenue_value = Helpers::EventTagUtils.get_revenue_value(event_tags)
+          revenue_value = Helpers::EventTagUtils.get_revenue_value(event_tags, @logger)
           event_object[:revenue] = revenue_value if revenue_value
 
-          numeric_value = Helpers::EventTagUtils.get_numeric_value(event_tags)
+          numeric_value = Helpers::EventTagUtils.get_numeric_value(event_tags, @logger)
           event_object[:value] = numeric_value if numeric_value
 
           event_object[:tags] = event_tags

lib/optimizely/helpers/event_tag_utils.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.
@@ -26,50 +26,63 @@ module EventTagUtils
       REVENUE_EVENT_METRIC_NAME = 'revenue'
       NUMERIC_EVENT_METRIC_NAME = 'value'
 
-      def string_numeric?(str)
-        !Float(str).nil?
-      rescue
-        false
-      end
-
-      def get_revenue_value(event_tags)
+      def get_revenue_value(event_tags, logger)
         # Grab the revenue value from the event tags. "revenue" is a reserved keyword.
-        # The revenue value must be an integer.
-        #
+        # The value will be parsed to an integer if possible.
+        # Example:
+        #   4.0 or "4.0" will be parsed to int(4).
+        #   4.1 will not be parsed and the method will return nil.
         # event_tags - Hash representing metadata associated with the event.
+        # logger - Optional component which provides a log method to log messages.
+        #
         # Returns revenue value as an integer number
         # Returns nil if revenue can't be retrieved from the event tags.
 
-        if event_tags.nil? || !Helpers::Validator.event_tags_valid?(event_tags)
+        if event_tags.nil?
+          logger.log(Logger::DEBUG, 'Event tags is undefined.')
           return nil
         end
 
-        return nil unless event_tags.key?(REVENUE_EVENT_METRIC_NAME)
+        unless Helpers::Validator.event_tags_valid?(event_tags)
+          logger.log(Logger::DEBUG, 'Event tags is not a hash.')
+          return nil
+        end
+
+        unless event_tags.key?(REVENUE_EVENT_METRIC_NAME)
+          logger.log(Logger::DEBUG, 'The revenue key is not defined in the event tags.')
+          return nil
+        end
+
+        if event_tags[REVENUE_EVENT_METRIC_NAME].nil?
+          logger.log(Logger::DEBUG, 'The revenue key is nil.')
+          return nil
+        end
 
-        logger = SimpleLogger.new
         raw_value = event_tags[REVENUE_EVENT_METRIC_NAME]
 
-        unless raw_value.is_a? Numeric
-          logger.log(Logger::WARN, "Failed to parse revenue value #{raw_value} from event tags.")
+        unless Helpers::Validator.string_numeric?(raw_value)
+          logger.log(Logger::WARN, 'Revenue value is not an integer or float, or is not a numeric string.')
           return nil
         end
 
-        if raw_value.is_a? Float
+        raw_value = raw_value.to_f if raw_value.is_a? String
+
+        unless raw_value == raw_value.to_i
           logger.log(Logger::WARN, "Failed to parse revenue value #{raw_value} from event tags.")
           return nil
         end
 
-        logger.log(Logger::INFO, "Parsed revenue value #{raw_value} from event tags.")
-        raw_value
+        logger.log(Logger::INFO, "Parsed revenue value #{raw_value.to_i} from event tags.")
+        raw_value.to_i
       end
 
-      def get_numeric_value(event_tags, logger = nil)
+      def get_numeric_value(event_tags, logger)
         # Grab the numeric event value from the event tags. "value" is a reserved keyword.
         # The value of 'value' can be a float or a numeric string
         #
         # event_tags - +Hash+ representing metadata associated with the event.
+        # logger - Optional component which provides a log method to log messages.
         # Returns  +Number+ | +nil+ if value can't be retrieved from the event tags.
-        logger = SimpleLogger.new if logger.nil?
 
         if event_tags.nil?
           logger.log(Logger::DEBUG, 'Event tags is undefined.')

spec/event_builder_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.
@@ -32,7 +32,7 @@
 
   before(:example) do
     config = Optimizely::ProjectConfig.new(@config_body_json, @logger, @error_handler)
-    @event_builder = Optimizely::EventBuilder.new(config)
+    @event_builder = Optimizely::EventBuilder.new(config, @logger)
 
     time_now = Time.now
     allow(Time).to receive(:now).and_return(time_now)
@@ -193,11 +193,13 @@
     expect(conversion_event.http_verb).to eq(:post)
   end
 
-  it 'should create a valid Event when create_conversion_event is called with invalid revenue event tag' do
+  it 'should create a valid Event when create_conversion_event is called with valid string revenue event tag' do
     event_tags = {'revenue' => '4200'}
 
     @expected_conversion_params[:visitors][0][:attributes] = []
-    @expected_conversion_params[:visitors][0][:snapshots][0][:events][0][:tags] = event_tags
+    @expected_conversion_params[:visitors][0][:snapshots][0][:events][0].merge!(revenue: 4200,
+                                                                                tags: event_tags)
+
     conversion_event = @event_builder.create_conversion_event('test_event', 'test_user', nil, event_tags,
                                                               '111127' => '111128')
     expect(conversion_event.params).to eq(@expected_conversion_params)

spec/event_tag_utils_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.
@@ -24,69 +24,187 @@
   let(:logger) { Optimizely::NoOpLogger.new }
 
   describe '.get_revenue_value' do
+    it 'should return nil if argument is undefined' do
+      expect(logger).to receive(:log).with(Logger::DEBUG,
+                                           'Event tags is undefined.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(nil, logger)).to be_nil
+    end
+
     it 'should return nil if argument is not a Hash' do
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(nil)).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(0.5)).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(65_536)).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(9_223_372_036_854_775_807)).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value('65536')).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(false)).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(true)).to be_nil
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value([])).to be_nil
+      expect(logger).to receive(:log).with(Logger::DEBUG,
+                                           'Event tags is not a hash.').exactly(7).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(0.5, logger)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(65_536, logger)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(9_223_372_036_854_775_807, logger)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value('65536', logger)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(false, logger)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(true, logger)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value([], logger)).to be_nil
     end
-    it 'should return nil if event tags does not contain the revenue' do
+
+    it 'should return nil if event tags does not contain the revenue key' do
       event_tags = {
         'non-revenue' => 5432
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(logger).to receive(:log)
+        .with(Logger::DEBUG, 'The revenue key is not defined in the event tags.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
     end
-    it 'should return nil if event tags contains the revenue with a string value' do
+
+    it 'should return nil if event tags contains the revenue key with nil value' do
       event_tags = {
-        'revenue' => 'string'
+        'revenue' => nil
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(logger).to receive(:log).with(Logger::DEBUG,
+                                           'The revenue key is nil.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
     end
+
     it 'should return nil if event tags contains the revenue with a string value' do
       event_tags = {
-        'revenue' => '65536'
+        'revenue' => 'string'
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Revenue value is not an integer or float, or is not a numeric string.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
     end
+
     it 'should return nil if event tags contains the revenue with a boolean true value' do
       event_tags = {
         'revenue' => true
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Revenue value is not an integer or float, or is not a numeric string.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
     end
+
     it 'should return nil if event tags contains the revenue with a boolean false value' do
       event_tags = {
         'revenue' => false
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Revenue value is not an integer or float, or is not a numeric string.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
     end
+
     it 'should return nil if event tags contains the revenue with a list value' do
       event_tags = {
         'revenue' => []
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Revenue value is not an integer or float, or is not a numeric string.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
     end
-    it 'should return nil if event tags contains the revenue with a float value' do
+
+    it 'should return nil if event tags contains the revenue with a invalid float value' do
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Failed to parse revenue value 0.5 from event tags.').exactly(1).times
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Failed to parse revenue value 2.5 from event tags.').exactly(1).times
       event_tags = {
         'revenue' => 0.5
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to be_nil
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
+      event_tags['revenue'] = 2.5
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
+    end
+
+    it 'should return nil if event tags contains the revenue with a invalid string numeric' do
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Failed to parse revenue value 0.5 from event tags.').exactly(1).times
+      expect(logger).to receive(:log)
+        .with(Logger::WARN, 'Failed to parse revenue value 2.5 from event tags.').exactly(1).times
+      event_tags = {
+        'revenue' => '0.5'
+      }
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
+      event_tags['revenue'] = '2.5'
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to be_nil
+    end
+
+    it 'should return correct value if event tags contains the revenue with a valid float value' do
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 65536 from event tags.').exactly(1).times
+      event_tags = {
+        'revenue' => 65_536.0
+      }
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(65_536)
     end
+
+    it 'should return correct value if event tags contains the revenue with a valid float string' do
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 65536 from event tags.').exactly(1).times
+      event_tags = {
+        'revenue' => '65536.0'
+      }
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(65_536)
+    end
+
+    it 'should return correct value if event tags contains the revenue with a valid integer string' do
+      event_tags = {
+        'revenue' => '65536'
+      }
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 65536 from event tags.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(65_536)
+    end
+
     it 'should return correct value if event tags contains the revenue with an integer value' do
       event_tags = {
         'revenue' => 65_536
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to eq(65_536)
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 65536 from event tags.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(65_536)
+    end
+
+    it 'should return correct value if event tags contains the revenue with an integer zero value' do
+      event_tags = {
+        'revenue' => 0
+      }
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 0 from event tags.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(0)
+    end
+
+    it 'should return correct value if event tags contains the revenue with an float zero value' do
+      event_tags = {
+        'revenue' => 0.0
+      }
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 0 from event tags.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(0.0)
     end
-    it 'should return correct value if event tags contains the revenue with a long value' do
+
+    it 'should return correct value if event tags contains the revenue with a long integer value' do
       event_tags = {
         'revenue' => 9_223_372_036_854_775_807
       }
-      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags)).to eq(9_223_372_036_854_775_807)
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, 'Parsed revenue value 9223372036854775807 from event tags.').exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(9_223_372_036_854_775_807)
+    end
+
+    it 'should return correct value if event tags contains the revenue with a long float value' do
+      value = 9_223_372_036_854_775_807.to_f
+      event_tags = {
+        'revenue' => value
+      }
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, "Parsed revenue value #{value.to_i} from event tags.").exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(value.to_i)
+    end
+
+    it 'should return correct value if event tags contains the revenue with a long string value' do
+      value = '9_223_372_036_854_775_807'
+      event_tags = {
+        'revenue' => value
+      }
+      # Float converts long number to E-notation i.e "9.223372036854776e+18"
+      value = value.to_f
+      expect(logger).to receive(:log)
+        .with(Logger::INFO, "Parsed revenue value #{value.to_i} from event tags.").exactly(1).times
+      expect(Optimizely::Helpers::EventTagUtils.get_revenue_value(event_tags, logger)).to eq(value.to_i)
     end
   end