Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/Optimizely.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import com.optimizely.ab.odp.ODPManager;
import com.optimizely.ab.odp.ODPSegmentManager;
import com.optimizely.ab.odp.ODPSegmentOption;
import com.optimizely.ab.odp.ODPUserKey;
import com.optimizely.ab.optimizelyconfig.OptimizelyConfig;
import com.optimizely.ab.optimizelyconfig.OptimizelyConfigManager;
import com.optimizely.ab.optimizelyconfig.OptimizelyConfigService;
Expand Down Expand Up @@ -1794,7 +1795,13 @@ public void identifyUser(@Nonnull String userId) {
}
ODPManager odpManager = getODPManager();
if (odpManager != null) {
odpManager.getEventManager().identifyUser(userId);
Map<String, String> identifiers = new HashMap<>();
if (ODPManager.isVuid(userId)) {
identifiers.put(ODPUserKey.VUID.getKeyString(), userId);
} else {
identifiers.put(ODPUserKey.FS_USER_ID.getKeyString(), userId);
}
odpManager.getEventManager().identifyUser(identifiers);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,18 @@ public void updateSettings(ODPConfig newConfig) {
}
}

/**
* @deprecated Use {@link #identifyUser(Map)} instead.
*/
@Deprecated
public void identifyUser(String userId) {
identifyUser(null, userId);
}

/**
* @deprecated Use {@link #identifyUser(Map)} instead.
*/
@Deprecated
public void identifyUser(@Nullable String vuid, @Nullable String userId) {
Map<String, String> identifiers = new HashMap<>();
if (vuid != null) {
Expand All @@ -127,7 +135,28 @@ public void identifyUser(@Nullable String vuid, @Nullable String userId) {
identifiers.put(ODPUserKey.FS_USER_ID.getKeyString(), userId);
}
}
ODPEvent event = new ODPEvent("fullstack", "identified", identifiers, null);
identifyUser(identifiers);
}

public void identifyUser(@Nonnull Map<String, String> identifiers) {
Comment thread
jaeopt marked this conversation as resolved.
if (identifiers == null) {
logger.debug("ODP identify event is not dispatched (fewer than 2 valid identifiers).");
return;
}

Map<String, String> validIdentifiers = new HashMap<>();
for (Map.Entry<String, String> entry : identifiers.entrySet()) {
if (entry.getValue() != null && !entry.getValue().isEmpty()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add a comment here, the reason for this condition. > 2 is unintuitive, some comment would be good

validIdentifiers.put(entry.getKey(), entry.getValue());
}
}

if (validIdentifiers.size() < 2) {
logger.debug("ODP identify event is not dispatched (fewer than 2 valid identifiers).");
return;
Comment thread
jaeopt marked this conversation as resolved.
}

ODPEvent event = new ODPEvent("fullstack", "identified", validIdentifiers, null);
sendEvent(event);
}

Expand Down
5 changes: 4 additions & 1 deletion core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -5084,7 +5084,10 @@ public void identifyUser() {
.build();

optimizely.identifyUser("the-user");
Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-user");
ArgumentCaptor<Map> identifiersCaptor = ArgumentCaptor.forClass(Map.class);
Mockito.verify(mockODPEventManager, times(1)).identifyUser(identifiersCaptor.capture());
Map<String, String> capturedIdentifiers = identifiersCaptor.getValue();
assertEquals("the-user", capturedIdentifiers.get("fs_user_id"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1970,7 +1970,7 @@ public void identifyUserErrorWhenConfigIsInvalid() {
.build();

optimizely.createUserContext("test-user");
verify(mockODPEventManager, never()).identifyUser("test-user");
verify(mockODPEventManager, never()).identifyUser(any(Map.class));
Mockito.reset(mockODPEventManager);

logbackVerifier.expectMessage(Level.ERROR, "Optimizely instance is not valid, failing identifyUser call.");
Expand All @@ -1992,13 +1992,16 @@ public void identifyUser() {
.build();

OptimizelyUserContext userContext = optimizely.createUserContext("test-user");
verify(mockODPEventManager).identifyUser("test-user");
ArgumentCaptor<Map> identifiersCaptor = ArgumentCaptor.forClass(Map.class);
verify(mockODPEventManager).identifyUser(identifiersCaptor.capture());
Map<String, String> capturedIdentifiers = identifiersCaptor.getValue();
assertEquals("test-user", capturedIdentifiers.get("fs_user_id"));

Mockito.reset(mockODPEventManager);
OptimizelyUserContext userContextClone = userContext.copy();

// identifyUser should not be called the new userContext is created through copy
verify(mockODPEventManager, never()).identifyUser("test-user");
// identifyUser should not be called when the new userContext is created through copy
verify(mockODPEventManager, never()).identifyUser(any(Map.class));

assertNotSame(userContextClone, userContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,10 @@ public void prepareCorrectPayloadForIdentifyUser() throws InterruptedException {
eventManager.updateSettings(odpConfig);
eventManager.start();
for (int i = 0; i < 2; i++) {
eventManager.identifyUser("the-vuid-" + i, "the-fs-user-id-" + i);
Map<String, String> identifiers = new HashMap<>();
identifiers.put("vuid", "the-vuid-" + i);
identifiers.put("fs_user_id", "the-fs-user-id-" + i);
eventManager.identifyUser(identifiers);
}

Thread.sleep(1500);
Expand Down Expand Up @@ -290,61 +293,88 @@ public void preparePayloadForIdentifyUserWithVariationsOfFsUserId() throws Inter
}

@Test
public void identifyUserWithVuidAndUserId() throws InterruptedException {
public void identifyUserWithMultipleIdentifiers() throws InterruptedException {
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));
ArgumentCaptor<ODPEvent> captor = ArgumentCaptor.forClass(ODPEvent.class);

eventManager.identifyUser("vuid_123", "test-user");
Map<String, String> identifiers = new HashMap<>();
identifiers.put("vuid", "vuid_123");
identifiers.put("fs_user_id", "test-user");
eventManager.identifyUser(identifiers);
verify(eventManager, times(1)).sendEvent(captor.capture());

ODPEvent event = captor.getValue();
Map<String, String> identifiers = event.getIdentifiers();
assertEquals(identifiers.size(), 2);
assertEquals(identifiers.get("vuid"), "vuid_123");
assertEquals(identifiers.get("fs_user_id"), "test-user");
Map<String, String> eventIdentifiers = event.getIdentifiers();
assertEquals(2, eventIdentifiers.size());
assertEquals("vuid_123", eventIdentifiers.get("vuid"));
assertEquals("test-user", eventIdentifiers.get("fs_user_id"));
}

@Test
public void identifyUserWithVuidOnly() throws InterruptedException {
public void identifyUserSkippedWithSingleIdentifier() throws InterruptedException {
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));
ArgumentCaptor<ODPEvent> captor = ArgumentCaptor.forClass(ODPEvent.class);

eventManager.identifyUser("vuid_123", null);
verify(eventManager, times(1)).sendEvent(captor.capture());
Map<String, String> identifiers = new HashMap<>();
identifiers.put("fs_user_id", "test-user");
eventManager.identifyUser(identifiers);
verify(eventManager, never()).sendEvent(any(ODPEvent.class));
logbackVerifier.expectMessage(Level.DEBUG, "ODP identify event is not dispatched (fewer than 2 valid identifiers).");
}

ODPEvent event = captor.getValue();
Map<String, String> identifiers = event.getIdentifiers();
assertEquals(identifiers.size(), 1);
assertEquals(identifiers.get("vuid"), "vuid_123");
@Test
public void identifyUserSkippedWithEmptyValues() throws InterruptedException {
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));

// Two keys but one has empty value - only 1 valid identifier
Map<String, String> identifiers = new HashMap<>();
identifiers.put("fs_user_id", "test-user");
identifiers.put("email", "");
eventManager.identifyUser(identifiers);
verify(eventManager, never()).sendEvent(any(ODPEvent.class));
logbackVerifier.expectMessage(Level.DEBUG, "ODP identify event is not dispatched (fewer than 2 valid identifiers).");
}

@Test
public void identifyUserWithUserIdOnly() throws InterruptedException {
public void identifyUserSkippedWithNullValues() throws InterruptedException {
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));
ArgumentCaptor<ODPEvent> captor = ArgumentCaptor.forClass(ODPEvent.class);

eventManager.identifyUser(null, "test-user");
verify(eventManager, times(1)).sendEvent(captor.capture());
// Two keys but one has null value - only 1 valid identifier
Map<String, String> identifiers = new HashMap<>();
identifiers.put("fs_user_id", "test-user");
identifiers.put("vuid", null);
eventManager.identifyUser(identifiers);
verify(eventManager, never()).sendEvent(any(ODPEvent.class));
logbackVerifier.expectMessage(Level.DEBUG, "ODP identify event is not dispatched (fewer than 2 valid identifiers).");
}

ODPEvent event = captor.getValue();
Map<String, String> identifiers = event.getIdentifiers();
assertEquals(identifiers.size(), 1);
assertEquals(identifiers.get("fs_user_id"), "test-user");
@Test
public void identifyUserSkippedWithEmptyMap() throws InterruptedException {
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));

Map<String, String> identifiers = new HashMap<>();
eventManager.identifyUser(identifiers);
verify(eventManager, never()).sendEvent(any(ODPEvent.class));
logbackVerifier.expectMessage(Level.DEBUG, "ODP identify event is not dispatched (fewer than 2 valid identifiers).");
}

@Test
public void identifyUserWithVuidAsUserId() throws InterruptedException {
public void identifyUserSendsWithThreeIdentifiers() throws InterruptedException {
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));
ArgumentCaptor<ODPEvent> captor = ArgumentCaptor.forClass(ODPEvent.class);

eventManager.identifyUser(null, "vuid_123");
Map<String, String> identifiers = new HashMap<>();
identifiers.put("vuid", "vuid_123");
identifiers.put("fs_user_id", "test-user");
identifiers.put("email", "test@example.com");
eventManager.identifyUser(identifiers);
verify(eventManager, times(1)).sendEvent(captor.capture());

ODPEvent event = captor.getValue();
Map<String, String> identifiers = event.getIdentifiers();
assertEquals(identifiers.size(), 1);
// SDK will convert userId to vuid when userId has a valid vuid format.
assertEquals(identifiers.get("vuid"), "vuid_123");
Map<String, String> eventIdentifiers = event.getIdentifiers();
assertEquals(3, eventIdentifiers.size());
assertEquals("vuid_123", eventIdentifiers.get("vuid"));
assertEquals("test-user", eventIdentifiers.get("fs_user_id"));
assertEquals("test@example.com", eventIdentifiers.get("email"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;

import static org.mockito.Matchers.*;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -75,13 +77,16 @@ public void shouldUseNewSettingsInEventManagerWhenODPConfigIsUpdated() throws In
ODPManager odpManager = ODPManager.builder().withApiManager(mockApiManager).build();
odpManager.updateSettings("test-host", "test-key", new HashSet<>(Arrays.asList("segment1", "segment2")));

odpManager.getEventManager().identifyUser("vuid", "fsuid");
Map<String, String> identifiers = new HashMap<>();
identifiers.put("vuid", "vuid_value");
identifiers.put("fs_user_id", "fsuid");
odpManager.getEventManager().identifyUser(identifiers);
Thread.sleep(2000);
verify(mockApiManager, times(1))
.sendEvents(eq("test-key"), eq("test-host/v3/events"), any());

odpManager.updateSettings("test-host-updated", "test-key-updated", new HashSet<>(Arrays.asList("segment1")));
odpManager.getEventManager().identifyUser("vuid", "fsuid");
odpManager.getEventManager().identifyUser(identifiers);
Thread.sleep(1200);
verify(mockApiManager, times(1))
.sendEvents(eq("test-key-updated"), eq("test-host-updated/v3/events"), any());
Expand Down
Loading