Skip to content

Commit 3157861

Browse files
mikeproeng37mikeproeng37
authored
fix(datafile-parsing): Prevent SDK from initializing if the datafile … (#205)
1 parent 9b5d6a8 commit 3157861

File tree

5 files changed

+230
-28
lines changed

5 files changed

+230
-28
lines changed

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

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -725,27 +725,6 @@ public boolean setForcedVariation(@Nonnull String experimentKey,
725725
return projectConfig;
726726
}
727727

728-
/**
729-
* @return a {@link ProjectConfig} instance given a json string
730-
*/
731-
private static ProjectConfig getProjectConfig(String datafile) throws ConfigParseException {
732-
if (datafile == null) {
733-
throw new ConfigParseException("Unable to parse null datafile.");
734-
}
735-
if (datafile.length() == 0) {
736-
throw new ConfigParseException("Unable to parse empty datafile.");
737-
}
738-
739-
ProjectConfig projectConfig = DefaultConfigParser.getInstance().parseProjectConfig(datafile);
740-
741-
if (projectConfig.getVersion().equals("1")) {
742-
throw new ConfigParseException("This version of the Java SDK does not support version 1 datafiles. " +
743-
"Please use a version 2 or 3 datafile with this SDK.");
744-
}
745-
746-
return projectConfig;
747-
}
748-
749728
@Nullable
750729
public UserProfileService getUserProfileService() {
751730
return userProfileService;
@@ -891,7 +870,9 @@ protected Builder withConfig(ProjectConfig projectConfig) {
891870

892871
public Optimizely build() throws ConfigParseException {
893872
if (projectConfig == null) {
894-
projectConfig = Optimizely.getProjectConfig(datafile);
873+
projectConfig = new ProjectConfig.Builder()
874+
.withDatafile(datafile)
875+
.build();
895876
}
896877

897878
if (bucketer == null) {

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

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,19 @@
2121
import com.optimizely.ab.UnknownExperimentException;
2222
import com.optimizely.ab.config.audience.Audience;
2323
import com.optimizely.ab.config.audience.Condition;
24+
import com.optimizely.ab.config.parser.ConfigParseException;
25+
import com.optimizely.ab.config.parser.DefaultConfigParser;
2426
import com.optimizely.ab.error.ErrorHandler;
2527
import com.optimizely.ab.error.NoOpErrorHandler;
2628
import com.optimizely.ab.error.RaiseExceptionErrorHandler;
27-
import com.optimizely.ab.internal.ControlAttribute;
2829
import org.slf4j.Logger;
2930
import org.slf4j.LoggerFactory;
3031

3132
import javax.annotation.CheckForNull;
3233
import javax.annotation.Nonnull;
3334
import javax.annotation.Nullable;
3435
import javax.annotation.concurrent.Immutable;
35-
import java.util.ArrayList;
36-
import java.util.Collections;
37-
import java.util.HashMap;
38-
import java.util.List;
39-
import java.util.Map;
36+
import java.util.*;
4037
import java.util.concurrent.ConcurrentHashMap;
4138

4239
/**
@@ -65,6 +62,12 @@ public String toString() {
6562
}
6663
}
6764

65+
private static final List<String> supportedVersions = Arrays.asList(
66+
Version.V2.version,
67+
Version.V3.version,
68+
Version.V4.version
69+
);
70+
6871
// logger
6972
private static final Logger logger = LoggerFactory.getLogger(ProjectConfig.class);
7073

@@ -599,4 +602,33 @@ public String toString() {
599602
", variationIdToExperimentMapping=" + variationIdToExperimentMapping +
600603
'}';
601604
}
605+
606+
public static class Builder {
607+
private String datafile;
608+
609+
public Builder withDatafile(String datafile) {
610+
this.datafile = datafile;
611+
return this;
612+
}
613+
614+
/**
615+
* @return a {@link ProjectConfig} instance given a JSON string datafile
616+
*/
617+
public ProjectConfig build() throws ConfigParseException{
618+
if (datafile == null) {
619+
throw new ConfigParseException("Unable to parse null datafile.");
620+
}
621+
if (datafile.isEmpty()) {
622+
throw new ConfigParseException("Unable to parse empty datafile.");
623+
}
624+
625+
ProjectConfig projectConfig = DefaultConfigParser.getInstance().parseProjectConfig(datafile);
626+
627+
if (!supportedVersions.contains(projectConfig.getVersion())) {
628+
throw new ConfigParseException("This version of the Java SDK does not support the given datafile version: " + projectConfig.getVersion());
629+
}
630+
631+
return projectConfig;
632+
}
633+
}
602634
}

‎core-api/src/test/java/com/optimizely/ab/config/ProjectConfigBuilderTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package com.optimizely.ab.config;
2+
3+
import com.optimizely.ab.config.parser.ConfigParseException;
4+
import org.junit.Rule;
5+
import org.junit.Test;
6+
import org.junit.rules.ExpectedException;
7+
8+
import static com.optimizely.ab.config.ProjectConfigTestUtils.invalidProjectConfigV5;
9+
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV4;
10+
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertNotNull;
12+
import static org.junit.Assert.assertNull;
13+
14+
/**
15+
* Tests for {@link com.optimizely.ab.config.ProjectConfig.Builder}.
16+
*/
17+
public class ProjectConfigBuilderTest {
18+
@Rule
19+
public ExpectedException thrown = ExpectedException.none();
20+
21+
@Test
22+
public void withNullDatafile() throws Exception {
23+
thrown.expect(ConfigParseException.class);
24+
new ProjectConfig.Builder()
25+
.withDatafile(null)
26+
.build();
27+
}
28+
29+
@Test
30+
public void withEmptyDatafile() throws Exception {
31+
thrown.expect(ConfigParseException.class);
32+
new ProjectConfig.Builder()
33+
.withDatafile("")
34+
.build();
35+
}
36+
37+
@Test
38+
public void withValidDatafile() throws Exception {
39+
ProjectConfig projectConfig = new ProjectConfig.Builder()
40+
.withDatafile(validConfigJsonV4())
41+
.build();
42+
assertNotNull(projectConfig);
43+
assertEquals("4", projectConfig.getVersion());
44+
}
45+
46+
@Test
47+
public void withUnsupportedDatafile() throws Exception {
48+
thrown.expect(ConfigParseException.class);
49+
new ProjectConfig.Builder()
50+
.withDatafile(invalidProjectConfigV5())
51+
.build();
52+
}
53+
}

‎core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,13 @@ public static ProjectConfig validProjectConfigV4() {
438438
return VALID_PROJECT_CONFIG_V4;
439439
}
440440

441+
/**
442+
* @return the expected {@link ProjectConfig} for the json produced by {@link #invalidProjectConfigV5()}
443+
*/
444+
public static String invalidProjectConfigV5() throws IOException {
445+
return Resources.toString(Resources.getResource("config/invalid-project-config-v5.json"), Charsets.UTF_8);
446+
}
447+
441448
/**
442449
* Asserts that the provided project configs are equivalent.
443450
*/

‎core-api/src/test/resources/config/invalid-project-config-v5.json‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
{
2+
"accountId": "789",
3+
"projectId": "1234",
4+
"version": "5",
5+
"revision": "42",
6+
"experiments": [
7+
{
8+
"id": "223",
9+
"key": "etag1",
10+
"status": "Running",
11+
"layerId": "1",
12+
"percentageIncluded": 9000,
13+
"audienceIds": [],
14+
"variations": [{
15+
"id": "276",
16+
"key": "vtag1",
17+
"variables": []
18+
}, {
19+
"id": "277",
20+
"key": "vtag2",
21+
"variables": []
22+
}],
23+
"forcedVariations": {
24+
"testUser1": "vtag1",
25+
"testUser2": "vtag2"
26+
},
27+
"trafficAllocation": [{
28+
"entityId": "276",
29+
"endOfRange": 3500
30+
}, {
31+
"entityId": "277",
32+
"endOfRange": 9000
33+
}]
34+
},
35+
{
36+
"id": "118",
37+
"key": "etag2",
38+
"status": "Not started",
39+
"layerId": "2",
40+
"audienceIds": [],
41+
"variations": [{
42+
"id": "278",
43+
"key": "vtag3",
44+
"variables": []
45+
}, {
46+
"id": "279",
47+
"key": "vtag4",
48+
"variables": []
49+
}],
50+
"forcedVariations": {},
51+
"trafficAllocation": [{
52+
"entityId": "278",
53+
"endOfRange": 4500
54+
}, {
55+
"entityId": "279",
56+
"endOfRange": 9000
57+
}]
58+
},
59+
{
60+
"id": "119",
61+
"key": "etag3",
62+
"status": "Launched",
63+
"layerId": "3",
64+
"audienceIds": [],
65+
"variations": [{
66+
"id": "280",
67+
"key": "vtag5"
68+
}, {
69+
"id": "281",
70+
"key": "vtag6"
71+
}],
72+
"forcedVariations": {},
73+
"trafficAllocation": [{
74+
"entityId": "280",
75+
"endOfRange": 5000
76+
}, {
77+
"entityId": "281",
78+
"endOfRange": 10000
79+
}]
80+
}
81+
],
82+
"groups": [],
83+
"audiences": [],
84+
"attributes": [
85+
{
86+
"id": "134",
87+
"key": "browser_type"
88+
}
89+
],
90+
"events": [
91+
{
92+
"id": "971",
93+
"key": "clicked_cart",
94+
"experimentIds": [
95+
"223"
96+
]
97+
},
98+
{
99+
"id": "098",
100+
"key": "Total Revenue",
101+
"experimentIds": [
102+
"223"
103+
]
104+
},
105+
{
106+
"id": "099",
107+
"key": "clicked_purchase",
108+
"experimentIds": [
109+
"118",
110+
"223"
111+
]
112+
},
113+
{
114+
"id": "100",
115+
"key": "launched_exp_event",
116+
"experimentIds": [
117+
"119"
118+
]
119+
},
120+
{
121+
"id": "101",
122+
"key": "event_with_launched_and_running_experiments",
123+
"experimentIds": [
124+
"119",
125+
"223"
126+
]
127+
}
128+
]
129+
}

0 commit comments

Comments
 (0)