From e5b18fb710c3d0167ec79f3b892f5a7a1bc6d1a4 Mon Sep 17 00:00:00 2001 From: Tanguy Date: Fri, 15 Jul 2022 12:23:35 +0200 Subject: [PATCH] bugfix: a leading field with a 'none' value was producing an incorrect encoding (#50) The field was omitted, but not the comma following it, resulting in an encoding such as '{, otherFields: ...}' --- json_serialization/reader.nim | 2 +- json_serialization/std/options.nim | 4 +- json_serialization/stew/results.nim | 4 +- json_serialization/writer.nim | 7 +-- tests/test_serialization.nim | 79 ++++++++++++++++++++++++++--- 5 files changed, 84 insertions(+), 12 deletions(-) diff --git a/json_serialization/reader.nim b/json_serialization/reader.nim index 00d7f6e..ba39bdc 100644 --- a/json_serialization/reader.nim +++ b/json_serialization/reader.nim @@ -418,7 +418,7 @@ func expectedFieldsBitmask*(TT: type): auto {.compileTime.} = enumAllSerializedFields const requiredWords = - (totalExpectedFields(T) + bitsPerWord - 1) div bitsPerWord + (totalSerializedFields(T) + bitsPerWord - 1) div bitsPerWord var res: array[requiredWords, uint] diff --git a/json_serialization/std/options.nim b/json_serialization/std/options.nim index 99c596b..414152a 100644 --- a/json_serialization/std/options.nim +++ b/json_serialization/std/options.nim @@ -4,11 +4,13 @@ export options template writeObjectField*(w: var JsonWriter, record: auto, fieldName: static string, - field: Option) = + field: Option): bool = mixin writeObjectField if field.isSome: writeObjectField(w, record, fieldName, field.get) + else: + false proc writeValue*(writer: var JsonWriter, value: Option) = mixin writeValue diff --git a/json_serialization/stew/results.nim b/json_serialization/stew/results.nim index 147d637..ad958e1 100644 --- a/json_serialization/stew/results.nim +++ b/json_serialization/stew/results.nim @@ -7,11 +7,13 @@ export template writeObjectField*[T](w: var JsonWriter, record: auto, fieldName: static string, - field: Result[T, void]) = + field: Result[T, void]): bool = mixin writeObjectField if field.isOk: writeObjectField(w, record, fieldName, field.get) + else: + false proc writeValue*[T](writer: var JsonWriter, value: Result[T, void]) = mixin writeValue diff --git a/json_serialization/writer.nim b/json_serialization/writer.nim index 6938a33..dabb89c 100644 --- a/json_serialization/writer.nim +++ b/json_serialization/writer.nim @@ -148,7 +148,7 @@ template isStringLike(v: auto): bool = false template writeObjectField*[FieldType, RecordType](w: var JsonWriter, record: RecordType, fieldName: static string, - field: FieldType) = + field: FieldType): bool = mixin writeFieldIMPL, writeValue type @@ -159,6 +159,7 @@ template writeObjectField*[FieldType, RecordType](w: var JsonWriter, w.writeValue(field) else: w.writeFieldIMPL(FieldTag[R, fieldName], field, record) + true proc writeValue*(w: var JsonWriter, value: auto) = mixin enumInstanceSerializedFields, writeValue @@ -238,8 +239,8 @@ proc writeValue*(w: var JsonWriter, value: auto) = w.beginRecord RecordType value.enumInstanceSerializedFields(fieldName, field): mixin writeObjectField - writeObjectField(w, value, fieldName, field) - w.state = AfterField + if writeObjectField(w, value, fieldName, field): + w.state = AfterField w.endRecord() else: diff --git a/tests/test_serialization.nim b/tests/test_serialization.nim index 45bac3d..094186e 100644 --- a/tests/test_serialization.nim +++ b/tests/test_serialization.nim @@ -74,13 +74,30 @@ type dup: bool HoldsResultOpt* = object - r*: ref Simple o*: Opt[Simple] + r*: ref Simple WithCustomFieldRule* = object str*: string intVal*: int + OtherOptionTest* = object + a*: Option[Meter] + b*: Option[Meter] + + NestedOptionTest* = object + c*: Option[OtherOptionTest] + d*: Option[OtherOptionTest] + + SeqOptionTest* = object + a*: seq[Option[Meter]] + b*: Meter + + OtherOptionTest2* = object + a*: Option[Meter] + b*: Option[Meter] + c*: Option[Meter] + var customVisit: TokenRegistry @@ -350,20 +367,70 @@ suite "toJson tests": let h1 = HoldsOption(o: some Simple(x: 1, y: "2", distance: Meter(3))) h2 = HoldsOption(r: newSimple(1, "2", Meter(3))) + h3 = Json.decode("""{"r":{"distance":3,"x":1,"y":"2"}}""", + HoldsOption, requireAllFields = true) Json.roundtripTest h1, """{"r":null,"o":{"distance":3,"x":1,"y":"2"}}""" Json.roundtripTest h2, """{"r":{"distance":3,"x":1,"y":"2"}}""" - let - h3 = Json.decode("""{"r":{"distance":3,"x":1,"y":"2"}}""", - HoldsOption, requireAllFields = true) - check h3 == h2 expect SerializationError: let h4 = Json.decode("""{"o":{"distance":3,"x":1,"y":"2"}}""", HoldsOption, requireAllFields = true) + test "Nested option types": + let + h3 = OtherOptionTest() + h4 = OtherOptionTest(a: some Meter(1)) + h5 = OtherOptionTest(b: some Meter(2)) + h6 = OtherOptionTest(a: some Meter(3), b: some Meter(4)) + + Json.roundtripTest h3, """{}""" + Json.roundtripTest h4, """{"a":1}""" + Json.roundtripTest h5, """{"b":2}""" + Json.roundtripTest h6, """{"a":3,"b":4}""" + + let + arr = @[some h3, some h4, some h5, some h6, none(OtherOptionTest)] + results = @[ + """{"c":{},"d":{}}""", + """{"c":{},"d":{"a":1}}""", + """{"c":{},"d":{"b":2}}""", + """{"c":{},"d":{"a":3,"b":4}}""", + """{"c":{}}""", + """{"c":{"a":1},"d":{}}""", + """{"c":{"a":1},"d":{"a":1}}""", + """{"c":{"a":1},"d":{"b":2}}""", + """{"c":{"a":1},"d":{"a":3,"b":4}}""", + """{"c":{"a":1}}""", + """{"c":{"b":2},"d":{}}""", + """{"c":{"b":2},"d":{"a":1}}""", + """{"c":{"b":2},"d":{"b":2}}""", + """{"c":{"b":2},"d":{"a":3,"b":4}}""", + """{"c":{"b":2}}""", + """{"c":{"a":3,"b":4},"d":{}}""", + """{"c":{"a":3,"b":4},"d":{"a":1}}""", + """{"c":{"a":3,"b":4},"d":{"b":2}}""", + """{"c":{"a":3,"b":4},"d":{"a":3,"b":4}}""", + """{"c":{"a":3,"b":4}}""", + """{"d":{}}""", + """{"d":{"a":1}}""", + """{"d":{"b":2}}""", + """{"d":{"a":3,"b":4}}""", + """{}""" + ] + + + var r = 0 + for a in arr: + for b in arr: + Json.roundtripTest NestedOptionTest(c: a, d: b), results[r] + r.inc + + Json.roundtripTest SeqOptionTest(a: @[some 5.Meter, none Meter], b: Meter(5)), """{"a":[5,null],"b":5}""" + Json.roundtripTest OtherOptionTest2(a: some 5.Meter, b: none Meter, c: some 10.Meter), """{"a":5,"c":10}""" + test "Result Opt types": check: false == static(isFieldExpected Opt[Simple]) @@ -374,7 +441,7 @@ suite "toJson tests": h1 = HoldsResultOpt(o: Opt[Simple].ok Simple(x: 1, y: "2", distance: Meter(3))) h2 = HoldsResultOpt(r: newSimple(1, "2", Meter(3))) - Json.roundtripTest h1, """{"r":null,"o":{"distance":3,"x":1,"y":"2"}}""" + Json.roundtripTest h1, """{"o":{"distance":3,"x":1,"y":"2"},"r":null}""" Json.roundtripTest h2, """{"r":{"distance":3,"x":1,"y":"2"}}""" let