From 759ce1c1b4d3188f4d483922614194271d1f6a5c Mon Sep 17 00:00:00 2001 From: Vignesh Raja Date: Tue, 24 Jan 2017 16:10:09 -0800 Subject: [PATCH 1/3] Add support for 'Launched' experiment status --- .../java/com/optimizely/ab/Optimizely.java | 27 ++++++---- .../com/optimizely/ab/config/Experiment.java | 27 ++++++++-- .../ab/event/internal/EventBuilderV2.java | 14 +++-- .../com/optimizely/ab/OptimizelyTestV2.java | 51 +++++++++++++++++++ .../ab/config/ProjectConfigTestUtils.java | 11 +++- .../ab/event/internal/EventBuilderV2Test.java | 34 +++++++++++++ .../config/no-audience-project-config-v2.json | 29 +++++++++++ 7 files changed, 173 insertions(+), 20 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 3f85edbff..a887ae088 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -172,18 +172,23 @@ private Optimizely(@Nonnull ProjectConfig projectConfig, return null; } - LogEvent impressionEvent = - eventBuilder.createImpressionEvent(projectConfig, experiment, variation, userId, attributes); - logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); - logger.debug("Dispatching impression event to URL {} with params {} and payload \"{}\".", - impressionEvent.getEndpointUrl(), impressionEvent.getRequestParams(), impressionEvent.getBody()); - try { - eventHandler.dispatchEvent(impressionEvent); - } catch (Exception e) { - logger.error("Unexpected exception in event dispatcher", e); - } + if (!experiment.isLaunched()) { + LogEvent impressionEvent = + eventBuilder.createImpressionEvent(projectConfig, experiment, variation, userId, attributes); + logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); + logger.debug( + "Dispatching impression event to URL {} with params {} and payload \"{}\".", + impressionEvent.getEndpointUrl(), impressionEvent.getRequestParams(), impressionEvent.getBody()); + try { + eventHandler.dispatchEvent(impressionEvent); + } catch (Exception e) { + logger.error("Unexpected exception in event dispatcher", e); + } - notificationBroadcaster.broadcastExperimentActivated(experiment, userId, attributes, variation); + notificationBroadcaster.broadcastExperimentActivated(experiment, userId, attributes, variation); + } else { + logger.info("Experiment has \"Launched\" status so not dispatching event during activation."); + } return variation; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java index f7bb62521..0bbbd9f03 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java +++ b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java @@ -51,9 +51,23 @@ public class Experiment implements IdKeyMapped { private final Map variationIdToVariationMap; private final Map userIdToVariationKeyMap; - // constant storing the status of a running experiment. Other possible statuses for an experiment - // include 'Not started', 'Paused', and 'Archived' - private static final String STATUS_RUNNING = "Running"; + public enum ExperimentStatus { + RUNNING ("Running"), + LAUNCHED ("Launched"), + PAUSED ("Paused"), + NOT_STARTED ("Not started"), + ARCHIVED ("Archived"); + + private final String experimentStatus; + + ExperimentStatus(String experimentStatus) { + this.experimentStatus = experimentStatus; + } + + public String toString() { + return experimentStatus; + } + } @JsonCreator public Experiment(@JsonProperty("id") String id, @@ -134,7 +148,12 @@ public String getGroupId() { } public boolean isRunning() { - return status.equals(STATUS_RUNNING); + return status.equals(ExperimentStatus.RUNNING.toString()) || + status.equals(ExperimentStatus.LAUNCHED.toString()); + } + + public boolean isLaunched() { + return status.equals(ExperimentStatus.LAUNCHED.toString()); } @Override diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java index 72d46425f..73139056d 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java @@ -199,10 +199,16 @@ private List createLayerStates(ProjectConfig projectConfig, Bucketer for (Experiment experiment : allExperiments) { if (experimentIds.contains(experiment.getId()) && ProjectValidationUtils.validatePreconditions(projectConfig, experiment, userId, attributes)) { - Variation bucketedVariation = bucketer.bucket(experiment, userId); - if (bucketedVariation != null) { - Decision decision = new Decision(bucketedVariation.getId(), false, experiment.getId()); - layerStates.add(new LayerState(experiment.getLayerId(), decision, true)); + if (!experiment.isLaunched()) { + Variation bucketedVariation = bucketer.bucket(experiment, userId); + if (bucketedVariation != null) { + Decision decision = new Decision(bucketedVariation.getId(), false, experiment.getId()); + layerStates.add(new LayerState(experiment.getLayerId(), decision, true)); + } + } else { + logger.info( + "Not tracking event \"{}\" for experiment \"{}\" because experiment has status \"Launched\".", + eventKey, experiment.getKey()); } } } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTestV2.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTestV2.java index c7f73eeef..87e6f4f6f 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTestV2.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTestV2.java @@ -669,6 +669,37 @@ public void activateDispatchEventThrowsException() throws Exception { optimizely.activate(experiment.getKey(), "userId"); } + /** + * Verify that {@link Optimizely#activate(String, String)} doesn't dispatch an event for an experiment with a + * "Launched" status. + */ + @Test + public void activateLaunchedExperimentDoesNotDispatchEvent() throws Exception { + String datafile = noAudienceProjectConfigJsonV2(); + ProjectConfig projectConfig = noAudienceProjectConfigV2(); + Experiment launchedExperiment = projectConfig.getExperiments().get(2); + + Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler) + .withBucketing(mockBucketer) + .withConfig(projectConfig) + .build(); + + Variation expectedVariation = launchedExperiment.getVariations().get(0); + + when(mockBucketer.bucket(launchedExperiment, "userId")) + .thenReturn(launchedExperiment.getVariations().get(0)); + + logbackVerifier.expectMessage(Level.INFO, + "Experiment has \"Launched\" status so not dispatching event during activation."); + Variation variation = optimizely.activate(launchedExperiment.getKey(), "userId"); + + assertNotNull(variation); + assertThat(variation.getKey(), is(expectedVariation.getKey())); + + // verify that we did NOT dispatch an event + verify(mockEventHandler, never()).dispatchEvent(any(LogEvent.class)); + } + //======== track tests ========// /** @@ -953,6 +984,26 @@ public void trackDispatchEventThrowsException() throws Exception { optimizely.track(eventType.getKey(), "userId"); } + /** + * Verify that {@link Optimizely#track(String, String)} doesn't make a dispatch for an event being used by a + * single experiment with a "Launched" status. + */ + @Test + public void trackLaunchedExperimentDoesNotDispatchEvent() throws Exception { + String datafile = noAudienceProjectConfigJsonV2(); + ProjectConfig projectConfig = noAudienceProjectConfigV2(); + EventType eventType = projectConfig.getEventTypes().get(3); + + Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler) + .withConfig(projectConfig) + .build(); + + optimizely.track(eventType.getKey(), "userId"); + + // verify that we did NOT dispatch an event + verify(mockEventHandler, never()).dispatchEvent(any(LogEvent.class)); + } + //======== getVariation tests ========// /** diff --git a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java index e645e4971..15459344e 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java @@ -295,6 +295,14 @@ private static ProjectConfig generateNoAudienceProjectConfigV2() { Collections.emptyMap(), asList(new TrafficAllocation("278", 4500), new TrafficAllocation("279", 9000)), + ""), + new Experiment("119", "etag3", "Launched", "3", + Collections.emptyList(), + asList(new Variation("280", "vtag5"), + new Variation("281", "vtag6")), + Collections.emptyMap(), + asList(new TrafficAllocation("280", 5000), + new TrafficAllocation("281", 10000)), "") ); @@ -304,7 +312,8 @@ private static ProjectConfig generateNoAudienceProjectConfigV2() { List multipleExperimentIds = asList("118", "223"); List events = asList(new EventType("971", "clicked_cart", singleExperimentId), new EventType("098", "Total Revenue", singleExperimentId), - new EventType("099", "clicked_purchase", multipleExperimentIds)); + new EventType("099", "clicked_purchase", multipleExperimentIds), + new EventType("100", "launched_exp_event", singletonList("119"))); return new ProjectConfig("789", "1234", "2", "42", Collections.emptyList(), experiments, attributes, events, Collections.emptyList()); diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java index 43027ae70..baf299f59 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab.event.internal; +import ch.qos.logback.classic.Level; import com.google.gson.Gson; import com.optimizely.ab.bucketing.Bucketer; @@ -33,8 +34,10 @@ import com.optimizely.ab.event.internal.payload.Feature; import com.optimizely.ab.event.internal.payload.Impression; import com.optimizely.ab.event.internal.payload.LayerState; +import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.internal.ProjectValidationUtils; +import org.junit.Rule; import org.junit.Test; import java.util.ArrayList; @@ -58,6 +61,9 @@ */ public class EventBuilderV2Test { + @Rule + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + private Gson gson = new Gson(); private EventBuilderV2 builder = new EventBuilderV2(); @@ -364,4 +370,32 @@ public void createConversionEventCustomClientEngineClientVersion() throws Except assertThat(conversion.getClientEngine(), is(ClientEngine.ANDROID_SDK.getClientEngineValue())); assertThat(conversion.getClientVersion(), is("0.0.0")); } + + /** + * Verify that {@link EventBuilderV2} doesn't add experiments with a "Launched" status to the bucket map + */ + @Test + public void createConversionEventForEventUsingLaunchedExperiment() throws Exception { + EventBuilderV2 builder = new EventBuilderV2(); + ProjectConfig projectConfig = ProjectConfigTestUtils.noAudienceProjectConfigV2(); + EventType eventType = projectConfig.getEventTypes().get(3); + String userId = "userId"; + + Bucketer mockBucketAlgorithm = mock(Bucketer.class); + for (Experiment experiment : projectConfig.getExperiments()) { + when(mockBucketAlgorithm.bucket(experiment, userId)) + .thenReturn(experiment.getVariations().get(0)); + } + + logbackVerifier.expectMessage(Level.INFO, + "Not tracking event \"launched_exp_event\" for experiment \"etag3\" because experiment has status " + + "\"Launched\"."); + LogEvent conversionEvent = builder.createConversionEvent(projectConfig, mockBucketAlgorithm, userId, + eventType.getId(), eventType.getKey(), + Collections.emptyMap()); + + // only 1 experiment uses the event and it has a "Launched" status so the bucket map is empty and the returned + // event will be null + assertNull(conversionEvent); + } } diff --git a/core-api/src/test/resources/config/no-audience-project-config-v2.json b/core-api/src/test/resources/config/no-audience-project-config-v2.json index b363a8c55..660b99f13 100644 --- a/core-api/src/test/resources/config/no-audience-project-config-v2.json +++ b/core-api/src/test/resources/config/no-audience-project-config-v2.json @@ -51,6 +51,28 @@ "entityId": "279", "endOfRange": 9000 }] + }, + { + "id": "119", + "key": "etag3", + "status": "Launched", + "layerId": "3", + "audienceIds": [], + "variations": [{ + "id": "280", + "key": "vtag5" + }, { + "id": "281", + "key": "vtag6" + }], + "forcedVariations": {}, + "trafficAllocation": [{ + "entityId": "280", + "endOfRange": 5000 + }, { + "entityId": "281", + "endOfRange": 10000 + }] } ], "groups": [], @@ -83,6 +105,13 @@ "118", "223" ] + }, + { + "id": "100", + "key": "launched_exp_event", + "experimentIds": [ + "119" + ] } ] } \ No newline at end of file From e669d01161a7a74fd8b0a2e5398b73b22dbf18ba Mon Sep 17 00:00:00 2001 From: Vignesh Raja Date: Wed, 25 Jan 2017 11:21:11 -0800 Subject: [PATCH 2/3] Change 'isRunning' to 'isActive' --- .../src/main/java/com/optimizely/ab/bucketing/Bucketer.java | 2 +- core-api/src/main/java/com/optimizely/ab/config/Experiment.java | 2 +- .../java/com/optimizely/ab/internal/ProjectValidationUtils.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java b/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java index 71f6eab84..b7199a113 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java @@ -240,7 +240,7 @@ public void cleanUserProfiles() { for (Map.Entry> record : records.entrySet()) { for (String experimentId : record.getValue().keySet()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); - if (experiment == null || !experiment.isRunning()) { + if (experiment == null || !experiment.isActive()) { userProfile.remove(record.getKey(), experimentId); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java index 0bbbd9f03..d597a92ac 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java +++ b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java @@ -147,7 +147,7 @@ public String getGroupId() { return groupId; } - public boolean isRunning() { + public boolean isActive() { return status.equals(ExperimentStatus.RUNNING.toString()) || status.equals(ExperimentStatus.LAUNCHED.toString()); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ProjectValidationUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ProjectValidationUtils.java index 1def80568..2ea00ea79 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ProjectValidationUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ProjectValidationUtils.java @@ -42,7 +42,7 @@ private ProjectValidationUtils() {} */ public static boolean validatePreconditions(ProjectConfig projectConfig, Experiment experiment, String userId, Map attributes) { - if (!experiment.isRunning()) { + if (!experiment.isActive()) { logger.info("Experiment \"{}\" is not running.", experiment.getKey(), userId); return false; } From 52d1725c92fe1fe20326dcccf7956b65a9beacae Mon Sep 17 00:00:00 2001 From: Vignesh Raja Date: Thu, 26 Jan 2017 13:15:46 -0800 Subject: [PATCH 3/3] Use 'isRunning' instead of 'isLaunched' --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 2 +- .../src/main/java/com/optimizely/ab/config/Experiment.java | 4 ++++ .../java/com/optimizely/ab/event/internal/EventBuilderV2.java | 2 +- 3 files changed, 6 insertions(+), 2 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 a887ae088..665fad3ce 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -172,7 +172,7 @@ private Optimizely(@Nonnull ProjectConfig projectConfig, return null; } - if (!experiment.isLaunched()) { + if (experiment.isRunning()) { LogEvent impressionEvent = eventBuilder.createImpressionEvent(projectConfig, experiment, variation, userId, attributes); logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); diff --git a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java index d597a92ac..6ac00d1e2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java +++ b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java @@ -152,6 +152,10 @@ public boolean isActive() { status.equals(ExperimentStatus.LAUNCHED.toString()); } + public boolean isRunning() { + return status.equals(ExperimentStatus.RUNNING.toString()); + } + public boolean isLaunched() { return status.equals(ExperimentStatus.LAUNCHED.toString()); } diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java index 73139056d..d3329316b 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java @@ -199,7 +199,7 @@ private List createLayerStates(ProjectConfig projectConfig, Bucketer for (Experiment experiment : allExperiments) { if (experimentIds.contains(experiment.getId()) && ProjectValidationUtils.validatePreconditions(projectConfig, experiment, userId, attributes)) { - if (!experiment.isLaunched()) { + if (experiment.isRunning()) { Variation bucketedVariation = bucketer.bucket(experiment, userId); if (bucketedVariation != null) { Decision decision = new Decision(bucketedVariation.getId(), false, experiment.getId());