Adds persisten buckter interface and hooks it up to buckter via composition

Author: Josh Deffibaugh

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -18,6 +18,7 @@
 
 import com.optimizely.ab.annotations.VisibleForTesting;
 import com.optimizely.ab.bucketing.Bucketer;
+import com.optimizely.ab.bucketing.PersistentBucketer;
 import com.optimizely.ab.config.Attribute;
 import com.optimizely.ab.config.EventType;
 import com.optimizely.ab.config.Experiment;
@@ -419,6 +420,7 @@ public static class Builder {
 
         private String datafile;
         private Bucketer bucketer;
+        private PersistentBucketer persistentBucketer;
         private ErrorHandler errorHandler;
         private EventHandler eventHandler;
         private EventBuilder eventBuilder;
@@ -435,6 +437,11 @@ public Builder withErrorHandler(ErrorHandler errorHandler) {
             return this;
         }
 
+        public Builder withPersistentBucketer(PersistentBucketer persistentBucketer) {
+            this.persistentBucketer = persistentBucketer;
+            return this;
+        }
+
         protected Builder withBucketing(Bucketer bucketer) {
             this.bucketer = bucketer;
             return this;
@@ -460,7 +467,7 @@ public Optimizely build() {
 
             // use the default bucketer and event builder, if no overrides were provided
             if (bucketer == null) {
-                bucketer = new Bucketer(projectConfig);
+                bucketer = new Bucketer(projectConfig, persistentBucketer);
             }
 
             if (eventBuilder == null) {

core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java

@@ -47,6 +47,8 @@ public class Bucketer {
 
     private final ProjectConfig projectConfig;
 
+    @Nullable private final PersistentBucketer persistentBucketer;
+
     private static final Logger logger = LoggerFactory.getLogger(Bucketer.class);
 
     private static final int MURMUR_HASH_SEED = 1;
@@ -58,7 +60,12 @@ public class Bucketer {
     static final int MAX_TRAFFIC_VALUE = 10000;
 
     public Bucketer(ProjectConfig projectConfig) {
+        this(projectConfig, null);
+    }
+
+    public Bucketer(ProjectConfig projectConfig, @Nullable PersistentBucketer persistentBucketer) {
         this.projectConfig = projectConfig;
+        this.persistentBucketer = persistentBucketer;
     }
 
     private String bucketToEntity(int bucketValue, List<TrafficAllocation> trafficAllocations) {
@@ -102,6 +109,19 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
                                         @Nonnull String userId) {
         // "salt" the bucket id using the experiment id
         String combinedBucketId = userId + experiment.getId();
+
+        // If a persistent bucketer instance is present then check it for a persisted variation
+        if (persistentBucketer != null) {
+            Variation variation = persistentBucketer.restoreActivation(projectConfig, userId, experiment);
+            if (variation != null) {
+                logger.info("Returning previously activated variation \"{}\" from persistent bucketer.", variation.getKey());
+                // A variation is stored for this combined bucket id
+                return variation;
+            } else {
+                logger.info("No previously activated variation returned from persistent bucketer.");
+            }
+        }
+
         String experimentKey = experiment.getKey();
 
         List<TrafficAllocation> trafficAllocations = experiment.getTrafficAllocation();
@@ -115,6 +135,17 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
             Variation bucketedVariation = experiment.getVariationIdToVariationMap().get(bucketedVariationId);
             logger.info("User \"{}\" is in variation \"{}\" of experiment \"{}\".", userId, bucketedVariation.getKey(),
                         experimentKey);
+
+            // If a persistent bucketer instance is present give it a variation to store
+            if (persistentBucketer != null) {
+                boolean saved = persistentBucketer.saveActivation(projectConfig, userId, experiment, bucketedVariation);
+                if (saved) {
+                    logger.info("Persisted variation \"{}\" of experiment \"{}\".", bucketedVariation.getKey(), experimentKey);
+                } else {
+                    logger.warn("Failed to persist variation \"{}\" of experiment \"{}\".", bucketedVariation.getKey(), experimentKey);
+                }
+            }
+
             return bucketedVariation;
         }
 
@@ -180,4 +211,9 @@ int generateBucketValue(int hashCode) {
         double ratio = (double)(hashCode & 0xFFFFFFFFL) / Math.pow(2, 32);
         return (int)Math.floor(MAX_TRAFFIC_VALUE * ratio);
     }
+
+    @Nullable
+    public PersistentBucketer getPersistentBucketer() {
+        return persistentBucketer;
+    }
 }

core-api/src/main/java/com/optimizely/ab/bucketing/PersistentBucketer.java

@@ -0,0 +1,55 @@
+package com.optimizely.ab.bucketing;
+
+import com.optimizely.ab.Optimizely;
+import com.optimizely.ab.config.Experiment;
+import com.optimizely.ab.config.ProjectConfig;
+import com.optimizely.ab.config.Variation;
+
+/**
+ * Interface for persisting variation activations for an experiment
+ *
+ * Persistent bucketing is useful when the variation for a user should not ever change, even if
+ * traffic allocation changes.
+ *
+ * Create an implementation of this interface and pass it to
+ * {@link com.optimizely.ab.Optimizely.Builder#withPersistentBucketer(PersistentBucketer)} to inject
+ * it into the returned {@link Optimizely} instance.  The default Optimizely bucketer will
+ * check the provided persistent bucketer for a persisted activation while bucketing.  If the a variation
+ * is returned that variation is used.  Return null for {@link this#restoreActivation(ProjectConfig, String, Experiment)}
+ * to tell the bucketer to bucket normally.
+ *
+ * Likewise, when the initial bucketing occurs the default Optimizely {@link Bucketer} will provide
+ * the {@link ProjectConfig projectConfig}, userId, {@link Experiment}, and
+ * {@link Variation} to {@link this#saveActivation(ProjectConfig, String, Experiment, Variation)}.
+ * These parameters should provide enough data to persist the activation and restore an instance of
+ * {@link Variation}
+ *
+ * If providing a custom {@link Bucketer} be sure to make it's bucketing methods respect the presence
+ * of a {@link PersistentBucketer} if desired.  {@link PersistentBucketer} instances should be composed
+ * inside of {@link Bucketer} instances.
+ *
+ *
+ */
+public interface PersistentBucketer {
+
+    /**
+     * Called after bucketing occurs, providing a chance to persist data about the activation
+     * @param projectConfig an instance of {@link ProjectConfig}
+     * @param userId a String representing a user ID
+     * @param experiment an {@link Experiment} instance
+     * @param variation a {@link Variation} id
+     * @return true if saved otherwise false
+     */
+    boolean saveActivation(ProjectConfig projectConfig, String userId, Experiment experiment, Variation variation);
+
+    /**
+     * Called before bucketing occurs, providing a chance to restore a previously bucketed {@link Variation}
+     * @param projectConfig an  instance of {@link ProjectConfig}
+     * @param userId a String representing a user ID
+     * @param experiment an {@link Experiment} instance
+     * @return an instance of {@link Variation}
+     *          The {@link ProjectConfig} instance can be used to look up the {@link Variation} instance
+     *          for the project via the userID string and the {@link Experiment} instance which has the experiment ID.
+     */
+    Variation restoreActivation(ProjectConfig projectConfig, String userId, Experiment experiment);
+}

core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java

@@ -16,6 +16,7 @@
  */
 package com.optimizely.ab;
 
+import com.optimizely.ab.bucketing.PersistentBucketer;
 import com.optimizely.ab.config.ProjectConfigTestUtils;
 import com.optimizely.ab.error.ErrorHandler;
 import com.optimizely.ab.error.NoOpErrorHandler;
@@ -35,6 +36,7 @@
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
 
 /**
  * Tests for {@link Optimizely#builder(String, EventHandler)}.
@@ -84,4 +86,14 @@ public void withDefaultErrorHandler() throws Exception {
 
         assertThat(optimizelyClient.errorHandler, instanceOf(NoOpErrorHandler.class));
     }
+
+    @Test
+    public void withPersistentBucketer() throws Exception {
+        PersistentBucketer persistentBucketer = mock(PersistentBucketer.class);
+        Optimizely optimizelyClient = Optimizely.builder(validConfigJson(), mockEventHandler)
+                .withPersistentBucketer(persistentBucketer)
+                .build();
+
+        assertThat(optimizelyClient.bucketer.getPersistentBucketer(), is(persistentBucketer));
+    }
 }

core-api/src/test/java/com/optimizely/ab/bucketing/BucketerTest.java

@@ -43,6 +43,9 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 /**
  * Tests for {@link Bucketer}.
@@ -440,6 +443,90 @@ public void bucketUserNotInOverlappingGroupExperiment() throws Exception {
         assertNull(algorithm.bucket(groupExperiment, "blah"));
     }
 
+    @Test public void bucketUserSaveActivationWithPersistentBucketer() throws Exception {
+        final AtomicInteger bucketValue = new AtomicInteger();
+        PersistentBucketer persistentBucketer = mock(PersistentBucketer.class);
+        Bucketer algorithm = mockPersistentBucketAlgorith(bucketValue, persistentBucketer);
+        bucketValue.set(3000);
+
+        ProjectConfig projectConfig = validProjectConfig();
+        List<Experiment> groupExperiments = projectConfig.getGroups().get(0).getExperiments();
+        Experiment groupExperiment = groupExperiments.get(0);
+        final Variation variation = groupExperiment.getVariations().get(0);
+
+        when(persistentBucketer.saveActivation(projectConfig, "blah", groupExperiment, variation)).thenReturn(true);
+
+        assertThat(algorithm.bucket(groupExperiment, "blah"),  is(variation));
+
+        logbackVerifier.expectMessage(Level.INFO,
+                "Persisted variation \"e2_vtag1\" of experiment \"group_etag2\".");
+
+        verify(persistentBucketer).saveActivation(projectConfig, "blah", groupExperiment, variation);
+    }
+
+    @Test public void bucketUserSaveActivationFailWithPersistentBucketer() throws Exception {
+        final AtomicInteger bucketValue = new AtomicInteger();
+        PersistentBucketer persistentBucketer = mock(PersistentBucketer.class);
+        Bucketer algorithm = mockPersistentBucketAlgorith(bucketValue, persistentBucketer);
+        bucketValue.set(3000);
+
+        ProjectConfig projectConfig = validProjectConfig();
+        List<Experiment> groupExperiments = projectConfig.getGroups().get(0).getExperiments();
+        Experiment groupExperiment = groupExperiments.get(0);
+        final Variation variation = groupExperiment.getVariations().get(0);
+
+        when(persistentBucketer.saveActivation(projectConfig, "blah", groupExperiment, variation)).thenReturn(false);
+
+        assertThat(algorithm.bucket(groupExperiment, "blah"),  is(variation));
+
+        logbackVerifier.expectMessage(Level.WARN,
+                "Failed to persist variation \"e2_vtag1\" of experiment \"group_etag2\".");
+
+        verify(persistentBucketer).saveActivation(projectConfig, "blah", groupExperiment, variation);
+    }
+
+    @Test public void bucketUserRestoreActivationWithPersistentBucketer() throws Exception {
+        final AtomicInteger bucketValue = new AtomicInteger();
+        PersistentBucketer persistentBucketer = mock(PersistentBucketer.class);
+        Bucketer algorithm = mockPersistentBucketAlgorith(bucketValue, persistentBucketer);
+        bucketValue.set(3000);
+
+        ProjectConfig projectConfig = validProjectConfig();
+        List<Experiment> groupExperiments = projectConfig.getGroups().get(0).getExperiments();
+        Experiment groupExperiment = groupExperiments.get(0);
+        final Variation variation = groupExperiment.getVariations().get(0);
+
+        when(persistentBucketer.restoreActivation(projectConfig, "blah", groupExperiment)).thenReturn(variation);
+
+        assertThat(algorithm.bucket(groupExperiment, "blah"),  is(variation));
+
+        logbackVerifier.expectMessage(Level.INFO,
+                "Returning previously activated variation \"e2_vtag1\" from persistent bucketer.");
+
+        verify(persistentBucketer).restoreActivation(projectConfig, "blah", groupExperiment);
+    }
+
+    @Test public void bucketUserRestoreActivationNullWithPersistentBucketer() throws Exception {
+        final AtomicInteger bucketValue = new AtomicInteger();
+        PersistentBucketer persistentBucketer = mock(PersistentBucketer.class);
+        Bucketer algorithm = mockPersistentBucketAlgorith(bucketValue, persistentBucketer);
+        bucketValue.set(3000);
+
+        ProjectConfig projectConfig = validProjectConfig();
+        List<Experiment> groupExperiments = projectConfig.getGroups().get(0).getExperiments();
+        Experiment groupExperiment = groupExperiments.get(0);
+        final Variation variation = groupExperiment.getVariations().get(0);
+
+        when(persistentBucketer.restoreActivation(projectConfig, "blah", groupExperiment)).thenReturn(null);
+
+        assertThat(algorithm.bucket(groupExperiment, "blah"),  is(variation));
+
+        logbackVerifier.expectMessage(Level.INFO,
+                "No previously activated variation returned from persistent bucketer.");
+
+        verify(persistentBucketer).restoreActivation(projectConfig, "blah", groupExperiment);
+    }
+
     //======== Helper methods ========//
 
     /**
@@ -455,4 +542,21 @@ int generateBucketValue(int hashCode) {
             }
         };
     }
+
+    /**
+     * Sets up a mock algorithm that returns an expected bucket value.
+     *
+     * Includes a composed {@link PersistentBucketer} mock instance
+     *
+     * @param bucketValue the expected bucket value holder
+     * @return the mock bucket algorithm
+     */
+    private Bucketer mockPersistentBucketAlgorith(final AtomicInteger bucketValue, final PersistentBucketer persistentBucketer) {
+        return new Bucketer(validProjectConfig(), persistentBucketer) {
+            @Override
+            int generateBucketValue(int hashCode) {
+                return bucketValue.get();
+            }
+        };
+    }
 }