Skip to content

Commit c1b088a

Browse files
wangjoshuahwangjoshuah
authored
Josh.wang/revise user profile (#80)
* Check for user profile in ProjectValidationUtils before audience evaluation * Return saved variation if it was stored before bucketing. The order of bucketing should be 1) Check Experiment is Active 2) If the user is whitelisted, return the whitelisted variation. 3) If the user profile is saved, return it 4) Check if the user passes audience targeting. 5) Bucket the user with murmur hash
1 parent a7a1652 commit c1b088a

18 files changed

+821
-585
lines changed

‎build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ plugins {
1919

2020
allprojects {
2121
group = 'com.optimizely.ab'
22+
apply plugin: 'idea'
2223
}
2324

2425
apply from: 'gradle/license.gradle'

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.optimizely.ab.annotations.VisibleForTesting;
2020
import com.optimizely.ab.bucketing.Bucketer;
2121
import com.optimizely.ab.bucketing.UserProfile;
22+
import com.optimizely.ab.internal.UserProfileUtils;
2223
import com.optimizely.ab.config.Attribute;
2324
import com.optimizely.ab.config.EventType;
2425
import com.optimizely.ab.config.Experiment;
@@ -94,22 +95,25 @@ public class Optimizely {
9495
@VisibleForTesting final EventHandler eventHandler;
9596
@VisibleForTesting final ErrorHandler errorHandler;
9697
@VisibleForTesting final NotificationBroadcaster notificationBroadcaster = new NotificationBroadcaster();
98+
@Nullable private final UserProfile userProfile;
9799

98100
private Optimizely(@Nonnull ProjectConfig projectConfig,
99101
@Nonnull Bucketer bucketer,
100102
@Nonnull EventHandler eventHandler,
101103
@Nonnull EventBuilder eventBuilder,
102-
@Nonnull ErrorHandler errorHandler) {
104+
@Nonnull ErrorHandler errorHandler,
105+
@Nullable UserProfile userProfile) {
103106
this.projectConfig = projectConfig;
104107
this.bucketer = bucketer;
105108
this.eventHandler = eventHandler;
106109
this.eventBuilder = eventBuilder;
107110
this.errorHandler = errorHandler;
111+
this.userProfile = userProfile;
108112
}
109113

110114
// Do work here that should be done once per Optimizely lifecycle
111115
@VisibleForTesting void initialize() {
112-
bucketer.cleanUserProfiles();
116+
UserProfileUtils.cleanUserProfiles(userProfile, projectConfig);
113117
}
114118

115119
//======== activate calls ========//
@@ -162,13 +166,8 @@ private Optimizely(@Nonnull ProjectConfig projectConfig,
162166
// attributes.
163167
attributes = filterAttributes(projectConfig, attributes);
164168

165-
if (!ProjectValidationUtils.validatePreconditions(projectConfig, experiment, userId, attributes)) {
166-
logger.info("Not activating user \"{}\" for experiment \"{}\".", userId, experiment.getKey());
167-
return null;
168-
}
169-
170169
// bucket the user to the given experiment and dispatch an impression event
171-
Variation variation = bucketer.bucket(experiment, userId);
170+
Variation variation = getVariation(projectConfig, experiment, attributes, userId);
172171
if (variation == null) {
173172
logger.info("Not activating user \"{}\" for experiment \"{}\".", userId, experiment.getKey());
174173
return null;
@@ -255,7 +254,7 @@ public void track(@Nonnull String eventName,
255254
}
256255

257256
// create the conversion event request parameters, then dispatch
258-
LogEvent conversionEvent = eventBuilder.createConversionEvent(currentConfig, bucketer, userId,
257+
LogEvent conversionEvent = eventBuilder.createConversionEvent(currentConfig, bucketer, userProfile, userId,
259258
eventType.getId(), eventType.getKey(), attributes,
260259
eventTags);
261260

@@ -440,7 +439,7 @@ public void track(@Nonnull String eventName,
440439
@Nonnull Map<String, String> attributes,
441440
@Nonnull String userId) {
442441

443-
if (!ProjectValidationUtils.validatePreconditions(projectConfig, experiment, userId, attributes)) {
442+
if (!ProjectValidationUtils.validatePreconditions(projectConfig, userProfile, experiment, userId, attributes)) {
444443
return null;
445444
}
446445

@@ -468,6 +467,12 @@ private static ProjectConfig getProjectConfig(String datafile) throws ConfigPars
468467
return DefaultConfigParser.getInstance().parseProjectConfig(datafile);
469468
}
470469

470+
@Nullable
471+
public UserProfile getUserProfile() {
472+
return userProfile;
473+
}
474+
475+
471476
//======== Notification listeners ========//
472477

473478
/**
@@ -739,7 +744,7 @@ public Optimizely build() throws ConfigParseException {
739744
errorHandler = new NoOpErrorHandler();
740745
}
741746

742-
Optimizely optimizely = new Optimizely(projectConfig, bucketer, eventHandler, eventBuilder, errorHandler);
747+
Optimizely optimizely = new Optimizely(projectConfig, bucketer, eventHandler, eventBuilder, errorHandler, userProfile);
743748
optimizely.initialize();
744749
return optimizely;
745750
}

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

Lines changed: 49 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,11 @@
2626
import org.slf4j.Logger;
2727
import org.slf4j.LoggerFactory;
2828

29-
import java.util.List;
30-
import java.util.Map;
31-
3229
import javax.annotation.Nonnull;
3330
import javax.annotation.Nullable;
3431
import javax.annotation.concurrent.Immutable;
32+
import java.util.List;
33+
import java.util.Map;
3534

3635
/**
3736
* Default Optimizely bucketing algorithm that evenly distributes users using the Murmur3 hash of some provided
@@ -46,8 +45,7 @@
4645
public class Bucketer {
4746

4847
private final ProjectConfig projectConfig;
49-
50-
@Nullable private final UserProfile userProfile;
48+
private final UserProfile userProfile;
5149

5250
private static final Logger logger = LoggerFactory.getLogger(Bucketer.class);
5351

@@ -112,27 +110,6 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
112110
String experimentKey = experiment.getKey();
113111
String combinedBucketId = userId + experimentId;
114112

115-
// If a user profile instance is present then check it for a saved variation
116-
if (userProfile != null) {
117-
String variationId = userProfile.lookup(userId, experimentId);
118-
if (variationId != null) {
119-
Variation savedVariation = projectConfig
120-
.getExperimentIdMapping()
121-
.get(experimentId)
122-
.getVariationIdToVariationMap()
123-
.get(variationId);
124-
logger.info("Returning previously activated variation \"{}\" of experiment \"{}\" "
125-
+ "for user \"{}\" from user profile.",
126-
savedVariation.getKey(), experimentKey, userId);
127-
// A variation is stored for this combined bucket id
128-
return savedVariation;
129-
} else {
130-
logger.info("No previously activated variation of experiment \"{}\" "
131-
+ "for user \"{}\" found in user profile.",
132-
experimentKey, userId);
133-
}
134-
}
135-
136113
List<TrafficAllocation> trafficAllocations = experiment.getTrafficAllocation();
137114

138115
int hashCode = MurmurHash3.murmurhash3_x86_32(combinedBucketId, 0, combinedBucketId.length(), MURMUR_HASH_SEED);
@@ -146,18 +123,6 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
146123
logger.info("User \"{}\" is in variation \"{}\" of experiment \"{}\".", userId, variationKey,
147124
experimentKey);
148125

149-
// If a user profile is present give it a variation to store
150-
if (userProfile != null) {
151-
boolean saved = userProfile.save(userId, experimentId, bucketedVariationId);
152-
if (saved) {
153-
logger.info("Saved variation \"{}\" of experiment \"{}\" for user \"{}\".",
154-
bucketedVariationId, experimentId, userId);
155-
} else {
156-
logger.warn("Failed to save variation \"{}\" of experiment \"{}\" for user \"{}\".",
157-
bucketedVariationId, experimentId, userId);
158-
}
159-
}
160-
161126
return bucketedVariation;
162127
}
163128

@@ -169,6 +134,7 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
169134
public @Nullable Variation bucket(@Nonnull Experiment experiment,
170135
@Nonnull String userId) {
171136

137+
// ---------- Check whitelist ----------
172138
// if a user has a forced variation mapping, return the respective variation
173139
Map<String, String> userIdToVariationKeyMap = experiment.getUserIdToVariationKeyMap();
174140
if (userIdToVariationKeyMap.containsKey(userId)) {
@@ -178,12 +144,37 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
178144
logger.info("User \"{}\" is forced in variation \"{}\".", userId, forcedVariationKey);
179145
} else {
180146
logger.error("Variation \"{}\" is not in the datafile. Not activating user \"{}\".", forcedVariationKey,
181-
userId);
147+
userId);
182148
}
183149

184150
return forcedVariation;
185151
}
186152

153+
// ---------- Check User Profile for Sticky Bucketing ----------
154+
// If a user profile instance is present then check it for a saved variation
155+
String experimentId = experiment.getId();
156+
String experimentKey = experiment.getKey();
157+
if (userProfile != null) {
158+
String variationId = userProfile.lookup(userId, experimentId);
159+
if (variationId != null) {
160+
Variation savedVariation = projectConfig
161+
.getExperimentIdMapping()
162+
.get(experimentId)
163+
.getVariationIdToVariationMap()
164+
.get(variationId);
165+
logger.info("Returning previously activated variation \"{}\" of experiment \"{}\" "
166+
+ "for user \"{}\" from user profile.",
167+
savedVariation.getKey(), experimentKey, userId);
168+
// A variation is stored for this combined bucket id
169+
return savedVariation;
170+
} else {
171+
logger.info("No previously activated variation of experiment \"{}\" "
172+
+ "for user \"{}\" found in user profile.",
173+
experimentKey, userId);
174+
}
175+
}
176+
177+
// ---------- Bucket User ----------
187178
String groupId = experiment.getGroupId();
188179
// check whether the experiment belongs to a group
189180
if (!groupId.isEmpty()) {
@@ -198,16 +189,32 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
198189
// don't perform further bucketing within the experiment
199190
if (!bucketedExperiment.getId().equals(experiment.getId())) {
200191
logger.info("User \"{}\" is not in experiment \"{}\" of group {}.", userId, experiment.getKey(),
201-
experimentGroup.getId());
192+
experimentGroup.getId());
202193
return null;
203194
}
204195

205196
logger.info("User \"{}\" is in experiment \"{}\" of group {}.", userId, experiment.getKey(),
206-
experimentGroup.getId());
197+
experimentGroup.getId());
207198
}
208199
}
209200

210-
return bucketToVariation(experiment, userId);
201+
Variation bucketedVariation = bucketToVariation(experiment, userId);
202+
203+
// ---------- Save Variation to User Profile ----------
204+
// If a user profile is present give it a variation to store
205+
if (userProfile != null && bucketedVariation != null) {
206+
String bucketedVariationId = bucketedVariation.getId();
207+
boolean saved = userProfile.save(userId, experimentId, bucketedVariationId);
208+
if (saved) {
209+
logger.info("Saved variation \"{}\" of experiment \"{}\" for user \"{}\".",
210+
bucketedVariationId, experimentId, userId);
211+
} else {
212+
logger.warn("Failed to save variation \"{}\" of experiment \"{}\" for user \"{}\".",
213+
bucketedVariationId, experimentId, userId);
214+
}
215+
}
216+
217+
return bucketedVariation;
211218
}
212219

213220
//======== Helper methods ========//
@@ -224,28 +231,5 @@ int generateBucketValue(int hashCode) {
224231
return (int)Math.floor(MAX_TRAFFIC_VALUE * ratio);
225232
}
226233

227-
@Nullable
228-
public UserProfile getUserProfile() {
229-
return userProfile;
230-
}
231234

232-
/**
233-
* Gives implementations of {@link UserProfile} a chance to remove records
234-
* of experiments that are deleted or not running.
235-
*/
236-
public void cleanUserProfiles() {
237-
if (userProfile != null) {
238-
Map<String, Map<String,String>> records = userProfile.getAllRecords();
239-
if (records != null) {
240-
for (Map.Entry<String,Map<String,String>> record : records.entrySet()) {
241-
for (String experimentId : record.getValue().keySet()) {
242-
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
243-
if (experiment == null || !experiment.isActive()) {
244-
userProfile.remove(record.getKey(), experimentId);
245-
}
246-
}
247-
}
248-
}
249-
}
250-
}
251235
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@
1717
package com.optimizely.ab.event.internal;
1818

1919
import com.optimizely.ab.bucketing.Bucketer;
20+
import com.optimizely.ab.bucketing.UserProfile;
2021
import com.optimizely.ab.config.Experiment;
2122
import com.optimizely.ab.config.ProjectConfig;
2223
import com.optimizely.ab.config.Variation;
2324
import com.optimizely.ab.event.LogEvent;
2425

2526
import javax.annotation.CheckForNull;
2627
import javax.annotation.Nonnull;
28+
import javax.annotation.Nullable;
2729

2830
import java.util.Collections;
2931
import java.util.Map;
@@ -38,15 +40,17 @@ public abstract LogEvent createImpressionEvent(@Nonnull ProjectConfig projectCon
3840

3941
public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
4042
@Nonnull Bucketer bucketer,
43+
@Nullable UserProfile userProfile,
4144
@Nonnull String userId,
4245
@Nonnull String eventId,
4346
@Nonnull String eventName,
4447
@Nonnull Map<String, String> attributes) {
45-
return createConversionEvent(projectConfig, bucketer, userId, eventId, eventName, attributes, Collections.<String, String>emptyMap());
48+
return createConversionEvent(projectConfig, bucketer, userProfile, userId, eventId, eventName, attributes, Collections.<String, String>emptyMap());
4649
}
4750

4851
public abstract LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
4952
@Nonnull Bucketer bucketer,
53+
@Nullable UserProfile userProfile,
5054
@Nonnull String userId,
5155
@Nonnull String eventId,
5256
@Nonnull String eventName,

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package com.optimizely.ab.event.internal;
1818

19+
import com.optimizely.ab.bucketing.UserProfile;
1920
import com.optimizely.ab.event.LogEvent;
2021
import com.optimizely.ab.internal.EventTagUtils;
2122
import com.optimizely.ab.internal.ProjectValidationUtils;
@@ -36,6 +37,7 @@
3637

3738
import javax.annotation.CheckForNull;
3839
import javax.annotation.Nonnull;
40+
import javax.annotation.Nullable;
3941

4042
import static com.optimizely.ab.event.LogEvent.RequestMethod;
4143

@@ -81,6 +83,7 @@ public LogEvent createImpressionEvent(@Nonnull ProjectConfig projectConfig,
8183

8284
public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
8385
@Nonnull Bucketer bucketer,
86+
@Nullable UserProfile userProfile,
8487
@Nonnull String userId,
8588
@Nonnull String eventId,
8689
@Nonnull String eventName,
@@ -89,7 +92,7 @@ public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig,
8992

9093
Map<String, String> requestParams = new HashMap<String, String>();
9194
List<Experiment> addedExperiments =
92-
addExperimentBucketMap(requestParams, projectConfig, bucketer, userId, eventName, attributes);
95+
addExperimentBucketMap(requestParams, projectConfig, bucketer, userProfile, userId, eventName, attributes);
9396

9497
if (addedExperiments.isEmpty()) {
9598
return null;
@@ -162,15 +165,15 @@ private void addProjectId(Map<String, String> requestParams, String projectId) {
162165
* @param attributes the user's attributes
163166
*/
164167
private List<Experiment> addExperimentBucketMap(Map<String, String> requestParams, ProjectConfig projectConfig,
165-
Bucketer bucketer, String userId, String goalKey,
168+
Bucketer bucketer, UserProfile userProfile, String userId, String goalKey,
166169
Map<String, String> attributes) {
167170
List<Experiment> allExperiments = projectConfig.getExperiments();
168171
List<String> experimentIds = projectConfig.getExperimentIdsForGoal(goalKey);
169172
List<Experiment> validExperiments = new ArrayList<Experiment>();
170173

171174
for (Experiment experiment : allExperiments) {
172175
if (experimentIds.contains(experiment.getId()) &&
173-
ProjectValidationUtils.validatePreconditions(projectConfig, experiment, userId, attributes)) {
176+
ProjectValidationUtils.validatePreconditions(projectConfig, userProfile, experiment, userId, attributes)) {
174177
Variation bucketedVariation = bucketer.bucket(experiment, userId);
175178
if (bucketedVariation != null) {
176179
requestParams.put(EXPERIMENT_PARAM_PREFIX + experiment.getId(), bucketedVariation.getId());

0 commit comments

Comments
 (0)