Skip to content

Commit a079824

Browse files
kaushalkapasikaushalkapasi
authored
fix: improve json parsing error handling (#231)
* fix: allow the Feature to accept non-String values when parsing an incoming User Configuration from the API * fix: log a warning and return empty dictionaries if the Features or Variables map parsing fails * fix: throw an error if the sdk fails to parse features or variables from the user config * chore: add tests for extra properties at all levels of the User Config
1 parent 017899e commit a079824

File tree

4 files changed

+341
-11
lines changed

4 files changed

+341
-11
lines changed

‎DevCycle.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
/* Begin PBXBuildFile section */
1010
0B721685290B21CD004D0AB7 /* SSEMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B721684290B21CD004D0AB7 /* SSEMessage.swift */; };
1111
0F2DD75A279EFA6B00540C9D /* CustomData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0F2DD759279EFA6B00540C9D /* CustomData.swift */; };
12+
1E9007402DF36DDF00800012 /* test_config_eval_reason.json in Resources */ = {isa = PBXBuildFile; fileRef = 1E90073F2DF36DDF00800012 /* test_config_eval_reason.json */; };
1213
52133B2028DDFB260007691D /* RequestConsolidatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52133B1F28DDFB260007691D /* RequestConsolidatorTests.swift */; };
1314
52133B2228DE00BC0007691D /* ProcessConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52133B2128DE00BC0007691D /* ProcessConfig.swift */; };
1415
52133B2428DE0FEB0007691D /* GetTestConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52133B2328DE0FEB0007691D /* GetTestConfig.swift */; };
@@ -83,6 +84,7 @@
8384
/* Begin PBXFileReference section */
8485
0B721684290B21CD004D0AB7 /* SSEMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SSEMessage.swift; sourceTree = "<group>"; };
8586
0F2DD759279EFA6B00540C9D /* CustomData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomData.swift; sourceTree = "<group>"; };
87+
1E90073F2DF36DDF00800012 /* test_config_eval_reason.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = test_config_eval_reason.json; sourceTree = "<group>"; };
8688
52133B1F28DDFB260007691D /* RequestConsolidatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RequestConsolidatorTests.swift; sourceTree = "<group>"; };
8789
52133B2128DE00BC0007691D /* ProcessConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProcessConfig.swift; sourceTree = "<group>"; };
8890
52133B2328DE0FEB0007691D /* GetTestConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GetTestConfig.swift; sourceTree = "<group>"; };
@@ -206,6 +208,7 @@
206208
52A1139E27AB235C000B8285 /* EventQueueTests.swift */,
207209
5264A78F2768E9F400FEDB43 /* UserConfigTests.swift */,
208210
5264A78D2768E94500FEDB43 /* test_config.json */,
211+
1E90073F2DF36DDF00800012 /* test_config_eval_reason.json */,
209212
0B721684290B21CD004D0AB7 /* SSEMessage.swift */,
210213
);
211214
path = Models;
@@ -416,6 +419,7 @@
416419
buildActionMask = 2147483647;
417420
files = (
418421
5264A78E2768E94600FEDB43 /* test_config.json in Resources */,
422+
1E9007402DF36DDF00800012 /* test_config_eval_reason.json in Resources */,
419423
);
420424
runOnlyForDeploymentPostprocessing = 0;
421425
};

‎DevCycle/Models/UserConfig.swift‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ enum UserConfigError: Error {
1010
case MissingInConfig(String)
1111
case MissingProperty(String)
1212
case InvalidVariableType(String)
13+
case InvalidProperty(String)
1314
}
1415

1516
public struct UserConfig {
@@ -47,7 +48,7 @@ public struct UserConfig {
4748
let variableKeys = Array(variablesMap.keys)
4849

4950
for key in featureKeys {
50-
if let featureDict = featureMap[key] as? [String:String]
51+
if let featureDict = featureMap[key] as? [String:Any]
5152
{
5253
let feature = try Feature(from: featureDict)
5354
featureMap[key] = feature
@@ -62,8 +63,19 @@ public struct UserConfig {
6263
}
6364
}
6465

65-
self.features = featureMap as! [String: Feature]
66-
self.variables = variablesMap as! [String: Variable]
66+
if let features = featureMap as? [String: Feature] {
67+
self.features = features
68+
} else {
69+
Log.warn("Invalid feature map format", tags: ["config", "JSONParsing"])
70+
throw UserConfigError.InvalidProperty("features")
71+
}
72+
73+
if let variables = variablesMap as? [String: Variable] {
74+
self.variables = variables
75+
} else {
76+
Log.warn("Invalid variables map format", tags: ["config", "JSONParsing"])
77+
throw UserConfigError.InvalidProperty("variables")
78+
}
6779
}
6880
}
6981

@@ -133,20 +145,31 @@ public struct Feature {
133145
public var variationName: String
134146
public var evalReason: String?
135147

136-
init (from dictionary: [String: String]) throws {
137-
guard let id = dictionary["_id"] else { throw UserConfigError.MissingProperty("_id in Feature object") }
138-
guard let variation = dictionary["_variation"] else { throw UserConfigError.MissingProperty("_variation in Feature object") }
139-
guard let key = dictionary["key"] else { throw UserConfigError.MissingProperty("key in Feature object") }
140-
guard let type = dictionary["type"] else { throw UserConfigError.MissingProperty("type in Feature object") }
141-
guard let variationKey = dictionary["variationKey"] else { throw UserConfigError.MissingProperty("variationKey in Feature object") }
142-
guard let variationName = dictionary["variationName"] else { throw UserConfigError.MissingProperty("variationName in Feature object") }
148+
init (from dictionary: [String: Any]) throws {
149+
guard let id = dictionary["_id"] as? String else {
150+
throw UserConfigError.MissingProperty("String: _id in Feature object")
151+
}
152+
guard let variation = dictionary["_variation"] as? String else {
153+
throw UserConfigError.MissingProperty("String: _variation in Feature object") }
154+
guard let key = dictionary["key"] as? String else {
155+
throw UserConfigError.MissingProperty("String: key in Feature object")
156+
}
157+
guard let type = dictionary["type"] as? String else {
158+
throw UserConfigError.MissingProperty("String: type in Feature object")
159+
}
160+
guard let variationKey = dictionary["variationKey"] as? String else {
161+
throw UserConfigError.MissingProperty("String: variationKey in Feature object")
162+
}
163+
guard let variationName = dictionary["variationName"] as? String else {
164+
throw UserConfigError.MissingProperty("String: variationName in Feature object")
165+
}
143166
self._id = id
144167
self._variation = variation
145168
self.key = key
146169
self.type = type
147170
self.variationKey = variationKey
148171
self.variationName = variationName
149-
self.evalReason = dictionary["evalReason"]
172+
self.evalReason = dictionary["evalReason"] as? String
150173
}
151174
}
152175

‎DevCycleTests/Models/UserConfigTests.swift‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,228 @@ class UserConfigTests: XCTestCase {
123123
XCTAssertNotNil(json)
124124
XCTAssertNotNil(nestedJson)
125125
}
126+
127+
128+
func testSuccessfulConfigParsingWithNonStringValuesOnFeatures() throws {
129+
let data = getConfigData(name: "test_config_eval_reason")
130+
let dictionary = try JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed) as! [String:Any]
131+
let config = try UserConfig(from: dictionary)
132+
let features = config.features
133+
XCTAssertNotNil(config)
134+
XCTAssertNotNil(features)
135+
}
136+
137+
func testSuccessfulConfigParsingWithManyUnusedProperties() throws {
138+
let data = """
139+
{
140+
"project": {
141+
"_id": "id1",
142+
"key": "default",
143+
"settings": {
144+
"unused": "data"
145+
}
146+
},
147+
"environment": {
148+
"_id": "id2",
149+
"key": "development",
150+
"metadata": {
151+
"testing": "unused data"
152+
}
153+
},
154+
"features": {
155+
"new-feature": {
156+
"_id": "id3",
157+
"key": "new-feature",
158+
"type": "release",
159+
"_variation": "id4",
160+
"variationKey": "id4-key",
161+
"variationName": "id4 name",
162+
"eval": {
163+
"reason": "TARGETING_MATCH",
164+
"details": "Platform AND App Version"
165+
},
166+
"evalReason": "we don't do this anymore"
167+
}
168+
},
169+
"featureVariationMap": {
170+
"id3": "id4"
171+
},
172+
"knownVariableKeys": [],
173+
"variables": {
174+
"bool-var": {
175+
"_id": "id5",
176+
"key": "bool-var",
177+
"type": "Boolean",
178+
"value": true,
179+
"eval": {
180+
"reason": "TARGETING_MATCH",
181+
"details": "Platform AND App Version"
182+
}
183+
},
184+
"json-var": {
185+
"_id": "id6",
186+
"key": "json-var",
187+
"type": "JSON",
188+
"value": {
189+
"key1": "value1",
190+
"key2": {
191+
"nestedKey1": "nestedValue1"
192+
}
193+
},
194+
"eval": {
195+
"reason": "TARGETING_MATCH",
196+
"details": "Platform AND App Version"
197+
}
198+
},
199+
"string-var": {
200+
"_id": "id7",
201+
"key": "string-var",
202+
"type": "String",
203+
"value": "string1",
204+
"eval": {
205+
"reason": "TARGETING_MATCH",
206+
"details": "Platform AND App Version"
207+
},
208+
"evalReason": "we really don't do this anymore"
209+
},
210+
"num-var": {
211+
"_id": "id8",
212+
"key": "num-var",
213+
"type": "Number",
214+
"value": 4,
215+
"eval": {
216+
"reason": "TARGETING_MATCH",
217+
"details": "Platform AND App Version"
218+
}
219+
}
220+
},
221+
"sse": {
222+
"url": "https://example.com",
223+
"inactivityDelay": 5,
224+
"questionable": {
225+
"unused": ["values", "here"]
226+
}
227+
},
228+
"welcome": "to the future",
229+
"the_answer": 42,
230+
"hitchhiker": false
231+
}
232+
""".data(using: .utf8)!
233+
let dictionary =
234+
try JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed)
235+
as! [String: Any]
236+
let config = try UserConfig(from: dictionary)
237+
XCTAssertNotNil(config)
238+
XCTAssertNotNil(config.project)
239+
XCTAssertNotNil(config.environment)
240+
XCTAssertNotNil(config.variables)
241+
XCTAssertNotNil(config.featureVariationMap)
242+
XCTAssertNotNil(config.features)
243+
XCTAssertNotNil(config.sse)
244+
}
245+
246+
func testConfigWithFeatureKeyAsNumber() throws {
247+
// Feature feature-1 has a Number `key` instead of a String
248+
let data = """
249+
{
250+
"project": {
251+
"_id": "id1",
252+
"key": "default"
253+
},
254+
"environment": {
255+
"_id": "id2",
256+
"key": "development"
257+
},
258+
"features": {
259+
"feature-1": {
260+
"_id": "id3",
261+
"key": 1,
262+
"type": "release",
263+
"_variation": "id4",
264+
"variationKey": "id4-key",
265+
"variationName": "id4 name",
266+
}
267+
},
268+
"featureVariationMap": {},
269+
"knownVariableKeys": [],
270+
"variables": {
271+
"bool-var": {
272+
"_id": "id5",
273+
"key": "bool-var",
274+
"type": "Boolean",
275+
"value": true,
276+
"eval": {
277+
"reason": "TARGETING_MATCH",
278+
"details": "Platform AND App Version"
279+
}
280+
}
281+
},
282+
"sse": {
283+
"url": "https://example.com",
284+
"inactivityDelay": 5
285+
}
286+
}
287+
""".data(using: .utf8)!
288+
let dictionary =
289+
try JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed)
290+
as! [String: Any]
291+
do {
292+
_ = try UserConfig(from: dictionary)
293+
} catch {
294+
let stringError = String(describing: UserConfigError.MissingProperty("String: key in Feature object"))
295+
XCTAssertEqual(String(describing: error), stringError)
296+
}
297+
}
298+
299+
func testConfigWithVariableIdMissing() throws {
300+
// Variable bool-var is missing the `_id` property
301+
let data = """
302+
{
303+
"project": {
304+
"_id": "id1",
305+
"key": "default"
306+
},
307+
"environment": {
308+
"_id": "id2",
309+
"key": "development"
310+
},
311+
"features": {
312+
"feature-1": {
313+
"_id": "id3",
314+
"key": "feature-1",
315+
"type": "release",
316+
"_variation": "id4",
317+
"variationKey": "id4-key",
318+
"variationName": "id4 name",
319+
}
320+
},
321+
"featureVariationMap": {},
322+
"knownVariableKeys": [],
323+
"variables": {
324+
"bool-var": {
325+
"key": "bool-var",
326+
"type": "Boolean",
327+
"value": true,
328+
"eval": {
329+
"reason": "TARGETING_MATCH",
330+
"details": "Platform AND App Version"
331+
}
332+
}
333+
},
334+
"sse": {
335+
"url": "https://example.com",
336+
"inactivityDelay": 5
337+
}
338+
}
339+
""".data(using: .utf8)!
340+
let dictionary =
341+
try JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed)
342+
as! [String: Any]
343+
do {
344+
_ = try UserConfig(from: dictionary)
345+
} catch {
346+
let stringError = String(describing: UserConfigError.MissingProperty("_id in Variable object"))
347+
XCTAssertEqual(String(describing: error), stringError)
348+
}
349+
}
126350
}

0 commit comments

Comments
 (0)