From afa0271e33c24c83208cd63e34c80d9398cd4164 Mon Sep 17 00:00:00 2001 From: Andriy Plokhotnyuk Date: Thu, 9 Jan 2025 18:45:48 +0100 Subject: [PATCH] Fix remaining inconsistency in JSON codecs for `Schema.CaseClass*` and `Schema.GenericRecord` types --- .../scala/zio/schema/codec/JsonCodec.scala | 59 ++++++++++--------- .../zio/schema/codec/JsonCodecSpec.scala | 39 +++++++----- 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/zio-schema-json/shared/src/main/scala/zio/schema/codec/JsonCodec.scala b/zio-schema-json/shared/src/main/scala/zio/schema/codec/JsonCodec.scala index 0f337e6d5..192f69587 100644 --- a/zio-schema-json/shared/src/main/scala/zio/schema/codec/JsonCodec.scala +++ b/zio-schema-json/shared/src/main/scala/zio/schema/codec/JsonCodec.scala @@ -805,6 +805,9 @@ object JsonCodec { case Json.Null => DynamicValue.NoneValue } + private def error(msg: String, trace: List[JsonError]): Nothing = + throw UnsafeJson(JsonError.Message(msg) :: trace) + private def enumDecoder[Z](parentSchema: Schema.Enum[Z]): ZJsonDecoder[Z] = { val caseNameAliases = new mutable.HashMap[String, Schema.Case[Z, Any]] parentSchema.cases.foreach { case_ => @@ -813,9 +816,6 @@ object JsonCodec { case_.caseNameAliases.foreach(a => caseNameAliases.put(a, schema)) } - def error(msg: String, trace: List[JsonError]): Nothing = - throw UnsafeJson(JsonError.Message(msg) :: trace) - if (parentSchema.cases.forall(_.schema.isInstanceOf[Schema.CaseClass0[_]])) { // if all cases are CaseClass0, decode as String if (caseNameAliases.size <= 64) { new ZJsonDecoder[Z] { @@ -970,18 +970,12 @@ object JsonCodec { schema: GenericRecord, discriminator: Option[String] ): ZJsonDecoder[ListMap[String, Any]] = { - val capacity = schema.fields.size * 2 val spansWithDecoders = - new util.HashMap[String, (JsonError.ObjectAccess, ZJsonDecoder[Any])](capacity) - val defaults = new util.HashMap[String, Any](capacity) + new util.HashMap[String, (JsonError.ObjectAccess, ZJsonDecoder[Any])](schema.fields.size * 2) schema.fields.foreach { field => - val fieldName = field.fieldName val spanWithDecoder = - (JsonError.ObjectAccess(fieldName), schemaDecoder(field.schema).asInstanceOf[ZJsonDecoder[Any]]) - field.nameAndAliases.foreach(x => spansWithDecoders.put(x, spanWithDecoder)) - if ((field.optional || field.transient) && field.defaultValue.isDefined) { - defaults.put(fieldName, field.defaultValue.get) - } + (JsonError.ObjectAccess(field.fieldName), schemaDecoder(field.schema).asInstanceOf[ZJsonDecoder[Any]]) + field.nameAndAliases.foreach(spansWithDecoders.put(_, spanWithDecoder)) } val skipExtraFields = !schema.annotations.exists(_.isInstanceOf[rejectExtraFields]) (trace: List[JsonError], in: RetractReader) => { @@ -1000,23 +994,37 @@ object JsonCodec { val dec = spanWithDecoder._2 val trace_ = span :: trace lexer.char(trace_, in, ':') - val fieldName = span.field + val fieldName = span.field // reuse strings with calculated hashCode val prev = map.put(fieldName, dec.unsafeDecode(trace_, in)) - if (prev != null) { - throw UnsafeJson(JsonError.Message("duplicate") :: trace_) - } + if (prev != null) error("duplicate", trace_) } else if (skipExtraFields || discriminator.contains(fieldNameOrAlias)) { lexer.char(trace, in, ':') lexer.skipValue(trace, in) - } else { - throw UnsafeJson(JsonError.Message(s"unexpected field: $fieldNameOrAlias") :: trace) - } + } else error("extra field", trace) continue = lexer.nextField(trace, in) } - val it = defaults.entrySet().iterator() - while (it.hasNext) { - val entry = it.next() - map.putIfAbsent(entry.getKey, entry.getValue) + schema.fields.foreach { field => + val fieldName = field.fieldName // reuse strings with calculated hashCode + if (map.get(fieldName) == null) { + map.put( // mitigation of a linking error for `map.computeIfAbsent` in Scala.js + fieldName, { + if ((field.optional || field.transient) && field.defaultValue.isDefined) { + field.defaultValue.get + } else { + var schema = field.schema + schema match { + case l: Schema.Lazy[_] => schema = l.schema + case _ => + } + schema match { + case _: Schema.Optional[_] => None + case collection: Schema.Collection[_, _] => collection.empty + case _ => error("missing", spansWithDecoders.get(fieldName)._1 :: trace) + } + } + } + ) + } } (ListMap.newBuilder[String, Any] ++= ({ // to avoid O(n) insert operations import scala.collection.JavaConverters.mapAsScalaMapConverter // use deprecated class for Scala 2.12 compatibility @@ -1095,10 +1103,7 @@ object JsonCodec { case (Some(a), Some(b)) => Fallback.Both(a, b) case (Some(a), _) => Fallback.Left(a) case (_, Some(b)) => Fallback.Right(b) - case _ => - throw UnsafeJson( - JsonError.Message("Fallback decoder was unable to decode both left and right sides") :: trace - ) + case _ => error("Fallback decoder was unable to decode both left and right sides", trace) } } } 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 ee7eb244c..35ce3e4db 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 @@ -477,9 +477,9 @@ object JsonCodecSpec extends ZIOSpecDefault { test("Do not encode transient field") { assertEncodes( RecordExample.schema, - RecordExample(f1 = Some("test"), f3 = Some("transient")), + RecordExample(f1 = "test", f3 = Some("transient"), f20 = None, f21 = Vector.empty, f22 = Nil), charSequenceToByteChunk( - """{"$f1":"test"}""".stripMargin + """{"$f1":"test","f20":null,"f21":[],"f22":[]}""".stripMargin ) ) } @@ -732,26 +732,33 @@ object JsonCodecSpec extends ZIOSpecDefault { charSequenceToByteChunk("""{"foo":"s","bar":1,"baz":2}""") ) }, - test("with missing fields") { + test("with empty optional and collection fields without default values") { assertDecodes( RecordExample.schema, - RecordExample(f1 = Some("test"), f2 = None), + RecordExample(f1 = "test", f2 = None, f20 = None, f21 = Vector.empty, f22 = Nil), charSequenceToByteChunk("""{"$f1":"test"}""") ) }, + test("missing required fields") { + assertDecodesToError( + RecordExample.schema, + """{}""", + JsonError.Message("missing") :: JsonError.ObjectAccess("$f1") :: Nil + ) + }, test("aliased field") { assertDecodes( RecordExample.schema, - RecordExample(f1 = Some("test"), f2 = Some("alias")), + RecordExample(f1 = "test", f2 = Some("alias"), f20 = None, f21 = Vector.empty, f22 = Nil), charSequenceToByteChunk("""{"$f1":"test", "field2":"alias"}""") ) }, test("reject extra fields") { assertDecodes( RecordExample.schema.annotate(rejectExtraFields()), - RecordExample(f1 = Some("test")), + RecordExample(f1 = "test", f20 = None, f21 = Vector.empty, f22 = Nil), charSequenceToByteChunk("""{"$f1":"test", "extraField":"extra"}""") - ).flip.map(err => assertTrue(err.getMessage() == "(unexpected field: extraField)")) + ).flip.map(err => assertTrue(err.getMessage() == "(extra field)")) }, test("reject duplicated fields") { assertDecodesToError( @@ -1697,7 +1704,7 @@ object JsonCodecSpec extends ZIOSpecDefault { assertDecodes(Schema[Command], Command.Cash, charSequenceToByteChunk("""{"type":"Cash","extraField":1}""")) &> assertDecodes(Schema[Command], Command.Cash, charSequenceToByteChunk("""{"extraField":1,"type":"Cash"}""")) ), - suite("of case objects with less than 64 cases")( + suite("of case objects with up to 64 cases")( test("without annotation")( assertEncodesThenDecodes(Schema[Color], Color.Red) ), @@ -1718,7 +1725,7 @@ object JsonCodecSpec extends ZIOSpecDefault { assertDecodesToError(Schema[Color], "\"not a color\"", JsonError.Message("unrecognized string") :: Nil) ) ), - suite("of case objects with 64 cases or more")( + suite("of case objects with more than 64 cases")( test("without annotation")( assertEncodesThenDecodes(Schema[BigEnum], BigEnum.Case69) ), @@ -2477,7 +2484,7 @@ object JsonCodecSpec extends ZIOSpecDefault { ) extends OneOf4 case class RecordExample( - @fieldName("$f1") f1: Option[String], // the only field that does not have a default value + @fieldName("$f1") f1: String, @fieldNameAliases("field2") f2: Option[String] = None, @transientField f3: Option[String] = None, f4: Option[String] = None, @@ -2493,12 +2500,12 @@ object JsonCodecSpec extends ZIOSpecDefault { f14: Option[String] = None, f15: Option[String] = None, f16: Option[String] = None, - f17: Option[String] = None, - f18: Option[String] = None, - f19: Option[String] = None, - f20: Option[String] = None, - f21: Option[String] = None, - f22: Option[String] = None, + f17: List[String] = Nil, + f18: Vector[String] = Vector.empty, + f19: Option[RecordExample] = None, + f20: Option[String], + f21: Vector[String], + f22: List[String], @fieldName("$f23") f23: Option[String] = None )