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..f81fe4b9 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,15 @@ 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) + # Filter out null and empty identifier values + valid_identifiers = {k: v for k, v in identifiers.items() if v is not None and v != ''} + + # Only send identify event when 2+ valid identifiers exist + if len(valid_identifiers) < 2: + self.logger.debug('ODP identify event is not dispatched (fewer than 2 valid identifiers).') + 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..6560cf8d 100644 --- a/optimizely/optimizely_user_context.py +++ b/optimizely/optimizely_user_context.py @@ -73,7 +73,8 @@ def __init__( ] = {} if self.client and identify: - self.client._identify_user(user_id) + from optimizely.helpers.enums import OdpManagerConfig + self.client._identify_user({OdpManagerConfig.KEY_FOR_USER_ID: user_id}) 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..80c36163 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'}) manager.send_event('t1', 'a1', {}, {}) mock_logger.error.assert_called_once_with('ODP is not enabled.') @@ -126,10 +126,11 @@ def test_identify_user_datafile_not_ready(self): manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) + identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'} with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: - manager.identify_user('user1') + manager.identify_user(identifiers) - mock_identify_user.assert_called_once_with('user1') + mock_identify_user.assert_called_once_with(identifiers) mock_logger.error.assert_not_called() def test_identify_user_odp_integrated(self): @@ -139,13 +140,14 @@ def test_identify_user_odp_integrated(self): manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger) manager.update_odp_config('key1', 'host1', []) + identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'} with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: - manager.identify_user('user1') + manager.identify_user(identifiers) mock_dispatch_event.assert_called_once_with({ 'type': 'fullstack', 'action': 'identified', - 'identifiers': {'fs_user_id': 'user1'}, + 'identifiers': {'fs_user_id': 'user1', 'email': 'user@example.com'}, 'data': { 'idempotence_id': mock.ANY, 'data_source_type': 'sdk', @@ -161,8 +163,9 @@ def test_identify_user_odp_not_integrated(self): manager = OdpManager(False, CustomCache(), event_manager=event_manager, logger=mock_logger) manager.update_odp_config(None, None, []) + identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'} with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: - manager.identify_user('user1') + manager.identify_user(identifiers) mock_dispatch_event.assert_not_called() mock_logger.error.assert_not_called() @@ -175,13 +178,94 @@ def test_identify_user_odp_disabled(self): manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.enabled = False + identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'} with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: - manager.identify_user('user1') + manager.identify_user(identifiers) mock_identify_user.assert_not_called() mock_logger.error.assert_not_called() mock_logger.debug.assert_called_with('ODP identify event is not dispatched (ODP disabled).') + def test_identify_user_single_identifier_skipped(self): + """Single identifier should NOT trigger an ODP 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 (fewer than 2 valid identifiers).') + + def test_identify_user_multiple_identifiers_sent(self): + """Multiple identifiers should trigger an ODP 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', []) + + identifiers = {'fs_user_id': 'user1', 'vuid': 'vuid-abc'} + with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: + manager.identify_user(identifiers) + + mock_identify_user.assert_called_once_with(identifiers) + + def test_identify_user_three_identifiers_sent(self): + """Three identifiers should trigger an ODP identify event with all identifiers.""" + 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', []) + + identifiers = {'fs_user_id': 'user1', 'vuid': 'vuid-abc', 'email': 'user@example.com'} + with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: + manager.identify_user(identifiers) + + mock_identify_user.assert_called_once_with(identifiers) + + def test_identify_user_null_empty_values_not_counted(self): + """Null and empty identifier values should not count toward the identifier total.""" + 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', []) + + # Two identifiers but one is empty - should not send + with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: + manager.identify_user({'fs_user_id': 'user1', 'email': ''}) + + mock_identify_user.assert_not_called() + mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).') + + mock_logger.reset_mock() + + # Two identifiers but one is None - should not send + with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: + manager.identify_user({'fs_user_id': 'user1', 'email': None}) + + mock_identify_user.assert_not_called() + mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).') + + def test_identify_user_zero_identifiers_skipped(self): + """Zero identifiers should NOT trigger an ODP 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({}) + + mock_identify_user.assert_not_called() + mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).') + def test_send_event_datafile_not_ready(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(mock_logger, OdpEventApiManager()) 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: