Skip to content

Commit dc464bf

Browse files
wangjoshuahwangjoshuah
authored
Refactor Event Builder (#88)
* `EventBuilder#createConversionEvent(ProjectConfig, Bucketer, UserProfile, ...` now accepts a different set of arguments: `(ProjectConfig, Map<Experiment, Variation>, ...)`. The event builder no longer calls `Bucketer#bucket()`, but relies on Optimizely to have already bucketed the user into all experiments for the event in `track()` and pass in the results in the `Map<Experiment, Variation>`. * New getter `ProjectConfig#getExperimentsForEventKey(String)` returns a list of experiments that the event is attached to as `List<Experiment>`.
1 parent 102c935 commit dc464bf

File tree

11 files changed

+945
-424
lines changed

11 files changed

+945
-424
lines changed

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

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ private Optimizely(@Nonnull ProjectConfig projectConfig,
159159
@Nonnull Experiment experiment,
160160
@Nonnull String userId,
161161
@Nonnull Map<String, String> attributes) {
162+
162163
// determine whether all the given attributes are present in the project config. If not, filter out the unknown
163164
// attributes.
164165
attributes = filterAttributes(projectConfig, attributes);
@@ -225,9 +226,9 @@ public void track(@Nonnull String eventName,
225226
}
226227

227228
public void track(@Nonnull String eventName,
228-
@Nonnull String userId,
229-
@Nonnull Map<String, String> attributes,
230-
@Nonnull Map<String, ?> eventTags) throws UnknownEventTypeException {
229+
@Nonnull String userId,
230+
@Nonnull Map<String, String> attributes,
231+
@Nonnull Map<String, ?> eventTags) throws UnknownEventTypeException {
231232

232233
ProjectConfig currentConfig = getProjectConfig();
233234

@@ -250,11 +251,31 @@ public void track(@Nonnull String eventName,
250251
eventValue = EventTagUtils.getRevenueValue(eventTags);
251252
}
252253

253-
// create the conversion event request parameters, then dispatch
254-
LogEvent conversionEvent = eventBuilder.createConversionEvent(currentConfig, bucketer, userProfile, userId,
255-
eventType.getId(), eventType.getKey(), attributes,
256-
eventTags);
254+
List<Experiment> experimentsForEvent = projectConfig.getExperimentsForEventKey(eventName);
255+
Map<Experiment, Variation> experimentVariationMap = new HashMap<Experiment, Variation>(experimentsForEvent.size());
256+
for (Experiment experiment : experimentsForEvent) {
257+
if (experiment.isRunning()) {
258+
Variation variation = getVariation(currentConfig, experiment, attributes, userId);
259+
if (variation != null) {
260+
experimentVariationMap.put(experiment, variation);
261+
}
262+
} else {
263+
logger.info(
264+
"Not tracking event \"{}\" for experiment \"{}\" because experiment has status \"Launched\".",
265+
eventType.getKey(), experiment.getKey());
266+
}
267+
}
257268

269+
// create the conversion event request parameters, then dispatch
270+
LogEvent conversionEvent = eventBuilder.createConversionEvent(
271+
projectConfig,
272+
experimentVariationMap,
273+
userId,
274+
eventType.getId(),
275+
eventType.getKey(),
276+
attributes,
277+
eventTags);
278+
258279
if (conversionEvent == null) {
259280
logger.info("There are no valid experiments for event \"{}\" to track.", eventName);
260281
logger.info("Not tracking event \"{}\" for user \"{}\".", eventName, userId);
@@ -415,7 +436,6 @@ public void track(@Nonnull String eventName,
415436
public @Nullable Variation getVariation(@Nonnull String experimentKey,
416437
@Nonnull String userId,
417438
@Nonnull Map<String, String> attributes) {
418-
419439
if (!validateUserId(userId)) {
420440
return null;
421441
}
@@ -436,6 +456,10 @@ public void track(@Nonnull String eventName,
436456
@Nonnull Map<String, String> attributes,
437457
@Nonnull String userId) {
438458

459+
// determine whether all the given attributes are present in the project config. If not, filter out the unknown
460+
// attributes.
461+
attributes = filterAttributes(projectConfig, attributes);
462+
439463
if (!ProjectValidationUtils.validatePreconditions(projectConfig, userProfile, experiment, userId, attributes)) {
440464
return null;
441465
}

‎core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,15 @@
1717
package com.optimizely.ab.config;
1818

1919
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
20-
2120
import com.optimizely.ab.config.audience.Audience;
2221
import com.optimizely.ab.config.audience.Condition;
2322

23+
import javax.annotation.concurrent.Immutable;
2424
import java.util.ArrayList;
2525
import java.util.Collections;
2626
import java.util.List;
2727
import java.util.Map;
2828

29-
import javax.annotation.concurrent.Immutable;
30-
3129
/**
3230
* Represents the Optimizely Project configuration.
3331
*
@@ -165,10 +163,16 @@ public List<Experiment> getExperiments() {
165163
return experiments;
166164
}
167165

168-
public List<String> getExperimentIdsForGoal(String goalKey) {
169-
EventType goal;
170-
if ((goal = eventNameMapping.get(goalKey)) != null) {
171-
return goal.getExperimentIds();
166+
public List<Experiment> getExperimentsForEventKey(String eventKey) {
167+
EventType event = eventNameMapping.get(eventKey);
168+
if (event != null) {
169+
List<String> experimentIds = event.getExperimentIds();
170+
List<Experiment> experiments = new ArrayList<Experiment>(experimentIds.size());
171+
for (String experimentId : experimentIds) {
172+
experiments.add(experimentIdMapping.get(experimentId));
173+
}
174+
175+
return experiments;
172176
}
173177

174178
return Collections.emptyList();

‎core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilder.java

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,12 @@
1616
*/
1717
package com.optimizely.ab.event.internal;
1818

19-
import com.optimizely.ab.bucketing.Bucketer;
20-
import com.optimizely.ab.bucketing.UserProfile;
2119
import com.optimizely.ab.config.Experiment;
2220
import com.optimizely.ab.config.ProjectConfig;
2321
import com.optimizely.ab.config.Variation;
2422
import com.optimizely.ab.event.LogEvent;
2523

26-
import javax.annotation.CheckForNull;
2724
import javax.annotation.Nonnull;
28-
import javax.annotation.Nullable;
29-
30-
import java.util.Collections;
3125
import java.util.Map;
3226

3327
public abstract class EventBuilder {
@@ -38,19 +32,8 @@ public abstract LogEvent createImpressionEvent(@Nonnull ProjectConfig projectCon
3832
@Nonnull String userId,
3933
@Nonnull Map<String, String> attributes);
4034

41-
public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
42-
@Nonnull Bucketer bucketer,
43-
@Nullable UserProfile userProfile,
44-
@Nonnull String userId,
45-
@Nonnull String eventId,
46-
@Nonnull String eventName,
47-
@Nonnull Map<String, String> attributes) {
48-
return createConversionEvent(projectConfig, bucketer, userProfile, userId, eventId, eventName, attributes, Collections.<String, String>emptyMap());
49-
}
50-
5135
public abstract LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
52-
@Nonnull Bucketer bucketer,
53-
@Nullable UserProfile userProfile,
36+
@Nonnull Map<Experiment, Variation> experimentVariationMap,
5437
@Nonnull String userId,
5538
@Nonnull String eventId,
5639
@Nonnull String eventName,

‎core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java

Lines changed: 27 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -17,32 +17,25 @@
1717
package com.optimizely.ab.event.internal;
1818

1919
import com.optimizely.ab.annotations.VisibleForTesting;
20-
import com.optimizely.ab.bucketing.Bucketer;
21-
import com.optimizely.ab.bucketing.UserProfile;
2220
import com.optimizely.ab.config.Attribute;
2321
import com.optimizely.ab.config.Experiment;
2422
import com.optimizely.ab.config.ProjectConfig;
2523
import com.optimizely.ab.config.Variation;
2624
import com.optimizely.ab.event.LogEvent;
2725
import com.optimizely.ab.event.internal.payload.Conversion;
2826
import com.optimizely.ab.event.internal.payload.Decision;
29-
import com.optimizely.ab.event.internal.payload.EventMetric;
3027
import com.optimizely.ab.event.internal.payload.Event.ClientEngine;
28+
import com.optimizely.ab.event.internal.payload.EventMetric;
3129
import com.optimizely.ab.event.internal.payload.Feature;
3230
import com.optimizely.ab.event.internal.payload.Impression;
3331
import com.optimizely.ab.event.internal.payload.LayerState;
3432
import com.optimizely.ab.event.internal.serializer.DefaultJsonSerializer;
3533
import com.optimizely.ab.event.internal.serializer.Serializer;
3634
import com.optimizely.ab.internal.EventTagUtils;
37-
import com.optimizely.ab.internal.ProjectValidationUtils;
38-
3935
import org.slf4j.Logger;
4036
import org.slf4j.LoggerFactory;
4137

42-
import javax.annotation.CheckForNull;
4338
import javax.annotation.Nonnull;
44-
import javax.annotation.Nullable;
45-
4639
import java.util.ArrayList;
4740
import java.util.Collections;
4841
import java.util.List;
@@ -106,45 +99,41 @@ public LogEvent createImpressionEvent(@Nonnull ProjectConfig projectConfig,
10699
}
107100

108101
public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
109-
@Nonnull Bucketer bucketer,
110-
@Nullable UserProfile userProfile,
102+
@Nonnull Map<Experiment, Variation> experimentVariationMap,
111103
@Nonnull String userId,
112104
@Nonnull String eventId,
113105
@Nonnull String eventName,
114106
@Nonnull Map<String, String> attributes,
115107
@Nonnull Map<String, ?> eventTags) {
116108

117-
Conversion conversionPayload = new Conversion();
118-
conversionPayload.setVisitorId(userId);
119-
conversionPayload.setTimestamp(System.currentTimeMillis());
120-
conversionPayload.setProjectId(projectConfig.getProjectId());
121-
conversionPayload.setAccountId(projectConfig.getAccountId());
122-
conversionPayload.setUserFeatures(createUserFeatures(attributes, projectConfig));
123-
124-
List<LayerState> layerStates = createLayerStates(projectConfig, bucketer, userProfile, userId, eventName, attributes);
125-
if (layerStates.isEmpty()) {
109+
if (experimentVariationMap.isEmpty()) {
126110
return null;
127111
}
128-
conversionPayload.setLayerStates(layerStates);
129112

130-
conversionPayload.setEventEntityId(eventId);
131-
conversionPayload.setEventName(eventName);
113+
List<LayerState> layerStates = createLayerStates(projectConfig, experimentVariationMap);
132114

133115
Long eventValue = EventTagUtils.getRevenueValue(eventTags);
116+
List<EventMetric> eventMetrics = Collections.emptyList();
134117
if (eventValue != null) {
135-
conversionPayload.setEventMetrics(
136-
Collections.singletonList(new EventMetric(EventMetric.REVENUE_METRIC_TYPE, eventValue)));
137-
} else {
138-
conversionPayload.setEventMetrics(Collections.<EventMetric>emptyList());
118+
eventMetrics = Collections.singletonList(new EventMetric(EventMetric.REVENUE_METRIC_TYPE, eventValue));
139119
}
140120

141-
conversionPayload.setEventFeatures(Collections.<Feature>emptyList());
142-
conversionPayload.setIsGlobalHoldback(false);
121+
Conversion conversionPayload = new Conversion();
122+
conversionPayload.setAccountId(projectConfig.getAccountId());
143123
conversionPayload.setAnonymizeIP(projectConfig.getAnonymizeIP());
144124
conversionPayload.setClientEngine(clientEngine);
145125
conversionPayload.setClientVersion(clientVersion);
146-
conversionPayload.setRevision(projectConfig.getRevision());
126+
conversionPayload.setEventEntityId(eventId);
147127
conversionPayload.setEventFeatures(createEventFeatures(eventTags));
128+
conversionPayload.setEventName(eventName);
129+
conversionPayload.setEventMetrics(eventMetrics);
130+
conversionPayload.setIsGlobalHoldback(false);
131+
conversionPayload.setLayerStates(layerStates);
132+
conversionPayload.setProjectId(projectConfig.getProjectId());
133+
conversionPayload.setRevision(projectConfig.getRevision());
134+
conversionPayload.setTimestamp(System.currentTimeMillis());
135+
conversionPayload.setUserFeatures(createUserFeatures(attributes, projectConfig));
136+
conversionPayload.setVisitorId(userId);
148137

149138
String payload = this.serializer.serialize(conversionPayload);
150139
return new LogEvent(RequestMethod.POST, CONVERSION_ENDPOINT, Collections.<String, String>emptyMap(), payload);
@@ -208,33 +197,18 @@ private List<Feature> createEventFeatures(Map<String, ?> eventTags) {
208197
* no good reason.
209198
*
210199
* @param projectConfig the current project config
211-
* @param bucketer the bucketing algorithm to use
212-
* @param userId the user's id for the impression event
213-
* @param eventKey the goal that the bucket map will be filtered by
214-
* @param attributes the user's attributes
200+
* @param experimentVariationMap the mapping of experiments associated with this event
201+
* and the variations the user was bucketed into for that experiment
202+
*
215203
*/
216-
private List<LayerState> createLayerStates(ProjectConfig projectConfig, Bucketer bucketer, UserProfile userProfile, String userId,
217-
String eventKey, Map<String, String> attributes) {
218-
List<Experiment> allExperiments = projectConfig.getExperiments();
219-
List<String> experimentIds = projectConfig.getExperimentIdsForGoal(eventKey);
204+
private List<LayerState> createLayerStates(ProjectConfig projectConfig, Map<Experiment, Variation> experimentVariationMap) {
220205
List<LayerState> layerStates = new ArrayList<LayerState>();
221206

222-
for (Experiment experiment : allExperiments) {
223-
if (experimentIds.contains(experiment.getId()) &&
224-
ProjectValidationUtils.validatePreconditions(projectConfig, userProfile, experiment, userId, attributes)) {
225-
if (experiment.isRunning()) {
226-
Variation bucketedVariation = bucketer.bucket(experiment, userId);
227-
if (bucketedVariation != null) {
228-
Decision decision = new Decision(bucketedVariation.getId(), false, experiment.getId());
229-
layerStates.add(
230-
new LayerState(experiment.getLayerId(), projectConfig.getRevision(), decision, true));
231-
}
232-
} else {
233-
logger.info(
234-
"Not tracking event \"{}\" for experiment \"{}\" because experiment has status \"Launched\".",
235-
eventKey, experiment.getKey());
236-
}
237-
}
207+
for (Map.Entry<Experiment, Variation> entry : experimentVariationMap.entrySet()) {
208+
Experiment experiment = entry.getKey();
209+
Variation variation = entry.getValue();
210+
Decision decision = new Decision(variation.getId(), false, experiment.getId());
211+
layerStates.add(new LayerState(experiment.getLayerId(), projectConfig.getRevision(), decision, true));
238212
}
239213

240214
return layerStates;

0 commit comments

Comments
 (0)