Skip to content

Commit 5295be5

Browse files
committed
[FSSDK-12721] Skip ODP identify event for single-identifier users
Change identify_user call chain to accept identifiers dict instead of single user_id string. Only send ODP identify event when 2+ valid (non-null, non-empty) identifiers exist. Filter out null/empty values before counting. Add tests for single, multiple, zero identifiers and null/empty value filtering.
1 parent 013ddf0 commit 5295be5

7 files changed

Lines changed: 110 additions & 17 deletions

File tree

optimizely/odp/odp_event_manager.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,9 @@ def dispatch(self, event: OdpEvent) -> None:
267267
except Full:
268268
self.logger.warning(Errors.ODP_EVENT_FAILED.format("Queue is full"))
269269

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

274274
def update_config(self) -> None:
275275
"""Adds update config signal to event_queue."""

optimizely/odp/odp_manager.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,23 @@ def fetch_qualified_segments(self, user_id: str, options: list[str]) -> Optional
7373

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

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

84-
self.event_manager.identify_user(user_id)
84+
# Filter out null and empty identifier values
85+
valid_identifiers = {k: v for k, v in identifiers.items() if v is not None and v != ''}
86+
87+
# Only send identify event when 2+ valid identifiers exist
88+
if len(valid_identifiers) < 2:
89+
self.logger.debug('ODP identify event is not dispatched (fewer than 2 valid identifiers).')
90+
return
91+
92+
self.event_manager.identify_user(valid_identifiers)
8593

8694
def send_event(self, type: str, action: str, identifiers: dict[str, str], data: dict[str, Any]) -> None:
8795
"""

optimizely/optimizely.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,7 +1541,7 @@ def _update_odp_config_on_datafile_update(self) -> None:
15411541
config.all_segments
15421542
)
15431543

1544-
def _identify_user(self, user_id: str) -> None:
1544+
def _identify_user(self, identifiers: dict[str, str]) -> None:
15451545
if not self.is_valid:
15461546
self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('identify_user'))
15471547
return
@@ -1551,7 +1551,7 @@ def _identify_user(self, user_id: str) -> None:
15511551
self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('identify_user'))
15521552
return
15531553

1554-
self.odp_manager.identify_user(user_id)
1554+
self.odp_manager.identify_user(identifiers)
15551555

15561556
def _fetch_qualified_segments(self, user_id: str, options: Optional[list[str]] = None) -> Optional[list[str]]:
15571557
if not self.is_valid:

optimizely/optimizely_user_context.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ def __init__(
7373
] = {}
7474

7575
if self.client and identify:
76-
self.client._identify_user(user_id)
76+
from optimizely.helpers.enums import OdpManagerConfig
77+
self.client._identify_user({OdpManagerConfig.KEY_FOR_USER_ID: user_id})
7778

7879
class OptimizelyDecisionContext:
7980
""" Using class with attributes here instead of namedtuple because

tests/test_odp_manager.py

Lines changed: 91 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def test_configurations_disable_odp(self):
4848
mock_logger.reset_mock()
4949

5050
# these call should be dropped gracefully with None
51-
manager.identify_user('user1')
51+
manager.identify_user({'fs_user_id': 'user1'})
5252

5353
manager.send_event('t1', 'a1', {}, {})
5454
mock_logger.error.assert_called_once_with('ODP is not enabled.')
@@ -126,10 +126,11 @@ def test_identify_user_datafile_not_ready(self):
126126

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

129+
identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'}
129130
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
130-
manager.identify_user('user1')
131+
manager.identify_user(identifiers)
131132

132-
mock_identify_user.assert_called_once_with('user1')
133+
mock_identify_user.assert_called_once_with(identifiers)
133134
mock_logger.error.assert_not_called()
134135

135136
def test_identify_user_odp_integrated(self):
@@ -139,13 +140,14 @@ def test_identify_user_odp_integrated(self):
139140
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
140141
manager.update_odp_config('key1', 'host1', [])
141142

143+
identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'}
142144
with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
143-
manager.identify_user('user1')
145+
manager.identify_user(identifiers)
144146

145147
mock_dispatch_event.assert_called_once_with({
146148
'type': 'fullstack',
147149
'action': 'identified',
148-
'identifiers': {'fs_user_id': 'user1'},
150+
'identifiers': {'fs_user_id': 'user1', 'email': 'user@example.com'},
149151
'data': {
150152
'idempotence_id': mock.ANY,
151153
'data_source_type': 'sdk',
@@ -161,8 +163,9 @@ def test_identify_user_odp_not_integrated(self):
161163
manager = OdpManager(False, CustomCache(), event_manager=event_manager, logger=mock_logger)
162164
manager.update_odp_config(None, None, [])
163165

166+
identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'}
164167
with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
165-
manager.identify_user('user1')
168+
manager.identify_user(identifiers)
166169

167170
mock_dispatch_event.assert_not_called()
168171
mock_logger.error.assert_not_called()
@@ -175,13 +178,94 @@ def test_identify_user_odp_disabled(self):
175178
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)
176179
manager.enabled = False
177180

181+
identifiers = {'fs_user_id': 'user1', 'email': 'user@example.com'}
178182
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
179-
manager.identify_user('user1')
183+
manager.identify_user(identifiers)
180184

181185
mock_identify_user.assert_not_called()
182186
mock_logger.error.assert_not_called()
183187
mock_logger.debug.assert_called_with('ODP identify event is not dispatched (ODP disabled).')
184188

189+
def test_identify_user_single_identifier_skipped(self):
190+
"""Single identifier should NOT trigger an ODP identify event."""
191+
mock_logger = mock.MagicMock()
192+
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())
193+
194+
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
195+
manager.update_odp_config('key1', 'host1', [])
196+
197+
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
198+
manager.identify_user({'fs_user_id': 'user1'})
199+
200+
mock_identify_user.assert_not_called()
201+
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).')
202+
203+
def test_identify_user_multiple_identifiers_sent(self):
204+
"""Multiple identifiers should trigger an ODP identify event."""
205+
mock_logger = mock.MagicMock()
206+
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())
207+
208+
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
209+
manager.update_odp_config('key1', 'host1', [])
210+
211+
identifiers = {'fs_user_id': 'user1', 'vuid': 'vuid-abc'}
212+
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
213+
manager.identify_user(identifiers)
214+
215+
mock_identify_user.assert_called_once_with(identifiers)
216+
217+
def test_identify_user_three_identifiers_sent(self):
218+
"""Three identifiers should trigger an ODP identify event with all identifiers."""
219+
mock_logger = mock.MagicMock()
220+
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())
221+
222+
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
223+
manager.update_odp_config('key1', 'host1', [])
224+
225+
identifiers = {'fs_user_id': 'user1', 'vuid': 'vuid-abc', 'email': 'user@example.com'}
226+
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
227+
manager.identify_user(identifiers)
228+
229+
mock_identify_user.assert_called_once_with(identifiers)
230+
231+
def test_identify_user_null_empty_values_not_counted(self):
232+
"""Null and empty identifier values should not count toward the identifier total."""
233+
mock_logger = mock.MagicMock()
234+
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())
235+
236+
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
237+
manager.update_odp_config('key1', 'host1', [])
238+
239+
# Two identifiers but one is empty - should not send
240+
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
241+
manager.identify_user({'fs_user_id': 'user1', 'email': ''})
242+
243+
mock_identify_user.assert_not_called()
244+
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).')
245+
246+
mock_logger.reset_mock()
247+
248+
# Two identifiers but one is None - should not send
249+
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
250+
manager.identify_user({'fs_user_id': 'user1', 'email': None})
251+
252+
mock_identify_user.assert_not_called()
253+
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).')
254+
255+
def test_identify_user_zero_identifiers_skipped(self):
256+
"""Zero identifiers should NOT trigger an ODP identify event."""
257+
mock_logger = mock.MagicMock()
258+
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())
259+
260+
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
261+
manager.update_odp_config('key1', 'host1', [])
262+
263+
with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
264+
manager.identify_user({})
265+
266+
mock_identify_user.assert_not_called()
267+
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (fewer than 2 valid identifiers).')
268+
185269
def test_send_event_datafile_not_ready(self):
186270
mock_logger = mock.MagicMock()
187271
event_manager = OdpEventManager(mock_logger, OdpEventApiManager())

tests/test_optimizely.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5343,7 +5343,7 @@ def test_send_identify_event__when_called_with_odp_enabled(self):
53435343
with mock.patch.object(client, '_identify_user') as identify:
53445344
client.create_user_context('user-id')
53455345

5346-
identify.assert_called_once_with('user-id')
5346+
identify.assert_called_once_with({'fs_user_id': 'user-id'})
53475347
mock_logger.error.assert_not_called()
53485348
client.close()
53495349

tests/test_user_context.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,7 +2094,7 @@ def test_send_identify_event_when_user_context_created(self):
20942094
with mock.patch.object(client, '_identify_user') as identify:
20952095
OptimizelyUserContext(client, mock_logger, 'user-id')
20962096

2097-
identify.assert_called_once_with('user-id')
2097+
identify.assert_called_once_with({'fs_user_id': 'user-id'})
20982098
mock_logger.error.assert_not_called()
20992099
client.close()
21002100

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

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

21102110
with mock.patch.object(client, '_identify_user') as identify:

0 commit comments

Comments
 (0)