Skip to content

Commit bd9ba4a

Browse files
authored
Refactor ProjectValidationUtils to ExperimentUtils (#91)
* refactored ProjectValidationUtils to ExperimentUtils. Then put back a deprecated ExperimentUtils.java file * totally revamp the experiment util tests
1 parent b4bebde commit bd9ba4a

File tree

8 files changed

+272
-109
lines changed

8 files changed

+272
-109
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import com.optimizely.ab.event.internal.EventBuilderV2;
3939
import com.optimizely.ab.event.internal.payload.Event.ClientEngine;
4040
import com.optimizely.ab.internal.EventTagUtils;
41-
import com.optimizely.ab.internal.ProjectValidationUtils;
41+
import com.optimizely.ab.internal.ExperimentUtils;
4242
import com.optimizely.ab.internal.ReservedEventKey;
4343
import com.optimizely.ab.internal.UserProfileUtils;
4444
import com.optimizely.ab.notification.NotificationBroadcaster;
@@ -460,7 +460,7 @@ public void track(@Nonnull String eventName,
460460
// attributes.
461461
attributes = filterAttributes(projectConfig, attributes);
462462

463-
if (!ProjectValidationUtils.validatePreconditions(projectConfig, userProfile, experiment, userId, attributes)) {
463+
if (!ExperimentUtils.isExperimentActive(experiment)) {
464464
return null;
465465
}
466466

@@ -478,7 +478,7 @@ public void track(@Nonnull String eventName,
478478
return variation;
479479
}
480480

481-
if (ProjectValidationUtils.isUserInExperiment(projectConfig, experiment, attributes)) {
481+
if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, attributes)) {
482482
return bucketer.bucket(experiment, userId);
483483
}
484484
logger.info("User \"{}\" does not meet conditions to be in experiment \"{}\".", userId, experiment.getKey());
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
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.internal;
18+
19+
import com.optimizely.ab.config.Experiment;
20+
import com.optimizely.ab.config.ProjectConfig;
21+
import com.optimizely.ab.config.audience.Condition;
22+
import org.slf4j.Logger;
23+
import org.slf4j.LoggerFactory;
24+
25+
import javax.annotation.Nonnull;
26+
import java.util.List;
27+
import java.util.Map;
28+
29+
public final class ExperimentUtils {
30+
31+
private static final Logger logger = LoggerFactory.getLogger(ExperimentUtils.class);
32+
33+
private ExperimentUtils() {}
34+
35+
/**
36+
* Helper method to validate all pre-conditions before bucketing a user.
37+
*
38+
* @param experiment the experiment we are validating pre-conditions for
39+
* @return whether the pre-conditions are satisfied
40+
*/
41+
public static boolean isExperimentActive(@Nonnull Experiment experiment) {
42+
43+
if (!experiment.isActive()) {
44+
logger.info("Experiment \"{}\" is not running.", experiment.getKey());
45+
return false;
46+
}
47+
48+
return true;
49+
}
50+
51+
/**
52+
* Determines whether a user satisfies audience conditions for the experiment.
53+
*
54+
* @param projectConfig the current projectConfig
55+
* @param experiment the experiment we are evaluating audiences for
56+
* @param attributes the attributes of the user
57+
* @return whether the user meets the criteria for the experiment
58+
*/
59+
public static boolean isUserInExperiment(@Nonnull ProjectConfig projectConfig,
60+
@Nonnull Experiment experiment,
61+
@Nonnull Map<String, String> attributes) {
62+
List<String> experimentAudienceIds = experiment.getAudienceIds();
63+
64+
// if there are no audiences, ALL users should be part of the experiment
65+
if (experimentAudienceIds.isEmpty()) {
66+
return true;
67+
}
68+
69+
// if there are audiences, but no user attributes, the user is not in the experiment.
70+
if (attributes.isEmpty()) {
71+
return false;
72+
}
73+
74+
for (String audienceId : experimentAudienceIds) {
75+
Condition conditions = projectConfig.getAudienceConditionsFromId(audienceId);
76+
if (conditions.evaluate(attributes)) {
77+
return true;
78+
}
79+
}
80+
81+
return false;
82+
}
83+
}

core-api/src/main/java/com/optimizely/ab/internal/ProjectValidationUtils.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.List;
2828
import java.util.Map;
2929

30+
@Deprecated
3031
public final class ProjectValidationUtils {
3132

3233
private static final Logger logger = LoggerFactory.getLogger(ProjectValidationUtils.class);
@@ -42,10 +43,13 @@ private ProjectValidationUtils() {}
4243
* @param attributes the attributes of the user
4344
* @return whether the pre-conditions are satisfied
4445
*/
46+
@Deprecated
4547
public static boolean validatePreconditions(ProjectConfig projectConfig, UserProfile userProfile,
4648
Experiment experiment, @Nonnull String userId,
4749
Map<String, String> attributes) {
4850

51+
logger.info("ProjectValidationUtils#validatePreconditions is now deprecated. Please use ExperimentUtils.isExperimentActive");
52+
4953
if (!experiment.isActive()) {
5054
logger.info("Experiment \"{}\" is not running.", experiment.getKey(), userId);
5155
return false;
@@ -62,8 +66,11 @@ public static boolean validatePreconditions(ProjectConfig projectConfig, UserPro
6266
* @param attributes the attributes of the user
6367
* @return whether the user meets the criteria for the experiment
6468
*/
69+
@Deprecated
6570
public static boolean isUserInExperiment(ProjectConfig projectConfig, Experiment experiment,
6671
Map<String, String> attributes) {
72+
logger.info("ProjectValidationUtils#isUserInExperiment is now deprecated. Please use ExperimentUtils.isUserInExperiment");
73+
6774
List<String> experimentAudienceIds = experiment.getAudienceIds();
6875

6976
// if there are no audiences, ALL users should be part of the experiment

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import com.optimizely.ab.event.internal.EventBuilder;
3636
import com.optimizely.ab.event.internal.EventBuilderV2;
3737
import com.optimizely.ab.internal.LogbackVerifier;
38-
import com.optimizely.ab.internal.ProjectValidationUtils;
38+
import com.optimizely.ab.internal.ExperimentUtils;
3939
import com.optimizely.ab.internal.ReservedEventKey;
4040
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
4141
import org.junit.BeforeClass;
@@ -785,15 +785,13 @@ public void trackEventEndToEnd() throws Exception {
785785
.thenReturn(experiment.getVariations().get(0));
786786
}
787787

788-
Map<String, String> emptyAttributes = Collections.emptyMap();
789-
790788
// call track
791789
optimizely.track(eventType.getKey(), "userId");
792790

793791
// verify that the bucketing algorithm was called only on experiments corresponding to the specified goal.
794792
List<Experiment> experimentsForEvent = noAudienceProjectConfig.getExperimentsForEventKey(eventType.getKey());
795793
for (Experiment experiment : allExperiments) {
796-
if (ProjectValidationUtils.validatePreconditions(noAudienceProjectConfig, null, experiment, "userId", emptyAttributes) &&
794+
if (ExperimentUtils.isExperimentActive(experiment) &&
797795
experimentsForEvent.contains(experiment)) {
798796
verify(mockBucketer).bucket(experiment, "userId");
799797
} else {

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
import com.optimizely.ab.event.internal.EventBuilder;
3737
import com.optimizely.ab.event.internal.EventBuilderV2;
3838
import com.optimizely.ab.internal.LogbackVerifier;
39-
import com.optimizely.ab.internal.ProjectValidationUtils;
39+
import com.optimizely.ab.internal.ExperimentUtils;
4040
import com.optimizely.ab.internal.ReservedEventKey;
4141
import com.optimizely.ab.notification.NotificationListener;
4242
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -761,15 +761,13 @@ public void trackEventEndToEnd() throws Exception {
761761
.thenReturn(experiment.getVariations().get(0));
762762
}
763763

764-
Map<String, String> emptyAttributes = Collections.emptyMap();
765-
766764
// call track
767765
optimizely.track(eventType.getKey(), "userId");
768766

769767
// verify that the bucketing algorithm was called only on experiments corresponding to the specified goal.
770768
List<Experiment> experimentsForEvent = noAudienceProjectConfig.getExperimentsForEventKey(eventType.getKey());
771769
for (Experiment experiment : allExperiments) {
772-
if (ProjectValidationUtils.validatePreconditions(noAudienceProjectConfig, null, experiment, "userId", emptyAttributes) &&
770+
if (ExperimentUtils.isExperimentActive(experiment) &&
773771
experimentsForEvent.contains(experiment)) {
774772
verify(mockBucketer).bucket(experiment, "userId");
775773
} else {

core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import com.optimizely.ab.event.internal.payload.Feature;
3333
import com.optimizely.ab.event.internal.payload.Impression;
3434
import com.optimizely.ab.event.internal.payload.LayerState;
35-
import com.optimizely.ab.internal.ProjectValidationUtils;
35+
import com.optimizely.ab.internal.ExperimentUtils;
3636
import com.optimizely.ab.internal.ReservedEventKey;
3737
import org.junit.BeforeClass;
3838
import org.junit.Test;
@@ -228,7 +228,7 @@ public void createConversionEvent() throws Exception {
228228
List<LayerState> expectedLayerStates = new ArrayList<LayerState>();
229229

230230
for (Experiment experiment : experimentsForEventKey) {
231-
if (ProjectValidationUtils.validatePreconditions(validProjectConfig, null, experiment, userId, attributeMap)) {
231+
if (ExperimentUtils.isExperimentActive(experiment)) {
232232
LayerState layerState = new LayerState(experiment.getLayerId(), validProjectConfig.getRevision(),
233233
new Decision(experiment.getVariations().get(0).getId(), false, experiment.getId()), true);
234234
expectedLayerStates.add(layerState);
@@ -499,7 +499,7 @@ public static Map<Experiment, Variation> createExperimentVariationMap(ProjectCon
499499
List<Experiment> eventExperiments = projectConfig.getExperimentsForEventKey(eventName);
500500
Map<Experiment, Variation> experimentVariationMap = new HashMap<Experiment, Variation>(eventExperiments.size());
501501
for (Experiment experiment : eventExperiments) {
502-
if (ProjectValidationUtils.validatePreconditions(projectConfig, userProfile, experiment, userId, attributes)
502+
if (ExperimentUtils.isExperimentActive(experiment)
503503
&& experiment.isRunning()) {
504504
Variation variation = bucketer.bucket(experiment, userId);
505505
if (variation != null) {
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
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.internal;
18+
19+
import com.optimizely.ab.config.audience.Audience;
20+
import com.optimizely.ab.config.audience.Condition;
21+
import com.optimizely.ab.config.Experiment;
22+
import com.optimizely.ab.config.Experiment.ExperimentStatus;
23+
import com.optimizely.ab.config.ProjectConfig;
24+
import com.optimizely.ab.config.TrafficAllocation;
25+
import com.optimizely.ab.config.Variation;
26+
import org.junit.BeforeClass;
27+
import org.junit.Test;
28+
29+
import java.io.IOException;
30+
import java.util.Collections;
31+
import java.util.Map;
32+
33+
import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigV2;
34+
import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV2;
35+
import static com.optimizely.ab.internal.ExperimentUtils.isExperimentActive;
36+
import static com.optimizely.ab.internal.ExperimentUtils.isUserInExperiment;
37+
import static org.junit.Assert.assertFalse;
38+
import static org.junit.Assert.assertTrue;
39+
40+
/**
41+
* Test the Experiment Utils methods.
42+
*/
43+
public class ExperimentUtilsTest {
44+
45+
private static ProjectConfig projectConfig;
46+
private static ProjectConfig noAudienceProjectConfig;
47+
48+
@BeforeClass
49+
public static void setUp() throws IOException {
50+
projectConfig = validProjectConfigV2();
51+
noAudienceProjectConfig = noAudienceProjectConfigV2();
52+
}
53+
54+
/**
55+
* If the {@link Experiment#status} is {@link ExperimentStatus#RUNNING},
56+
* then {@link ExperimentUtils#isExperimentActive(Experiment)} should return true.
57+
*/
58+
@Test
59+
public void isExperimentActiveReturnsTrueWhenTheExperimentIsRunning() {
60+
Experiment mockExperiment = makeMockExperimentWithStatus(ExperimentStatus.RUNNING);
61+
62+
assertTrue(isExperimentActive(mockExperiment));
63+
}
64+
65+
/**
66+
* If the {@link Experiment#status} is {@link ExperimentStatus#LAUNCHED},
67+
* then {@link ExperimentUtils#isExperimentActive(Experiment)} should return true.
68+
*/
69+
@Test
70+
public void isExperimentActiveReturnsTrueWhenTheExperimentIsLaunched() {
71+
Experiment mockExperiment = makeMockExperimentWithStatus(ExperimentStatus.LAUNCHED);
72+
73+
assertTrue(isExperimentActive(mockExperiment));
74+
}
75+
76+
/**
77+
* If the {@link Experiment#status} is {@link ExperimentStatus#PAUSED},
78+
* then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false.
79+
*/
80+
@Test
81+
public void isExperimentActiveReturnsFalseWhenTheExperimentIsPaused() {
82+
Experiment mockExperiment = makeMockExperimentWithStatus(ExperimentStatus.PAUSED);
83+
84+
assertFalse(isExperimentActive(mockExperiment));
85+
}
86+
87+
/**
88+
* If the {@link Experiment#status} is {@link ExperimentStatus#ARCHIVED},
89+
* then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false.
90+
*/
91+
@Test
92+
public void isExperimentActiveReturnsFalseWhenTheExperimentIsArchived() {
93+
Experiment mockExperiment = makeMockExperimentWithStatus(ExperimentStatus.ARCHIVED);
94+
95+
assertFalse(isExperimentActive(mockExperiment));
96+
}
97+
98+
/**
99+
* If the {@link Experiment#status} is {@link ExperimentStatus#NOT_STARTED},
100+
* then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false.
101+
*/
102+
@Test
103+
public void isExperimentActiveReturnsFalseWhenTheExperimentIsNotStarted() {
104+
Experiment mockExperiment = makeMockExperimentWithStatus(ExperimentStatus.NOT_STARTED);
105+
106+
assertFalse(isExperimentActive(mockExperiment));
107+
}
108+
109+
/**
110+
* If the {@link Experiment} does not have any {@link Audience}s,
111+
* then {@link ExperimentUtils#isUserInExperiment(ProjectConfig, Experiment, Map)} should return true;
112+
*/
113+
@Test
114+
public void isUserInExperimentReturnsTrueIfExperimentHasNoAudiences() {
115+
Experiment experiment = noAudienceProjectConfig.getExperiments().get(0);
116+
117+
assertTrue(isUserInExperiment(noAudienceProjectConfig, experiment, Collections.<String, String>emptyMap()));
118+
}
119+
120+
/**
121+
* If the {@link Experiment} contains at least one {@link Audience}, but attributes is empty,
122+
* then {@link ExperimentUtils#isUserInExperiment(ProjectConfig, Experiment, Map)} should return false.
123+
*/
124+
@Test
125+
public void isUserInExperimentReturnsFalseIfExperimentHasAudiencesButUserHasNoAttributes() {
126+
Experiment experiment = projectConfig.getExperiments().get(0);
127+
128+
assertFalse(isUserInExperiment(projectConfig, experiment, Collections.<String, String>emptyMap()));
129+
}
130+
131+
/**
132+
* If the attributes satisfies at least one {@link Condition} in an {@link Audience} of the {@link Experiment},
133+
* then {@link ExperimentUtils#isUserInExperiment(ProjectConfig, Experiment, Map)} should return true.
134+
*/
135+
@Test
136+
public void isUserInExperimentReturnsTrueIfUserSatisfiesAnAudience() {
137+
Experiment experiment = projectConfig.getExperiments().get(0);
138+
Map<String, String> attributes = Collections.singletonMap("browser_type", "chrome");
139+
140+
assertTrue(isUserInExperiment(projectConfig, experiment, attributes));
141+
}
142+
143+
/**
144+
* If the attributes satisfies no {@link Condition} of any {@link Audience} of the {@link Experiment},
145+
* then {@link ExperimentUtils#isUserInExperiment(ProjectConfig, Experiment, Map)} should return false.
146+
*/
147+
@Test
148+
public void isUserInExperimentReturnsTrueIfUserDoesNotSatisfyAnyAudiences() {
149+
Experiment experiment = projectConfig.getExperiments().get(0);
150+
Map<String, String> attributes = Collections.singletonMap("browser_type", "firefox");
151+
152+
assertFalse(isUserInExperiment(projectConfig, experiment, attributes));
153+
}
154+
155+
/**
156+
* Helper method to create an {@link Experiment} object with the provided status.
157+
*
158+
* @param status What the desired {@link Experiment#status} should be.
159+
* @return The newly created {@link Experiment}.
160+
*/
161+
private Experiment makeMockExperimentWithStatus(ExperimentStatus status) {
162+
return new Experiment("12345",
163+
"mockExperimentKey",
164+
status.toString(),
165+
"layerId",
166+
Collections.<String>emptyList(),
167+
Collections.<Variation>emptyList(),
168+
Collections.<String, String>emptyMap(),
169+
Collections.<TrafficAllocation>emptyList()
170+
);
171+
}
172+
}

0 commit comments

Comments
 (0)