Skip to content

Commit c453845

Browse files
authored
Create a Decision Service (#92)
* move decision logic to a dedicated decision service. * do not let decision service to be overriden. Adjust the copyright * Move tests to DecisionServiceTest.java. * User Profile save happens in decision service. * remove bucketer from the optimizely builder
1 parent 5a6c849 commit c453845

File tree

6 files changed

+586
-475
lines changed

6 files changed

+586
-475
lines changed

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

Lines changed: 118 additions & 118 deletions
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import javax.annotation.Nullable;
3131
import javax.annotation.concurrent.Immutable;
3232
import java.util.List;
33-
import java.util.Map;
3433

3534
/**
3635
* Default Optimizely bucketing algorithm that evenly distributes users using the Murmur3 hash of some provided
@@ -45,7 +44,6 @@
4544
public class Bucketer {
4645

4746
private final ProjectConfig projectConfig;
48-
private final UserProfile userProfile;
4947

5048
private static final Logger logger = LoggerFactory.getLogger(Bucketer.class);
5149

@@ -58,12 +56,7 @@ public class Bucketer {
5856
static final int MAX_TRAFFIC_VALUE = 10000;
5957

6058
public Bucketer(ProjectConfig projectConfig) {
61-
this(projectConfig, null);
62-
}
63-
64-
public Bucketer(ProjectConfig projectConfig, @Nullable UserProfile userProfile) {
6559
this.projectConfig = projectConfig;
66-
this.userProfile = userProfile;
6760
}
6861

6962
private String bucketToEntity(int bucketValue, List<TrafficAllocation> trafficAllocations) {
@@ -139,8 +132,6 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
139132
*/
140133
public @Nullable Variation bucket(@Nonnull Experiment experiment,
141134
@Nonnull String userId) {
142-
String experimentId = experiment.getId();
143-
144135
// ---------- Bucket User ----------
145136
String groupId = experiment.getGroupId();
146137
// check whether the experiment belongs to a group
@@ -165,85 +156,9 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
165156
}
166157
}
167158

168-
Variation bucketedVariation = bucketToVariation(experiment, userId);
169-
170-
// ---------- Save Variation to User Profile ----------
171-
// If a user profile is present give it a variation to store
172-
if (userProfile != null && bucketedVariation != null) {
173-
String bucketedVariationId = bucketedVariation.getId();
174-
boolean saved = userProfile.save(userId, experimentId, bucketedVariationId);
175-
if (saved) {
176-
logger.info("Saved variation \"{}\" of experiment \"{}\" for user \"{}\".",
177-
bucketedVariationId, experimentId, userId);
178-
} else {
179-
logger.warn("Failed to save variation \"{}\" of experiment \"{}\" for user \"{}\".",
180-
bucketedVariationId, experimentId, userId);
181-
}
182-
}
183-
184-
return bucketedVariation;
185-
}
186-
187-
/**
188-
* Get the variation the user has been whitelisted into.
189-
* @param experiment {@link Experiment} in which user is to be bucketed.
190-
* @param userId User Identifier
191-
* @return null if the user is not whitelisted into any variation
192-
* {@link Variation} the user is bucketed into if the user has a specified whitelisted variation.
193-
*/
194-
public @Nullable Variation getForcedVariation(@Nonnull Experiment experiment, @Nonnull String userId) {
195-
// if a user has a forced variation mapping, return the respective variation
196-
Map<String, String> userIdToVariationKeyMap = experiment.getUserIdToVariationKeyMap();
197-
if (userIdToVariationKeyMap.containsKey(userId)) {
198-
String forcedVariationKey = userIdToVariationKeyMap.get(userId);
199-
Variation forcedVariation = experiment.getVariationKeyToVariationMap().get(forcedVariationKey);
200-
if (forcedVariation != null) {
201-
logger.info("User \"{}\" is forced in variation \"{}\".", userId, forcedVariationKey);
202-
} else {
203-
logger.error("Variation \"{}\" is not in the datafile. Not activating user \"{}\".", forcedVariationKey,
204-
userId);
205-
}
206-
207-
return forcedVariation;
208-
}
209-
210-
return null;
159+
return bucketToVariation(experiment, userId);
211160
}
212161

213-
/**
214-
* Get the {@link Variation} that has been stored for the user in the {@link UserProfile} implementation.
215-
* @param experiment {@link Experiment} in which the user was bucketed.
216-
* @param userId User Identifier
217-
* @return null if the {@link UserProfile} implementation is null or the user was not previously bucketed.
218-
* else return the {@link Variation} the user was previously bucketed into.
219-
*/
220-
public @Nullable Variation getStoredVariation(@Nonnull Experiment experiment, @Nonnull String userId) {
221-
// ---------- Check User Profile for Sticky Bucketing ----------
222-
// If a user profile instance is present then check it for a saved variation
223-
String experimentId = experiment.getId();
224-
String experimentKey = experiment.getKey();
225-
if (userProfile != null) {
226-
String variationId = userProfile.lookup(userId, experimentId);
227-
if (variationId != null) {
228-
Variation savedVariation = projectConfig
229-
.getExperimentIdMapping()
230-
.get(experimentId)
231-
.getVariationIdToVariationMap()
232-
.get(variationId);
233-
logger.info("Returning previously activated variation \"{}\" of experiment \"{}\" "
234-
+ "for user \"{}\" from user profile.",
235-
savedVariation.getKey(), experimentKey, userId);
236-
// A variation is stored for this combined bucket id
237-
return savedVariation;
238-
} else {
239-
logger.info("No previously activated variation of experiment \"{}\" "
240-
+ "for user \"{}\" found in user profile.",
241-
experimentKey, userId);
242-
}
243-
}
244-
245-
return null;
246-
}
247162

248163
//======== Helper methods ========//
249164

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/**
2+
*
3+
* Copyright 2017, Optimizely and contributors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package com.optimizely.ab.bucketing;
18+
19+
import com.optimizely.ab.config.Experiment;
20+
import com.optimizely.ab.config.ProjectConfig;
21+
import com.optimizely.ab.config.Variation;
22+
import com.optimizely.ab.internal.ExperimentUtils;
23+
import org.slf4j.Logger;
24+
import org.slf4j.LoggerFactory;
25+
26+
import javax.annotation.Nonnull;
27+
import javax.annotation.Nullable;
28+
import java.util.Map;
29+
30+
/**
31+
* Optimizely's decision service that determines which variation of an experiment the user will be allocated to.
32+
*
33+
* The decision service contains all logic around how a user decision is made. This includes all of the following:
34+
* 1. Checking experiment status
35+
* 2. Checking whitelisting
36+
* 3. Checking sticky bucketing
37+
* 4. Checking audience targeting
38+
* 5. Using Murmurhash3 to bucket the user.
39+
*/
40+
public class DecisionService {
41+
42+
private final Bucketer bucketer;
43+
private final ProjectConfig projectConfig;
44+
private final UserProfile userProfile;
45+
private static final Logger logger = LoggerFactory.getLogger(DecisionService.class);
46+
47+
/**
48+
* Initialize a decision service for the Optimizely client.
49+
* @param bucketer Base bucketer to allocate new users to an experiment.
50+
* @param projectConfig Optimizely Project Config representing the datafile.
51+
* @param userProfile UserProfile implementation for storing decisions.
52+
*/
53+
public DecisionService(@Nonnull Bucketer bucketer,
54+
@Nonnull ProjectConfig projectConfig,
55+
@Nullable UserProfile userProfile) {
56+
this.bucketer = bucketer;
57+
this.projectConfig = projectConfig;
58+
this.userProfile = userProfile;
59+
}
60+
61+
/**
62+
* Get a {@link Variation} of an {@link Experiment} for a user to be allocated into.
63+
*
64+
* @param experiment The Experiment the user will be bucketed into.
65+
* @param userId The userId of the user.
66+
* @param filteredAttributes The user's attributes. This should be filtered to just attributes in the Datafile.
67+
* @return The {@link Variation} the user is allocated into.
68+
*/
69+
public @Nullable Variation getVariation(@Nonnull Experiment experiment,
70+
@Nonnull String userId,
71+
@Nonnull Map<String, String> filteredAttributes) {
72+
73+
if (!ExperimentUtils.isExperimentActive(experiment)) {
74+
return null;
75+
}
76+
77+
Variation variation;
78+
79+
// check for whitelisting
80+
variation = getWhitelistedVariation(experiment, userId);
81+
if (variation != null) {
82+
return variation;
83+
}
84+
85+
// check if user exists in user profile
86+
variation = getStoredVariation(experiment, userId);
87+
if (variation != null) {
88+
return variation;
89+
}
90+
91+
if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, filteredAttributes)) {
92+
Variation bucketedVariation = bucketer.bucket(experiment, userId);
93+
94+
if (bucketedVariation != null) {
95+
storeVariation(experiment, bucketedVariation, userId);
96+
}
97+
98+
return bucketedVariation;
99+
}
100+
logger.info("User \"{}\" does not meet conditions to be in experiment \"{}\".", userId, experiment.getKey());
101+
102+
return null;
103+
}
104+
105+
/**
106+
* Get the variation the user has been whitelisted into.
107+
* @param experiment {@link Experiment} in which user is to be bucketed.
108+
* @param userId User Identifier
109+
* @return null if the user is not whitelisted into any variation
110+
* {@link Variation} the user is bucketed into if the user has a specified whitelisted variation.
111+
*/
112+
@Nullable Variation getWhitelistedVariation(@Nonnull Experiment experiment, @Nonnull String userId) {
113+
// if a user has a forced variation mapping, return the respective variation
114+
Map<String, String> userIdToVariationKeyMap = experiment.getUserIdToVariationKeyMap();
115+
if (userIdToVariationKeyMap.containsKey(userId)) {
116+
String forcedVariationKey = userIdToVariationKeyMap.get(userId);
117+
Variation forcedVariation = experiment.getVariationKeyToVariationMap().get(forcedVariationKey);
118+
if (forcedVariation != null) {
119+
logger.info("User \"{}\" is forced in variation \"{}\".", userId, forcedVariationKey);
120+
} else {
121+
logger.error("Variation \"{}\" is not in the datafile. Not activating user \"{}\".", forcedVariationKey,
122+
userId);
123+
}
124+
125+
return forcedVariation;
126+
}
127+
128+
return null;
129+
}
130+
131+
/**
132+
* Get the {@link Variation} that has been stored for the user in the {@link UserProfile} implementation.
133+
* @param experiment {@link Experiment} in which the user was bucketed.
134+
* @param userId User Identifier
135+
* @return null if the {@link UserProfile} implementation is null or the user was not previously bucketed.
136+
* else return the {@link Variation} the user was previously bucketed into.
137+
*/
138+
@Nullable Variation getStoredVariation(@Nonnull Experiment experiment, @Nonnull String userId) {
139+
// ---------- Check User Profile for Sticky Bucketing ----------
140+
// If a user profile instance is present then check it for a saved variation
141+
String experimentId = experiment.getId();
142+
String experimentKey = experiment.getKey();
143+
if (userProfile != null) {
144+
String variationId = userProfile.lookup(userId, experimentId);
145+
if (variationId != null) {
146+
Variation savedVariation = projectConfig
147+
.getExperimentIdMapping()
148+
.get(experimentId)
149+
.getVariationIdToVariationMap()
150+
.get(variationId);
151+
logger.info("Returning previously activated variation \"{}\" of experiment \"{}\" "
152+
+ "for user \"{}\" from user profile.",
153+
savedVariation.getKey(), experimentKey, userId);
154+
// A variation is stored for this combined bucket id
155+
return savedVariation;
156+
} else {
157+
logger.info("No previously activated variation of experiment \"{}\" "
158+
+ "for user \"{}\" found in user profile.",
159+
experimentKey, userId);
160+
}
161+
}
162+
163+
return null;
164+
}
165+
166+
/**
167+
* Store a {@link Variation} of an {@link Experiment} for a user in the {@link UserProfile}.
168+
*
169+
* @param experiment The experiment the user was buck
170+
* @param variation The Variation to store.
171+
* @param userId The ID of the user.
172+
*/
173+
void storeVariation(@Nonnull Experiment experiment, @Nonnull Variation variation, @Nonnull String userId) {
174+
String experimentId = experiment.getId();
175+
// ---------- Save Variation to User Profile ----------
176+
// If a user profile is present give it a variation to store
177+
if (userProfile != null) {
178+
String bucketedVariationId = variation.getId();
179+
boolean saved = userProfile.save(userId, experimentId, bucketedVariationId);
180+
if (saved) {
181+
logger.info("Saved variation \"{}\" of experiment \"{}\" for user \"{}\".",
182+
bucketedVariationId, experimentId, userId);
183+
} else {
184+
logger.warn("Failed to save variation \"{}\" of experiment \"{}\" for user \"{}\".",
185+
bucketedVariationId, experimentId, userId);
186+
}
187+
}
188+
}
189+
}

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

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@
8787
import static org.mockito.Mockito.doThrow;
8888
import static org.mockito.Mockito.mock;
8989
import static org.mockito.Mockito.never;
90-
import static org.mockito.Mockito.spy;
9190
import static org.mockito.Mockito.times;
9291
import static org.mockito.Mockito.verify;
9392
import static org.mockito.Mockito.when;
@@ -1983,35 +1982,6 @@ public void getVariationForGroupExperimentWithNonMatchingAttributes() throws Exc
19831982
Collections.singletonMap("browser_type", "firefox")));
19841983
}
19851984

1986-
/**
1987-
* Verify that {@link Optimizely#getVariation(String, String, Map)} gives precedence to forced variation bucketing
1988-
* over user profile and audience evaluation for murmurhash3 bucketing.
1989-
*/
1990-
@Test
1991-
public void getVariationForcedVariationPrecedesUserProfileAndAudienceEval() throws Exception {
1992-
Bucketer bucketer = spy(new Bucketer(validProjectConfig));
1993-
Experiment experiment = validProjectConfig.getExperiments().get(0);
1994-
Variation expectedVariation = experiment.getVariations().get(0);
1995-
String whitelistedUserId = "testUser1";
1996-
1997-
Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler)
1998-
.withBucketing(bucketer)
1999-
.withConfig(validProjectConfig)
2000-
.build();
2001-
2002-
// user excluded without audiences and whitelisting
2003-
assertNull(optimizely.getVariation(experiment.getKey(), genericUserId));
2004-
2005-
logbackVerifier.expectMessage(Level.INFO, "User \"" + whitelistedUserId + "\" is forced in variation \"vtag1\".");
2006-
2007-
// no attributes provided for a experiment that has an audience
2008-
assertThat(optimizely.getVariation(experiment.getKey(), whitelistedUserId), is(expectedVariation));
2009-
2010-
verify(bucketer).getForcedVariation(experiment, whitelistedUserId);
2011-
verify(bucketer, never()).getStoredVariation(experiment, whitelistedUserId);
2012-
verify(bucketer, never()).bucket(experiment, whitelistedUserId);
2013-
}
2014-
20151985
/**
20161986
* Verify that {@link Optimizely#getVariation(String, String)} gives precedence to experiment status over forced
20171987
* variation bucketing.

0 commit comments

Comments
 (0)