Skip to content

Commit d034f10

Browse files
thomaszurkan-optimizelythomaszurkan-optimizely
authored
(fix):fix gson parser if null in datafile (#258)
* fix gson parser if null in datafile * update for all parsers. use latest libs, setup for all parsers * added unit test for all parsers to handle null featureEnabled. Just in case * fix nits * remove jackson maven comment from gradle build for quickstart
1 parent f9de142 commit d034f10

File tree

15 files changed

+284
-36
lines changed

15 files changed

+284
-36
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ private static List<Variation> parseVariations(JsonArray variationJson, JsonDese
5858
String id = variationObject.get("id").getAsString();
5959
String key = variationObject.get("key").getAsString();
6060
Boolean featureEnabled = false;
61-
if (variationObject.has("featureEnabled"))
61+
if (variationObject.has("featureEnabled") && !variationObject.get("featureEnabled").isJsonNull()) {
6262
featureEnabled = variationObject.get("featureEnabled").getAsBoolean();
63+
}
6364

6465
List<LiveVariableUsageInstance> variableUsageInstances = null;
6566
// this is an existence check rather than a version check since it's difficult to pass data

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,9 @@ private List<Variation> parseVariations(JSONArray variationJson) {
210210
String key = variationObject.getString("key");
211211
Boolean featureEnabled = false;
212212

213-
if (variationObject.has("featureEnabled"))
213+
if (variationObject.has("featureEnabled") && !variationObject.isNull("featureEnabled")) {
214214
featureEnabled = variationObject.getBoolean("featureEnabled");
215+
}
215216

216217
List<LiveVariableUsageInstance> liveVariableUsageInstances = null;
217218
if (variationObject.has("variables")) {

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

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,17 @@
2626
import com.optimizely.ab.config.audience.OrCondition;
2727
import com.optimizely.ab.config.audience.UserAttribute;
2828
import org.json.simple.JSONObject;
29+
import org.slf4j.Logger;
30+
import org.slf4j.LoggerFactory;
2931

3032
import java.util.ArrayList;
3133
import java.util.List;
3234
import java.util.Map;
3335

3436
public class ConditionUtils {
3537

38+
static Logger logger = LoggerFactory.getLogger("ConditionUtil");
39+
3640
static public <T> Condition parseConditions(Class<T> clazz, Object object) throws InvalidAudienceCondition {
3741

3842
if (object instanceof List) {
@@ -45,43 +49,64 @@ static public <T> Condition parseConditions(Class<T> clazz, Object object) throw
4549
} else {
4650
throw new InvalidAudienceCondition(String.format("Expected AudienceIdCondition got %s", clazz.getCanonicalName()));
4751
}
48-
} else if (object instanceof LinkedTreeMap) { // gson
49-
if (clazz != UserAttribute.class) {
50-
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
51-
52-
}
52+
} else {
53+
try {
54+
if (object instanceof LinkedTreeMap) { // gson
55+
if (clazz != UserAttribute.class) {
56+
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
5357

54-
LinkedTreeMap<String, ?> conditionMap = (LinkedTreeMap<String, ?>) object;
55-
return new UserAttribute((String) conditionMap.get("name"), (String) conditionMap.get("type"),
56-
(String) conditionMap.get("match"), conditionMap.get("value"));
57-
} else if (object instanceof JSONObject) {
58-
if (clazz != UserAttribute.class) {
59-
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
58+
}
6059

60+
LinkedTreeMap<String, ?> conditionMap = (LinkedTreeMap<String, ?>) object;
61+
return new UserAttribute((String) conditionMap.get("name"), (String) conditionMap.get("type"),
62+
(String) conditionMap.get("match"), conditionMap.get("value"));
63+
}
64+
}
65+
catch (NoClassDefFoundError ex) {
66+
// no gson loaded... not sure we need to log this if they don't use gson.
67+
logger.debug("parser: gson library not loaded");
6168
}
6269

63-
JSONObject conditionMap = (JSONObject) object;
64-
return new UserAttribute((String) conditionMap.get("name"), (String) conditionMap.get("type"),
65-
(String) conditionMap.get("match"), conditionMap.get("value"));
66-
} else if (object instanceof org.json.JSONArray) {
67-
return ConditionUtils.<T>parseConditions(clazz, (org.json.JSONArray) object);
68-
} else if (object instanceof org.json.JSONObject) {
69-
if (clazz != UserAttribute.class) {
70-
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
70+
try {
71+
if (object instanceof JSONObject) { // simple json
72+
if (clazz != UserAttribute.class) {
73+
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
7174

75+
}
76+
77+
JSONObject conditionMap = (JSONObject) object;
78+
return new UserAttribute((String) conditionMap.get("name"), (String) conditionMap.get("type"),
79+
(String) conditionMap.get("match"), conditionMap.get("value"));
80+
}
81+
}
82+
catch (NoClassDefFoundError ex) {
83+
logger.debug("parser: simple json not found");
7284
}
73-
org.json.JSONObject conditionMap = (org.json.JSONObject) object;
74-
String match = null;
75-
Object value = null;
76-
if (conditionMap.has("match")) {
77-
match = (String) conditionMap.get("match");
85+
86+
try {
87+
if (object instanceof org.json.JSONArray) { // json
88+
return ConditionUtils.<T>parseConditions(clazz, (org.json.JSONArray) object);
89+
} else if (object instanceof org.json.JSONObject){ //json
90+
if (clazz != UserAttribute.class) {
91+
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
92+
93+
}
94+
org.json.JSONObject conditionMap = (org.json.JSONObject) object;
95+
String match = null;
96+
Object value = null;
97+
if (conditionMap.has("match")) {
98+
match = (String) conditionMap.get("match");
99+
}
100+
if (conditionMap.has("value")) {
101+
value = conditionMap.get("value");
102+
}
103+
return new UserAttribute((String) conditionMap.get("name"), (String) conditionMap.get("type"),
104+
match, value);
105+
}
78106
}
79-
if (conditionMap.has("value")) {
80-
value = conditionMap.get("value");
107+
catch (NoClassDefFoundError ex) {
108+
logger.debug("parser: json package not found.");
81109
}
82-
return new UserAttribute((String) conditionMap.get("name"), (String) conditionMap.get("type"),
83-
match, value);
84-
} else { // looking for audience conditions in audience
85110
if (clazz != UserAttribute.class) {
86111
throw new InvalidAudienceCondition(String.format("Expected UserAttributes got %s", clazz.getCanonicalName()));
87112

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,10 @@ public static String validConfigJsonV4() throws IOException {
429429
return Resources.toString(Resources.getResource("config/valid-project-config-v4.json"), Charsets.UTF_8);
430430
}
431431

432+
public static String nullFeatureEnabledConfigJsonV4() throws IOException {
433+
return Resources.toString(Resources.getResource("config/null-featureEnabled-config-v4.json"), Charsets.UTF_8);
434+
}
435+
432436
/**
433437
* @return the expected {@link ProjectConfig} for the json produced by {@link #validConfigJsonV2()} ()}
434438
*/

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.lang.reflect.Type;
3535
import java.util.List;
3636

37+
import static com.optimizely.ab.config.ProjectConfigTestUtils.nullFeatureEnabledConfigJsonV4;
3738
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV2;
3839
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV3;
3940
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV4;
@@ -78,6 +79,19 @@ public void parseProjectConfigV4() throws Exception {
7879
verifyProjectConfig(actual, expected);
7980
}
8081

82+
@Test
83+
public void parseNullFeatureEnabledProjectConfigV4() throws Exception {
84+
GsonConfigParser parser = new GsonConfigParser();
85+
ProjectConfig actual = parser.parseProjectConfig(nullFeatureEnabledConfigJsonV4());
86+
87+
assertNotNull(actual);
88+
89+
assertNotNull(actual.getExperiments());
90+
91+
assertNotNull(actual.getFeatureFlags());
92+
93+
}
94+
8195
@Test
8296
public void parseAudience() throws Exception {
8397
JsonObject jsonObject = new JsonObject();

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.junit.Test;
2929
import org.junit.rules.ExpectedException;
3030

31+
import static com.optimizely.ab.config.ProjectConfigTestUtils.nullFeatureEnabledConfigJsonV4;
3132
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV2;
3233
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV3;
3334
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV4;
@@ -72,6 +73,20 @@ public void parseProjectConfigV4() throws Exception {
7273
verifyProjectConfig(actual, expected);
7374
}
7475

76+
@Test
77+
public void parseNullFeatureEnabledProjectConfigV4() throws Exception {
78+
JacksonConfigParser parser = new JacksonConfigParser();
79+
ProjectConfig actual = parser.parseProjectConfig(nullFeatureEnabledConfigJsonV4());
80+
81+
assertNotNull(actual);
82+
83+
assertNotNull(actual.getExperiments());
84+
85+
assertNotNull(actual.getFeatureFlags());
86+
87+
}
88+
89+
7590
@Test
7691
public void parseAudience() throws Exception {
7792
String audienceString =

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.junit.Test;
3131
import org.junit.rules.ExpectedException;
3232

33+
import static com.optimizely.ab.config.ProjectConfigTestUtils.nullFeatureEnabledConfigJsonV4;
3334
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV2;
3435
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV4;
3536
import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV2;
@@ -74,6 +75,19 @@ public void parseProjectConfigV4() throws Exception {
7475
verifyProjectConfig(actual, expected);
7576
}
7677

78+
@Test
79+
public void parseNullFeatureEnabledProjectConfigV4() throws Exception {
80+
JsonConfigParser parser = new JsonConfigParser();
81+
ProjectConfig actual = parser.parseProjectConfig(nullFeatureEnabledConfigJsonV4());
82+
83+
assertNotNull(actual);
84+
85+
assertNotNull(actual.getExperiments());
86+
87+
assertNotNull(actual.getFeatureFlags());
88+
89+
}
90+
7791
@Test
7892
public void parseAudience() throws Exception {
7993
JSONObject jsonObject = new JSONObject();

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.junit.Test;
3131
import org.junit.rules.ExpectedException;
3232

33+
import static com.optimizely.ab.config.ProjectConfigTestUtils.nullFeatureEnabledConfigJsonV4;
3334
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV2;
3435
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV4;
3536
import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV2;
@@ -74,6 +75,19 @@ public void parseProjectConfigV4() throws Exception {
7475
verifyProjectConfig(actual, expected);
7576
}
7677

78+
@Test
79+
public void parseNullFeatureEnabledProjectConfigV4() throws Exception {
80+
JsonSimpleConfigParser parser = new JsonSimpleConfigParser();
81+
ProjectConfig actual = parser.parseProjectConfig(nullFeatureEnabledConfigJsonV4());
82+
83+
assertNotNull(actual);
84+
85+
assertNotNull(actual.getExperiments());
86+
87+
assertNotNull(actual.getFeatureFlags());
88+
89+
}
90+
7791
@Test
7892
public void parseAudience() throws Exception {
7993
JSONObject jsonObject = new JSONObject();

0 commit comments

Comments
 (0)