[FSSDK-9074] fix: odp event validation (#334)

andrewleap-optimizely · web-flow · commit b8cee28fd496 · 2023-04-11T13:48:05.000-04:00
* fix odp send event validation

* add unit tests
diff --git a/lib/optimizely.rb b/lib/optimizely.rb
@@ -895,7 +895,7 @@ def get_optimizely_config
 
     # Send an event to the ODP server.
     #
-    # @param action - the event action name.
+    # @param action - the event action name. Cannot be nil or empty string.
     # @param identifiers - a hash for identifiers. The caller must provide at least one key-value pair.
     # @param type - the event type (default = "fullstack").
     # @param data - a hash for associated data. The default event data will be added to this data before sending to the ODP server.
@@ -911,6 +911,13 @@ def send_odp_event(action:, identifiers:, type: Helpers::Constants::ODP_MANAGER_
         return
       end
 
+      if action.nil? || action.empty?
+        @logger.log(Logger::ERROR, Helpers::Constants::ODP_LOGS[:ODP_INVALID_ACTION])
+        return
+      end
+
+      type = Helpers::Constants::ODP_MANAGER_CONFIG[:EVENT_TYPE] if type.nil? || type.empty?
+
       @odp_manager.send_event(type: type, action: action, identifiers: identifiers, data: data)
     end
 
diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb
@@ -387,7 +387,8 @@ module Constants
         ODP_EVENT_FAILED: 'ODP event send failed (%s).',
         ODP_NOT_ENABLED: 'ODP is not enabled.',
         ODP_NOT_INTEGRATED: 'ODP is not integrated.',
-        ODP_INVALID_DATA: 'ODP data is not valid.'
+        ODP_INVALID_DATA: 'ODP data is not valid.',
+        ODP_INVALID_ACTION: 'ODP action is not valid (cannot be empty).'
       }.freeze
 
       DECISION_NOTIFICATION_TYPES = {
diff --git a/lib/optimizely/odp/odp_event.rb b/lib/optimizely/odp/odp_event.rb
@@ -17,14 +17,18 @@
 #
 
 require 'json'
+require_relative '../helpers/constants'
 
 module Optimizely
   class OdpEvent
     # Representation of an odp event which can be sent to the Optimizely odp platform.
+
+    KEY_FOR_USER_ID = Helpers::Constants::ODP_MANAGER_CONFIG[:KEY_FOR_USER_ID]
+
     def initialize(type:, action:, identifiers:, data:)
       @type = type
       @action = action
-      @identifiers = identifiers
+      @identifiers = convert_identifiers(identifiers)
       @data = add_common_event_data(data)
     end
 
@@ -39,6 +43,22 @@ def add_common_event_data(custom_data)
       data
     end
 
+    def convert_identifiers(identifiers)
+      # Convert incorrect case/separator of identifier key `fs_user_id`
+      # (ie. `fs-user-id`, `FS_USER_ID`).
+
+      identifiers.clone.each_key do |key|
+        break if key == KEY_FOR_USER_ID
+
+        if ['fs-user-id', KEY_FOR_USER_ID].include?(key.downcase)
+          identifiers[KEY_FOR_USER_ID] = identifiers.delete(key)
+          break
+        end
+      end
+
+      identifiers
+    end
+
     def to_json(*_args)
       {
         type: @type,
diff --git a/spec/odp/odp_event_manager_spec.rb b/spec/odp/odp_event_manager_spec.rb
@@ -92,6 +92,20 @@
       event[:data]['invalid-item'] = {}
       expect(Optimizely::Helpers::Validator.odp_data_types_valid?(event[:data])).to be false
     end
+
+    it 'should convert invalid event identifier' do
+      event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'fs-user-id' => 'great'}, data: {})
+      expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great'})
+
+      event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'FS-user-ID' => 'great'}, data: {})
+      expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great'})
+
+      event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'FS_USER_ID' => 'great', 'fs.user.id' => 'wow'}, data: {})
+      expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great', 'fs.user.id' => 'wow'})
+
+      event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'fs_user_id' => 'great', 'fsuserid' => 'wow'}, data: {})
+      expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great', 'fsuserid' => 'wow'})
+    end
   end
 
   describe '#initialize' do
diff --git a/spec/project_spec.rb b/spec/project_spec.rb
@@ -4916,5 +4916,39 @@ class InvalidEventManager; end # rubocop:disable Lint/ConstantDefinitionInBlock
 
       project.close
     end
+
+    it 'should log error with nil action' do
+      expect(spy_logger).to receive(:log).once.with(Logger::ERROR, 'ODP action is not valid (cannot be empty).')
+      project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
+      project.send_odp_event(type: 'wow', action: nil, identifiers: {amazing: 'fantastic'}, data: {})
+      project.close
+    end
+
+    it 'should log error with empty string action' do
+      expect(spy_logger).to receive(:log).once.with(Logger::ERROR, 'ODP action is not valid (cannot be empty).')
+      project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
+      project.send_odp_event(type: 'wow', action: '', identifiers: {amazing: 'fantastic'}, data: {})
+      project.close
+    end
+
+    it 'should use default with nil type' do
+      project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
+      expect(project.odp_manager).to receive('send_event').with(type: 'fullstack', action: 'great', identifiers: {amazing: 'fantastic'}, data: {})
+      project.send_odp_event(type: nil, action: 'great', identifiers: {amazing: 'fantastic'}, data: {})
+
+      expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything)
+
+      project.close
+    end
+
+    it 'should use default with empty string type' do
+      project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
+      expect(project.odp_manager).to receive('send_event').with(type: 'fullstack', action: 'great', identifiers: {amazing: 'fantastic'}, data: {})
+      project.send_odp_event(type: '', action: 'great', identifiers: {amazing: 'fantastic'}, data: {})
+
+      expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything)
+
+      project.close
+    end
   end
 end
