Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.'


Expand Down
17 changes: 16 additions & 1 deletion optimizely/odp/odp_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]]

Expand All @@ -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:
Expand All @@ -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:
Expand Down
9 changes: 8 additions & 1 deletion optimizely/optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'))
Expand Down
13 changes: 13 additions & 0 deletions tests/test_odp_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
40 changes: 40 additions & 0 deletions tests/test_optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()