Fix for DynamicValue.Record field order mixed up by serialization (#284)

* Reproducer for Dynamic Record field order issue * Fix
zio · Jun 10, 2022 · 7fd45d2 · 7fd45d2
1 parent f3623b3
commit 7fd45d2
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 21 deletions.
diff --git a/tests/shared/src/test/scala/zio/schema/DynamicValueSpec.scala b/tests/shared/src/test/scala/zio/schema/DynamicValueSpec.scala
@@ -71,7 +71,7 @@ object DynamicValueSpec extends ZIOSpecDefault {
       },
       test("round-trip semiDynamic") {
         val gen = for {
-          schemaAndGen       <- SchemaGen.anyGenericRecordAndGen
+          schemaAndGen       <- SchemaGen.anyGenericRecordAndGen()
           (schema, valueGen) = schemaAndGen
           value              <- valueGen
         } yield schema -> value

diff --git a/tests/shared/src/test/scala/zio/schema/SchemaGen.scala b/tests/shared/src/test/scala/zio/schema/SchemaGen.scala
@@ -9,7 +9,7 @@ import zio.test.{ Gen, Sized }
 
 object SchemaGen {
 
-  val anyLabel: Gen[Sized, String] = Gen.alphaNumericStringBounded(1, 3)
+  val anyLabel: Gen[Sized, String] = Gen.alphaNumericStringBounded(1, 15)
 
   def anyStructure(
     schemaGen: Gen[Sized, Schema[_]]
@@ -25,9 +25,9 @@ object SchemaGen {
       }
     }
 
-  def anyStructure[A](schema: Schema[A]): Gen[Sized, Seq[Schema.Field[A]]] =
+  def anyStructure[A](schema: Schema[A], max: Int = 3): Gen[Sized, Seq[Schema.Field[A]]] =
     Gen
-      .setOfBounded(1, 3)(
+      .setOfBounded(1, max)(
         anyLabel.map(Schema.Field(_, schema))
       )
       .map(_.toSeq)
@@ -215,10 +215,10 @@ object SchemaGen {
 
   type GenericRecordAndGen = (Schema[ListMap[String, _]], Gen[Sized, ListMap[String, _]])
 
-  val anyGenericRecordAndGen: Gen[Sized, GenericRecordAndGen] =
+  def anyGenericRecordAndGen(maxFieldCount: Int = 3): Gen[Sized, GenericRecordAndGen] =
     for {
       (schema, gen) <- anyPrimitiveAndGen
-      structure     <- anyStructure(schema)
+      structure     <- anyStructure(schema, maxFieldCount)
     } yield {
       val valueGen = Gen
         .const(structure.map(_.label))
@@ -234,17 +234,17 @@ object SchemaGen {
 
   type RecordAndValue = (Schema[ListMap[String, _]], ListMap[String, _])
 
-  val anyRecordAndValue: Gen[Sized, RecordAndValue] =
+  def anyRecordAndValue(maxFieldCount: Int = 3): Gen[Sized, RecordAndValue] =
     for {
-      (schema, gen) <- anyGenericRecordAndGen
+      (schema, gen) <- anyGenericRecordAndGen(maxFieldCount)
       value         <- gen
     } yield schema -> value
 
   val anyRecordOfRecordsAndValue: Gen[Sized, RecordAndValue] =
     for {
-      (schema1, gen1) <- anyGenericRecordAndGen
-      (schema2, gen2) <- anyGenericRecordAndGen
-      (schema3, gen3) <- anyGenericRecordAndGen
+      (schema1, gen1) <- anyGenericRecordAndGen()
+      (schema2, gen2) <- anyGenericRecordAndGen()
+      (schema3, gen3) <- anyGenericRecordAndGen()
       keys            <- Gen.setOfN(3)(anyLabel).map(_.toSeq)
       (key1, value1)  <- Gen.const(keys(0)).zip(gen1)
       (key2, value2)  <- Gen.const(keys(1)).zip(gen2)

diff --git a/zio-schema-json/shared/src/test/scala/zio/schema/codec/JsonCodecSpec.scala b/zio-schema-json/shared/src/test/scala/zio/schema/codec/JsonCodecSpec.scala
@@ -288,7 +288,7 @@ object JsonCodecSpec extends ZIOSpecDefault {
       },
       test("of records") {
         check(for {
-          (left, a)       <- SchemaGen.anyRecordAndValue
+          (left, a)       <- SchemaGen.anyRecordAndValue()
           primitiveSchema <- SchemaGen.anyPrimitive
         } yield (Schema.EitherSchema(left, primitiveSchema), Left(a))) {
           case (schema, value) => assertEncodesThenDecodes(schema, value)
@@ -326,7 +326,7 @@ object JsonCodecSpec extends ZIOSpecDefault {
         }
       },
       test("of record") {
-        check(SchemaGen.anyRecordAndValue) {
+        check(SchemaGen.anyRecordAndValue()) {
           case (schema, value) =>
             assertEncodesThenDecodes(Schema.Optional(schema), Some(value)) &>
               assertEncodesThenDecodes(Schema.Optional(schema), None)
@@ -442,12 +442,12 @@ object JsonCodecSpec extends ZIOSpecDefault {
     ),
     suite("record")(
       test("any") {
-        check(SchemaGen.anyRecordAndValue) {
+        check(SchemaGen.anyRecordAndValue()) {
           case (schema, value) => assertEncodesThenDecodes(schema, value)
         }
       },
       test("minimal test case") {
-        SchemaGen.anyRecordAndValue.runHead.flatMap {
+        SchemaGen.anyRecordAndValue().runHead.flatMap {
           case Some((schema, value)) =>
             val key      = new String(Array('\u0007', '\n'))
             val embedded = Schema.record(Schema.Field(key, schema))
@@ -462,7 +462,7 @@ object JsonCodecSpec extends ZIOSpecDefault {
         }
       },
       test("of primitives") {
-        check(SchemaGen.anyRecordAndValue) {
+        check(SchemaGen.anyRecordAndValue()) {
           case (schema, value) => assertEncodesThenDecodes(schema, value)
         }
       },
@@ -616,7 +616,25 @@ object JsonCodecSpec extends ZIOSpecDefault {
             compare = compareRandomRecords
           )
         }
-      }
+      },
+      test("deserialized dynamic record converted to typed value") {
+        check(SchemaGen.anyRecordAndValue(maxFieldCount = 15)) {
+          case (schema, value) =>
+            val dyn = DynamicValue.fromSchemaAndValue(schema, value)
+            ZStream
+              .succeed(dyn)
+              .via(JsonCodec.encoder(Schema.dynamicValue))
+              .via(JsonCodec.decoder(Schema.dynamicValue))
+              .map(_.toTypedValue(schema))
+              .runHead
+              .map { result =>
+                val resultList = result.get.toOption.get.toList
+                assertTrue(
+                  resultList == value.toList
+                )
+              }
+        }
+      } @@ TestAspect.sized(1000) @@ TestAspect.samples(1000)
     )
   )
 

diff --git a/zio-schema/shared/src/main/scala/zio/schema/DynamicValue.scala b/zio-schema/shared/src/main/scala/zio/schema/DynamicValue.scala
@@ -2071,10 +2071,11 @@ private[schema] object DynamicValueSchema { self =>
   private val recordCase: Schema.Case[DynamicValue.Record, DynamicValue] =
     Schema.Case(
       "Record",
-      Schema.CaseClass1[Map[String, DynamicValue], DynamicValue.Record](
-        Schema.Field("values", Schema.defer(Schema.map(Schema.primitive[String], DynamicValueSchema()))),
-        map => DynamicValue.Record(ListMap(map.toSeq: _*)),
-        record => record.values
+      Schema.CaseClass1[Chunk[(String, DynamicValue)], DynamicValue.Record](
+        Schema
+          .Field("values", Schema.defer(Schema.chunk(Schema.tuple2(Schema.primitive[String], DynamicValueSchema())))),
+        chunk => DynamicValue.Record(ListMap(chunk.toSeq: _*)),
+        record => Chunk.fromIterable(record.values)
       ),
       _.asInstanceOf[DynamicValue.Record]
     )