Skip to content

Commit 23268f8

Browse files
author
Vignesh Raja
authored
Persist experiment and variation IDs instead of keys in UserProfile (#59)
1 parent 1d2d45c commit 23268f8

File tree

3 files changed

+38
-35
lines changed

3 files changed

+38
-35
lines changed

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

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,18 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
114114

115115
// If a user profile instance is present then check it for a saved variation
116116
if (userProfile != null) {
117-
String variationKey = userProfile.lookup(userId, experimentKey);
118-
if (variationKey != 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);
119124
logger.info("Returning previously activated variation \"{}\" of experiment \"{}\" "
120125
+ "for user \"{}\" from user profile.",
121-
variationKey, experimentKey, userId);
126+
savedVariation.getKey(), experimentKey, userId);
122127
// A variation is stored for this combined bucket id
123-
return projectConfig
124-
.getExperimentIdMapping()
125-
.get(experimentId)
126-
.getVariationKeyToVariationMap()
127-
.get(variationKey);
128+
return savedVariation;
128129
} else {
129130
logger.info("No previously activated variation of experiment \"{}\" "
130131
+ "for user \"{}\" found in user profile.",
@@ -142,18 +143,18 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
142143
if (bucketedVariationId != null) {
143144
Variation bucketedVariation = experiment.getVariationIdToVariationMap().get(bucketedVariationId);
144145
String variationKey = bucketedVariation.getKey();
145-
logger.info("User \"{}\" is in variation \"{}\" of experiment \"{}\".", userId, variationKey,
146+
logger.info("User \"{}\" is in variation \"{}\" of experiment \"{}\".", userId, variationKey,
146147
experimentKey);
147148

148149
// If a user profile is present give it a variation to store
149150
if (userProfile != null) {
150-
boolean saved = userProfile.save(userId, experiment.getKey(), variationKey);
151+
boolean saved = userProfile.save(userId, experimentId, bucketedVariationId);
151152
if (saved) {
152153
logger.info("Saved variation \"{}\" of experiment \"{}\" for user \"{}\".",
153-
variationKey, experimentKey, userId);
154+
bucketedVariationId, experimentId, userId);
154155
} else {
155156
logger.warn("Failed to save variation \"{}\" of experiment \"{}\" for user \"{}\".",
156-
variationKey, experimentKey, userId);
157+
bucketedVariationId, experimentId, userId);
157158
}
158159
}
159160

@@ -237,10 +238,10 @@ public void cleanUserProfiles() {
237238
Map<String, Map<String,String>> records = userProfile.getAllRecords();
238239
if (records != null) {
239240
for (Map.Entry<String,Map<String,String>> record : records.entrySet()) {
240-
for (String experimentKey : record.getValue().keySet()) {
241-
Experiment experiment = projectConfig.getExperimentKeyMapping().get(experimentKey);
241+
for (String experimentId : record.getValue().keySet()) {
242+
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
242243
if (experiment == null || !experiment.isRunning()) {
243-
userProfile.remove(record.getKey(), experimentKey);
244+
userProfile.remove(record.getKey(), experimentId);
244245
}
245246
}
246247
}

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,20 @@ public interface UserProfile {
3131
* Called when implementors should save an activation
3232
*
3333
* @param userId the user id of the activation
34-
* @param experimentKey the experiment key of the activation
35-
* @param variationKey the variation key of the activation
34+
* @param experimentId the experiment ID of the activation
35+
* @param variationId the variation ID of the activation
3636
* @return true if saving of the record was successful
3737
*/
38-
boolean save(String userId, String experimentKey, String variationKey);
38+
boolean save(String userId, String experimentId, String variationId);
3939

4040
/**
4141
* Called by the bucketer to check for a record of the previous activation
4242
*
4343
* @param userId the user is id of the next activation
44-
* @param experimentKey the experiment id of the next activation
45-
* @return the variation key of the next activation, or null if no record exists
44+
* @param experimentId the experiment ID of the next activation
45+
* @return the variation ID of the next activation, or null if no record exists
4646
*/
47-
String lookup(String userId, String experimentKey);
47+
String lookup(String userId, String experimentId);
4848

4949
/**
5050
* Called when user profile should be removed
@@ -53,18 +53,18 @@ public interface UserProfile {
5353
* deleted.
5454
*
5555
* @param userId the user id of the record to remove
56-
* @param experimentKey the experiment key of the record to remove
56+
* @param experimentId the experiment ID of the record to remove
5757
* @return true if the record was removed
5858
*/
59-
boolean remove(String userId, String experimentKey);
59+
boolean remove(String userId, String experimentId);
6060

6161
/**
6262
* Called by bucketer to get a mapping of all stored records
6363
*
6464
* This mapping is needed so that the bucketer can {@link #remove(String, String)} outdated
6565
* records.
66-
* @return a map of userIds to a map of experiment keys to variation keys
66+
* @return a map of user IDs to a map of experiment IDs to variation IDs
6767
*/
68-
Map<String, Map<String,String>> getAllRecords();
68+
Map<String, Map<String, String>> getAllRecords();
6969

7070
}

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -460,14 +460,15 @@ public void bucketUserNotInOverlappingGroupExperiment() throws Exception {
460460
Experiment groupExperiment = groupExperiments.get(0);
461461
final Variation variation = groupExperiment.getVariations().get(0);
462462

463-
when(userProfile.save("blah", groupExperiment.getKey(), variation.getKey())).thenReturn(true);
463+
when(userProfile.save("blah", groupExperiment.getId(), variation.getId())).thenReturn(true);
464464

465465
assertThat(algorithm.bucket(groupExperiment, "blah"), is(variation));
466466

467467
logbackVerifier.expectMessage(Level.INFO,
468-
"Saved variation \"e2_vtag1\" of experiment \"group_etag2\" for user \"blah\".");
468+
String.format("Saved variation \"%s\" of experiment \"%s\" for user \"blah\".", variation.getId(),
469+
groupExperiment.getId()));
469470

470-
verify(userProfile).save("blah", groupExperiment.getKey(), variation.getKey());
471+
verify(userProfile).save("blah", groupExperiment.getId(), variation.getId());
471472
}
472473

473474
/**
@@ -485,14 +486,15 @@ public void bucketUserNotInOverlappingGroupExperiment() throws Exception {
485486
Experiment groupExperiment = groupExperiments.get(0);
486487
final Variation variation = groupExperiment.getVariations().get(0);
487488

488-
when(userProfile.save("blah", groupExperiment.getKey(), variation.getKey())).thenReturn(false);
489+
when(userProfile.save("blah", groupExperiment.getId(), variation.getId())).thenReturn(false);
489490

490491
assertThat(algorithm.bucket(groupExperiment, "blah"), is(variation));
491492

492493
logbackVerifier.expectMessage(Level.WARN,
493-
"Failed to save variation \"e2_vtag1\" of experiment \"group_etag2\" for user \"blah\".");
494+
String.format("Failed to save variation \"%s\" of experiment \"%s\" for user \"blah\".",
495+
variation.getId(), groupExperiment.getId()));
494496

495-
verify(userProfile).save("blah", groupExperiment.getKey(), variation.getKey());
497+
verify(userProfile).save("blah", groupExperiment.getId(), variation.getId());
496498
}
497499

498500
/**
@@ -510,15 +512,15 @@ public void bucketUserNotInOverlappingGroupExperiment() throws Exception {
510512
Experiment groupExperiment = groupExperiments.get(0);
511513
final Variation variation = groupExperiment.getVariations().get(0);
512514

513-
when(userProfile.lookup("blah", groupExperiment.getKey())).thenReturn(variation.getKey());
515+
when(userProfile.lookup("blah", groupExperiment.getId())).thenReturn(variation.getId());
514516

515517
assertThat(algorithm.bucket(groupExperiment, "blah"), is(variation));
516518

517519
logbackVerifier.expectMessage(Level.INFO,
518520
"Returning previously activated variation \"e2_vtag1\" of experiment \"group_etag2\""
519521
+ " for user \"blah\" from user profile.");
520522

521-
verify(userProfile).lookup("blah", groupExperiment.getKey());
523+
verify(userProfile).lookup("blah", groupExperiment.getId());
522524
}
523525

524526
/**
@@ -536,13 +538,13 @@ public void bucketUserNotInOverlappingGroupExperiment() throws Exception {
536538
Experiment groupExperiment = groupExperiments.get(0);
537539
final Variation variation = groupExperiment.getVariations().get(0);
538540

539-
when(userProfile.lookup("blah", groupExperiment.getKey())).thenReturn(null);
541+
when(userProfile.lookup("blah", groupExperiment.getId())).thenReturn(null);
540542

541543
assertThat(algorithm.bucket(groupExperiment, "blah"), is(variation));
542544

543545
logbackVerifier.expectMessage(Level.INFO, "No previously activated variation of experiment " +
544546
"\"group_etag2\" for user \"blah\" found in user profile.");
545-
verify(userProfile).lookup("blah", groupExperiment.getKey());
547+
verify(userProfile).lookup("blah", groupExperiment.getId());
546548
}
547549

548550
/**

0 commit comments

Comments
 (0)