Skip to content
Closed
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
4 changes: 2 additions & 2 deletions optimizely/odp/odp_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ def dispatch(self, event: OdpEvent) -> None:
except Full:
self.logger.warning(Errors.ODP_EVENT_FAILED.format("Queue is full"))

def identify_user(self, user_id: str) -> None:
def identify_user(self, identifiers: dict[str, str]) -> None:
self.send_event(OdpManagerConfig.EVENT_TYPE, 'identified',
{OdpManagerConfig.KEY_FOR_USER_ID: user_id}, {})
identifiers, {})

def update_config(self) -> None:
"""Adds update config signal to event_queue."""
Expand Down
9 changes: 7 additions & 2 deletions optimizely/odp/odp_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,20 @@ def fetch_qualified_segments(self, user_id: str, options: list[str]) -> Optional

return self.segment_manager.fetch_qualified_segments(user_key, user_value, options)

def identify_user(self, user_id: str) -> None:
def identify_user(self, identifiers: dict[str, str]) -> None:
if not self.enabled or not self.event_manager:
self.logger.debug('ODP identify event is not dispatched (ODP disabled).')
return
if self.odp_config.odp_state() == OdpConfigState.NOT_INTEGRATED:
self.logger.debug('ODP identify event is not dispatched (ODP not integrated).')
return

self.event_manager.identify_user(user_id)
valid_identifiers = {k: v for k, v in identifiers.items() if v}
if len(valid_identifiers) < 2:
self.logger.debug('ODP identify event is not dispatched (only one identifier provided).')
return

self.event_manager.identify_user(valid_identifiers)

def send_event(self, type: str, action: str, identifiers: dict[str, str], data: dict[str, Any]) -> None:
"""
Expand Down
4 changes: 2 additions & 2 deletions optimizely/optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,7 @@ def _update_odp_config_on_datafile_update(self) -> None:
config.all_segments
)

def _identify_user(self, user_id: str) -> None:
def _identify_user(self, identifiers: dict[str, str]) -> None:
if not self.is_valid:
self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('identify_user'))
return
Expand All @@ -1551,7 +1551,7 @@ def _identify_user(self, user_id: str) -> None:
self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('identify_user'))
return

self.odp_manager.identify_user(user_id)
self.odp_manager.identify_user(identifiers)

def _fetch_qualified_segments(self, user_id: str, options: Optional[list[str]] = None) -> Optional[list[str]]:
if not self.is_valid:
Expand Down
4 changes: 3 additions & 1 deletion optimizely/optimizely_user_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from typing import TYPE_CHECKING, Any, Callable, Optional, NewType, Dict

from optimizely.decision import optimizely_decision
from optimizely.helpers.enums import OdpManagerConfig

if TYPE_CHECKING:
# prevent circular dependency by skipping import at runtime
Expand Down Expand Up @@ -73,7 +74,8 @@ def __init__(
] = {}

if self.client and identify:
self.client._identify_user(user_id)
identifiers = {OdpManagerConfig.KEY_FOR_USER_ID: user_id}
self.client._identify_user(identifiers)

class OptimizelyDecisionContext:
""" Using class with attributes here instead of namedtuple because
Expand Down
66 changes: 59 additions & 7 deletions tests/test_odp_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_configurations_disable_odp(self):
mock_logger.reset_mock()

# these call should be dropped gracefully with None
manager.identify_user('user1')
manager.identify_user({'fs_user_id': 'user1', 'email': 'user1@example.com'})

manager.send_event('t1', 'a1', {}, {})
mock_logger.error.assert_called_once_with('ODP is not enabled.')
Expand Down Expand Up @@ -120,16 +120,68 @@ def test_fetch_qualified_segments__seg_cache_and_seg_mgr_are_none(self):
mock_logger.error.assert_not_called()
mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', [])

def test_identify_user_single_identifier_skipped(self):
"""Single identifier should NOT dispatch an identify event."""
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())

manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
manager.identify_user({'fs_user_id': 'user1'})

mock_identify_user.assert_not_called()
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (only one identifier provided).')

def test_identify_user_empty_values_not_counted(self):
"""Identifiers with empty or null values should not count toward the minimum."""
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())

manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
manager.identify_user({'fs_user_id': 'user1', 'email': '', 'vuid': ''})

mock_identify_user.assert_not_called()
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (only one identifier provided).')

def test_identify_user_multiple_identifiers_sent(self):
"""Multiple valid identifiers should dispatch an identify event."""
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())

manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
manager.identify_user({'fs_user_id': 'user1', 'email': 'user1@example.com'})

mock_dispatch_event.assert_called_once_with({
'type': 'fullstack',
'action': 'identified',
'identifiers': {'fs_user_id': 'user1', 'email': 'user1@example.com'},
'data': {
'idempotence_id': mock.ANY,
'data_source_type': 'sdk',
'data_source': 'python-sdk',
'data_source_version': version.__version__
}})
mock_logger.error.assert_not_called()

def test_identify_user_datafile_not_ready(self):
"""When datafile is not ready but ODP config allows, identifiers with 2+ valid entries should forward."""
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger)

manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)

with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
manager.identify_user('user1')
manager.identify_user({'fs_user_id': 'user1', 'email': 'user1@example.com'})

mock_identify_user.assert_called_once_with('user1')
mock_identify_user.assert_called_once_with({'fs_user_id': 'user1', 'email': 'user1@example.com'})
mock_logger.error.assert_not_called()

def test_identify_user_odp_integrated(self):
Expand All @@ -140,12 +192,12 @@ def test_identify_user_odp_integrated(self):
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
manager.identify_user('user1')
manager.identify_user({'fs_user_id': 'user1', 'vuid': 'vuid123'})

mock_dispatch_event.assert_called_once_with({
'type': 'fullstack',
'action': 'identified',
'identifiers': {'fs_user_id': 'user1'},
'identifiers': {'fs_user_id': 'user1', 'vuid': 'vuid123'},
'data': {
'idempotence_id': mock.ANY,
'data_source_type': 'sdk',
Expand All @@ -162,7 +214,7 @@ def test_identify_user_odp_not_integrated(self):
manager.update_odp_config(None, None, [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
manager.identify_user('user1')
manager.identify_user({'fs_user_id': 'user1', 'email': 'user1@example.com'})

mock_dispatch_event.assert_not_called()
mock_logger.error.assert_not_called()
Expand All @@ -176,7 +228,7 @@ def test_identify_user_odp_disabled(self):
manager.enabled = False

with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
manager.identify_user('user1')
manager.identify_user({'fs_user_id': 'user1', 'email': 'user1@example.com'})

mock_identify_user.assert_not_called()
mock_logger.error.assert_not_called()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -5343,7 +5343,7 @@ def test_send_identify_event__when_called_with_odp_enabled(self):
with mock.patch.object(client, '_identify_user') as identify:
client.create_user_context('user-id')

identify.assert_called_once_with('user-id')
identify.assert_called_once_with({'fs_user_id': 'user-id'})
mock_logger.error.assert_not_called()
client.close()

Expand Down
4 changes: 2 additions & 2 deletions tests/test_user_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -2094,7 +2094,7 @@ def test_send_identify_event_when_user_context_created(self):
with mock.patch.object(client, '_identify_user') as identify:
OptimizelyUserContext(client, mock_logger, 'user-id')

identify.assert_called_once_with('user-id')
identify.assert_called_once_with({'fs_user_id': 'user-id'})
mock_logger.error.assert_not_called()
client.close()

Expand All @@ -2104,7 +2104,7 @@ def test_identify_is_skipped_with_decisions(self):
with mock.patch.object(client, '_identify_user') as identify:
user_context = OptimizelyUserContext(client, mock_logger, 'user-id')

identify.assert_called_once_with('user-id')
identify.assert_called_once_with({'fs_user_id': 'user-id'})
mock_logger.error.assert_not_called()

with mock.patch.object(client, '_identify_user') as identify:
Expand Down
Loading