diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index 3fb961ac..2163f2d7 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -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.""" diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py index a6e26253..024fb1d1 100644 --- a/optimizely/odp/odp_manager.py +++ b/optimizely/odp/odp_manager.py @@ -73,7 +73,7 @@ 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 @@ -81,7 +81,12 @@ def identify_user(self, user_id: str) -> None: 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: """ diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 4b014e7f..04162823 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -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 @@ -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: diff --git a/optimizely/optimizely_user_context.py b/optimizely/optimizely_user_context.py index e88c0f52..379dfb0e 100644 --- a/optimizely/optimizely_user_context.py +++ b/optimizely/optimizely_user_context.py @@ -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 @@ -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 diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index ae0e4a1a..8e3f33f4 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -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.') @@ -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): @@ -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', @@ -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() @@ -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() diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index c2de186f..0340ef72 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -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() diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 3ae9be0d..3edb072c 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -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() @@ -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: