[AI-FSSDK] [FSSDK-12721] Skip ODP identify event for single-identifier users

Author: jaeopt

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."""

optimizely/odp/odp_manager.py

@@ -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:
         """

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:

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

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()

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()
 

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: