Skip to content

[AI-FSSDK] [FSSDK-12670] Block ODP identify events with single identifier #409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jaeopt wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[AI-FSSDK] [FSSDK-12670] Block ODP identify events with single identifier #409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48097c0
[FSSDK-12670] Block ODP identify events with single identifier
jaeopt May 26, 2026
9324adb
[FSSDK-12670] Address review feedback: fix log message
jaeopt May 27, 2026
be8686a
[FSSDK-12670] Retrigger CI
jaeopt May 27, 2026
8149c82
[FSSDK-12670] Add comment explaining the < 2 identifiers guard
jaeopt May 27, 2026
fe0e47b
[FSSDK-12670] retrigger CI
jaeopt May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,15 @@ public void ShouldLogWhenOdpNotIntegratedAndIdentifyUserCalled()
eventManager.UpdateSettings(mockOdpConfig.
Object); // auto-start after update; Logs 1x here

eventManager.IdentifyUser(FS_USER_ID); // Logs 1x here too
eventManager.IdentifyUser(FS_USER_ID); // Blocked by single identifier check

_mockLogger.Verify(
l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE),
Times.Exactly(3)); // during Start() and SendEvent()
Times.Exactly(2)); // during Start() and UpdateSettings()
_mockLogger.Verify(
l => l.Log(LogLevel.DEBUG,
"ODP identify event is not dispatched (fewer than 2 valid identifiers)."),
Times.Once); // IdentifyUser blocked before reaching SendEvent
}

[Test]
Expand Down Expand Up @@ -606,14 +610,12 @@ public void ShouldFlushAllScheduledEventsBeforeStopping()
}

[Test]
public void ShouldPrepareCorrectPayloadForIdentifyUser()
public void ShouldNotSendIdentifyEventWithSingleIdentifier()
{
const string USER_ID = "test_fs_user_id";
var cde = new CountdownEvent(1);
var eventsCollector = new List<List<OdpEvent>>();
_mockApiManager.Setup(api => api.SendEvents(It.IsAny<string>(), It.IsAny<string>(),
Capture.In(eventsCollector))).
Callback(() => cde.Signal());
Capture.In(eventsCollector)));
var eventManager = new OdpEventManager.Builder().
WithOdpEventApiManager(_mockApiManager.Object).
WithLogger(_mockLogger.Object).
Expand All @@ -623,20 +625,16 @@ public void ShouldPrepareCorrectPayloadForIdentifyUser()
eventManager.UpdateSettings(_odpConfig);

eventManager.IdentifyUser(USER_ID);
cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS);

var eventsSentToApi = eventsCollector.FirstOrDefault();
var actualEvent = eventsSentToApi?.FirstOrDefault();
Assert.IsNotNull(actualEvent);
Assert.AreEqual(Constants.ODP_EVENT_TYPE, actualEvent.Type);
Assert.AreEqual("identified", actualEvent.Action);
Assert.AreEqual(USER_ID, actualEvent.Identifiers[Constants.FS_USER_ID]);
var eventData = actualEvent.Data;
Assert.AreEqual(Guid.NewGuid().ToString().Length,
eventData["idempotence_id"].ToString().Length);
Assert.AreEqual("sdk", eventData["data_source_type"]);
Assert.AreEqual("csharp-sdk", eventData["data_source"]);
Assert.IsNotNull(eventData["data_source_version"]);
// Allow time for any async processing
Thread.Sleep(500);

// No events should be sent since only one identifier (fs_user_id) was provided
Assert.IsEmpty(eventsCollector);
_mockLogger.Verify(
l => l.Log(LogLevel.DEBUG,
"ODP identify event is not dispatched (fewer than 2 valid identifiers)."),
Times.Once);
}

[Test]
Expand Down
16 changes: 15 additions & 1 deletion OptimizelySDK/Odp/OdpEventManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,21 @@ public void IdentifyUser(string userId)
{ Constants.FS_USER_ID, userId },
};

var odpEvent = new OdpEvent(Constants.ODP_EVENT_TYPE, "identified", identifiers);
// Filter out null/empty identifier values and require 2+ valid identifiers
var validIdentifiers = identifiers
.Where(kvp => !string.IsNullOrEmpty(kvp.Value))
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);

// Identify requires 2+ identifiers to link (e.g., vuid + fs_user_id).
// A single identifier has no cross-reference value and generates unnecessary traffic.
if (validIdentifiers.Count < 2)
Copy link
Copy Markdown
Contributor

@junaed-optimizely junaed-optimizely Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaeopt, this condition will always evaluate to true in the current C# SDK. IdentifyUser(string userId) constructs a dictionary with exactly one entry (fs_user_id), so after filtering, validIdentifiers.Count is either 1 or 0. It can never reach 2.

This effectively makes IdentifyUser a permanent no-op. Is this intentional?

Copy link
Copy Markdown
Contributor Author

@jaeopt jaeopt Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@junaed-optimizely yes a good catch - it's true for all server-sdks. We had an option to clean up all but decide to keep this way since we have a chance to change this behavior or add other identities.

{
_logger.Log(LogLevel.DEBUG,
"ODP identify event is not dispatched (fewer than 2 valid identifiers).");
return;
}

var odpEvent = new OdpEvent(Constants.ODP_EVENT_TYPE, "identified", validIdentifiers);
SendEvent(odpEvent);
}

Expand Down
Loading