From 7fd45d2b5cb3f68dced66c469a36a26dd9fa2987 Mon Sep 17 00:00:00 2001 From: Daniel Vigovszky Date: Fri, 10 Jun 2022 23:19:07 +0200 Subject: [PATCH] Fix for DynamicValue.Record field order mixed up by serialization (#284) * Reproducer for Dynamic Record field order issue * Fix --- .../scala/zio/schema/DynamicValueSpec.scala | 2 +- .../src/test/scala/zio/schema/SchemaGen.scala | 20 ++++++------- .../zio/schema/codec/JsonCodecSpec.scala | 30 +++++++++++++++---- .../main/scala/zio/schema/DynamicValue.scala | 9 +++--- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/tests/shared/src/test/scala/zio/schema/DynamicValueSpec.scala b/tests/shared/src/test/scala/zio/schema/DynamicValueSpec.scala index ccbc7af44..069f2e97f 100644 --- 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 index c2ff02922..41c07f7d1 100644 --- 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 index 8dac87f96..d8da01abf 100644 --- 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 index d0319945e..b0065df38 100644 --- 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] )