Resilience against null fields (#78)

* Resilience against fields with null value * writeField helper also handle optional fields correctly * Use uint4 for test_parser's parseValue * Add parseObjectWithoutSkip and parseObjectSkipNullFields
status-im · Jan 17, 2024 · d9394dc · d9394dc
1 parent b14f5b5
commit d9394dc
Show file tree

Hide file tree

Showing 6 changed files with 199 additions and 21 deletions.
diff --git a/README.md b/README.md
@@ -98,6 +98,9 @@ type
 
 createJsonFlavor OptJson
 OptionalFields.useDefaultSerializationIn OptJson
+
+`omitOptionalFields` is used by the Writer to ignore fields with null value.
+`skipNullFields` is used by the Reader to ignore fields with null value.
 ```
 
 ## Decoder example
@@ -229,6 +232,8 @@ parseValue(r: var JsonReader, val: var JsonValueRef)
 parseArray(r: var JsonReader; body: untyped)
 parseArray(r: var JsonReader; idx: untyped; body: untyped)
 parseObject(r: var JsonReader, key: untyped, body: untyped)
+parseObjectWithoutSkip(r: var JsonReader, key: untyped, body: untyped)
+parseObjectSkipNullFields(r: var JsonReader, key: untyped, body: untyped)
 parseObjectCustomKey(r: var JsonReader, keyAction: untyped, body: untyped)
 parseJsonNode(r: var JsonReader): JsonNode
 skipSingleJsValue(r: var JsonReader)
@@ -242,6 +247,9 @@ beginRecord(w: var JsonWriter, T: type)
 beginRecord(w: var JsonWriter)
 endRecord(w: var JsonWriter)
 
+writeObject(w: var JsonWriter, T: type)
+writeObject(w: var JsonWriter)
+
 writeFieldName(w: var JsonWriter, name: string)
 writeField(w: var JsonWriter, name: string, value: auto)
 

diff --git a/json_serialization/parser.nim b/json_serialization/parser.nim
@@ -291,20 +291,14 @@ proc parseFloat*(r: var JsonReader, T: type SomeFloat): T
 
 proc parseAsString*(r: var JsonReader, val: var string)
        {.gcsafe, raises: [IOError, JsonReaderError].} =
-  mixin flavorSkipNullFields
-  type
-    Reader = typeof r
-    Flavor = Reader.Flavor
-  const skipNullFields = flavorSkipNullFields(Flavor)
-
   case r.tokKind
   of JsonValueKind.String:
     escapeJson(r.parseString(), val)
   of JsonValueKind.Number:
     r.lex.scanNumber(val)
     r.checkError
   of JsonValueKind.Object:
-    parseObjectImpl(r.lex, skipNullFields):
+    parseObjectImpl(r.lex, false):
       # initial action
       val.add '{'
     do: # closing action
@@ -399,6 +393,32 @@ template parseObject*(r: var JsonReader, key: untyped, body: untyped) =
   do: # error action
     r.raiseParserError()
 
+template parseObjectWithoutSkip*(r: var JsonReader, key: untyped, body: untyped) =
+  if r.tokKind != JsonValueKind.Object:
+    r.raiseParserError(errCurlyLeExpected)
+  parseObjectImpl(r.lex, false): discard # initial action
+  do: discard # closing action
+  do: discard # comma action
+  do: # key action
+    let key {.inject.} = r.parseString()
+  do: # value action
+    body
+  do: # error action
+    r.raiseParserError()
+
+template parseObjectSkipNullFields*(r: var JsonReader, key: untyped, body: untyped) =
+  if r.tokKind != JsonValueKind.Object:
+    r.raiseParserError(errCurlyLeExpected)
+  parseObjectImpl(r.lex, true): discard # initial action
+  do: discard # closing action
+  do: discard # comma action
+  do: # key action
+    let key {.inject.} = r.parseString()
+  do: # value action
+    body
+  do: # error action
+    r.raiseParserError()
+
 template parseObjectCustomKey*(r: var JsonReader, keyAction: untyped, body: untyped) =
   mixin flavorSkipNullFields
   type
@@ -432,12 +452,6 @@ proc readJsonNodeField(r: var JsonReader, field: var JsonNode)
   field = r.parseJsonNode()
 
 proc parseJsonNode(r: var JsonReader): JsonNode =
-  mixin flavorSkipNullFields
-  type
-    Reader = typeof r
-    Flavor = Reader.Flavor
-  const skipNullFields = flavorSkipNullFields(Flavor)
-
   case r.tokKind
   of JsonValueKind.String:
     result = JsonNode(kind: JString, str: r.parseString())
@@ -452,7 +466,7 @@ proc parseJsonNode(r: var JsonReader): JsonNode =
         r.toInt(val, typeof(result.num), JsonReaderFlag.portableInt in r.lex.flags))
   of JsonValueKind.Object:
     result = JsonNode(kind: JObject)
-    parseObjectImpl(r.lex, skipNullFields): discard # initial action
+    parseObjectImpl(r.lex, false): discard # initial action
     do: discard # closing action
     do: discard # comma action
     do: # key action

diff --git a/json_serialization/writer.nim b/json_serialization/writer.nim
@@ -44,6 +44,10 @@ proc beginRecord*(w: var JsonWriter, T: type)
 proc beginRecord*(w: var JsonWriter)
 proc writeValue*(w: var JsonWriter, value: auto) {.gcsafe, raises: [IOError].}
 
+# If it's an optional field, test for it's value before write something.
+# If it's non optional field, the field is always written.
+template shouldWriteObjectField*[FieldType](field: FieldType): bool = true
+
 template append(x: untyped) =
   write w.stream, x
 
@@ -78,11 +82,21 @@ proc writeFieldName*(w: var JsonWriter, name: string) =
 proc writeField*(
     w: var JsonWriter, name: string, value: auto) {.raises: [IOError].} =
   mixin writeValue
+  mixin flavorOmitsOptionalFields, shouldWriteObjectField
 
-  w.writeFieldName(name)
-  w.writeValue(value)
+  type
+    Writer = typeof w
+    Flavor = Writer.Flavor
 
-  w.state = AfterField
+  when flavorOmitsOptionalFields(Flavor):
+    if shouldWriteObjectField(value):
+      w.writeFieldName(name)
+      w.writeValue(value)
+      w.state = AfterField
+  else:
+    w.writeFieldName(name)
+    w.writeValue(value)
+    w.state = AfterField
 
 template fieldWritten*(w: var JsonWriter) =
   w.state = AfterField
@@ -149,17 +163,23 @@ proc writeIterable*(w: var JsonWriter, collection: auto) =
 proc writeArray*[T](w: var JsonWriter, elements: openArray[T]) =
   writeIterable(w, elements)
 
+template writeObject*(w: var JsonWriter, T: type, body: untyped) =
+  w.beginRecord(T)
+  body
+  w.endRecord()
+
+template writeObject*(w: var JsonWriter, body: untyped) =
+  w.beginRecord()
+  body
+  w.endRecord()
+
 # this construct catches `array[N, char]` which otherwise won't decompose into
 # openArray[char] - we treat any array-like thing-of-characters as a string in
 # the output
 template isStringLike(v: string|cstring|openArray[char]|seq[char]): bool = true
 template isStringLike[N](v: array[N, char]): bool = true
 template isStringLike(v: auto): bool = false
 
-# If it's an optional field, test for it's value before write something.
-# If it's non optional field, the field is always written.
-template shouldWriteObjectField*[FieldType](field: FieldType): bool = true
-
 template writeObjectField*[FieldType, RecordType](w: var JsonWriter,
                                                   record: RecordType,
                                                   fieldName: static string,

diff --git a/tests/test_parser.nim b/tests/test_parser.nim
@@ -15,10 +15,17 @@ import
   ../json_serialization/value_ops,
   ./utils
 
+createJsonFlavor NullFields,
+  skipNullFields = true
+
 func toReader(input: string): JsonReader[DefaultFlavor] =
   var stream = unsafeMemoryInput(input)
   JsonReader[DefaultFlavor].init(stream)
 
+func toReaderNullFields(input: string): JsonReader[NullFields] =
+  var stream = unsafeMemoryInput(input)
+  JsonReader[NullFields].init(stream)
+
 suite "Custom iterators":
   test "customIntValueIt":
     var value: int
@@ -243,6 +250,62 @@ suite "Public parser":
         if name notin allowedToFail:
           testParseAsString(fileName)
 
+  test "parseAsString of null fields":
+    var r = toReaderNullFields("""{"something":null, "bool":null, "string":null}""")
+    let res = r.parseAsString()
+    check res.string == """{"something":null,"bool":null,"string":null}"""
+
+    var y = toReader("""{"something":null, "bool":null, "string":null}""")
+    let yy = y.parseAsString()
+    check yy.string == """{"something":null,"bool":null,"string":null}"""
+
+  proc execParseObject(r: var JsonReader): int =
+    r.parseObject(key):
+      discard key
+      let val = r.parseAsString()
+      discard val
+      inc result
+
+  test "parseObject of null fields":
+    var r = toReaderNullFields("""{"something":null, "bool":true, "string":null}""")
+    check execParseObject(r) == 1
+
+    var y = toReader("""{"something":null,"bool":true,"string":"moon"}""")
+    check execParseObject(y) == 3
+
+    var z = toReaderNullFields("""{"something":null,"bool":true,"string":"moon"}""")
+    check execParseObject(z) == 2
+
+  test "parseJsonNode of null fields":
+    var r = toReaderNullFields("""{"something":null, "bool":true, "string":null}""")
+    let n = r.parseJsonNode()
+    check:
+      n["something"].kind == JNull
+      n["bool"].kind == JBool
+      n["string"].kind == JNull
+
+    var y = toReader("""{"something":null,"bool":true,"string":"moon"}""")
+    let z = y.parseJsonNode()
+    check:
+      z["something"].kind == JNull
+      z["bool"].kind == JBool
+      z["string"].kind == JString
+
+  test "parseValue of null fields":
+    var r = toReaderNullFields("""{"something":null, "bool":true, "string":null}""")
+    let n = r.parseValue(uint64)
+    check:
+      n["something"].kind == JsonValueKind.Null
+      n["bool"].kind == JsonValueKind.Bool
+      n["string"].kind == JsonValueKind.Null
+
+    var y = toReader("""{"something":null,"bool":true,"string":"moon"}""")
+    let z = y.parseValue(uint64)
+    check:
+      z["something"].kind == JsonValueKind.Null
+      z["bool"].kind == JsonValueKind.Bool
+      z["string"].kind == JsonValueKind.String
+
   test "JsonValueRef comparison":
     var x = JsonValueRef[uint64](kind: JsonValueKind.Null)
     var n = JsonValueRef[uint64](nil)

diff --git a/tests/test_reader.nim b/tests/test_reader.nim
@@ -14,10 +14,17 @@ import
   serialization,
   ../json_serialization/reader
 
+createJsonFlavor NullFields,
+  skipNullFields = true
+
 func toReader(input: string): JsonReader[DefaultFlavor] =
   var stream = unsafeMemoryInput(input)
   JsonReader[DefaultFlavor].init(stream)
 
+func toReaderNullFields(input: string): JsonReader[NullFields] =
+  var stream = unsafeMemoryInput(input)
+  JsonReader[NullFields].init(stream)
+
 const
   jsonText = """
 
@@ -171,3 +178,34 @@ suite "JsonReader basic test":
       val.`bool`.boolVal == true
       val.`null`.kind == JsonValueKind.Null
       val.`array`.string == """[true,567.89,"string in array",null,[123]]"""
+
+  proc execReadObjectFields(r: var JsonReader): int =
+    for key in r.readObjectFields():
+      let val = r.parseAsString()
+      discard val
+      inc result
+
+  test "readObjectFields of null fields":
+    var r = toReaderNullFields("""{"something":null, "bool":true, "string":null}""")
+    check execReadObjectFields(r) == 1
+
+    var y = toReader("""{"something":null,"bool":true,"string":"moon"}""")
+    check execReadObjectFields(y) == 3
+
+    var z = toReaderNullFields("""{"something":null,"bool":true,"string":"moon"}""")
+    check execReadObjectFields(z) == 2
+
+  proc execReadObject(r: var JsonReader): int =
+    for k, v in r.readObject(string, int):
+      inc result
+
+  test "readObjectFields of null fields":
+    var r = toReaderNullFields("""{"something":null, "bool":123, "string":null}""")
+    check execReadObject(r) == 1
+
+    expect JsonReaderError:
+      var y = toReader("""{"something":null,"bool":78,"string":345}""")
+      check execReadObject(y) == 3
+
+    var z = toReaderNullFields("""{"something":null,"bool":999,"string":100}""")
+    check execReadObject(z) == 2
diff --git a/tests/test_writer.nim b/tests/test_writer.nim
@@ -19,6 +19,11 @@ type
     b: Option[string]
     c: int
 
+  OWOF = object
+    a: Opt[int]
+    b: Option[string]
+    c: int
+
 createJsonFlavor YourJson,
   omitOptionalFields = false
 
@@ -28,6 +33,13 @@ createJsonFlavor MyJson,
 ObjectWithOptionalFields.useDefaultSerializationIn YourJson
 ObjectWithOptionalFields.useDefaultSerializationIn MyJson
 
+proc writeValue*(w: var JsonWriter, val: OWOF)
+                  {.gcsafe, raises: [IOError].} =
+  w.writeObject(OWOF):
+    w.writeField("a", val.a)
+    w.writeField("b", val.b)
+    w.writeField("c", val.c)
+
 suite "Test writer":
   test "stdlib option top level some YourJson":
     var val = some(123)
@@ -133,3 +145,26 @@ suite "Test writer":
 
     let yy = MyJson.encode(y)
     check yy.string == """{"c":999}"""
+
+  test "writeField with object with optional fields":
+    let x = OWOF(
+      a: Opt.some(123),
+      b: some("nano"),
+      c: 456,
+    )
+
+    let y = OWOF(
+      a: Opt.none(int),
+      b: none(string),
+      c: 999,
+    )
+
+    let xx = MyJson.encode(x)
+    check xx.string == """{"a":123,"b":"nano","c":456}"""
+    let yy = MyJson.encode(y)
+    check yy.string == """{"c":999}"""
+
+    let uu = YourJson.encode(x)
+    check uu.string == """{"a":123,"b":"nano","c":456}"""
+    let vv = YourJson.encode(y)
+    check vv.string == """{"a":null,"b":null,"c":999}"""