Skip to content

Commit

Permalink
refactor so that any java 8 or later can use lambdas for Activate and… (
Browse files Browse the repository at this point in the history
#175)

* refactor so that any java 8 or later can use lambdas for Activate and Track listeners.

* make some updates according to mike's comments

* add some unit tests to up code coverage
  • Loading branch information
thomaszurkan-optimizely authored Mar 30, 2018
1 parent be515ba commit 00c2312
Show file tree
Hide file tree
Showing 11 changed files with 206 additions and 616 deletions.
39 changes: 0 additions & 39 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -245,12 +241,9 @@ public void track(@Nonnull String eventName,
// attributes.
Map<String, String> 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.<String, String>emptyMap();
} else {
eventValue = EventTagUtils.getRevenueValue(eventTags);
}

List<Experiment> experimentsForEvent = projectConfig.getExperimentsForEventKey(eventName);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 ========//

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, String> attributes,
@Nonnull Variation variation,
@Nonnull LogEvent event) ;

}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -78,6 +82,43 @@ public NotificationCenter() {
// we used a list so that notification order can mean something.
private Map<NotificationType, ArrayList<NotificationHolder>> notificationsListeners =new HashMap<NotificationType, ArrayList<NotificationHolder>>();

/**
* 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<String, String> 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<String, String> attributes, @Nonnull Map<String, ?> eventTags, @Nonnull LogEvent event) {
trackNotificationListenerInterface.onTrack(eventKey, userId, attributes, eventTags, event);
}
});
}
}

/**
* Add a notification listener to the notification center.
Expand All @@ -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)) {
Expand All @@ -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) {
Expand All @@ -130,17 +171,17 @@ 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);
}
}

/**
* 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();
}

Expand Down
Loading

0 comments on commit 00c2312

Please sign in to comment.