Skip to content

Commit 8dc9a74

Browse files
mnoman09NomanShoaib
mnoman09
and
NomanShoaib
authored
[FSSDK-9076] fix: send odp event empty values (#513)
* [FSSDK-9076] fix: send odp event empty values * nit fix --------- Co-authored-by: NomanShoaib <[email protected]>
1 parent 2022b49 commit 8dc9a74

File tree

6 files changed

+226
-4
lines changed

6 files changed

+226
-4
lines changed

‎core-api/src/main/java/com/optimizely/ab/Optimizely.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,11 @@ public ODPManager getODPManager() {
14791479
*/
14801480
public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map<String, String> identifiers, @Nullable Map<String, Object> data) {
14811481
if (odpManager != null) {
1482+
if (action == null || action.trim().isEmpty()) {
1483+
logger.error("ODP action is not valid (cannot be empty).");
1484+
return;
1485+
}
1486+
14821487
ODPEvent event = new ODPEvent(type, action, identifiers, data);
14831488
odpManager.getEventManager().sendEvent(event);
14841489
} else {

‎core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class ODPEvent {
3030
@Nonnull private Map<String, Object> data;
3131

3232
public ODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map<String, String> identifiers, @Nullable Map<String, Object> data) {
33-
this.type = type == null ? EVENT_TYPE_FULLSTACK : type;
33+
this.type = type == null || type.trim().isEmpty() ? EVENT_TYPE_FULLSTACK : type;
3434
this.action = action;
3535
this.identifiers = identifiers != null ? identifiers : Collections.emptyMap();
3636
this.data = data != null ? data : Collections.emptyMap();

‎core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ public class ODPEventManager {
3535
private static final int DEFAULT_FLUSH_INTERVAL = 1000;
3636
private static final int MAX_RETRIES = 3;
3737
private static final String EVENT_URL_PATH = "/v3/events";
38+
private static final List<String> FS_USER_ID_MATCHES = new ArrayList<>(Arrays.asList(
39+
ODPUserKey.FS_USER_ID.getKeyString(),
40+
ODPUserKey.FS_USER_ID_ALIAS.getKeyString()
41+
));
3842
private static final Object SHUTDOWN_SIGNAL = new Object();
3943

4044
private final int queueSize;
@@ -121,7 +125,7 @@ public void identifyUser(@Nullable String vuid, @Nullable String userId) {
121125

122126
public void sendEvent(ODPEvent event) {
123127
event.setData(augmentCommonData(event.getData()));
124-
event.setIdentifiers(augmentCommonIdentifiers(event.getIdentifiers()));
128+
event.setIdentifiers(convertCriticalIdentifiers(augmentCommonIdentifiers(event.getIdentifiers())));
125129

126130
if (!event.isIdentifiersValid()) {
127131
logger.error("ODP event send failed (event identifiers must have at least one key-value pair)");
@@ -133,6 +137,7 @@ public void sendEvent(ODPEvent event) {
133137
return;
134138
}
135139

140+
136141
processEvent(event);
137142
}
138143

@@ -158,6 +163,27 @@ protected Map<String, String> augmentCommonIdentifiers(Map<String, String> sourc
158163
Map<String, String> identifiers = new HashMap<>();
159164
identifiers.putAll(userCommonIdentifiers);
160165
identifiers.putAll(sourceIdentifiers);
166+
167+
return identifiers;
168+
}
169+
170+
private static Map<String, String> convertCriticalIdentifiers(Map<String, String> identifiers) {
171+
172+
if (identifiers.containsKey(ODPUserKey.FS_USER_ID.getKeyString())) {
173+
return identifiers;
174+
}
175+
176+
List<Map.Entry<String, String>> identifiersList = new ArrayList<>(identifiers.entrySet());
177+
178+
for (Map.Entry<String, String> kvp : identifiersList) {
179+
180+
if (FS_USER_ID_MATCHES.contains(kvp.getKey().toLowerCase())) {
181+
identifiers.remove(kvp.getKey());
182+
identifiers.put(ODPUserKey.FS_USER_ID.getKeyString(), kvp.getValue());
183+
break;
184+
}
185+
}
186+
161187
return identifiers;
162188
}
163189

‎core-api/src/main/java/com/optimizely/ab/odp/ODPUserKey.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2022, Optimizely
3+
* Copyright 2022-2023, Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -20,7 +20,9 @@ public enum ODPUserKey {
2020

2121
VUID("vuid"),
2222

23-
FS_USER_ID("fs_user_id");
23+
FS_USER_ID("fs_user_id"),
24+
25+
FS_USER_ID_ALIAS("fs-user-id");
2426

2527
private final String keyString;
2628

‎core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4762,6 +4762,128 @@ public void sendODPEvent() {
47624762
assertEquals(data, eventArgument.getValue().getData());
47634763
}
47644764

4765+
@Test
4766+
@SuppressFBWarnings(value = "NP_NONNULL_PARAM_VIOLATION", justification = "Testing nullness contract violation")
4767+
public void sendODPEventErrorNullAction() {
4768+
ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class);
4769+
Mockito.when(mockProjectConfigManager.getConfig()).thenReturn(validProjectConfig);
4770+
ODPEventManager mockODPEventManager = mock(ODPEventManager.class);
4771+
ODPManager mockODPManager = mock(ODPManager.class);
4772+
4773+
Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager);
4774+
Optimizely optimizely = Optimizely.builder()
4775+
.withConfigManager(mockProjectConfigManager)
4776+
.withODPManager(mockODPManager)
4777+
.build();
4778+
4779+
verify(mockODPEventManager).start();
4780+
4781+
Map<String, String> identifiers = new HashMap<>();
4782+
identifiers.put("id1", "value1");
4783+
identifiers.put("id2", "value2");
4784+
4785+
Map<String, Object> data = new HashMap<>();
4786+
data.put("sdk", "java");
4787+
data.put("revision", 52);
4788+
4789+
optimizely.sendODPEvent("fullstack", null, identifiers, data);
4790+
logbackVerifier.expectMessage(Level.ERROR, "ODP action is not valid (cannot be empty).");
4791+
}
4792+
4793+
@Test
4794+
public void sendODPEventErrorEmptyAction() {
4795+
ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class);
4796+
Mockito.when(mockProjectConfigManager.getConfig()).thenReturn(validProjectConfig);
4797+
ODPEventManager mockODPEventManager = mock(ODPEventManager.class);
4798+
ODPManager mockODPManager = mock(ODPManager.class);
4799+
4800+
Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager);
4801+
Optimizely optimizely = Optimizely.builder()
4802+
.withConfigManager(mockProjectConfigManager)
4803+
.withODPManager(mockODPManager)
4804+
.build();
4805+
4806+
verify(mockODPEventManager).start();
4807+
4808+
Map<String, String> identifiers = new HashMap<>();
4809+
identifiers.put("id1", "value1");
4810+
identifiers.put("id2", "value2");
4811+
4812+
Map<String, Object> data = new HashMap<>();
4813+
data.put("sdk", "java");
4814+
data.put("revision", 52);
4815+
4816+
optimizely.sendODPEvent("fullstack", "", identifiers, data);
4817+
logbackVerifier.expectMessage(Level.ERROR, "ODP action is not valid (cannot be empty).");
4818+
}
4819+
4820+
@Test
4821+
@SuppressFBWarnings(value = "NP_NONNULL_PARAM_VIOLATION", justification = "Testing nullness contract violation")
4822+
public void sendODPEventNullType() {
4823+
ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class);
4824+
Mockito.when(mockProjectConfigManager.getConfig()).thenReturn(validProjectConfig);
4825+
ODPEventManager mockODPEventManager = mock(ODPEventManager.class);
4826+
ODPManager mockODPManager = mock(ODPManager.class);
4827+
4828+
Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager);
4829+
Optimizely optimizely = Optimizely.builder()
4830+
.withConfigManager(mockProjectConfigManager)
4831+
.withODPManager(mockODPManager)
4832+
.build();
4833+
4834+
verify(mockODPEventManager).start();
4835+
4836+
Map<String, String> identifiers = new HashMap<>();
4837+
identifiers.put("id1", "value1");
4838+
identifiers.put("id2", "value2");
4839+
4840+
Map<String, Object> data = new HashMap<>();
4841+
data.put("sdk", "java");
4842+
data.put("revision", 52);
4843+
4844+
optimizely.sendODPEvent(null, "identify", identifiers, data);
4845+
ArgumentCaptor<ODPEvent> eventArgument = ArgumentCaptor.forClass(ODPEvent.class);
4846+
verify(mockODPEventManager).sendEvent(eventArgument.capture());
4847+
4848+
assertEquals("fullstack", eventArgument.getValue().getType());
4849+
assertEquals("identify", eventArgument.getValue().getAction());
4850+
assertEquals(identifiers, eventArgument.getValue().getIdentifiers());
4851+
assertEquals(data, eventArgument.getValue().getData());
4852+
}
4853+
4854+
@Test
4855+
public void sendODPEventEmptyType() {
4856+
ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class);
4857+
Mockito.when(mockProjectConfigManager.getConfig()).thenReturn(validProjectConfig);
4858+
ODPEventManager mockODPEventManager = mock(ODPEventManager.class);
4859+
ODPManager mockODPManager = mock(ODPManager.class);
4860+
4861+
Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager);
4862+
Optimizely optimizely = Optimizely.builder()
4863+
.withConfigManager(mockProjectConfigManager)
4864+
.withODPManager(mockODPManager)
4865+
.build();
4866+
4867+
verify(mockODPEventManager).start();
4868+
4869+
Map<String, String> identifiers = new HashMap<>();
4870+
identifiers.put("id1", "value1");
4871+
identifiers.put("id2", "value2");
4872+
4873+
Map<String, Object> data = new HashMap<>();
4874+
data.put("sdk", "java");
4875+
data.put("revision", 52);
4876+
4877+
optimizely.sendODPEvent("", "identify", identifiers, data);
4878+
ArgumentCaptor<ODPEvent> eventArgument = ArgumentCaptor.forClass(ODPEvent.class);
4879+
verify(mockODPEventManager).sendEvent(eventArgument.capture());
4880+
4881+
assertEquals("fullstack", eventArgument.getValue().getType());
4882+
assertEquals("identify", eventArgument.getValue().getAction());
4883+
assertEquals(identifiers, eventArgument.getValue().getIdentifiers());
4884+
assertEquals(data, eventArgument.getValue().getData());
4885+
}
4886+
47654887
@Test
47664888
public void sendODPEventError() {
47674889
ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class);

‎core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,73 @@ public void prepareCorrectPayloadForIdentifyUser() throws InterruptedException {
227227
}
228228
}
229229

230+
@Test
231+
public void preparePayloadForIdentifyUserWithVariationsOfFsUserId() throws InterruptedException {
232+
Mockito.reset(mockApiManager);
233+
Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202);
234+
int flushInterval = 1;
235+
ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null);
236+
ODPEventManager eventManager = new ODPEventManager(mockApiManager, null, flushInterval);
237+
eventManager.updateSettings(odpConfig);
238+
eventManager.start();
239+
ODPEvent event1 = new ODPEvent("fullstack",
240+
"identified",
241+
new HashMap<String, String>() {{
242+
put("fs-user-id", "123");
243+
}}, null);
244+
245+
ODPEvent event2 = new ODPEvent("fullstack",
246+
"identified",
247+
new HashMap<String, String>() {{
248+
put("FS-user-ID", "123");
249+
}}, null);
250+
251+
ODPEvent event3 = new ODPEvent("fullstack",
252+
"identified",
253+
new HashMap<String, String>() {{
254+
put("FS_USER_ID", "123");
255+
put("fs.user.id", "456");
256+
}}, null);
257+
258+
ODPEvent event4 = new ODPEvent("fullstack",
259+
"identified",
260+
new HashMap<String, String>() {{
261+
put("fs_user_id", "123");
262+
put("fsuserid", "456");
263+
}}, null);
264+
List<Map<String, String>> expectedIdentifiers = new ArrayList<Map<String, String>>() {{
265+
add(new HashMap<String, String>() {{
266+
put("fs_user_id", "123");
267+
}});
268+
add(new HashMap<String, String>() {{
269+
put("fs_user_id", "123");
270+
}});
271+
add(new HashMap<String, String>() {{
272+
put("fs_user_id", "123");
273+
put("fs.user.id", "456");
274+
}});
275+
add(new HashMap<String, String>() {{
276+
put("fs_user_id", "123");
277+
put("fsuserid", "456");
278+
}});
279+
}};
280+
eventManager.sendEvent(event1);
281+
eventManager.sendEvent(event2);
282+
eventManager.sendEvent(event3);
283+
eventManager.sendEvent(event4);
284+
285+
Thread.sleep(1500);
286+
Mockito.verify(mockApiManager, times(1)).sendEvents(eq("key"), eq("http://www.odp-host.com/v3/events"), payloadCaptor.capture());
287+
288+
String payload = payloadCaptor.getValue();
289+
JSONArray events = new JSONArray(payload);
290+
assertEquals(4, events.length());
291+
for (int i = 0; i < events.length(); i++) {
292+
JSONObject event = events.getJSONObject(i);
293+
assertEquals(event.getJSONObject("identifiers").toMap(), expectedIdentifiers.get(i));
294+
}
295+
}
296+
230297
@Test
231298
public void identifyUserWithVuidAndUserId() throws InterruptedException {
232299
ODPEventManager eventManager = spy(new ODPEventManager(mockApiManager));

0 commit comments

Comments
 (0)