Skip to content

Commit 7a5fd99

Browse files
elliotykimElliot Kim
elliotykim
authored and
Elliot Kim
committed
filterAttributes returns an empty map if attributes parameter is null (#79)
1 parent 545286d commit 7a5fd99

File tree

3 files changed

+441
-3
lines changed

3 files changed

+441
-3
lines changed

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,10 +631,16 @@ private LiveVariable getLiveVariableOrThrow(ProjectConfig projectConfig, String
631631
*
632632
* @param projectConfig the current project config
633633
* @param attributes the attributes map to validate and potentially filter
634-
* @return the filtered attributes map (containing only attributes that are present in the project config)
635-
*
634+
* @return the filtered attributes map (containing only attributes that are present in the project config) or an
635+
* empty map if a null attributes object is passed in
636636
*/
637-
private Map<String, String> filterAttributes(ProjectConfig projectConfig, Map<String, String> attributes) {
637+
private Map<String, String> filterAttributes(@Nonnull ProjectConfig projectConfig,
638+
@Nonnull Map<String, String> attributes) {
639+
if (attributes == null) {
640+
logger.warn("Attributes is null when non-null was expected. Defaulting to an empty attributes map.");
641+
return Collections.<String, String>emptyMap();
642+
}
643+
638644
List<String> unknownAttributes = null;
639645

640646
Map<String, Attribute> attributeKeyMapping = projectConfig.getAttributeKeyMapping();

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

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,117 @@ public void activateWithUnknownAttribute() throws Exception {
433433
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
434434
}
435435

436+
/**
437+
* Verify that {@link Optimizely#activate(String, String, Map)} ignores null attributes.
438+
*/
439+
@Test
440+
@SuppressFBWarnings(
441+
value="NP_NONNULL_PARAM_VIOLATION",
442+
justification="testing nullness contract violation")
443+
public void activateWithNullAttributes() throws Exception {
444+
String datafile = noAudienceProjectConfigJsonV2();
445+
ProjectConfig projectConfig = noAudienceProjectConfigV2();
446+
Experiment activatedExperiment = projectConfig.getExperiments().get(0);
447+
Variation bucketedVariation = activatedExperiment.getVariations().get(0);
448+
449+
// setup a mock event builder to return expected impression params
450+
EventBuilder mockEventBuilder = mock(EventBuilder.class);
451+
452+
Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler)
453+
.withBucketing(mockBucketer)
454+
.withEventBuilder(mockEventBuilder)
455+
.withConfig(projectConfig)
456+
.withErrorHandler(mockErrorHandler)
457+
.build();
458+
459+
Map<String, String> testParams = new HashMap<String, String>();
460+
testParams.put("test", "params");
461+
LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, "");
462+
when(mockEventBuilder.createImpressionEvent(eq(projectConfig), eq(activatedExperiment), eq(bucketedVariation),
463+
eq("userId"), eq(Collections.<String, String>emptyMap()),
464+
isNull(String.class)))
465+
.thenReturn(logEventToDispatch);
466+
467+
when(mockBucketer.bucket(activatedExperiment, "userId"))
468+
.thenReturn(bucketedVariation);
469+
470+
// activate the experiment
471+
Map<String, String> attributes = null;
472+
Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), "userId", attributes);
473+
474+
logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map.");
475+
476+
// verify that the bucketing algorithm was called correctly
477+
verify(mockBucketer).bucket(activatedExperiment, "userId");
478+
assertThat(actualVariation, is(bucketedVariation));
479+
480+
// setup the attribute map captor (so we can verify its content)
481+
ArgumentCaptor<Map> attributeCaptor = ArgumentCaptor.forClass(Map.class);
482+
verify(mockEventBuilder).createImpressionEvent(eq(projectConfig), eq(activatedExperiment),
483+
eq(bucketedVariation), eq("userId"), attributeCaptor.capture(),
484+
isNull(String.class));
485+
486+
Map<String, String> actualValue = attributeCaptor.getValue();
487+
assertThat(actualValue, is(Collections.<String, String>emptyMap()));
488+
489+
// verify that dispatchEvent was called with the correct LogEvent object
490+
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
491+
}
492+
493+
/**
494+
* Verify that {@link Optimizely#activate(String, String, Map)} gracefully handles null attribute values.
495+
*/
496+
@Test
497+
public void activateWithNullAttributeValues() throws Exception {
498+
String datafile = validConfigJsonV2();
499+
ProjectConfig projectConfig = validProjectConfigV2();
500+
Experiment activatedExperiment = projectConfig.getExperiments().get(0);
501+
Variation bucketedVariation = activatedExperiment.getVariations().get(0);
502+
Attribute attribute = projectConfig.getAttributes().get(0);
503+
504+
// setup a mock event builder to return expected impression params
505+
EventBuilder mockEventBuilder = mock(EventBuilder.class);
506+
507+
Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler)
508+
.withBucketing(mockBucketer)
509+
.withEventBuilder(mockEventBuilder)
510+
.withConfig(projectConfig)
511+
.withErrorHandler(mockErrorHandler)
512+
.build();
513+
514+
Map<String, String> testParams = new HashMap<String, String>();
515+
testParams.put("test", "params");
516+
LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, "");
517+
when(mockEventBuilder.createImpressionEvent(eq(projectConfig), eq(activatedExperiment), eq(bucketedVariation),
518+
eq("userId"), anyMapOf(String.class, String.class),
519+
isNull(String.class)))
520+
.thenReturn(logEventToDispatch);
521+
522+
when(mockBucketer.bucket(activatedExperiment, "userId"))
523+
.thenReturn(bucketedVariation);
524+
525+
// activate the experiment
526+
Map<String, String> attributes = new HashMap<String, String>();
527+
attributes.put(attribute.getKey(), null);
528+
Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), "userId", attributes);
529+
530+
// verify that the bucketing algorithm was called correctly
531+
verify(mockBucketer).bucket(activatedExperiment, "userId");
532+
assertThat(actualVariation, is(bucketedVariation));
533+
534+
// setup the attribute map captor (so we can verify its content)
535+
ArgumentCaptor<Map> attributeCaptor = ArgumentCaptor.forClass(Map.class);
536+
verify(mockEventBuilder).createImpressionEvent(eq(projectConfig), eq(activatedExperiment),
537+
eq(bucketedVariation), eq("userId"), attributeCaptor.capture(),
538+
isNull(String.class));
539+
540+
Map<String, String> actualValue = attributeCaptor.getValue();
541+
assertThat(actualValue, hasEntry(attribute.getKey(), null));
542+
543+
// verify that dispatchEvent was called with the correct LogEvent object
544+
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
545+
}
546+
436547
/**
437548
* Verify that {@link Optimizely#activate(String, String)} returns null when the experiment id corresponds to a
438549
* non-running experiment.
@@ -883,6 +994,111 @@ public void trackEventWithAttributes() throws Exception {
883994
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
884995
}
885996

997+
/**
998+
* Verify that {@link Optimizely#track(String, String)} ignores null attributes.
999+
*/
1000+
@Test
1001+
@SuppressFBWarnings(
1002+
value="NP_NONNULL_PARAM_VIOLATION",
1003+
justification="testing nullness contract violation")
1004+
public void trackEventWithNullAttributes() throws Exception {
1005+
String datafile = noAudienceProjectConfigJsonV2();
1006+
ProjectConfig projectConfig = noAudienceProjectConfigV2();
1007+
EventType eventType = projectConfig.getEventTypes().get(0);
1008+
1009+
// setup a mock event builder to return expected conversion params
1010+
EventBuilder mockEventBuilder = mock(EventBuilder.class);
1011+
1012+
Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler)
1013+
.withBucketing(mockBucketer)
1014+
.withEventBuilder(mockEventBuilder)
1015+
.withConfig(projectConfig)
1016+
.withErrorHandler(mockErrorHandler)
1017+
.build();
1018+
1019+
Map<String, String> testParams = new HashMap<String, String>();
1020+
testParams.put("test", "params");
1021+
LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, "");
1022+
when(mockEventBuilder.createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
1023+
eq(eventType.getId()), eq(eventType.getKey()),
1024+
eq(Collections.<String, String>emptyMap()), isNull(Long.class),
1025+
isNull(String.class)))
1026+
.thenReturn(logEventToDispatch);
1027+
1028+
logbackVerifier.expectMessage(Level.INFO, "Tracking event \"clicked_cart\" for user \"userId\".");
1029+
logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " +
1030+
testParams + " and payload \"\"");
1031+
1032+
// call track
1033+
Map<String, String> attributes = null;
1034+
optimizely.track(eventType.getKey(), "userId", attributes);
1035+
1036+
logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map.");
1037+
1038+
// setup the attribute map captor (so we can verify its content)
1039+
ArgumentCaptor<Map> attributeCaptor = ArgumentCaptor.forClass(Map.class);
1040+
1041+
// verify that the event builder was called with the expected attributes
1042+
verify(mockEventBuilder).createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
1043+
eq(eventType.getId()), eq(eventType.getKey()),
1044+
attributeCaptor.capture(), isNull(Long.class),
1045+
isNull(String.class));
1046+
1047+
Map<String, String> actualValue = attributeCaptor.getValue();
1048+
assertThat(actualValue, is(Collections.<String, String>emptyMap()));
1049+
1050+
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
1051+
}
1052+
1053+
/**
1054+
* Verify that {@link Optimizely#track(String, String)} gracefully handles null attribute values.
1055+
*/
1056+
@Test
1057+
public void trackEventWithNullAttributeValues() throws Exception {
1058+
String datafile = validConfigJsonV2();
1059+
ProjectConfig projectConfig = validProjectConfigV2();
1060+
EventType eventType = projectConfig.getEventTypes().get(0);
1061+
1062+
// setup a mock event builder to return expected conversion params
1063+
EventBuilder mockEventBuilder = mock(EventBuilder.class);
1064+
1065+
Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler)
1066+
.withBucketing(mockBucketer)
1067+
.withEventBuilder(mockEventBuilder)
1068+
.withConfig(projectConfig)
1069+
.withErrorHandler(mockErrorHandler)
1070+
.build();
1071+
1072+
Map<String, String> testParams = new HashMap<String, String>();
1073+
testParams.put("test", "params");
1074+
LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, "");
1075+
when(mockEventBuilder.createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
1076+
eq(eventType.getId()), eq(eventType.getKey()),
1077+
anyMapOf(String.class, String.class), isNull(Long.class),
1078+
isNull(String.class)))
1079+
.thenReturn(logEventToDispatch);
1080+
1081+
logbackVerifier.expectMessage(Level.INFO, "Tracking event \"clicked_cart\" for user \"userId\".");
1082+
logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " +
1083+
testParams + " and payload \"\"");
1084+
1085+
// call track
1086+
Map<String, String> attributes = new HashMap<String, String>();
1087+
attributes.put("test", null);
1088+
optimizely.track(eventType.getKey(), "userId", attributes);
1089+
1090+
// setup the attribute map captor (so we can verify its content)
1091+
ArgumentCaptor<Map> attributeCaptor = ArgumentCaptor.forClass(Map.class);
1092+
1093+
// verify that the event builder was called with the expected attributes
1094+
verify(mockEventBuilder).createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
1095+
eq(eventType.getId()), eq(eventType.getKey()),
1096+
attributeCaptor.capture(), isNull(Long.class),
1097+
isNull(String.class));
1098+
1099+
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
1100+
}
1101+
8861102
/**
8871103
* Verify that {@link Optimizely#track(String, String)} handles the case where an unknown attribute
8881104
* (i.e., not in the config) is passed through.

0 commit comments

Comments
 (0)