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 3907345bc..5cd4509e7 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -39,7 +39,6 @@ import com.optimizely.ab.event.internal.EventBuilder; import com.optimizely.ab.event.internal.payload.EventBatch.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; @@ -90,7 +89,6 @@ public class Optimizely { @VisibleForTesting final ProjectConfig projectConfig; @VisibleForTesting final EventHandler eventHandler; @VisibleForTesting final ErrorHandler errorHandler; - @VisibleForTesting final NotificationBroadcaster notificationBroadcaster = new NotificationBroadcaster(); public final NotificationCenter notificationCenter = new NotificationCenter(); @Nullable private final UserProfileService userProfileService; @@ -205,8 +203,6 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, logger.error("Unexpected exception in event dispatcher", e); } - notificationBroadcaster.broadcastExperimentActivated(experiment, userId, filteredAttributes, variation); - notificationCenter.sendNotifications(NotificationCenter.NotificationType.Activate, experiment, userId, filteredAttributes, variation, impressionEvent); } else { @@ -245,12 +241,9 @@ public void track(@Nonnull String eventName, // attributes. Map filteredAttributes = filterAttributes(currentConfig, attributes); - Long eventValue = null; if (eventTags == null) { logger.warn("Event tags is null when non-null was expected. Defaulting to an empty event tags map."); eventTags = Collections.emptyMap(); - } else { - eventValue = EventTagUtils.getRevenueValue(eventTags); } List experimentsForEvent = projectConfig.getExperimentsForEventKey(eventName); @@ -293,8 +286,6 @@ public void track(@Nonnull String eventName, logger.error("Unexpected exception in event dispatcher", e); } - notificationBroadcaster.broadcastEventTracked(eventName, userId, filteredAttributes, eventValue, - conversionEvent); notificationCenter.sendNotifications(NotificationCenter.NotificationType.Track, eventName, userId, filteredAttributes, eventTags, conversionEvent); } @@ -729,36 +720,6 @@ public UserProfileService getUserProfileService() { return userProfileService; } - //======== Notification listeners ========// - - /** - * Add a {@link NotificationListener} if it does not exist already. - * - * @param listener listener to add - */ - @Deprecated - public void addNotificationListener(@Nonnull NotificationListener listener) { - notificationBroadcaster.addListener(listener); - } - - /** - * Remove a {@link NotificationListener} if it exists. - * - * @param listener listener to remove - */ - @Deprecated - public void removeNotificationListener(@Nonnull NotificationListener listener) { - notificationBroadcaster.removeListener(listener); - } - - /** - * Remove all {@link NotificationListener}. - */ - @Deprecated - public void clearNotificationListeners() { - notificationBroadcaster.clearListeners(); - } - //======== Helper methods ========// /** diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java index e0843d745..883bb6963 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java @@ -25,7 +25,7 @@ import java.util.Map; -public abstract class ActivateNotificationListener extends NotificationListener { +public abstract class ActivateNotificationListener implements NotificationListener, ActivateNotificationListenerInterface { /** * 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/ActivateNotificationListenerInterface.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java new file mode 100644 index 000000000..fdae1fc5d --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java @@ -0,0 +1,25 @@ +package com.optimizely.ab.notification; + +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; + +import javax.annotation.Nonnull; +import java.util.Map; + +public interface ActivateNotificationListenerInterface { + /** + * 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 void onActivate(@Nonnull Experiment experiment, + @Nonnull String userId, + @Nonnull Map attributes, + @Nonnull Variation variation, + @Nonnull LogEvent event) ; + +} 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 deleted file mode 100644 index 3455a1bb1..000000000 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationBroadcaster.java +++ /dev/null @@ -1,117 +0,0 @@ -/** - * - * Copyright 2016-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.annotations.VisibleForTesting; -import com.optimizely.ab.config.Experiment; -import com.optimizely.ab.config.Variation; -import com.optimizely.ab.event.LogEvent; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.HashSet; -import java.util.Map; - -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; - -/** - * Manages Optimizely SDK notification listeners and broadcasts messages. - */ -@Deprecated -public class NotificationBroadcaster { - - private static final Logger logger = LoggerFactory.getLogger(NotificationBroadcaster.class); - - @VisibleForTesting final HashSet listeners = - new HashSet(); - - /** - * Add a listener if it does not exist already. - * - * @param listener listener to add - */ - public void addListener(@Nonnull NotificationListener listener) { - if (listeners.contains(listener)) { - logger.debug("Notification listener was not added because it already existed"); - return; - } - - listeners.add(listener); - logger.debug("Notification listener was added"); - } - - /** - * Remove a listener if it exists. - * - * @param listener listener to remove - */ - public void removeListener(@Nonnull NotificationListener listener) { - if (listeners.contains(listener)) { - listeners.remove(listener); - logger.debug("Notification listener was removed"); - return; - } - - logger.debug("Notification listener was not removed because it did not exist"); - } - - /** - * Remove all listeners. - */ - public void clearListeners() { - listeners.clear(); - logger.debug("Notification listeners were cleared"); - } - - /** - * Notify listeners that an Optimizely event has been tracked. - * - * @param eventKey the key of the tracked event - * @param userId the ID of the user - * @param attributes a map of attributes about the event - * @param eventValue an integer to be aggregated for the event - * @param logEvent the log event sent to the event dispatcher - */ - public void broadcastEventTracked(@Nonnull String eventKey, - @Nonnull String userId, - @Nonnull Map attributes, - @CheckForNull Long eventValue, - @Nonnull LogEvent logEvent) { - for (final NotificationListener iterListener : listeners) { - iterListener.onEventTracked(eventKey, userId, attributes, eventValue, logEvent); - } - } - - /** - * Notify listeners that an Optimizely experiment has been activated. - * - * @param experiment the key of the activated experiment - * @param userId the id of the user - * @param attributes a map of attributes about the user - * @param variation the key of the variation that was bucketed - */ - public void broadcastExperimentActivated(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull Map attributes, - @Nonnull Variation variation) { - for (final NotificationListener iterListener : listeners) { - iterListener.onExperimentActivated(experiment, userId, attributes, 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 7cd270925..9b68c75ba 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,6 +1,6 @@ /** * - * Copyright 2017, Optimizely and contributors + * Copyright 2017-2018, 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. @@ -16,9 +16,13 @@ */ package com.optimizely.ab.notification; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nonnull; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; @@ -78,6 +82,43 @@ public NotificationCenter() { // we used a list so that notification order can mean something. private Map> notificationsListeners =new HashMap>(); + /** + * Convenience method to support lambdas as callbacks in later version of Java (8+). + * @param activateNotificationListenerInterface + * @return greater than zero if added. + */ + public int addActivateNotificationListener(final ActivateNotificationListenerInterface activateNotificationListenerInterface) { + if (activateNotificationListenerInterface instanceof ActivateNotificationListener) { + return addNotificationListener(NotificationType.Activate, (NotificationListener)activateNotificationListenerInterface); + } + else { + return addNotificationListener(NotificationType.Activate, new ActivateNotificationListener() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + activateNotificationListenerInterface.onActivate(experiment, userId, attributes, variation, event); + } + }); + } + } + + /** + * Convenience method to support lambdas as callbacks in later versions of Java (8+) + * @param trackNotificationListenerInterface + * @return greater than zero if added. + */ + public int addTrackNotificationListener(final TrackNotificationListenerInterface trackNotificationListenerInterface) { + if (trackNotificationListenerInterface instanceof TrackNotificationListener) { + return addNotificationListener(NotificationType.Activate, (NotificationListener)trackNotificationListenerInterface); + } + else { + return addNotificationListener(NotificationType.Track, new TrackNotificationListener() { + @Override + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + trackNotificationListenerInterface.onTrack(eventKey, userId, attributes, eventTags, event); + } + }); + } + } /** * Add a notification listener to the notification center. @@ -86,7 +127,7 @@ public NotificationCenter() { * @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 notificationListener) { + public int addNotificationListener(NotificationType notificationType, NotificationListener notificationListener) { Class clazz = notificationType.notificationTypeClass; if (clazz == null || !clazz.isInstance(notificationListener)) { @@ -107,11 +148,11 @@ public int addNotification(NotificationType notificationType, NotificationListen } /** - * Remove the notification listener based on the notificationId passed back from addNotification. + * Remove the notification listener based on the notificationId passed back from addNotificationListener. * @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) { + public boolean removeNotificationListener(int notificationID) { for (NotificationType type : NotificationType.values()) { for (NotificationHolder holder : notificationsListeners.get(type)) { if (holder.notificationId == notificationID) { @@ -130,9 +171,9 @@ public boolean removeNotification(int notificationID) { /** * Clear out all the notification listeners. */ - public void clearAllNotifications() { + public void clearAllNotificationListeners() { for (NotificationType type : NotificationType.values()) { - clearNotifications(type); + clearNotificationListeners(type); } } @@ -140,7 +181,7 @@ public void clearAllNotifications() { * Clear notification listeners by notification type. * @param notificationType type of notificationsListeners to remove. */ - public void clearNotifications(NotificationType notificationType) { + public void clearNotificationListeners(NotificationType notificationType) { notificationsListeners.get(notificationType).clear(); } 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 3072f3921..687870979 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 @@ -26,53 +26,17 @@ import javax.annotation.Nonnull; /** - * Abstract class for Optimizely notification listeners. + * An interface class for Optimizely notification listeners. *

- * We use an abstract class here instead of an interface for convenience of use and backwards - * compatibility in the future. An interface would force consumers to implement every method defined - * on it as well as update their application code with new method implementations every time new - * methods are added to the interface in the SDK. An abstract classes allows consumers to override - * just the methods they need. + * We changed this from a abstract class to a interface to support lambdas moving forward in Java 8 and beyond. */ -public abstract class NotificationListener { +public interface NotificationListener { /** - * Listener that is called after an event is tracked. - * - * @param eventKey the key of the tracked event - * @param userId the ID of the user - * @param attributes a map of attributes about the event - * @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, - @CheckForNull Long eventValue, - @Nonnull LogEvent logEvent) { - } - - /** - * Listener that is called after an experiment has been activated. - * - * @param experiment the activated experiment - * @param userId the id of the user - * @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 ActivateNotificationListener} + * This is the base method of notification. Implementation classes such as {@link ActivateNotificationListener} * 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); + public void notify(Object... args); } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java index bdf3c19fd..16809f716 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java @@ -24,8 +24,7 @@ /** * This class handles the track event notification. */ -public abstract class TrackNotificationListener extends NotificationListener { - +public abstract class TrackNotificationListener implements NotificationListener, TrackNotificationListenerInterface { /** * 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. diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java new file mode 100644 index 000000000..74df611dd --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java @@ -0,0 +1,23 @@ +package com.optimizely.ab.notification; + +import com.optimizely.ab.event.LogEvent; + +import javax.annotation.Nonnull; +import java.util.Map; + +public interface TrackNotificationListenerInterface { + /** + * 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 void onTrack(@Nonnull String eventKey, + @Nonnull String userId, + @Nonnull Map attributes, + @Nonnull Map eventTags, + @Nonnull LogEvent event) ; + +} 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 956d0c454..f1b2bb2ff 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2232,7 +2232,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ }; - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + int notificationId = optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, activateNotification); Map testParams = new HashMap(); testParams.put("test", "params"); @@ -2248,7 +2248,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ // activate the experiment Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, testUserAttributes); - assertTrue(optimizely.notificationCenter.removeNotification(notificationId)); + assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); // verify that the bucketing algorithm was called correctly verify(mockBucketer).bucket(activatedExperiment, testBucketingId); assertThat(actualVariation, is(bucketedVariation)); @@ -2298,13 +2298,13 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ }; - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); + int notificationId = optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, activateNotification); // activate the experiment Map attributes = null; Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, attributes); - optimizely.notificationCenter.removeNotification(notificationId); + optimizely.notificationCenter.removeNotificationListener(notificationId); logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map."); @@ -2325,248 +2325,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ } /** - * 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); - - when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, bucketedVariation, genericUserId, - attributes)) - .thenReturn(logEventToDispatch); - - // Add and remove listener - NotificationListener listener = mock(NotificationListener.class); - optimizely.addNotificationListener(listener); - optimizely.removeNotificationListener(listener); - - // Check if listener is notified after an experiment is activated - Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); - verify(listener, never()) - .onExperimentActivated(activatedExperiment, genericUserId, attributes, actualVariation); - - // Check if listener is notified after a live variable is accessed - boolean activateExperiment = true; - verify(listener, never()) - .onExperimentActivated(activatedExperiment, genericUserId, attributes, actualVariation); - - // 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(listener, never()) - .onEventTracked(eventKey, genericUserId, attributes, null, 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 clearNotificationListeners() 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); - - NotificationListener listener = mock(NotificationListener.class); - optimizely.addNotificationListener(listener); - optimizely.clearNotificationListeners(); - - // 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(listener, never()) - .onExperimentActivated(activatedExperiment, genericUserId, attributes, actualVariation); - - // Check if listener is notified after a live variable is accessed - boolean activateExperiment = true; - verify(listener, never()) - .onExperimentActivated(activatedExperiment, genericUserId, attributes, actualVariation); - - // 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(listener, never()) - .onEventTracked(eventKey, genericUserId, attributes, null, logEventToDispatch); - } - - /** - * Verify that {@link com.optimizely.ab.notification.NotificationCenter#addNotification( + * Verify that {@link com.optimizely.ab.notification.NotificationCenter#addNotificationListener( * com.optimizely.ab.notification.NotificationCenter.NotificationType, * com.optimizely.ab.notification.NotificationListener)} properly used * and the listener is @@ -2611,7 +2370,7 @@ public void addNotificationListenerFromNotificationCenter() throws Exception { // Add listener ActivateNotificationListener listener = mock(ActivateNotificationListener.class); - optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, listener); + optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, listener); // Check if listener is notified when experiment is activated Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); @@ -2640,7 +2399,7 @@ public void addNotificationListenerFromNotificationCenter() throws Exception { TrackNotificationListener trackNotification = mock(TrackNotificationListener.class); - optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Track, trackNotification); optimizely.track(eventKey, genericUserId, attributes); verify(trackNotification, times(1)) @@ -2683,12 +2442,12 @@ public void removeNotificationListenerNotificationCenter() throws Exception { // Add and remove listener ActivateNotificationListener activateNotification = mock(ActivateNotificationListener.class); - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); - assertTrue(optimizely.notificationCenter.removeNotification(notificationId)); + int notificationId = optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, activateNotification); + assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); TrackNotificationListener trackNotification = mock(TrackNotificationListener.class); - notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); - assertTrue(optimizely.notificationCenter.removeNotification(notificationId)); + notificationId = optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Track, trackNotification); + assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); // Check if listener is notified after an experiment is activated Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); @@ -2776,10 +2535,10 @@ public void clearNotificationListenersNotificationCenter() throws Exception { ActivateNotificationListener activateNotification = mock(ActivateNotificationListener.class); TrackNotificationListener trackNotification = mock(TrackNotificationListener.class); - optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, activateNotification); - optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, activateNotification); + optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Track, trackNotification); - optimizely.notificationCenter.clearAllNotifications(); + optimizely.notificationCenter.clearAllNotificationListeners(); // Check if listener is notified after an experiment is activated Variation actualVariation = optimizely.activate(activatedExperiment, genericUserId, attributes); @@ -2882,12 +2641,12 @@ public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull M } }; - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + int notificationId = optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Track, trackNotification); // call track optimizely.track(eventType.getKey(), genericUserId, attributes); - optimizely.notificationCenter.removeNotification(notificationId); + optimizely.notificationCenter.removeNotificationListener(notificationId); // setup the attribute map captor (so we can verify its content) ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); @@ -2968,13 +2727,13 @@ public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull M } }; - int notificationId = optimizely.notificationCenter.addNotification(NotificationCenter.NotificationType.Track, trackNotification); + int notificationId = optimizely.notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Track, trackNotification); // call track Map attributes = null; optimizely.track(eventType.getKey(), genericUserId, attributes); - optimizely.notificationCenter.removeNotification(notificationId); + optimizely.notificationCenter.removeNotificationListener(notificationId); logbackVerifier.expectMessage(Level.WARN, "Attributes is null when non-null was expected. Defaulting to an empty attributes map."); diff --git a/core-api/src/test/java/com/optimizely/ab/notification/NotificationBroadcasterTest.java b/core-api/src/test/java/com/optimizely/ab/notification/NotificationBroadcasterTest.java deleted file mode 100644 index d754c4233..000000000 --- a/core-api/src/test/java/com/optimizely/ab/notification/NotificationBroadcasterTest.java +++ /dev/null @@ -1,146 +0,0 @@ -/** - * - * Copyright 2016-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; -import com.optimizely.ab.config.Variation; -import com.optimizely.ab.event.LogEvent; - -import org.junit.Before; -import org.junit.Test; - -import java.util.Collections; -import java.util.Map; - -import static junit.framework.Assert.assertEquals; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; - -/** - * Tests for {@link NotificationBroadcaster}. - */ -public class NotificationBroadcasterTest { - - private NotificationBroadcaster notificationBroadcaster; - private NotificationListener listener; - private NotificationListener listener2; - - @Before - public void initialize() { - notificationBroadcaster = new NotificationBroadcaster(); - listener = mock(NotificationListener.class); - listener2 = mock(NotificationListener.class); - } - - /** - * Verify that {@link NotificationBroadcaster#addListener(NotificationListener)} correctly adds - * a new listener. - */ - @Test - public void addListener() throws Exception { - notificationBroadcaster.addListener(listener); - assertEquals("addListener did not add the listener", - 1, notificationBroadcaster.listeners.size()); - } - - /** - * Verify that {@link NotificationBroadcaster#addListener(NotificationListener)} does not add a - * listener that has already been added. - */ - @Test - public void addListenerAlreadyExists() throws Exception { - notificationBroadcaster.addListener(listener); - notificationBroadcaster.addListener(listener); - assertEquals("addListener did not add the listener just once", - 1, notificationBroadcaster.listeners.size()); - } - - /** - * Verify that {@link NotificationBroadcaster#removeListener(NotificationListener)} removes a - * listener that has been added. - */ - @Test - public void removeListener() throws Exception { - notificationBroadcaster.addListener(listener); - notificationBroadcaster.removeListener(listener); - assertEquals("removeListener did not remove the listener", - 0, notificationBroadcaster.listeners.size()); - } - - /** - * Verify that {@link NotificationBroadcaster#removeListener(NotificationListener)} does not - * remove a listener that was not added. - */ - @Test - public void removeListenerNotAdded() throws Exception { - notificationBroadcaster.addListener(listener); - notificationBroadcaster.removeListener(listener2); - assertEquals("removeListener did not remove just the listener that was added", - 1, notificationBroadcaster.listeners.size()); - } - - /** - * Verify that {@link NotificationBroadcaster#clearListeners()} removes all listeners. - */ - @Test - public void clearListeners() throws Exception { - notificationBroadcaster.addListener(listener); - notificationBroadcaster.addListener(listener2); - notificationBroadcaster.clearListeners(); - assertEquals("clearListeners did not remove all listeners that were added", - 0, notificationBroadcaster.listeners.size()); - } - - /** - * Verify that {@link NotificationBroadcaster#broadcastEventTracked(String, String, Map, Long, LogEvent)} - * notifies all listeners. - */ - @Test - public void broadcastEventTracked() throws Exception { - notificationBroadcaster.addListener(listener); - notificationBroadcaster.addListener(listener2); - - String eventKey = "event1"; - String userId = "user1"; - Map attributes = Collections.emptyMap(); - Long eventValue = 0L; - LogEvent logEvent = mock(LogEvent.class); - notificationBroadcaster.broadcastEventTracked( - eventKey, userId, attributes, eventValue, logEvent); - verify(listener).onEventTracked(eventKey, userId, attributes, eventValue, logEvent); - verify(listener2).onEventTracked(eventKey, userId, attributes, eventValue, logEvent); - } - - /** - * Verify that {@link NotificationBroadcaster#broadcastExperimentActivated(Experiment, String, Map, Variation)} - * notifies all listeners. - */ - @Test - public void broadcastExperimentActivated() throws Exception { - notificationBroadcaster.addListener(listener); - notificationBroadcaster.addListener(listener2); - - Experiment experiment = mock(Experiment.class); - String userId = "user1"; - Map attributes = Collections.emptyMap(); - Variation variation = mock(Variation.class); - notificationBroadcaster.broadcastExperimentActivated( - experiment, userId, attributes, variation); - verify(listener).onExperimentActivated(experiment, userId, attributes, variation); - verify(listener2).onExperimentActivated(experiment, userId, attributes, variation); - } -} 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 f6a4eb1cd..08958be64 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 @@ -1,6 +1,9 @@ package com.optimizely.ab.notification; import ch.qos.logback.classic.Level; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; import com.optimizely.ab.internal.LogbackVerifier; import org.junit.Before; import org.junit.Rule; @@ -9,6 +12,8 @@ import javax.annotation.Nonnull; import java.util.Map; +import static junit.framework.TestCase.assertNotSame; +import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; @@ -30,18 +35,94 @@ public void initialize() { @Test public void testAddWrongTrackNotificationListener() { - int notificationId = notificationCenter.addNotification(NotificationCenter.NotificationType.Activate, trackNotification); + int notificationId = notificationCenter.addNotificationListener(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)); + assertFalse(notificationCenter.removeNotificationListener(notificationId)); } @Test public void testAddWrongActivateNotificationListener() { - int notificationId = notificationCenter.addNotification(NotificationCenter.NotificationType.Track, activateNotification); + int notificationId = notificationCenter.addNotificationListener(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)); + assertFalse(notificationCenter.removeNotificationListener(notificationId)); } + + @Test + public void testAddActivateNotificationTwice() { + ActivateNotificationListener listener = new ActivateNotificationListener() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + + } + }; + int notificationId = notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, listener); + int notificationId2 = notificationCenter.addNotificationListener(NotificationCenter.NotificationType.Activate, listener); + logbackVerifier.expectMessage(Level.WARN,"Notificication listener was already added"); + assertEquals(notificationId2, -1); + assertTrue(notificationCenter.removeNotificationListener(notificationId)); + notificationCenter.clearAllNotificationListeners(); + } + + @Test + public void testAddActivateNotification() { + int notificationId = notificationCenter.addActivateNotificationListener(new ActivateNotificationListenerInterface() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + + } + }); + assertNotSame(notificationId, -1); + assertTrue(notificationCenter.removeNotificationListener(notificationId)); + notificationCenter.clearAllNotificationListeners(); + } + + @Test + public void testAddTrackNotification() { + int notificationId = notificationCenter.addTrackNotificationListener(new TrackNotificationListenerInterface() { + @Override + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + + } + }); + assertNotSame(notificationId, -1); + assertTrue(notificationCenter.removeNotificationListener(notificationId)); + notificationCenter.clearAllNotificationListeners(); + } + + @Test + public void testNotificationTypeClasses() { + assertEquals(NotificationCenter.NotificationType.Activate.getNotificationTypeClass(), + ActivateNotificationListener.class); + assertEquals(NotificationCenter.NotificationType.Track.getNotificationTypeClass(), TrackNotificationListener.class); + } + + @Test + public void testAddTrackNotificationInterface() { + int notificationId = notificationCenter.addTrackNotificationListener(new TrackNotificationListenerInterface() { + @Override + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + + } + }); + assertNotSame(notificationId, -1); + assertTrue(notificationCenter.removeNotificationListener(notificationId)); + notificationCenter.clearAllNotificationListeners(); + } + + @Test + public void testAddActivateNotificationInterface() { + int notificationId = notificationCenter.addActivateNotificationListener(new ActivateNotificationListenerInterface() { + @Override + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + + } + }); + assertNotSame(notificationId, -1); + assertTrue(notificationCenter.removeNotificationListener(notificationId)); + notificationCenter.clearAllNotificationListeners(); + } + }