From c88d86d271bdbe8b91e695032a6d5bfe0fa83b37 Mon Sep 17 00:00:00 2001 From: Benoit Louy Date: Thu, 5 Sep 2024 11:46:43 -0400 Subject: [PATCH 1/5] add support for jsonUnknown trait to Document decoder --- .../test/src/smithy4s/DocumentSpec.scala | 210 ++++++++++++++++++ .../DocumentDecoderSchemaVisitor.scala | 155 +++++++++---- 2 files changed, 319 insertions(+), 46 deletions(-) diff --git a/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala b/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala index fa3f749ae..cf16a9cb3 100644 --- a/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala @@ -20,6 +20,7 @@ import smithy.api.JsonName import smithy.api.Default import smithy4s.example.IntList import alloy.Discriminated +import alloy.JsonUnknown import munit._ import smithy4s.example.DefaultNullsOperationOutput import alloy.Untagged @@ -663,4 +664,213 @@ class DocumentSpec() extends FunSuite { assertEquals(niceSyntaxDocument, expectedDocument) } + case class JsonUnknownExample( + s: String, + i: Int, + others: Map[String, Document] + ) + + object JsonUnknownExample { + implicit val jsonUnknownExampleSchema: Schema[JsonUnknownExample] = { + val s = string.required[JsonUnknownExample]("s", _.s) + val i = int.required[JsonUnknownExample]("i", _.i) + val others = map(string, document) + .required[JsonUnknownExample]("others", _.others) + .addHints(JsonUnknown()) + struct(s, i, others)(JsonUnknownExample.apply) + } + } + + object JsonUnknownExampleWithDefault { + implicit val jsonUnknownExampleSchema: Schema[JsonUnknownExample] = { + val s = string.required[JsonUnknownExample]("s", _.s) + val i = int.required[JsonUnknownExample]("i", _.i) + val others = map(string, document) + .required[JsonUnknownExample]("others", _.others) + .addHints( + JsonUnknown(), + Default(Document.obj("default" -> Document.fromBoolean(true))) + ) + struct(s, i, others)(JsonUnknownExample.apply) + } + } + + case class JsonUnknownExampleOptional( + s: String, + i: Int, + others: Option[Map[String, Document]] + ) + + object JsonUnknownExampleOptional { + implicit val jsonUnknownExampleOptionalSchema + : Schema[JsonUnknownExampleOptional] = { + val s = string.required[JsonUnknownExampleOptional]("s", _.s) + val i = int.required[JsonUnknownExampleOptional]("i", _.i) + val others = map(string, document) + .optional[JsonUnknownExampleOptional]("others", _.others) + .addHints(JsonUnknown()) + struct(s, i, others)(JsonUnknownExampleOptional.apply) + } + } + + object JsonUnknownExampleOptionalWithDefault { + implicit val jsonUnknownExampleOptionalSchema + : Schema[JsonUnknownExampleOptional] = { + val s = string.required[JsonUnknownExampleOptional]("s", _.s) + val i = int.required[JsonUnknownExampleOptional]("i", _.i) + val others = map(string, document) + .optional[JsonUnknownExampleOptional]("others", _.others) + .addHints( + JsonUnknown(), + Default(Document.obj("default" -> Document.fromBoolean(true))) + ) + struct(s, i, others)(JsonUnknownExampleOptional.apply) + } + } + + test("unknown field decoding: no unknown field in payload") { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67) + ) + val expected = JsonUnknownExample("foo", 67, Map.empty) + + val res = Document.Decoder + .fromSchema(JsonUnknownExample.jsonUnknownExampleSchema) + .decode(doc) + + assertEquals(res, Right(expected)) + } + + test("unknown field decoding: no unknown field in payload with default") { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67) + ) + val expected = JsonUnknownExample( + "foo", + 67, + Map("default" -> Document.fromBoolean(true)) + ) + + val res = Document.Decoder + .fromSchema(JsonUnknownExampleWithDefault.jsonUnknownExampleSchema) + .decode(doc) + + assertEquals(res, Right(expected)) + } + + test( + "unknown field decoding: no unknown field in payload, optional field" + ) { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67) + ) + val expected = JsonUnknownExampleOptional("foo", 67, None) + + val res = Document.Decoder + .fromSchema(JsonUnknownExampleOptional.jsonUnknownExampleOptionalSchema) + .decode(doc) + + assertEquals(res, Right(expected)) + } + + test( + "unknown field decoding: no unknown field in payload, optional field with default" + ) { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67) + ) + val expected = JsonUnknownExampleOptional( + "foo", + 67, + Some(Map("default" -> Document.fromBoolean(true))) + ) + + val res = Document.Decoder + .fromSchema( + JsonUnknownExampleOptionalWithDefault.jsonUnknownExampleOptionalSchema + ) + .decode(doc) + + assertEquals(res, Right(expected)) + } + + test("unknown field decoding: with unknown fields in payload") { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67), + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75) + ) + val expected = JsonUnknownExample( + "foo", + 67, + Map( + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75) + ) + ) + + val res = Document.Decoder + .fromSchema(JsonUnknownExample.jsonUnknownExampleSchema) + .decode(doc) + + assertEquals(res, Right(expected)) + } + + test( + "unknown field decoding: with unknown fields in payload, optional field" + ) { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67), + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75) + ) + val expected = JsonUnknownExampleOptional( + "foo", + 67, + Some( + Map( + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75) + ) + ) + ) + + val res = Document.Decoder + .fromSchema(JsonUnknownExampleOptional.jsonUnknownExampleOptionalSchema) + .decode(doc) + + assertEquals(res, Right(expected)) + } + + test("unknown field decoding: with unknow field explicitely set in payload") { + val doc = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67), + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75), + "others" -> Document.obj() + ) + val expected = JsonUnknownExample( + "foo", + 67, + Map( + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75), + "others" -> Document.obj() + ) + ) + + val res = Document.Decoder + .fromSchema(JsonUnknownExample.jsonUnknownExampleSchema) + .decode(doc) + + assertEquals(res, Right(expected)) + } + } diff --git a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala index 0f4fcb1dd..8f9a58df3 100644 --- a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala +++ b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala @@ -32,8 +32,11 @@ import smithy4s.schema._ import java.util.Base64 import java.util.UUID +import java.{util => ju} import scala.collection.immutable.ListMap import alloy.Untagged +import alloy.JsonUnknown +import scala.collection.mutable.ListBuffer trait DocumentDecoder[A] { self => def apply(history: List[PayloadPath.Segment], document: Document): A @@ -308,6 +311,9 @@ class DocumentDecoderSchemaVisitor( } } + private def isForJsonUnknown[Z, A](field: Field[Z, A]): Boolean = + field.hints.has(JsonUnknown) + override def struct[S]( shapeId: ShapeId, hints: Hints, @@ -317,58 +323,115 @@ class DocumentDecoderSchemaVisitor( def jsonLabel[A](field: Field[S, A]): String = field.hints.get(JsonName).map(_.value).getOrElse(field.label) - def fieldDecoder[A]( - field: Field[S, A] - ): ( - List[PayloadPath.Segment], - Any => Unit, - Map[String, Document] - ) => Unit = { + type Handler = + (List[PayloadPath.Segment], Document, ju.HashMap[String, Any]) => Unit + + val labelelledFields = fields.map { field => val jLabel = jsonLabel(field) + val decoded = field.schema.getDefaultValue + val default = decoded.orNull + (field, jLabel, default) + } + + def fieldHandler[A](field: Field[S, A], jLabel: String): Handler = { + val decoder = apply(field.schema) + val label = field.label + (parentPath, in, mmap) => + val _ = mmap.put( + label, { + val path = PayloadPath.Segment(jLabel) :: parentPath + decoder(path, in) + } + ) + } - field.getDefaultValue match { - case Some(defaultValue) => - ( - pp: List[PayloadPath.Segment], - buffer: Any => Unit, - fields: Map[String, Document] - ) => - val path = PayloadPath.Segment(jLabel) :: pp - fields - .get(jLabel) match { - case Some(document) => - buffer(apply(field.schema)(path, document)) - case None => - buffer(defaultValue) + val (fieldsForUnknown, knownFields) = labelelledFields.partition { + case (field, _, _) => isForJsonUnknown(field) + } + + val handlers = + new ju.HashMap[String, Handler](knownFields.length << 1, 0.5f) { + knownFields.foreach { case (field, jLabel, _) => + put(jLabel, fieldHandler(field, jLabel)) + } + } + + if (fieldsForUnknown.isEmpty) { + DocumentDecoder.instance("Structure", "Object") { + case (pp, DObject(value)) => + val buffer = new ju.HashMap[String, Any](handlers.size << 1, 0.5f) + value.foreach { case (key, value) => + val handler = handlers.get(key) + if (handler != null) { + handler(pp, value, buffer) } - case None => - ( - pp: List[PayloadPath.Segment], - buffer: Any => Unit, - fields: Map[String, Document] - ) => - val path = PayloadPath.Segment(jLabel) :: pp - fields - .get(jLabel) match { - case Some(document) => - buffer(apply(field.schema)(path, document)) - case None => - throw new PayloadError( - PayloadPath(path.reverse), - "", - "Required field not found" - ) + } + val orderedBuffer = Vector.newBuilder[Any] + labelelledFields.foreach { case (field, jLabel, default) => + orderedBuffer += { + val value = buffer.get(field.label) + if (value == null) { + if (default == null) { + throw new PayloadError( + PayloadPath((PayloadPath.Segment(jLabel) :: pp).reverse), + jLabel, + "Missing required field" + ) + } else default + } else value } + } + make(orderedBuffer.result()) + } + } else { + val fieldForUnknownDocDecoders = fieldsForUnknown.map { + case (field, jLabel, _) => + jLabel -> apply(field.schema) + }.toMap + DocumentDecoder.instance("Structure", "Object") { + case (pp, DObject(value)) => + val buffer = new ju.HashMap[String, Any](handlers.size << 1, 0.5f) + val unknownValues = ListBuffer[(String, Document)]() + value.foreach { case (key, value) => + val handler = handlers.get(key) + if (handler == null) { + unknownValues += (key -> value) + } else { + handler(pp, value, buffer) + } + } + val orderedBuffer = Vector.newBuilder[Any] + val unknownValue = if (unknownValues.nonEmpty) Document.obj(unknownValues) else null + labelelledFields.foreach { case (field, jLabel, default) => + orderedBuffer += { + fieldForUnknownDocDecoders.get(jLabel) match { + case None => + val value = buffer.get(field.label) + if (value == null) { + if (default == null) { + throw new PayloadError( + PayloadPath( + (PayloadPath.Segment(jLabel) :: pp).reverse + ), + jLabel, + "Missing required field" + ) + } else default + } else value + case Some(decoder) => + if (unknownValue == null) { + if (default == null) { + decoder(Nil, Document.obj()) + } else default + } else { + decoder(Nil, unknownValue) + } + } + } + } + make(orderedBuffer.result()) } - } - - val fieldDecoders = fields.map(field => fieldDecoder(field)) - DocumentDecoder.instance("Structure", "Object") { - case (pp, DObject(value)) => - val buffer = Vector.newBuilder[Any] - fieldDecoders.foreach(fd => fd(pp, buffer.+=(_), value)) - make(buffer.result()) } } From 4eb52906dfb6b257803fd8f4ea77a1035aaaab39 Mon Sep 17 00:00:00 2001 From: Benoit Louy Date: Thu, 5 Sep 2024 12:40:04 -0400 Subject: [PATCH 2/5] add support for jsonUnknown trait to Document encoder --- .../test/src/smithy4s/DocumentSpec.scala | 26 ++++++++++++++ .../DocumentEncoderSchemaVisitor.scala | 36 ++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala b/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala index cf16a9cb3..c1b96fecf 100644 --- a/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala @@ -873,4 +873,30 @@ class DocumentSpec() extends FunSuite { assertEquals(res, Right(expected)) } + test("unknown field encoding") { + val in = JsonUnknownExample( + "foo", + 67, + Map( + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75), + "others" -> Document.obj() + ) + ) + + val expected = Document.obj( + "s" -> Document.fromString("foo"), + "i" -> Document.fromInt(67), + "someField" -> Document.obj("a" -> Document.fromString("b")), + "someOtherField" -> Document.fromInt(75), + "others" -> Document.obj() + ) + + val doc = Document.Encoder + .fromSchema(JsonUnknownExample.jsonUnknownExampleSchema) + .encode(in) + + assertEquals(doc, expected) + } + } diff --git a/modules/core/src/smithy4s/internals/DocumentEncoderSchemaVisitor.scala b/modules/core/src/smithy4s/internals/DocumentEncoderSchemaVisitor.scala index c6b555d02..bc329006c 100644 --- a/modules/core/src/smithy4s/internals/DocumentEncoderSchemaVisitor.scala +++ b/modules/core/src/smithy4s/internals/DocumentEncoderSchemaVisitor.scala @@ -23,6 +23,7 @@ import smithy.api.TimestampFormat.DATE_TIME import smithy.api.TimestampFormat.EPOCH_SECONDS import smithy.api.TimestampFormat.HTTP_DATE import alloy.Discriminated +import alloy.JsonUnknown import smithy4s.capability.EncoderK import smithy4s.schema._ @@ -186,6 +187,9 @@ class DocumentEncoderSchemaVisitor( from(e => DString(total(e).stringValue)) } + private def isForJsonUnknown(field: Field[_, _]): Boolean = + field.hints.has(JsonUnknown) + override def struct[S]( shapeId: ShapeId, hints: Hints, @@ -215,7 +219,37 @@ class DocumentEncoderSchemaVisitor( } } - val encoders = fields.map(field => fieldEncoder(field)) + def jsonUnknownFieldEncoder[A]( + field: Field[S, A] + ): (S, Builder[(String, Document), Map[String, Document]]) => Unit = { + val encoder = apply(field.schema) + (s, builder) => { + if (explicitDefaultsEncoding) { + encoder(field.get(s)) match { + case Document.DObject(value) => value.foreach(builder += _) + case _ => + throw new IllegalArgumentException( + s"Failed encoding field ${field.label} because it cannot be converted to a JSON object" + ) + } + } else { + field.foreachUnlessDefault(s) { a => + encoder(a) match { + case Document.DObject(value) => value.foreach(builder += _) + case _ => + throw new IllegalArgumentException( + s"Failed encoding field ${field.label} because it cannot be converted to a JSON object" + ) + } + } + } + } + } + + val (fieldsForUnknown, knownFields) = fields.partition(isForJsonUnknown) + + val encoders = knownFields.map(field => fieldEncoder(field)) ++ + fieldsForUnknown.map(field => jsonUnknownFieldEncoder(field)) new DocumentEncoder[S] { def apply(s: S): Document = { val builder = Map.newBuilder[String, Document] From a9154d94ebb7e5ce1fd663c135e27a149baaae6e Mon Sep 17 00:00:00 2001 From: Benoit Louy Date: Thu, 5 Sep 2024 12:53:42 -0400 Subject: [PATCH 3/5] fix scala 2.12 compilation --- .../src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala index 8f9a58df3..26793b543 100644 --- a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala +++ b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala @@ -386,7 +386,7 @@ class DocumentDecoderSchemaVisitor( } else { val fieldForUnknownDocDecoders = fieldsForUnknown.map { case (field, jLabel, _) => - jLabel -> apply(field.schema) + jLabel -> apply(field.schema).asInstanceOf[DocumentDecoder[Any]] }.toMap DocumentDecoder.instance("Structure", "Object") { case (pp, DObject(value)) => From 7076fd71d0e25cb1020020e43832adf2399f5c6e Mon Sep 17 00:00:00 2001 From: Benoit Louy Date: Thu, 5 Sep 2024 13:14:53 -0400 Subject: [PATCH 4/5] use same error message --- .../smithy4s/internals/DocumentDecoderSchemaVisitor.scala | 7 ++++--- .../src-jvm/smithy4s/dynamic/DynamicJsonServerSpec.scala | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala index 26793b543..814234ad6 100644 --- a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala +++ b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala @@ -375,7 +375,7 @@ class DocumentDecoderSchemaVisitor( throw new PayloadError( PayloadPath((PayloadPath.Segment(jLabel) :: pp).reverse), jLabel, - "Missing required field" + "Required field not found" ) } else default } else value @@ -401,7 +401,8 @@ class DocumentDecoderSchemaVisitor( } } val orderedBuffer = Vector.newBuilder[Any] - val unknownValue = if (unknownValues.nonEmpty) Document.obj(unknownValues) else null + val unknownValue = + if (unknownValues.nonEmpty) Document.obj(unknownValues) else null labelelledFields.foreach { case (field, jLabel, default) => orderedBuffer += { fieldForUnknownDocDecoders.get(jLabel) match { @@ -414,7 +415,7 @@ class DocumentDecoderSchemaVisitor( (PayloadPath.Segment(jLabel) :: pp).reverse ), jLabel, - "Missing required field" + "Required field not found" ) } else default } else value diff --git a/modules/dynamic/test/src-jvm/smithy4s/dynamic/DynamicJsonServerSpec.scala b/modules/dynamic/test/src-jvm/smithy4s/dynamic/DynamicJsonServerSpec.scala index 621b33ac3..122e3f8af 100644 --- a/modules/dynamic/test/src-jvm/smithy4s/dynamic/DynamicJsonServerSpec.scala +++ b/modules/dynamic/test/src-jvm/smithy4s/dynamic/DynamicJsonServerSpec.scala @@ -81,7 +81,7 @@ class DynamicJsonServerSpec() extends DummyIO.Suite { testJsonIO("Dynamic service is correctly wired: Bad Json Input") { jsonIO => val expected = PayloadError( PayloadPath("key"), - "", + "key", "Required field not found" ) From 8f69871deb6159645772bccdba37aa3976f25166 Mon Sep 17 00:00:00 2001 From: Benoit Louy Date: Fri, 6 Sep 2024 09:16:23 -0400 Subject: [PATCH 5/5] fix typo --- .../smithy4s/internals/DocumentDecoderSchemaVisitor.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala index 814234ad6..e959a169b 100644 --- a/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala +++ b/modules/core/src/smithy4s/internals/DocumentDecoderSchemaVisitor.scala @@ -326,7 +326,7 @@ class DocumentDecoderSchemaVisitor( type Handler = (List[PayloadPath.Segment], Document, ju.HashMap[String, Any]) => Unit - val labelelledFields = fields.map { field => + val labelledFields = fields.map { field => val jLabel = jsonLabel(field) val decoded = field.schema.getDefaultValue val default = decoded.orNull @@ -345,7 +345,7 @@ class DocumentDecoderSchemaVisitor( ) } - val (fieldsForUnknown, knownFields) = labelelledFields.partition { + val (fieldsForUnknown, knownFields) = labelledFields.partition { case (field, _, _) => isForJsonUnknown(field) } @@ -367,7 +367,7 @@ class DocumentDecoderSchemaVisitor( } } val orderedBuffer = Vector.newBuilder[Any] - labelelledFields.foreach { case (field, jLabel, default) => + labelledFields.foreach { case (field, jLabel, default) => orderedBuffer += { val value = buffer.get(field.label) if (value == null) { @@ -403,7 +403,7 @@ class DocumentDecoderSchemaVisitor( val orderedBuffer = Vector.newBuilder[Any] val unknownValue = if (unknownValues.nonEmpty) Document.obj(unknownValues) else null - labelelledFields.foreach { case (field, jLabel, default) => + labelledFields.foreach { case (field, jLabel, default) => orderedBuffer += { fieldForUnknownDocDecoders.get(jLabel) match { case None =>