Skip to content

Commit 43e7e95

Browse files
elliotykimelliotykim
authored
filterAttributes returns an empty map if attributes parameter is null (#79)
1 parent 545286d commit 43e7e95

File tree

2 files changed

+225
-7
lines changed

2 files changed

+225
-7
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/OptimizelyTestV3.java‎

Lines changed: 216 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,6 @@ public void activateWithUnknownExperimentKeyAndRaiseExceptionErrorHandler() thro
322322
* Verify that {@link Optimizely#activate(String, String, Map)} passes through attributes.
323323
*/
324324
@Test
325-
@SuppressWarnings("unchecked")
326325
public void activateWithAttributes() throws Exception {
327326
String datafile = validConfigJsonV3();
328327
ProjectConfig projectConfig = validProjectConfigV3();
@@ -379,7 +378,6 @@ public void activateWithAttributes() throws Exception {
379378
* In this case, the activate call should remove the unknown attribute from the given map.
380379
*/
381380
@Test
382-
@SuppressWarnings("unchecked")
383381
public void activateWithUnknownAttribute() throws Exception {
384382
String datafile = validConfigJsonV3();
385383
ProjectConfig projectConfig = validProjectConfigV3();
@@ -437,6 +435,117 @@ public void activateWithUnknownAttribute() throws Exception {
437435
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
438436
}
439437

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

932+
/**
933+
* Verify that {@link Optimizely#track(String, String)} ignores null attributes.
934+
*/
935+
@Test
936+
@SuppressFBWarnings(
937+
value="NP_NONNULL_PARAM_VIOLATION",
938+
justification="testing nullness contract violation")
939+
public void trackEventWithNullAttributes() throws Exception {
940+
String datafile = noAudienceProjectConfigJsonV3();
941+
ProjectConfig projectConfig = noAudienceProjectConfigV3();
942+
EventType eventType = projectConfig.getEventTypes().get(0);
943+
944+
// setup a mock event builder to return expected conversion params
945+
EventBuilder mockEventBuilder = mock(EventBuilder.class);
946+
947+
Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler)
948+
.withBucketing(mockBucketer)
949+
.withEventBuilder(mockEventBuilder)
950+
.withConfig(projectConfig)
951+
.withErrorHandler(mockErrorHandler)
952+
.build();
953+
954+
Map<String, String> testParams = new HashMap<String, String>();
955+
testParams.put("test", "params");
956+
LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, "");
957+
when(mockEventBuilder.createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
958+
eq(eventType.getId()), eq(eventType.getKey()),
959+
eq(Collections.<String, String>emptyMap()), isNull(Long.class),
960+
isNull(String.class)))
961+
.thenReturn(logEventToDispatch);
962+
963+
logbackVerifier.expectMessage(Level.INFO, "Tracking event \"clicked_cart\" for user \"userId\".");
964+
logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " +
965+
testParams + " and payload \"\"");
966+
967+
// call track
968+
Map<String, String> attributes = null;
969+
optimizely.track(eventType.getKey(), "userId", attributes);
970+
971+
logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map.");
972+
973+
// setup the attribute map captor (so we can verify its content)
974+
ArgumentCaptor<Map> attributeCaptor = ArgumentCaptor.forClass(Map.class);
975+
976+
// verify that the event builder was called with the expected attributes
977+
verify(mockEventBuilder).createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
978+
eq(eventType.getId()), eq(eventType.getKey()),
979+
attributeCaptor.capture(), isNull(Long.class),
980+
isNull(String.class));
981+
982+
Map<String, String> actualValue = attributeCaptor.getValue();
983+
assertThat(actualValue, is(Collections.<String, String>emptyMap()));
984+
985+
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
986+
}
987+
988+
/**
989+
* Verify that {@link Optimizely#track(String, String)} gracefully handles null attribute values.
990+
*/
991+
@Test
992+
public void trackEventWithNullAttributeValues() throws Exception {
993+
String datafile = validConfigJsonV3();
994+
ProjectConfig projectConfig = validProjectConfigV3();
995+
EventType eventType = projectConfig.getEventTypes().get(0);
996+
997+
// setup a mock event builder to return expected conversion params
998+
EventBuilder mockEventBuilder = mock(EventBuilder.class);
999+
1000+
Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler)
1001+
.withBucketing(mockBucketer)
1002+
.withEventBuilder(mockEventBuilder)
1003+
.withConfig(projectConfig)
1004+
.withErrorHandler(mockErrorHandler)
1005+
.build();
1006+
1007+
Map<String, String> testParams = new HashMap<String, String>();
1008+
testParams.put("test", "params");
1009+
LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, "");
1010+
when(mockEventBuilder.createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
1011+
eq(eventType.getId()), eq(eventType.getKey()),
1012+
anyMapOf(String.class, String.class), isNull(Long.class),
1013+
isNull(String.class)))
1014+
.thenReturn(logEventToDispatch);
1015+
1016+
logbackVerifier.expectMessage(Level.INFO, "Tracking event \"clicked_cart\" for user \"userId\".");
1017+
logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " +
1018+
testParams + " and payload \"\"");
1019+
1020+
// call track
1021+
Map<String, String> attributes = new HashMap<String, String>();
1022+
attributes.put("test", null);
1023+
optimizely.track(eventType.getKey(), "userId", attributes);
1024+
1025+
// setup the attribute map captor (so we can verify its content)
1026+
ArgumentCaptor<Map> attributeCaptor = ArgumentCaptor.forClass(Map.class);
1027+
1028+
// verify that the event builder was called with the expected attributes
1029+
verify(mockEventBuilder).createConversionEvent(eq(projectConfig), eq(mockBucketer), eq("userId"),
1030+
eq(eventType.getId()), eq(eventType.getKey()),
1031+
attributeCaptor.capture(), isNull(Long.class),
1032+
isNull(String.class));
1033+
1034+
verify(mockEventHandler).dispatchEvent(logEventToDispatch);
1035+
}
1036+
8241037
/**
8251038
* Verify that {@link Optimizely#track(String, String)} handles the case where an unknown attribute
8261039
* (i.e., not in the config) is passed through.
8271040
*
8281041
* In this case, the track event call should remove the unknown attribute from the given map.
8291042
*/
8301043
@Test
831-
@SuppressWarnings("unchecked")
8321044
public void trackEventWithUnknownAttribute() throws Exception {
8331045
String datafile = validConfigJsonV3();
8341046
ProjectConfig projectConfig = validProjectConfigV3();

0 commit comments

Comments
 (0)