Skip to content

Commit 72f188f

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 72f188f

File tree

2 files changed

+225
-3
lines changed

2 files changed

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

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

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

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

0 commit comments

Comments
 (0)