From 1d3e8a13b6337b87eacf4172680fe10616670bb8 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 12 Dec 2017 21:54:34 -0800 Subject: [PATCH 01/10] notification center --- .../java/com/optimizely/ab/Optimizely.java | 8 ++ .../ab/notification/ActivateNotification.java | 37 ++++++++ .../ab/notification/Notification.java | 12 +++ .../notification/NotificationBroadcaster.java | 1 + .../ab/notification/NotificationCenter.java | 90 +++++++++++++++++++ .../ab/notification/NotificationListener.java | 1 + .../ab/notification/TrackNotification.java | 31 +++++++ 7 files changed, 180 insertions(+) create mode 100644 core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java create mode 100644 core-api/src/main/java/com/optimizely/ab/notification/Notification.java create mode 100644 core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java create mode 100644 core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 44da0eb97..d46144c9b 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -41,6 +41,7 @@ import com.optimizely.ab.event.internal.payload.Event.ClientEngine; import com.optimizely.ab.internal.EventTagUtils; import com.optimizely.ab.notification.NotificationBroadcaster; +import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.notification.NotificationListener; import org.slf4j.Logger; @@ -93,6 +94,8 @@ public class Optimizely { @VisibleForTesting final EventHandler eventHandler; @VisibleForTesting final ErrorHandler errorHandler; @VisibleForTesting final NotificationBroadcaster notificationBroadcaster = new NotificationBroadcaster(); + @VisibleForTesting final NotificationCenter notificationCenter = new NotificationCenter(); + @Nullable private final UserProfileService userProfileService; private Optimizely(@Nonnull ProjectConfig projectConfig, @@ -206,6 +209,9 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, } notificationBroadcaster.broadcastExperimentActivated(experiment, userId, filteredAttributes, variation); + + notificationCenter.sendNotifications(NotificationCenter.NotificationType.Activate, experiment, userId, + filteredAttributes, variation, impressionEvent); } else { logger.info("Experiment has \"Launched\" status so not dispatching event during activation."); } @@ -292,6 +298,8 @@ public void track(@Nonnull String eventName, notificationBroadcaster.broadcastEventTracked(eventName, userId, filteredAttributes, eventValue, conversionEvent); + notificationCenter.sendNotifications(NotificationCenter.NotificationType.Track, eventName, userId, + filteredAttributes, eventValue, conversionEvent); } //======== FeatureFlag APIs ========// diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java new file mode 100644 index 000000000..3002cbc0d --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java @@ -0,0 +1,37 @@ +package com.optimizely.ab.notification; + +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; +import sun.rmi.runtime.Log; + +import java.util.Map; + + +public abstract class ActivateNotification implements Notification { + + @Override + public void notify(Object... args) { + assert(args[0] instanceof Experiment); + Experiment experiment = (Experiment) args[0]; + assert(args[1] instanceof String); + String userId = (String) args[1]; + assert(args[2] instanceof java.util.Map); + Map attributes = (Map) args[2]; + assert(args[3] instanceof Variation); + Variation variation = (Variation) args[3]; + assert(args[4] instanceof LogEvent); + LogEvent logEvent = (LogEvent) args[4]; + + onActivate(experiment, userId, attributes, variation, logEvent); + } + + // Notice the argument list for decision + abstract void onActivate(@javax.annotation.Nonnull Experiment experiment, + @javax.annotation.Nonnull String userId, + @javax.annotation.Nonnull Map attributes, + @javax.annotation.Nonnull Variation variation, + @javax.annotation.Nonnull LogEvent event) ; + +} + diff --git a/core-api/src/main/java/com/optimizely/ab/notification/Notification.java b/core-api/src/main/java/com/optimizely/ab/notification/Notification.java new file mode 100644 index 000000000..04426dbb4 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/notification/Notification.java @@ -0,0 +1,12 @@ +package com.optimizely.ab.notification; + +import javax.annotation.Nonnull; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; +import java.util.Map; + +public interface Notification { + + public void notify(Object ...args); +} diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationBroadcaster.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationBroadcaster.java index bb77468df..3455a1bb1 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationBroadcaster.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationBroadcaster.java @@ -33,6 +33,7 @@ /** * Manages Optimizely SDK notification listeners and broadcasts messages. */ +@Deprecated public class NotificationBroadcaster { private static final Logger logger = LoggerFactory.getLogger(NotificationBroadcaster.class); diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java new file mode 100644 index 000000000..cd8444106 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -0,0 +1,90 @@ +package com.optimizely.ab.notification; + +import java.util.ArrayList; +import java.util.Map; +import java.util.HashMap; + +public class NotificationCenter { + public enum NotificationType { + Activate, // Activate was called. + Track, // track an impression event + }; + + + // the notification id is incremented and is assigned as the callback id, it can then be used to remove the notification. + private int notificationID = 1; + // use tuple if available. + private class NotificationHolder + { + int notificationId; + Notification notification; + + NotificationHolder(int id, Notification notification) { + notificationId = id; + this.notification = notification; + } + } + + public NotificationCenter() { + notifications.put(NotificationType.Activate, new ArrayList()); + notifications.put(NotificationType.Track, new ArrayList()); + } +// dictionary of enum to notification lists. So, DECISION would have an array of notification holders. + private Map> notifications =new HashMap>(); + + // add a notification to your dictionary. Look at python implementation for details. +// return notification id + public int addNotification(NotificationType notificaitonType, Notification notification) { + for (NotificationHolder holder : notifications.get(notificaitonType)) { + if (holder.notification == notification) { + return -1; + } + } + int id = notificationID; + notifications.get(notificaitonType).add(new NotificationHolder(notificationID++, notification )); + return id; + } + + // remove notification by id + public boolean removeNotifiation(int notificationID) { + ArrayList types = new ArrayList(); + types.add(NotificationType.Activate); + types.add(NotificationType.Track); + + for (NotificationType type : types) { + for (NotificationHolder holder : notifications.get(type)) { + if (holder.notificationId == notificationID) { + notifications.get(type).remove(holder); + return true; + } + } + } + + return false; + } + + // clean out all notifications. + public void cleanAllNotifications() { + ArrayList types = new ArrayList(); + types.add(NotificationType.Activate); + types.add(NotificationType.Track); + + for (NotificationType type : types) { + notifications.get(type).clear(); + } + } + + // clean out notifications by notification type. + public void clearNotifications(NotificationType notificationType) { + notifications.get(notificationType).clear(); + } + + // fire a notificaiton of a certain type. The arg list changes depending on the type of notification sent. + public void sendNotifications(NotificationType notificationType, Object ...args) { + ArrayList holders = notifications.get(notificationType); + for (NotificationHolder holder : holders) { + holder.notification.notify(args); + } + } + +} diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java index 2fe6e6bc1..7b436c5ab 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java @@ -34,6 +34,7 @@ * methods are added to the interface in the SDK. An abstract classes allows consumers to override * just the methods they need. */ +@Deprecated public abstract class NotificationListener { /** diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java new file mode 100644 index 000000000..1b09fbcd7 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java @@ -0,0 +1,31 @@ +package com.optimizely.ab.notification; + +import java.util.Map; +import javax.annotation.Nonnull; + +import com.optimizely.ab.event.LogEvent; + +public abstract class TrackNotification implements Notification { + + public void notify(Object... args) { + assert(args[0] instanceof String); + String eventKey = (String) args[0]; + assert(args[1] instanceof String); + String userId = (String) args[1]; + assert(args[2] instanceof java.util.Map); + Map attributes = (Map) args[2]; + assert(args[3] instanceof java.util.Map); + Map eventTags = (Map) args[3]; + assert(args[4] instanceof LogEvent); + LogEvent logEvent = (LogEvent) args[4]; + + onTrack(eventKey, userId,attributes,eventTags, logEvent); + } + + // Notice the argument list for decision + abstract void onTrack(@Nonnull String eventKey, + @Nonnull String userId, + @Nonnull Map attributes, + @Nonnull Map eventTags, + @Nonnull LogEvent event) ; +} From b27165dc04f8a6f1a471280a507c2df8fd9e518e Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 12 Dec 2017 22:32:53 -0800 Subject: [PATCH 02/10] update tests and notifications --- .../ab/notification/ActivateNotification.java | 5 +- .../ab/notification/NotificationCenter.java | 2 +- .../ab/notification/TrackNotification.java | 5 +- .../com/optimizely/ab/OptimizelyTest.java | 66 +++++++++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java index 3002cbc0d..9522cc74a 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java @@ -3,7 +3,6 @@ import com.optimizely.ab.config.Experiment; import com.optimizely.ab.config.Variation; import com.optimizely.ab.event.LogEvent; -import sun.rmi.runtime.Log; import java.util.Map; @@ -11,7 +10,7 @@ public abstract class ActivateNotification implements Notification { @Override - public void notify(Object... args) { + public final void notify(Object... args) { assert(args[0] instanceof Experiment); Experiment experiment = (Experiment) args[0]; assert(args[1] instanceof String); @@ -27,7 +26,7 @@ public void notify(Object... args) { } // Notice the argument list for decision - abstract void onActivate(@javax.annotation.Nonnull Experiment experiment, + public abstract void onActivate(@javax.annotation.Nonnull Experiment experiment, @javax.annotation.Nonnull String userId, @javax.annotation.Nonnull Map attributes, @javax.annotation.Nonnull Variation variation, diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index cd8444106..024d750c1 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -14,7 +14,7 @@ public enum NotificationType { // the notification id is incremented and is assigned as the callback id, it can then be used to remove the notification. private int notificationID = 1; // use tuple if available. - private class NotificationHolder + private static class NotificationHolder { int notificationId; Notification notification; diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java index 1b09fbcd7..97b24c9ec 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java @@ -7,7 +7,8 @@ public abstract class TrackNotification implements Notification { - public void notify(Object... args) { + @Override + public final void notify(Object... args) { assert(args[0] instanceof String); String eventKey = (String) args[0]; assert(args[1] instanceof String); @@ -23,7 +24,7 @@ public void notify(Object... args) { } // Notice the argument list for decision - abstract void onTrack(@Nonnull String eventKey, + public abstract void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index d8720913d..fb388467b 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -37,6 +37,8 @@ import com.optimizely.ab.event.internal.EventBuilder; import com.optimizely.ab.event.internal.EventBuilderV2; import com.optimizely.ab.internal.LogbackVerifier; +import com.optimizely.ab.notification.ActivateNotification; +import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.notification.NotificationListener; import org.junit.Rule; @@ -62,6 +64,8 @@ import ch.qos.logback.classic.Level; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import javax.annotation.Nonnull; + import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigJsonV2; import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigJsonV3; import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigV2; @@ -531,6 +535,68 @@ public void activateWithExperimentKey() throws Exception { verify(mockEventHandler).dispatchEvent(logEventToDispatch); } + /** + * Verify that the {@link Optimizely#activate(String, String, Map)} call + * correctly builds an endpoint url and request params + * and passes them through {@link EventHandler#dispatchEvent(LogEvent)}. + */ + @Test + public void activateWithListener() throws Exception { + final Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + final Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + ActivateNotification activateNotification = new ActivateNotification() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + assertEquals(experiment.getKey(), activatedExperiment.getKey()); + assertEquals(bucketedVariation.getKey(), variation.getKey()); + assertEquals(userId, testUserId); + } + }; + + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + + Map testUserAttributes = new HashMap(); + if (datafileVersion >= 4) { + testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + testUserAttributes.put("browser_type", "chrome"); + } + + testUserAttributes.put(testBucketingIdKey, testBucketingId); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), + eq(testUserId), eq(testUserAttributes))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, testBucketingId)) + .thenReturn(bucketedVariation); + + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); + + optimizely.notificationCenter.removeNotifiation(notificationId); + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, testBucketingId); + assertThat(actualVariation, is(bucketedVariation)); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + /** * Verify that {@link Optimizely#activate(String, String)} handles the case where an unknown experiment * (i.e., not in the config) is passed through and a {@link NoOpErrorHandler} is used by default. From 393a3433f13a3e101aa0ea52454278a877fe37dc Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 13 Dec 2017 10:54:16 -0800 Subject: [PATCH 03/10] test for all callback values --- .../com/optimizely/ab/OptimizelyTest.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index fb388467b..bd382d762 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -553,27 +553,33 @@ public void activateWithListener() throws Exception { .withErrorHandler(mockErrorHandler) .build(); + final Map testUserAttributes = new HashMap(); + if (datafileVersion >= 4) { + testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + testUserAttributes.put("browser_type", "chrome"); + } + + testUserAttributes.put(testBucketingIdKey, testBucketingId); + ActivateNotification activateNotification = new ActivateNotification() { @Override public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { assertEquals(experiment.getKey(), activatedExperiment.getKey()); assertEquals(bucketedVariation.getKey(), variation.getKey()); assertEquals(userId, testUserId); + for (Map.Entry entry : attributes.entrySet()) { + assertEquals(testUserAttributes.get(entry.getKey()), entry.getValue()); + } + + assertEquals(event.getRequestMethod(), RequestMethod.GET); } + }; int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); - Map testUserAttributes = new HashMap(); - if (datafileVersion >= 4) { - testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); - } - else { - testUserAttributes.put("browser_type", "chrome"); - } - - testUserAttributes.put(testBucketingIdKey, testBucketingId); - Map testParams = new HashMap(); testParams.put("test", "params"); LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); From dfabf59d36f37f7847cc82e072c319b5533ba4ed Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 13 Dec 2017 13:38:17 -0800 Subject: [PATCH 04/10] fix some typos and add logging along with try/catch for sendNotifications. --- .../java/com/optimizely/ab/Optimizely.java | 2 +- .../ab/notification/NotificationCenter.java | 21 ++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index d46144c9b..0f72c9553 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -94,7 +94,7 @@ public class Optimizely { @VisibleForTesting final EventHandler eventHandler; @VisibleForTesting final ErrorHandler errorHandler; @VisibleForTesting final NotificationBroadcaster notificationBroadcaster = new NotificationBroadcaster(); - @VisibleForTesting final NotificationCenter notificationCenter = new NotificationCenter(); + public final NotificationCenter notificationCenter = new NotificationCenter(logger); @Nullable private final UserProfileService userProfileService; diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index 024d750c1..35a1903e5 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -1,5 +1,7 @@ package com.optimizely.ab.notification; +import org.slf4j.Logger; + import java.util.ArrayList; import java.util.Map; import java.util.HashMap; @@ -13,6 +15,9 @@ public enum NotificationType { // the notification id is incremented and is assigned as the callback id, it can then be used to remove the notification. private int notificationID = 1; + + final private Logger logger; + // use tuple if available. private static class NotificationHolder { @@ -25,7 +30,8 @@ private static class NotificationHolder } } - public NotificationCenter() { + public NotificationCenter(Logger logger) { + this.logger = logger; notifications.put(NotificationType.Activate, new ArrayList()); notifications.put(NotificationType.Track, new ArrayList()); } @@ -34,14 +40,14 @@ public NotificationCenter() { // add a notification to your dictionary. Look at python implementation for details. // return notification id - public int addNotification(NotificationType notificaitonType, Notification notification) { - for (NotificationHolder holder : notifications.get(notificaitonType)) { + public int addNotification(NotificationType notificationType, Notification notification) { + for (NotificationHolder holder : notifications.get(notificationType)) { if (holder.notification == notification) { return -1; } } int id = notificationID; - notifications.get(notificaitonType).add(new NotificationHolder(notificationID++, notification )); + notifications.get(notificationType).add(new NotificationHolder(notificationID++, notification )); return id; } @@ -83,7 +89,12 @@ public void clearNotifications(NotificationType notificationType) { public void sendNotifications(NotificationType notificationType, Object ...args) { ArrayList holders = notifications.get(notificationType); for (NotificationHolder holder : holders) { - holder.notification.notify(args); + try { + holder.notification.notify(args); + } + catch (Exception e) { + logger.error("Unexpected exception calling notification listener {}", holder.notificationId, e); + } } } From 5f6b56c80e60047b4e5010f58a1c01e158731c08 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 13 Dec 2017 15:09:16 -0800 Subject: [PATCH 05/10] adding another listener unit test --- .../ab/notification/NotificationCenter.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 72 ++++++++++++++++++- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index 35a1903e5..c0483d6b3 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -52,7 +52,7 @@ public int addNotification(NotificationType notificationType, Notification notif } // remove notification by id - public boolean removeNotifiation(int notificationID) { + public boolean removeNotification(int notificationID) { ArrayList types = new ArrayList(); types.add(NotificationType.Activate); types.add(NotificationType.Track); diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index bd382d762..bf09e5079 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -522,8 +522,7 @@ public void activateWithExperimentKey() throws Exception { when(mockBucketer.bucket(activatedExperiment, testBucketingId)) .thenReturn(bucketedVariation); - - + // activate the experiment Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); @@ -594,7 +593,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ // activate the experiment Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); - optimizely.notificationCenter.removeNotifiation(notificationId); + optimizely.notificationCenter.removeNotification(notificationId); // verify that the bucketing algorithm was called correctly verify(mockBucketer).bucket(activatedExperiment, testBucketingId); assertThat(actualVariation, is(bucketedVariation)); @@ -823,6 +822,73 @@ public void activateWithNullAttributes() throws Exception { verify(mockEventHandler).dispatchEvent(logEventToDispatch); } + @Test + @SuppressFBWarnings( + value="NP_NONNULL_PARAM_VIOLATION", + justification="testing nullness contract violation") + public void activateWithListenerNullAttributes() throws Exception { + final Experiment activatedExperiment = noAudienceProjectConfig.getExperiments().get(0); + final Variation bucketedVariation = activatedExperiment.getVariations().get(0); + + // setup a mock event builder to return expected impression params + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(noAudienceDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(noAudienceProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(eq(noAudienceProjectConfig), eq(activatedExperiment), eq(bucketedVariation), + eq(testUserId), eq(Collections.emptyMap()))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, testUserId)) + .thenReturn(bucketedVariation); + + ActivateNotification activateNotification = new ActivateNotification() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + assertEquals(experiment.getKey(), activatedExperiment.getKey()); + assertEquals(bucketedVariation.getKey(), variation.getKey()); + assertEquals(userId, testUserId); + assertTrue(attributes.isEmpty()); + + assertEquals(event.getRequestMethod(), RequestMethod.GET); + } + + }; + + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + + // activate the experiment + Map attributes = null; + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, attributes); + + optimizely.notificationCenter.removeNotification(notificationId); + + logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map."); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, testUserId); + assertThat(actualVariation, is(bucketedVariation)); + + // setup the attribute map captor (so we can verify its content) + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + verify(mockEventBuilder).createImpressionEvent(eq(noAudienceProjectConfig), eq(activatedExperiment), + eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); + + Map actualValue = attributeCaptor.getValue(); + assertThat(actualValue, is(Collections.emptyMap())); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + /** * Verify that {@link Optimizely#activate(String, String, Map)} gracefully handles null attribute values. */ From 1d567cbb4303d5baebdadcbf8c5393d185851390 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 13 Dec 2017 16:15:26 -0800 Subject: [PATCH 06/10] finish unit tests --- .../java/com/optimizely/ab/Optimizely.java | 2 +- .../ab/notification/NotificationCenter.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 831 +++++++++++++----- 3 files changed, 631 insertions(+), 204 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 0f72c9553..99d43b9c5 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -299,7 +299,7 @@ public void track(@Nonnull String eventName, notificationBroadcaster.broadcastEventTracked(eventName, userId, filteredAttributes, eventValue, conversionEvent); notificationCenter.sendNotifications(NotificationCenter.NotificationType.Track, eventName, userId, - filteredAttributes, eventValue, conversionEvent); + filteredAttributes, eventTags, conversionEvent); } //======== FeatureFlag APIs ========// diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index c0483d6b3..4599a6234 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -70,7 +70,7 @@ public boolean removeNotification(int notificationID) { } // clean out all notifications. - public void cleanAllNotifications() { + public void clearAllNotifications() { ArrayList types = new ArrayList(); types.add(NotificationType.Activate); types.add(NotificationType.Track); diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index bf09e5079..44f30f82a 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -41,6 +41,7 @@ import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.notification.NotificationListener; +import com.optimizely.ab.notification.TrackNotification; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -534,74 +535,6 @@ public void activateWithExperimentKey() throws Exception { verify(mockEventHandler).dispatchEvent(logEventToDispatch); } - /** - * Verify that the {@link Optimizely#activate(String, String, Map)} call - * correctly builds an endpoint url and request params - * and passes them through {@link EventHandler#dispatchEvent(LogEvent)}. - */ - @Test - public void activateWithListener() throws Exception { - final Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); - final Variation bucketedVariation = activatedExperiment.getVariations().get(0); - EventBuilder mockEventBuilder = mock(EventBuilder.class); - - Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) - .withBucketing(mockBucketer) - .withEventBuilder(mockEventBuilder) - .withConfig(validProjectConfig) - .withErrorHandler(mockErrorHandler) - .build(); - - final Map testUserAttributes = new HashMap(); - if (datafileVersion >= 4) { - testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); - } - else { - testUserAttributes.put("browser_type", "chrome"); - } - - testUserAttributes.put(testBucketingIdKey, testBucketingId); - - ActivateNotification activateNotification = new ActivateNotification() { - @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { - assertEquals(experiment.getKey(), activatedExperiment.getKey()); - assertEquals(bucketedVariation.getKey(), variation.getKey()); - assertEquals(userId, testUserId); - for (Map.Entry entry : attributes.entrySet()) { - assertEquals(testUserAttributes.get(entry.getKey()), entry.getValue()); - } - - assertEquals(event.getRequestMethod(), RequestMethod.GET); - } - - }; - - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); - - Map testParams = new HashMap(); - testParams.put("test", "params"); - LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); - when(mockEventBuilder.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), - eq(testUserId), eq(testUserAttributes))) - .thenReturn(logEventToDispatch); - - when(mockBucketer.bucket(activatedExperiment, testBucketingId)) - .thenReturn(bucketedVariation); - - - // activate the experiment - Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); - - optimizely.notificationCenter.removeNotification(notificationId); - // verify that the bucketing algorithm was called correctly - verify(mockBucketer).bucket(activatedExperiment, testBucketingId); - assertThat(actualVariation, is(bucketedVariation)); - - // verify that dispatchEvent was called with the correct LogEvent object - verify(mockEventHandler).dispatchEvent(logEventToDispatch); - } - /** * Verify that {@link Optimizely#activate(String, String)} handles the case where an unknown experiment * (i.e., not in the config) is passed through and a {@link NoOpErrorHandler} is used by default. @@ -822,73 +755,6 @@ public void activateWithNullAttributes() throws Exception { verify(mockEventHandler).dispatchEvent(logEventToDispatch); } - @Test - @SuppressFBWarnings( - value="NP_NONNULL_PARAM_VIOLATION", - justification="testing nullness contract violation") - public void activateWithListenerNullAttributes() throws Exception { - final Experiment activatedExperiment = noAudienceProjectConfig.getExperiments().get(0); - final Variation bucketedVariation = activatedExperiment.getVariations().get(0); - - // setup a mock event builder to return expected impression params - EventBuilder mockEventBuilder = mock(EventBuilder.class); - - Optimizely optimizely = Optimizely.builder(noAudienceDatafile, mockEventHandler) - .withBucketing(mockBucketer) - .withEventBuilder(mockEventBuilder) - .withConfig(noAudienceProjectConfig) - .withErrorHandler(mockErrorHandler) - .build(); - - Map testParams = new HashMap(); - testParams.put("test", "params"); - LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); - when(mockEventBuilder.createImpressionEvent(eq(noAudienceProjectConfig), eq(activatedExperiment), eq(bucketedVariation), - eq(testUserId), eq(Collections.emptyMap()))) - .thenReturn(logEventToDispatch); - - when(mockBucketer.bucket(activatedExperiment, testUserId)) - .thenReturn(bucketedVariation); - - ActivateNotification activateNotification = new ActivateNotification() { - @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { - assertEquals(experiment.getKey(), activatedExperiment.getKey()); - assertEquals(bucketedVariation.getKey(), variation.getKey()); - assertEquals(userId, testUserId); - assertTrue(attributes.isEmpty()); - - assertEquals(event.getRequestMethod(), RequestMethod.GET); - } - - }; - - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); - - // activate the experiment - Map attributes = null; - Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, attributes); - - optimizely.notificationCenter.removeNotification(notificationId); - - logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map."); - - // verify that the bucketing algorithm was called correctly - verify(mockBucketer).bucket(activatedExperiment, testUserId); - assertThat(actualVariation, is(bucketedVariation)); - - // setup the attribute map captor (so we can verify its content) - ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); - verify(mockEventBuilder).createImpressionEvent(eq(noAudienceProjectConfig), eq(activatedExperiment), - eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); - - Map actualValue = attributeCaptor.getValue(); - assertThat(actualValue, is(Collections.emptyMap())); - - // verify that dispatchEvent was called with the correct LogEvent object - verify(mockEventHandler).dispatchEvent(logEventToDispatch); - } - /** * Verify that {@link Optimizely#activate(String, String, Map)} gracefully handles null attribute values. */ @@ -2325,107 +2191,242 @@ public void getVariationExperimentStatusPrecedesForcedVariation() throws Excepti //======== Notification listeners ========// /** - * Verify that {@link Optimizely#addNotificationListener(NotificationListener)} properly calls - * through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the listener is - * added and notified when an experiment is activated. + * Verify that the {@link Optimizely#activate(String, String, Map)} call + * correctly builds an endpoint url and request params + * and passes them through {@link EventHandler#dispatchEvent(LogEvent)}. */ @Test - public void addNotificationListener() throws Exception { - Experiment activatedExperiment; - EventType eventType; - if (datafileVersion >= 4) { - activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_BASIC_EXPERIMENT_KEY); - eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); - } - else { - activatedExperiment = validProjectConfig.getExperiments().get(0); - eventType = validProjectConfig.getEventTypes().get(0); - } - Variation bucketedVariation = activatedExperiment.getVariations().get(0); + public void activateWithListener() throws Exception { + final Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + final Variation bucketedVariation = activatedExperiment.getVariations().get(0); EventBuilder mockEventBuilder = mock(EventBuilder.class); Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) - .withDecisionService(mockDecisionService) + .withBucketing(mockBucketer) .withEventBuilder(mockEventBuilder) .withConfig(validProjectConfig) .withErrorHandler(mockErrorHandler) .build(); - Map attributes = Collections.emptyMap(); + final Map testUserAttributes = new HashMap(); + if (datafileVersion >= 4) { + testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + testUserAttributes.put("browser_type", "chrome"); + } + + testUserAttributes.put(testBucketingIdKey, testBucketingId); + + ActivateNotification activateNotification = new ActivateNotification() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + assertEquals(experiment.getKey(), activatedExperiment.getKey()); + assertEquals(bucketedVariation.getKey(), variation.getKey()); + assertEquals(userId, testUserId); + for (Map.Entry entry : attributes.entrySet()) { + assertEquals(testUserAttributes.get(entry.getKey()), entry.getValue()); + } + + assertEquals(event.getRequestMethod(), RequestMethod.GET); + } + + }; + + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); Map testParams = new HashMap(); testParams.put("test", "params"); LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); - when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, - bucketedVariation, genericUserId, attributes)) + when(mockEventBuilder.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), + eq(testUserId), eq(testUserAttributes))) .thenReturn(logEventToDispatch); - when(mockDecisionService.getVariation( - eq(activatedExperiment), - eq(genericUserId), - eq(Collections.emptyMap()))) + when(mockBucketer.bucket(activatedExperiment, testBucketingId)) .thenReturn(bucketedVariation); - // Add listener - NotificationListener listener = mock(NotificationListener.class); - optimizely.addNotificationListener(listener); - - // Check if listener is notified when experiment is activated - Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); - verify(listener, times(1)) - .onExperimentActivated(activatedExperiment, genericUserId, attributes, actualVariation); - // Check if listener is notified after an event is tracked - String eventKey = eventType.getKey(); + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); - Map experimentVariationMap = createExperimentVariationMap( - validProjectConfig, - mockDecisionService, - eventType.getKey(), - genericUserId, - attributes); - when(mockEventBuilder.createConversionEvent( - eq(validProjectConfig), - eq(experimentVariationMap), - eq(genericUserId), - eq(eventType.getId()), - eq(eventKey), - eq(attributes), - anyMapOf(String.class, Object.class))) - .thenReturn(logEventToDispatch); + optimizely.notificationCenter.removeNotification(notificationId); + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, testBucketingId); + assertThat(actualVariation, is(bucketedVariation)); - optimizely.track(eventKey, genericUserId, attributes); - verify(listener, times(1)) - .onEventTracked(eventKey, genericUserId, attributes, null, logEventToDispatch); + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); } - /** - * Verify that {@link Optimizely#removeNotificationListener(NotificationListener)} properly - * calls through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the - * listener is removed and no longer notified when an experiment is activated. - */ @Test - public void removeNotificationListener() throws Exception { - Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); - Variation bucketedVariation = activatedExperiment.getVariations().get(0); + @SuppressFBWarnings( + value="NP_NONNULL_PARAM_VIOLATION", + justification="testing nullness contract violation") + public void activateWithListenerNullAttributes() throws Exception { + final Experiment activatedExperiment = noAudienceProjectConfig.getExperiments().get(0); + final Variation bucketedVariation = activatedExperiment.getVariations().get(0); + + // setup a mock event builder to return expected impression params EventBuilder mockEventBuilder = mock(EventBuilder.class); - Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + Optimizely optimizely = Optimizely.builder(noAudienceDatafile, mockEventHandler) .withBucketing(mockBucketer) .withEventBuilder(mockEventBuilder) - .withConfig(validProjectConfig) + .withConfig(noAudienceProjectConfig) .withErrorHandler(mockErrorHandler) .build(); - Map attributes = new HashMap(); - attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); - Map testParams = new HashMap(); testParams.put("test", "params"); LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); - when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, - bucketedVariation, genericUserId, attributes)) - .thenReturn(logEventToDispatch); + when(mockEventBuilder.createImpressionEvent(eq(noAudienceProjectConfig), eq(activatedExperiment), eq(bucketedVariation), + eq(testUserId), eq(Collections.emptyMap()))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, testUserId)) + .thenReturn(bucketedVariation); + + ActivateNotification activateNotification = new ActivateNotification() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + assertEquals(experiment.getKey(), activatedExperiment.getKey()); + assertEquals(bucketedVariation.getKey(), variation.getKey()); + assertEquals(userId, testUserId); + assertTrue(attributes.isEmpty()); + + assertEquals(event.getRequestMethod(), RequestMethod.GET); + } + + }; + + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + + // activate the experiment + Map attributes = null; + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, attributes); + + optimizely.notificationCenter.removeNotification(notificationId); + + logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map."); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, testUserId); + assertThat(actualVariation, is(bucketedVariation)); + + // setup the attribute map captor (so we can verify its content) + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + verify(mockEventBuilder).createImpressionEvent(eq(noAudienceProjectConfig), eq(activatedExperiment), + eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); + + Map actualValue = attributeCaptor.getValue(); + assertThat(actualValue, is(Collections.emptyMap())); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + + /** + * Verify that {@link Optimizely#addNotificationListener(NotificationListener)} properly calls + * through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the listener is + * added and notified when an experiment is activated. + */ + @Test + public void addNotificationListener() throws Exception { + Experiment activatedExperiment; + EventType eventType; + if (datafileVersion >= 4) { + activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_BASIC_EXPERIMENT_KEY); + eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); + } + else { + activatedExperiment = validProjectConfig.getExperiments().get(0); + eventType = validProjectConfig.getEventTypes().get(0); + } + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withDecisionService(mockDecisionService) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map attributes = Collections.emptyMap(); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, + bucketedVariation, genericUserId, attributes)) + .thenReturn(logEventToDispatch); + + when(mockDecisionService.getVariation( + eq(activatedExperiment), + eq(genericUserId), + eq(Collections.emptyMap()))) + .thenReturn(bucketedVariation); + + // Add listener + NotificationListener listener = mock(NotificationListener.class); + optimizely.addNotificationListener(listener); + + // Check if listener is notified when experiment is activated + Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); + verify(listener, times(1)) + .onExperimentActivated(activatedExperiment, genericUserId, attributes, actualVariation); + + // Check if listener is notified after an event is tracked + String eventKey = eventType.getKey(); + + Map experimentVariationMap = createExperimentVariationMap( + validProjectConfig, + mockDecisionService, + eventType.getKey(), + genericUserId, + attributes); + when(mockEventBuilder.createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventKey), + eq(attributes), + anyMapOf(String.class, Object.class))) + .thenReturn(logEventToDispatch); + + optimizely.track(eventKey, genericUserId, attributes); + verify(listener, times(1)) + .onEventTracked(eventKey, genericUserId, attributes, null, logEventToDispatch); + } + + /** + * Verify that {@link Optimizely#removeNotificationListener(NotificationListener)} properly + * calls through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the + * listener is removed and no longer notified when an experiment is activated. + */ + @Test + public void removeNotificationListener() throws Exception { + Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map attributes = new HashMap(); + attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, + bucketedVariation, genericUserId, attributes)) + .thenReturn(logEventToDispatch); when(mockBucketer.bucket(activatedExperiment, genericUserId)) .thenReturn(bucketedVariation); @@ -2565,6 +2566,432 @@ public void clearNotificationListeners() throws Exception { .onEventTracked(eventKey, genericUserId, attributes, null, logEventToDispatch); } + /** + * Verify that {@link Optimizely#addNotificationListener(NotificationListener)} properly calls + * through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the listener is + * added and notified when an experiment is activated. + */ + @Test + public void addNotificationListenerFromNotificationCenter() throws Exception { + Experiment activatedExperiment; + EventType eventType; + if (datafileVersion >= 4) { + activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_BASIC_EXPERIMENT_KEY); + eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); + } + else { + activatedExperiment = validProjectConfig.getExperiments().get(0); + eventType = validProjectConfig.getEventTypes().get(0); + } + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withDecisionService(mockDecisionService) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map attributes = Collections.emptyMap(); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, + bucketedVariation, genericUserId, attributes)) + .thenReturn(logEventToDispatch); + + when(mockDecisionService.getVariation( + eq(activatedExperiment), + eq(genericUserId), + eq(Collections.emptyMap()))) + .thenReturn(bucketedVariation); + + // Add listener + ActivateNotification listener = mock(ActivateNotification.class); + optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, listener); + + // Check if listener is notified when experiment is activated + Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); + verify(listener, times(1)) + .onActivate(activatedExperiment, genericUserId, attributes, bucketedVariation, logEventToDispatch); + + assertEquals(actualVariation.getKey(), bucketedVariation.getKey()); + // Check if listener is notified after an event is tracked + String eventKey = eventType.getKey(); + + Map experimentVariationMap = createExperimentVariationMap( + validProjectConfig, + mockDecisionService, + eventType.getKey(), + genericUserId, + attributes); + when(mockEventBuilder.createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventKey), + eq(attributes), + anyMapOf(String.class, Object.class))) + .thenReturn(logEventToDispatch); + + TrackNotification trackNotification = mock(TrackNotification.class); + + optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + + optimizely.track(eventKey, genericUserId, attributes); + verify(trackNotification, times(1)) + .onTrack(eventKey, genericUserId, attributes, Collections.EMPTY_MAP, logEventToDispatch); + } + + /** + * Verify that {@link Optimizely#removeNotificationListener(NotificationListener)} properly + * calls through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the + * listener is removed and no longer notified when an experiment is activated. + */ + @Test + public void removeNotificationListenerNotificationCenter() throws Exception { + Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map attributes = new HashMap(); + attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, + bucketedVariation, genericUserId, attributes)) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, genericUserId)) + .thenReturn(bucketedVariation); + + when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, bucketedVariation, genericUserId, + attributes)) + .thenReturn(logEventToDispatch); + + // Add and remove listener + ActivateNotification activateNotification = mock(ActivateNotification.class); + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + optimizely.notificationCenter.removeNotification(notificationId); + + TrackNotification trackNotification = mock(TrackNotification.class); + notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + optimizely.notificationCenter.removeNotification(notificationId); + + // Check if listener is notified after an experiment is activated + Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); + verify(activateNotification, never()) + .onActivate(activatedExperiment, genericUserId, attributes, actualVariation, logEventToDispatch); + + // Check if listener is notified after a live variable is accessed + boolean activateExperiment = true; + verify(activateNotification, never()) + .onActivate(activatedExperiment, genericUserId, attributes, actualVariation, logEventToDispatch); + + // Check if listener is notified after an event is tracked + EventType eventType = validProjectConfig.getEventTypes().get(0); + String eventKey = eventType.getKey(); + + Map experimentVariationMap = createExperimentVariationMap( + validProjectConfig, + mockDecisionService, + eventType.getKey(), + genericUserId, + attributes); + when(mockEventBuilder.createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventKey), + eq(attributes), + anyMapOf(String.class, Object.class))) + .thenReturn(logEventToDispatch); + + optimizely.track(eventKey, genericUserId, attributes); + verify(trackNotification, never()) + .onTrack(eventKey, genericUserId, attributes, Collections.EMPTY_MAP, logEventToDispatch); + } + + /** + * Verify that {@link Optimizely#clearNotificationListeners()} properly calls through to + * {@link com.optimizely.ab.notification.NotificationBroadcaster} and all listeners are removed + * and no longer notified when an experiment is activated. + */ + @Test + public void clearNotificationListenersNotificationCenter() throws Exception { + Experiment activatedExperiment; + Map attributes = new HashMap(); + if (datafileVersion >= 4) { + activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY); + attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + activatedExperiment = validProjectConfig.getExperiments().get(0); + attributes.put("browser_type", "chrome"); + } + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, + bucketedVariation, genericUserId, attributes)) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, genericUserId)) + .thenReturn(bucketedVariation); + + // set up argument captor for the attributes map to compare map equality + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + + when(mockEventBuilder.createImpressionEvent( + eq(validProjectConfig), + eq(activatedExperiment), + eq(bucketedVariation), + eq(genericUserId), + attributeCaptor.capture() + )).thenReturn(logEventToDispatch); + + ActivateNotification activateNotification = mock(ActivateNotification.class); + TrackNotification trackNotification = mock(TrackNotification.class); + + optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + + optimizely.notificationCenter.clearAllNotifications(); + + // Check if listener is notified after an experiment is activated + Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); + + // check that the argument that was captured by the mockEventBuilder attribute captor, + // was equal to the attributes passed in to activate + assertEquals(attributes, attributeCaptor.getValue()); + verify(activateNotification, never()) + .onActivate(activatedExperiment, genericUserId, attributes, actualVariation, logEventToDispatch); + + // Check if listener is notified after a live variable is accessed + boolean activateExperiment = true; + verify(activateNotification, never()) + .onActivate(activatedExperiment, genericUserId, attributes, actualVariation, logEventToDispatch); + + // Check if listener is notified after a event is tracked + EventType eventType = validProjectConfig.getEventTypes().get(0); + String eventKey = eventType.getKey(); + + Map experimentVariationMap = createExperimentVariationMap( + validProjectConfig, + mockDecisionService, + eventType.getKey(), + OptimizelyTest.genericUserId, + attributes); + when(mockEventBuilder.createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(OptimizelyTest.genericUserId), + eq(eventType.getId()), + eq(eventKey), + eq(attributes), + anyMapOf(String.class, Object.class))) + .thenReturn(logEventToDispatch); + + optimizely.track(eventKey, genericUserId, attributes); + verify(trackNotification, never()) + .onTrack(eventKey, genericUserId, attributes, Collections.EMPTY_MAP, logEventToDispatch); + } + + @Test + @SuppressWarnings("unchecked") + public void trackEventWithListenerAttributes() throws Exception { + final Attribute attribute = validProjectConfig.getAttributes().get(0); + final EventType eventType; + if (datafileVersion >= 4) { + eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); + } + else { + eventType = validProjectConfig.getEventTypes().get(0); + } + + // setup a mock event builder to return expected conversion params + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + final Map attributes = ImmutableMap.of(attribute.getKey(), "attributeValue"); + Map experimentVariationMap = createExperimentVariationMap( + validProjectConfig, + mockDecisionService, + eventType.getKey(), + genericUserId, + attributes); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventType.getKey()), + anyMapOf(String.class, String.class), + eq(Collections.emptyMap()))) + .thenReturn(logEventToDispatch); + + logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + + "\" for user \"" + genericUserId + "\"."); + logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + + testParams + " and payload \"\""); + + TrackNotification trackNotification = new TrackNotification() { + @Override + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map _attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + assertEquals(eventType.getKey(), eventKey); + assertEquals(genericUserId, userId); + assertEquals(attributes, _attributes); + assertTrue(eventTags.isEmpty()); + } + }; + + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + + // call track + optimizely.track(eventType.getKey(), genericUserId, attributes); + + optimizely.notificationCenter.removeNotification(notificationId); + + // setup the attribute map captor (so we can verify its content) + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + + // verify that the event builder was called with the expected attributes + verify(mockEventBuilder).createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventType.getKey()), + attributeCaptor.capture(), + eq(Collections.emptyMap())); + + Map actualValue = attributeCaptor.getValue(); + assertThat(actualValue, hasEntry(attribute.getKey(), "attributeValue")); + + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + + /** + * Verify that {@link Optimizely#track(String, String)} ignores null attributes. + */ + @Test + @SuppressFBWarnings( + value="NP_NONNULL_PARAM_VIOLATION", + justification="testing nullness contract violation") + public void trackEventWitListenerhNullAttributes() throws Exception { + final EventType eventType; + if (datafileVersion >= 4) { + eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); + } + else { + eventType = validProjectConfig.getEventTypes().get(0); + } + + // setup a mock event builder to return expected conversion params + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + Map testParams = new HashMap(); + testParams.put("test", "params"); + Map experimentVariationMap = createExperimentVariationMap( + validProjectConfig, + mockDecisionService, + eventType.getKey(), + genericUserId, + Collections.emptyMap()); + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventType.getKey()), + eq(Collections.emptyMap()), + eq(Collections.emptyMap()))) + .thenReturn(logEventToDispatch); + + logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + + "\" for user \"" + genericUserId + "\"."); + logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + + testParams + " and payload \"\""); + + TrackNotification trackNotification = new TrackNotification() { + @Override + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + assertEquals(eventType.getKey(), eventKey); + assertEquals(genericUserId, userId); + assertTrue(attributes.isEmpty()); + assertTrue(eventTags.isEmpty()); + } + }; + + int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + + // call track + Map attributes = null; + optimizely.track(eventType.getKey(), genericUserId, attributes); + + optimizely.notificationCenter.removeNotification(notificationId); + + logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map."); + + // setup the attribute map captor (so we can verify its content) + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + + // verify that the event builder was called with the expected attributes + verify(mockEventBuilder).createConversionEvent( + eq(validProjectConfig), + eq(experimentVariationMap), + eq(genericUserId), + eq(eventType.getId()), + eq(eventType.getKey()), + attributeCaptor.capture(), + eq(Collections.emptyMap())); + + Map actualValue = attributeCaptor.getValue(); + assertThat(actualValue, is(Collections.emptyMap())); + + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + //======== Feature Accessor Tests ========// /** From 5da84ac34535cc0984481778132ff8d77886968b Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 14 Dec 2017 16:21:52 -0800 Subject: [PATCH 07/10] tweak notificaion type, add javadocs and comments --- .../ab/notification/ActivateNotification.java | 30 ++++- .../ab/notification/Notification.java | 31 +++++- .../ab/notification/NotificationCenter.java | 103 ++++++++++++++---- .../ab/notification/TrackNotification.java | 32 +++++- .../com/optimizely/ab/OptimizelyTest.java | 22 ++-- 5 files changed, 180 insertions(+), 38 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java index 9522cc74a..73c426545 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java @@ -1,3 +1,20 @@ +/** + * + * Copyright 2017, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.optimizely.ab.notification; import com.optimizely.ab.config.Experiment; @@ -9,6 +26,10 @@ public abstract class ActivateNotification implements Notification { + /** + * Base notify called with var args. This method parses the parameters and calls the abstract method. + * @param args - variable argument list based on the type of notification. + */ @Override public final void notify(Object... args) { assert(args[0] instanceof Experiment); @@ -25,7 +46,14 @@ public final void notify(Object... args) { onActivate(experiment, userId, attributes, variation, logEvent); } - // Notice the argument list for decision + /** + * onActivate called when an activate was triggered + * @param experiment - The experiment object being activated. + * @param userId - The userId passed into activate. + * @param attributes - The filtered attribute list passed into activate + * @param variation - The variation that was returned from activate. + * @param event - The impression event that was triggered. + */ public abstract void onActivate(@javax.annotation.Nonnull Experiment experiment, @javax.annotation.Nonnull String userId, @javax.annotation.Nonnull Map attributes, diff --git a/core-api/src/main/java/com/optimizely/ab/notification/Notification.java b/core-api/src/main/java/com/optimizely/ab/notification/Notification.java index 04426dbb4..83a3f28eb 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/Notification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/Notification.java @@ -1,12 +1,31 @@ -package com.optimizely.ab.notification; +/** + * + * Copyright 2017, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ -import javax.annotation.Nonnull; -import com.optimizely.ab.config.Experiment; -import com.optimizely.ab.config.Variation; -import com.optimizely.ab.event.LogEvent; -import java.util.Map; +package com.optimizely.ab.notification; +/** + * Interface used to process notifications. Notifications can pass in any argument list. + * Abstract implementations take the called notify and make sure the parameters are as expected. + */ public interface Notification { + /** + * notify called when a notification is triggered + * @param args - variable argument list based on the type of notification. + */ public void notify(Object ...args); } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index 4599a6234..d686f5f16 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2017, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.notification; import org.slf4j.Logger; @@ -6,10 +22,28 @@ import java.util.Map; import java.util.HashMap; +/** + * This class handles impression and conversion notifications. It replaces NotificationBroadcaster and is intended to be + * more flexible. + */ public class NotificationCenter { + /** + * NotificationType is used for the notification types supported. + */ public enum NotificationType { - Activate, // Activate was called. - Track, // track an impression event + + Activate(ActivateNotification.class), // Activate was called. Track an impression event + Track(TrackNotification.class); // Track was called. Track a conversion event + + private Class notificationClass; + + NotificationType(Class notificationClass) { + this.notificationClass = notificationClass; + } + + public Class getNotificationClass() { + return notificationClass; + } }; @@ -18,7 +52,7 @@ public enum NotificationType { final private Logger logger; - // use tuple if available. + // notification holder holds the id as well as the notification. private static class NotificationHolder { int notificationId; @@ -30,57 +64,82 @@ private static class NotificationHolder } } + /** + * Instantiate a new NotificationCenter + * @param logger pass in logger to use. + */ public NotificationCenter(Logger logger) { this.logger = logger; notifications.put(NotificationType.Activate, new ArrayList()); notifications.put(NotificationType.Track, new ArrayList()); } -// dictionary of enum to notification lists. So, DECISION would have an array of notification holders. + + // private list of notification by notification type. + // we used a list so that notification order can mean something. private Map> notifications =new HashMap>(); - // add a notification to your dictionary. Look at python implementation for details. -// return notification id + + /** + * Add a notification listener to the notification center. + * + * @param notificationType - enum NotificationType to add. + * @param notification - Notification to add. + * @return the notification id used to remove the notification. It is greater than 0 on success. + */ public int addNotification(NotificationType notificationType, Notification notification) { + + Class clazz = notificationType.notificationClass; + if (clazz == null || !clazz.isInstance(notification)) { + logger.warn("Notification listener was the wrong type. It was not added to the notification center."); + return -1; + } + for (NotificationHolder holder : notifications.get(notificationType)) { if (holder.notification == notification) { + logger.warn("Notificication listener was already added"); return -1; } } - int id = notificationID; - notifications.get(notificationType).add(new NotificationHolder(notificationID++, notification )); + int id = notificationID++; + notifications.get(notificationType).add(new NotificationHolder(id, notification )); + logger.info("Notification listener {} was added with id {}", notification.toString(), id); return id; } - // remove notification by id + /** + * Remove the notification listener based on the notificationId passed back from addNotification. + * @param notificationID the id passed back from add notification. + * @return true if removed otherwise false (if the notification is already registered, it returns false). + */ public boolean removeNotification(int notificationID) { - ArrayList types = new ArrayList(); - types.add(NotificationType.Activate); - types.add(NotificationType.Track); - - for (NotificationType type : types) { + for (NotificationType type : NotificationType.values()) { for (NotificationHolder holder : notifications.get(type)) { if (holder.notificationId == notificationID) { notifications.get(type).remove(holder); + logger.info("Notification listener removed {}", notificationID); return true; } } } + logger.warn("Notification listener with id {} not found", notificationID); + return false; } - // clean out all notifications. + /** + * Clear out all the notification listeners. + */ public void clearAllNotifications() { - ArrayList types = new ArrayList(); - types.add(NotificationType.Activate); - types.add(NotificationType.Track); - - for (NotificationType type : types) { - notifications.get(type).clear(); + for (NotificationType type : NotificationType.values()) { + clearNotifications(type); } } - // clean out notifications by notification type. + /** + * Clear notification listeners by notification type. + * @param notificationType type of notifications to remove. + */ public void clearNotifications(NotificationType notificationType) { notifications.get(notificationType).clear(); } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java index 97b24c9ec..d1743e710 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2017, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.notification; import java.util.Map; @@ -5,8 +21,15 @@ import com.optimizely.ab.event.LogEvent; +/** + * This class handles the track event notification. + */ public abstract class TrackNotification implements Notification { + /** + * Base notify called with var args. This method parses the parameters and calls the abstract method. + * @param args - variable argument list based on the type of notification. + */ @Override public final void notify(Object... args) { assert(args[0] instanceof String); @@ -23,7 +46,14 @@ public final void notify(Object... args) { onTrack(eventKey, userId,attributes,eventTags, logEvent); } - // Notice the argument list for decision + /** + * onTrack is called when a track event is triggered + * @param eventKey - The event key that was triggered. + * @param userId - user id passed into track. + * @param attributes - filtered attributes list after passed into track + * @param eventTags - event tags if any were passed in. + * @param event - The event being recorded. + */ public abstract void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 44f30f82a..dae50821b 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2567,8 +2567,10 @@ public void clearNotificationListeners() throws Exception { } /** - * Verify that {@link Optimizely#addNotificationListener(NotificationListener)} properly calls - * through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the listener is + * Verify that {@link com.optimizely.ab.notification.NotificationCenter#addNotification( + * com.optimizely.ab.notification.NotificationCenter.NotificationType, + * com.optimizely.ab.notification.Notification)} properly used + * and the listener is * added and notified when an experiment is activated. */ @Test @@ -2647,9 +2649,8 @@ public void addNotificationListenerFromNotificationCenter() throws Exception { } /** - * Verify that {@link Optimizely#removeNotificationListener(NotificationListener)} properly - * calls through to {@link com.optimizely.ab.notification.NotificationBroadcaster} and the - * listener is removed and no longer notified when an experiment is activated. + * Verify that {@link com.optimizely.ab.notification.NotificationCenter} properly + * calls and the listener is removed and no longer notified when an experiment is activated. */ @Test public void removeNotificationListenerNotificationCenter() throws Exception { @@ -2726,8 +2727,8 @@ public void removeNotificationListenerNotificationCenter() throws Exception { } /** - * Verify that {@link Optimizely#clearNotificationListeners()} properly calls through to - * {@link com.optimizely.ab.notification.NotificationBroadcaster} and all listeners are removed + * Verify that {@link com.optimizely.ab.notification.NotificationCenter} + * clearAllListerners removes all listeners * and no longer notified when an experiment is activated. */ @Test @@ -2820,6 +2821,11 @@ public void clearNotificationListenersNotificationCenter() throws Exception { .onTrack(eventKey, genericUserId, attributes, Collections.EMPTY_MAP, logEventToDispatch); } + /** + * Add notificaiton listener for track {@link com.optimizely.ab.notification.NotificationCenter}. Verify called and + * remove. + * @throws Exception + */ @Test @SuppressWarnings("unchecked") public void trackEventWithListenerAttributes() throws Exception { @@ -2904,7 +2910,7 @@ public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull M } /** - * Verify that {@link Optimizely#track(String, String)} ignores null attributes. + * Track with listener and verify that {@link Optimizely#track(String, String)} ignores null attributes. */ @Test @SuppressFBWarnings( From e86f3eb7a254ceca209337a6469aac016c063fc0 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Fri, 15 Dec 2017 14:16:57 -0800 Subject: [PATCH 08/10] added more tests and fix typo --- .../java/com/optimizely/ab/Optimizely.java | 3 ++ .../ab/notification/NotificationCenter.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 8 ++-- .../notification/NotificationCenterTest.java | 47 +++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 99d43b9c5..422345001 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -696,6 +696,7 @@ public UserProfileService getUserProfileService() { * * @param listener listener to add */ + @Deprecated public void addNotificationListener(@Nonnull NotificationListener listener) { notificationBroadcaster.addListener(listener); } @@ -705,6 +706,7 @@ public void addNotificationListener(@Nonnull NotificationListener listener) { * * @param listener listener to remove */ + @Deprecated public void removeNotificationListener(@Nonnull NotificationListener listener) { notificationBroadcaster.removeListener(listener); } @@ -712,6 +714,7 @@ public void removeNotificationListener(@Nonnull NotificationListener listener) { /** * Remove all {@link NotificationListener}. */ + @Deprecated public void clearNotificationListeners() { notificationBroadcaster.clearListeners(); } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index d686f5f16..80e2518ba 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -123,7 +123,7 @@ public boolean removeNotification(int notificationID) { } logger.warn("Notification listener with id {} not found", notificationID); - + return false; } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index dae50821b..7cf863fc0 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2249,7 +2249,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ // activate the experiment Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); - optimizely.notificationCenter.removeNotification(notificationId); + assertTrue(optimizely.notificationCenter.removeNotification(notificationId)); // verify that the bucketing algorithm was called correctly verify(mockBucketer).bucket(activatedExperiment, testBucketingId); assertThat(actualVariation, is(bucketedVariation)); @@ -2685,11 +2685,11 @@ public void removeNotificationListenerNotificationCenter() throws Exception { // Add and remove listener ActivateNotification activateNotification = mock(ActivateNotification.class); int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); - optimizely.notificationCenter.removeNotification(notificationId); + assertTrue(optimizely.notificationCenter.removeNotification(notificationId)); TrackNotification trackNotification = mock(TrackNotification.class); notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); - optimizely.notificationCenter.removeNotification(notificationId); + assertTrue(optimizely.notificationCenter.removeNotification(notificationId)); // Check if listener is notified after an experiment is activated Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); @@ -2916,7 +2916,7 @@ public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull M @SuppressFBWarnings( value="NP_NONNULL_PARAM_VIOLATION", justification="testing nullness contract violation") - public void trackEventWitListenerhNullAttributes() throws Exception { + public void trackEventWithListenerNullAttributes() throws Exception { final EventType eventType; if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); diff --git a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java new file mode 100644 index 000000000..29b5e8904 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java @@ -0,0 +1,47 @@ +package com.optimizely.ab.notification; + +import ch.qos.logback.classic.Level; +import com.optimizely.ab.internal.LogbackVerifier; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.Mockito.mock; + +public class NotificationCenterTest { + private NotificationCenter notificationCenter; + private ActivateNotification activateNotification; + private TrackNotification trackNotification; + private Logger logger; + + @Rule + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + + @Before + public void initialize() { + logger = LoggerFactory.getLogger("NotificationCenterTest"); + notificationCenter = new NotificationCenter(logger); + activateNotification = mock(ActivateNotification.class); + trackNotification = mock(TrackNotification.class); + } + + @Test + public void testAddWrongTrackNotificationListener() { + int notificationId = notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, trackNotification); + logbackVerifier.expectMessage(Level.WARN,"Notification listener was the wrong type. It was not added to the notification center."); + assertEquals(notificationId, -1); + assertFalse(notificationCenter.removeNotification(notificationId)); + } + + @Test + public void testAddWrongActivateNotificationListener() { + int notificationId = notificationCenter.addNotification(NotificationCenter.NotificationType.Track, activateNotification); + logbackVerifier.expectMessage(Level.WARN,"Notification listener was the wrong type. It was not added to the notification center."); + assertEquals(notificationId, -1); + assertFalse(notificationCenter.removeNotification(notificationId)); + } +} From 7d946ad41d7fe3d1fc918804006d766c11c24fb2 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Mon, 18 Dec 2017 14:59:56 -0800 Subject: [PATCH 09/10] refactor notification listener --- .../java/com/optimizely/ab/Optimizely.java | 2 +- .../ab/notification/ActivateNotification.java | 2 +- .../ab/notification/Notification.java | 31 ------------------- .../ab/notification/NotificationCenter.java | 16 +++++----- .../ab/notification/NotificationListener.java | 11 ++++++- .../ab/notification/TrackNotification.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 2 +- .../notification/NotificationCenterTest.java | 4 +-- 8 files changed, 23 insertions(+), 47 deletions(-) delete mode 100644 core-api/src/main/java/com/optimizely/ab/notification/Notification.java diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 422345001..efb7b82d6 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -94,7 +94,7 @@ public class Optimizely { @VisibleForTesting final EventHandler eventHandler; @VisibleForTesting final ErrorHandler errorHandler; @VisibleForTesting final NotificationBroadcaster notificationBroadcaster = new NotificationBroadcaster(); - public final NotificationCenter notificationCenter = new NotificationCenter(logger); + public final NotificationCenter notificationCenter = new NotificationCenter(); @Nullable private final UserProfileService userProfileService; diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java index 73c426545..9ee96afc9 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java @@ -24,7 +24,7 @@ import java.util.Map; -public abstract class ActivateNotification implements Notification { +public abstract class ActivateNotification extends NotificationListener { /** * Base notify called with var args. This method parses the parameters and calls the abstract method. diff --git a/core-api/src/main/java/com/optimizely/ab/notification/Notification.java b/core-api/src/main/java/com/optimizely/ab/notification/Notification.java deleted file mode 100644 index 83a3f28eb..000000000 --- a/core-api/src/main/java/com/optimizely/ab/notification/Notification.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * - * Copyright 2017, Optimizely and contributors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.optimizely.ab.notification; - -/** - * Interface used to process notifications. Notifications can pass in any argument list. - * Abstract implementations take the called notify and make sure the parameters are as expected. - */ -public interface Notification { - - /** - * notify called when a notification is triggered - * @param args - variable argument list based on the type of notification. - */ - public void notify(Object ...args); -} diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index 80e2518ba..8ade62f61 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -17,10 +17,12 @@ package com.optimizely.ab.notification; import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.ArrayList; -import java.util.Map; import java.util.HashMap; +import java.util.Map; + /** * This class handles impression and conversion notifications. It replaces NotificationBroadcaster and is intended to be @@ -50,15 +52,15 @@ public Class getNotificationClass() { // the notification id is incremented and is assigned as the callback id, it can then be used to remove the notification. private int notificationID = 1; - final private Logger logger; + final private static Logger logger = LoggerFactory.getLogger(NotificationCenter.class); // notification holder holds the id as well as the notification. private static class NotificationHolder { int notificationId; - Notification notification; + NotificationListener notification; - NotificationHolder(int id, Notification notification) { + NotificationHolder(int id, NotificationListener notification) { notificationId = id; this.notification = notification; } @@ -66,10 +68,8 @@ private static class NotificationHolder /** * Instantiate a new NotificationCenter - * @param logger pass in logger to use. */ - public NotificationCenter(Logger logger) { - this.logger = logger; + public NotificationCenter() { notifications.put(NotificationType.Activate, new ArrayList()); notifications.put(NotificationType.Track, new ArrayList()); } @@ -86,7 +86,7 @@ public NotificationCenter(Logger logger) { * @param notification - Notification to add. * @return the notification id used to remove the notification. It is greater than 0 on success. */ - public int addNotification(NotificationType notificationType, Notification notification) { + public int addNotification(NotificationType notificationType, NotificationListener notification) { Class clazz = notificationType.notificationClass; if (clazz == null || !clazz.isInstance(notification)) { diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java index 7b436c5ab..012e250d7 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java @@ -34,7 +34,6 @@ * methods are added to the interface in the SDK. An abstract classes allows consumers to override * just the methods they need. */ -@Deprecated public abstract class NotificationListener { /** @@ -46,6 +45,7 @@ public abstract class NotificationListener { * @param eventValue an integer to be aggregated for the event * @param logEvent the log event sent to the event dispatcher */ + @Deprecated public void onEventTracked(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @@ -61,9 +61,18 @@ public void onEventTracked(@Nonnull String eventKey, * @param attributes a map of attributes about the user * @param variation the key of the variation that was bucketed */ + @Deprecated public void onExperimentActivated(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation) { } + + /** + * This is the new method of notification. Implementation classes such as {@link com.optimizely.ab.notification.ActivateNotification} + * will implement this call and provide another method with the correct parameters + * Notify called when a notification is triggered via the {@link com.optimizely.ab.notification.NotificationCenter} + * @param args - variable argument list based on the type of notification. + */ + public abstract void notify(Object... args); } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java index d1743e710..3ba9e3153 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotification.java @@ -24,7 +24,7 @@ /** * This class handles the track event notification. */ -public abstract class TrackNotification implements Notification { +public abstract class TrackNotification extends NotificationListener { /** * Base notify called with var args. This method parses the parameters and calls the abstract method. diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 7cf863fc0..f5f4c27cd 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2569,7 +2569,7 @@ public void clearNotificationListeners() throws Exception { /** * Verify that {@link com.optimizely.ab.notification.NotificationCenter#addNotification( * com.optimizely.ab.notification.NotificationCenter.NotificationType, - * com.optimizely.ab.notification.Notification)} properly used + * com.optimizely.ab.notification.NotificationListener)} properly used * and the listener is * added and notified when an experiment is activated. */ diff --git a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java index 29b5e8904..e23e1e695 100644 --- a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java +++ b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java @@ -16,15 +16,13 @@ public class NotificationCenterTest { private NotificationCenter notificationCenter; private ActivateNotification activateNotification; private TrackNotification trackNotification; - private Logger logger; @Rule public LogbackVerifier logbackVerifier = new LogbackVerifier(); @Before public void initialize() { - logger = LoggerFactory.getLogger("NotificationCenterTest"); - notificationCenter = new NotificationCenter(logger); + notificationCenter = new NotificationCenter(); activateNotification = mock(ActivateNotification.class); trackNotification = mock(TrackNotification.class); } From be0fb0e42769893e05f71041e4de66dd3b6307af Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Mon, 18 Dec 2017 15:22:02 -0800 Subject: [PATCH 10/10] update from notification to notificationListener where appropriate --- .../ab/notification/NotificationCenter.java | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index 8ade62f61..a538fe3fa 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -25,7 +25,7 @@ /** - * This class handles impression and conversion notifications. It replaces NotificationBroadcaster and is intended to be + * This class handles impression and conversion notificationsListeners. It replaces NotificationBroadcaster and is intended to be * more flexible. */ public class NotificationCenter { @@ -37,20 +37,20 @@ public enum NotificationType { Activate(ActivateNotification.class), // Activate was called. Track an impression event Track(TrackNotification.class); // Track was called. Track a conversion event - private Class notificationClass; + private Class notificationTypeClass; NotificationType(Class notificationClass) { - this.notificationClass = notificationClass; + this.notificationTypeClass = notificationClass; } - public Class getNotificationClass() { - return notificationClass; + public Class getNotificationTypeClass() { + return notificationTypeClass; } }; // the notification id is incremented and is assigned as the callback id, it can then be used to remove the notification. - private int notificationID = 1; + private int notificationListenerID = 1; final private static Logger logger = LoggerFactory.getLogger(NotificationCenter.class); @@ -58,11 +58,11 @@ public Class getNotificationClass() { private static class NotificationHolder { int notificationId; - NotificationListener notification; + NotificationListener notificationListener; - NotificationHolder(int id, NotificationListener notification) { + NotificationHolder(int id, NotificationListener notificationListener) { notificationId = id; - this.notification = notification; + this.notificationListener = notificationListener; } } @@ -70,39 +70,39 @@ private static class NotificationHolder * Instantiate a new NotificationCenter */ public NotificationCenter() { - notifications.put(NotificationType.Activate, new ArrayList()); - notifications.put(NotificationType.Track, new ArrayList()); + notificationsListeners.put(NotificationType.Activate, new ArrayList()); + notificationsListeners.put(NotificationType.Track, new ArrayList()); } // private list of notification by notification type. // we used a list so that notification order can mean something. - private Map> notifications =new HashMap>(); + private Map> notificationsListeners =new HashMap>(); /** * Add a notification listener to the notification center. * * @param notificationType - enum NotificationType to add. - * @param notification - Notification to add. + * @param notificationListener - Notification to add. * @return the notification id used to remove the notification. It is greater than 0 on success. */ - public int addNotification(NotificationType notificationType, NotificationListener notification) { + public int addNotification(NotificationType notificationType, NotificationListener notificationListener) { - Class clazz = notificationType.notificationClass; - if (clazz == null || !clazz.isInstance(notification)) { + Class clazz = notificationType.notificationTypeClass; + if (clazz == null || !clazz.isInstance(notificationListener)) { logger.warn("Notification listener was the wrong type. It was not added to the notification center."); return -1; } - for (NotificationHolder holder : notifications.get(notificationType)) { - if (holder.notification == notification) { + for (NotificationHolder holder : notificationsListeners.get(notificationType)) { + if (holder.notificationListener == notificationListener) { logger.warn("Notificication listener was already added"); return -1; } } - int id = notificationID++; - notifications.get(notificationType).add(new NotificationHolder(id, notification )); - logger.info("Notification listener {} was added with id {}", notification.toString(), id); + int id = notificationListenerID++; + notificationsListeners.get(notificationType).add(new NotificationHolder(id, notificationListener )); + logger.info("Notification listener {} was added with id {}", notificationListener.toString(), id); return id; } @@ -113,9 +113,9 @@ public int addNotification(NotificationType notificationType, NotificationListen */ public boolean removeNotification(int notificationID) { for (NotificationType type : NotificationType.values()) { - for (NotificationHolder holder : notifications.get(type)) { + for (NotificationHolder holder : notificationsListeners.get(type)) { if (holder.notificationId == notificationID) { - notifications.get(type).remove(holder); + notificationsListeners.get(type).remove(holder); logger.info("Notification listener removed {}", notificationID); return true; } @@ -138,18 +138,18 @@ public void clearAllNotifications() { /** * Clear notification listeners by notification type. - * @param notificationType type of notifications to remove. + * @param notificationType type of notificationsListeners to remove. */ public void clearNotifications(NotificationType notificationType) { - notifications.get(notificationType).clear(); + notificationsListeners.get(notificationType).clear(); } // fire a notificaiton of a certain type. The arg list changes depending on the type of notification sent. public void sendNotifications(NotificationType notificationType, Object ...args) { - ArrayList holders = notifications.get(notificationType); + ArrayList holders = notificationsListeners.get(notificationType); for (NotificationHolder holder : holders) { try { - holder.notification.notify(args); + holder.notificationListener.notify(args); } catch (Exception e) { logger.error("Unexpected exception calling notification listener {}", holder.notificationId, e);