From 2bfc4e0c703f35f60581883901355edbbf1ae1fa Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Mon, 18 May 2026 11:54:51 -0700 Subject: [PATCH] [AI-FSSDK] [FSSDK-12670] Block ODP identify event for single identifier --- optimizely/odp/odp_event_manager.py | 23 +++++++++- optimizely/odp/odp_manager.py | 4 +- optimizely/optimizely.py | 4 +- optimizely/optimizely_user_context.py | 4 +- tests/test_odp_manager.py | 60 +++++++++++++++++++++++---- tests/test_optimizely.py | 2 +- tests/test_user_context.py | 4 +- 7 files changed, 84 insertions(+), 17 deletions(-) diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index 3fb961ac7..369637c47 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -267,9 +267,28 @@ 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: + """Send an identify event to ODP if there are multiple valid identifiers. + + An identify event is only useful when there are two or more identifiers + to link together in the ODP user profile. Sending a single identifier + provides no linking value and wastes a network call. + + Args: + identifiers: A dictionary of identifier key-value pairs + (e.g. {"fs_user_id": "abc", "email": "a@b.com"}). + """ + # Filter out identifiers with None or empty string values. + 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.send_event(OdpManagerConfig.EVENT_TYPE, 'identified', - {OdpManagerConfig.KEY_FOR_USER_ID: user_id}, {}) + valid_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 a6e262531..10c160e09 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,7 @@ 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) + self.event_manager.identify_user(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 4b014e7f2..04162823b 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 e88c0f521..379dfb0ed 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 ae0e4a1a3..c9483dc0d 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': 'test@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': 'test@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': 'test@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': 'test@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,56 @@ 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': 'test@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): + """Identify event should not be sent when only one identifier is provided.""" + 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'}) + + mock_dispatch_event.assert_not_called() + + def test_identify_user_empty_values_not_counted(self): + """Identifiers with empty or None 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, 'dispatch') as mock_dispatch_event: + manager.identify_user({'fs_user_id': 'user1', 'email': '', 'vuid': None}) + + mock_dispatch_event.assert_not_called() + + def test_identify_user_multiple_identifiers_sent(self): + """Identify event should be sent when two or more valid identifiers are provided.""" + 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': 'vuid123', 'email': 'a@b.com'} + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + manager.identify_user(identifiers) + + mock_dispatch_event.assert_called_once() + event = mock_dispatch_event.call_args[0][0] + self.assertEqual(event.identifiers, 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 c2de186fa..0340ef72a 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 3ae9be0dc..3edb072ce 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: