[FSSDK-9069] fix: odp event validation (#423)

* fix odp send event validation

* add unit tests

* update action missing error

Author: andrewleap-optimizely

Diff for: optimizely/helpers/enums.py

@@ -126,6 +126,7 @@ class Errors:
     ODP_NOT_INTEGRATED: Final = 'ODP is not integrated.'
     ODP_NOT_ENABLED: Final = 'ODP is not enabled.'
     ODP_INVALID_DATA: Final = 'ODP data is not valid.'
+    ODP_INVALID_ACTION: Final = 'ODP action is not valid (cannot be empty).'
     MISSING_SDK_KEY: Final = 'SDK key not provided/cannot be found in the datafile.'
 
 

Diff for: optimizely/odp/odp_event.py

@@ -17,6 +17,7 @@
 import uuid
 import json
 from optimizely import version
+from optimizely.helpers.enums import OdpManagerConfig
 
 OdpDataDict = Dict[str, Union[str, int, float, bool, None]]
 
@@ -27,7 +28,7 @@ class OdpEvent:
     def __init__(self, type: str, action: str, identifiers: dict[str, str], data: OdpDataDict) -> None:
         self.type = type
         self.action = action
-        self.identifiers = identifiers
+        self.identifiers = self._convert_identifers(identifiers)
         self.data = self._add_common_event_data(data)
 
     def __repr__(self) -> str:
@@ -51,6 +52,20 @@ def _add_common_event_data(self, custom_data: OdpDataDict) -> OdpDataDict:
         data.update(custom_data)
         return data
 
+    def _convert_identifers(self, identifiers: dict[str, str]) -> dict[str, str]:
+        """
+        Convert incorrect case/separator of identifier key `fs_user_id`
+        (ie. `fs-user-id`, `FS_USER_ID`).
+        """
+        for key in list(identifiers):
+            if key == OdpManagerConfig.KEY_FOR_USER_ID:
+                break
+            elif key.lower() in ("fs-user-id", OdpManagerConfig.KEY_FOR_USER_ID):
+                identifiers[OdpManagerConfig.KEY_FOR_USER_ID] = identifiers.pop(key)
+                break
+
+        return identifiers
+
 
 class OdpEventEncoder(json.JSONEncoder):
     def default(self, obj: object) -> Any:

Diff for: optimizely/optimizely.py

@@ -1387,7 +1387,7 @@ def send_odp_event(
         Send an event to the ODP server.
 
         Args:
-            action: The event action name.
+            action: The event action name. Cannot be None or empty string.
             identifiers: A dictionary for identifiers. The caller must provide at least one key-value pair.
             type: The event type. Default 'fullstack'.
             data: An optional dictionary for associated data. The default event data will be added to this data
@@ -1397,10 +1397,17 @@ def send_odp_event(
             self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('send_odp_event'))
             return
 
+        if action is None or action == "":
+            self.logger.error(enums.Errors.ODP_INVALID_ACTION)
+            return
+
         if not identifiers or not isinstance(identifiers, dict):
             self.logger.error('ODP events must have at least one key-value pair in identifiers.')
             return
 
+        if type is None or type == "":
+            type = enums.OdpManagerConfig.EVENT_TYPE
+
         config = self.config_manager.get_config()
         if not config:
             self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('send_odp_event'))

Diff for: tests/test_odp_event_manager.py

@@ -98,6 +98,19 @@ def test_invalid_odp_event(self, *args):
         event['data']['invalid-item'] = {}
         self.assertStrictFalse(validator.are_odp_data_types_valid(event['data']))
 
+    def test_odp_event_identifier_conversion(self, *args):
+        event = OdpEvent('type', 'action', {'fs-user-id': 'great'}, {})
+        self.assertDictEqual(event.identifiers, {'fs_user_id': 'great'})
+
+        event = OdpEvent('type', 'action', {'FS-user-ID': 'great'}, {})
+        self.assertDictEqual(event.identifiers, {'fs_user_id': 'great'})
+
+        event = OdpEvent('type', 'action', {'FS_USER_ID': 'great', 'fs.user.id': 'wow'}, {})
+        self.assertDictEqual(event.identifiers, {'fs_user_id': 'great', 'fs.user.id': 'wow'})
+
+        event = OdpEvent('type', 'action', {'fs_user_id': 'great', 'fsuserid': 'wow'}, {})
+        self.assertDictEqual(event.identifiers, {'fs_user_id': 'great', 'fsuserid': 'wow'})
+
     def test_odp_event_manager_success(self, *args):
         mock_logger = mock.Mock()
         event_manager = OdpEventManager(mock_logger)

Diff for: tests/test_optimizely.py

@@ -5483,3 +5483,43 @@ def test_send_odp_event__log_error_with_missing_integrations_data(self):
 
         mock_logger.error.assert_called_with('ODP is not integrated.')
         client.close()
+
+    def test_send_odp_event__log_error_with_action_none(self):
+        mock_logger = mock.Mock()
+        client = optimizely.Optimizely(json.dumps(self.config_dict_with_audience_segments), logger=mock_logger)
+
+        client.send_odp_event(type='wow', action=None, identifiers={'amazing': 'fantastic'}, data={})
+        client.close()
+
+        mock_logger.error.assert_called_once_with('ODP action is not valid (cannot be empty).')
+
+    def test_send_odp_event__log_error_with_action_empty_string(self):
+        mock_logger = mock.Mock()
+        client = optimizely.Optimizely(json.dumps(self.config_dict_with_audience_segments), logger=mock_logger)
+
+        client.send_odp_event(type='wow', action="", identifiers={'amazing': 'fantastic'}, data={})
+        client.close()
+
+        mock_logger.error.assert_called_once_with('ODP action is not valid (cannot be empty).')
+
+    def test_send_odp_event__default_type_when_none(self):
+        mock_logger = mock.Mock()
+
+        client = optimizely.Optimizely(json.dumps(self.config_dict_with_audience_segments), logger=mock_logger)
+        with mock.patch.object(client.odp_manager, 'send_event') as mock_send_event:
+            client.send_odp_event(type=None, action="great", identifiers={'amazing': 'fantastic'}, data={})
+            client.close()
+
+        mock_send_event.assert_called_with('fullstack', 'great', {'amazing': 'fantastic'}, {})
+        mock_logger.error.assert_not_called()
+
+    def test_send_odp_event__default_type_when_empty_string(self):
+        mock_logger = mock.Mock()
+
+        client = optimizely.Optimizely(json.dumps(self.config_dict_with_audience_segments), logger=mock_logger)
+        with mock.patch.object(client.odp_manager, 'send_event') as mock_send_event:
+            client.send_odp_event(type="", action="great", identifiers={'amazing': 'fantastic'}, data={})
+            client.close()
+
+        mock_send_event.assert_called_with('fullstack', 'great', {'amazing': 'fantastic'}, {})
+        mock_logger.error.assert_not_called()