From 2b355ca1024cf442460972986ac50518d1efde56 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 9 May 2024 16:32:05 +0300 Subject: [PATCH 01/11] Initial support for sparse lists in query params --- .../http/internals/MetadataSpec.scala | 38 +++++++++---------- .../smithy4s/http/internals/PathSpec.scala | 2 +- .../core/src/smithy4s/http/HttpEndpoint.scala | 4 +- modules/core/src/smithy4s/http/HttpUri.scala | 2 +- modules/core/src/smithy4s/http/Metadata.scala | 18 ++++++--- .../smithy4s/http/internals/MetaDecode.scala | 10 +++-- .../src/smithy4s/http/internals/package.scala | 9 +++-- .../src/smithy4s/http4s/kernel/package.scala | 18 +++++---- 8 files changed, 58 insertions(+), 43 deletions(-) diff --git a/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala b/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala index 43e55ebd3..153f66b46 100644 --- a/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala @@ -119,7 +119,7 @@ class MetadataSpec() extends FunSuite { val queries = Queries(str = Some("hello")) val finished = Queries(str = Some("hello"), slm = Some(Map("str" -> "hello"))) - val expected = Metadata(query = Map("str" -> List("hello"))) + val expected = Metadata(query = Map("str" -> List(Some("hello")))) checkQueryRoundTrip(queries, expected, finished) } @@ -131,7 +131,7 @@ class MetadataSpec() extends FunSuite { test("String length constraint violation") { val string = "1" * 11 val queries = ValidationChecks(str = Some(string)) - val expected = Metadata(query = Map("str" -> List(string))) + val expected = Metadata(query = Map("str" -> List(Some(string)))) checkRoundTripError( queries, expected, @@ -142,7 +142,7 @@ class MetadataSpec() extends FunSuite { test("List length constraint violation") { val list = List.fill(11)("str") val queries = ValidationChecks(lst = Some(list)) - val expected = Metadata(query = Map("lst" -> list)) + val expected = Metadata(query = Map("lst" -> list.map(Some(_)))) checkRoundTripError( queries, expected, @@ -153,7 +153,7 @@ class MetadataSpec() extends FunSuite { test("Integer range constraint violation") { val i = 11 val queries = ValidationChecks(int = Some(i)) - val expected = Metadata(query = Map("int" -> List(i.toString))) + val expected = Metadata(query = Map("int" -> List(Some(i.toString)))) checkRoundTripError( queries, expected, @@ -164,14 +164,14 @@ class MetadataSpec() extends FunSuite { test("Integer query parameter") { val queries = Queries(int = Some(123)) val finished = Queries(int = Some(123), slm = Some(Map("int" -> "123"))) - val expected = Metadata(query = Map("int" -> List("123"))) + val expected = Metadata(query = Map("int" -> List(Some("123")))) checkQueryRoundTrip(queries, expected, finished) } test("Boolean query parameter") { val queries = Queries(b = Some(true)) val finished = Queries(b = Some(true), slm = Some(Map("b" -> "true"))) - val expected = Metadata(query = Map("b" -> List("true"))) + val expected = Metadata(query = Map("b" -> List(Some("true")))) checkQueryRoundTrip(queries, expected, finished) } @@ -180,7 +180,7 @@ class MetadataSpec() extends FunSuite { val queries = Queries(ts1 = Some(ts)) val finished = Queries(ts1 = Some(ts), slm = Some(Map("ts1" -> epochString))) - val expected = Metadata(query = Map("ts1" -> List(epochString))) + val expected = Metadata(query = Map("ts1" -> List(Some(epochString)))) checkQueryRoundTrip(queries, expected, finished) } @@ -189,7 +189,7 @@ class MetadataSpec() extends FunSuite { val queries = Queries(ts2 = Some(ts)) val finished = Queries(ts2 = Some(ts), slm = Some(Map("ts2" -> epochString))) - val expected = Metadata(query = Map("ts2" -> List(epochString))) + val expected = Metadata(query = Map("ts2" -> List(Some(epochString)))) checkQueryRoundTrip(queries, expected, finished) } @@ -198,7 +198,7 @@ class MetadataSpec() extends FunSuite { val queries = Queries(ts3 = Some(ts)) val finished = Queries(ts3 = Some(ts), slm = Some(Map("ts3" -> "1234567890"))) - val expected = Metadata(query = Map("ts3" -> List("1234567890"))) + val expected = Metadata(query = Map("ts3" -> List(Some("1234567890")))) checkQueryRoundTrip(queries, expected, finished) } @@ -210,7 +210,7 @@ class MetadataSpec() extends FunSuite { slm = Some(Map("ts4" -> "Thu, 01 Jan 1970 00:00:00 GMT")) ) val expected = - Metadata(query = Map("ts4" -> List("Thu, 01 Jan 1970 00:00:00 GMT"))) + Metadata(query = Map("ts4" -> List(Some("Thu, 01 Jan 1970 00:00:00 GMT")))) checkQueryRoundTrip(queries, expected, finished) } @@ -218,7 +218,7 @@ class MetadataSpec() extends FunSuite { val map = Map("hello" -> "a", "world" -> "b") val queries = Queries(slm = Some(map)) - val expected = Metadata(query = map.fmap(Seq(_))) + val expected = Metadata(query = map.fmap(a => Seq(Some(a)))) checkRoundTrip(queries, expected) } @@ -226,7 +226,7 @@ class MetadataSpec() extends FunSuite { val list = List("hello", "world") val queries = Queries(sl = Some(list)) val finished = Queries(sl = Some(list), slm = Some(Map("sl" -> "hello"))) - val expected = Metadata(query = Map("sl" -> list)) + val expected = Metadata(query = Map("sl" -> list.map(Some(_)))) checkQueryRoundTrip(queries, expected, finished) } @@ -237,7 +237,7 @@ class MetadataSpec() extends FunSuite { ie = Some(smithy4s.example.Numbers.ONE), slm = Some(Map("nums" -> "1")) ) - val expected = Metadata(query = Map("nums" -> List("1"))) + val expected = Metadata(query = Map("nums" -> List(Some("1")))) checkQueryRoundTrip(queries, expected, finished) } @@ -248,7 +248,7 @@ class MetadataSpec() extends FunSuite { on = Some(smithy4s.example.OpenNums.$Unknown(101)), slm = Some(Map("openNums" -> "101")) ) - val expected = Metadata(query = Map("openNums" -> List("101"))) + val expected = Metadata(query = Map("openNums" -> List(Some("101")))) checkQueryRoundTrip(queries, expected, finished) } @@ -259,7 +259,7 @@ class MetadataSpec() extends FunSuite { on = Some(smithy4s.example.OpenNums.ONE), slm = Some(Map("openNums" -> "1")) ) - val expected = Metadata(query = Map("openNums" -> List("1"))) + val expected = Metadata(query = Map("openNums" -> List(Some("1")))) checkQueryRoundTrip(queries, expected, finished) } @@ -271,7 +271,7 @@ class MetadataSpec() extends FunSuite { ons = Some(smithy4s.example.OpenNumsStr.$Unknown("test")), slm = Some(Map("openNumsStr" -> "test")) ) - val expected = Metadata(query = Map("openNumsStr" -> List("test"))) + val expected = Metadata(query = Map("openNumsStr" -> List(Some("test")))) checkQueryRoundTrip(queries, expected, finished) } @@ -282,7 +282,7 @@ class MetadataSpec() extends FunSuite { ons = Some(smithy4s.example.OpenNumsStr.ONE), slm = Some(Map("openNumsStr" -> "ONE")) ) - val expected = Metadata(query = Map("openNumsStr" -> List("ONE"))) + val expected = Metadata(query = Map("openNumsStr" -> List(Some("ONE")))) checkQueryRoundTrip(queries, expected, finished) } @@ -439,7 +439,7 @@ class MetadataSpec() extends FunSuite { test("bad data gets caught") { val metadata = - Metadata(query = Map("ts3" -> List("Thu, 01 Jan 1970 00:00:00 GMT"))) + Metadata(query = Map("ts3" -> List(Some("Thu, 01 Jan 1970 00:00:00 GMT")))) val result = Metadata.decode[Queries](metadata) val expected = MetadataError.WrongType( "ts3", @@ -462,7 +462,7 @@ class MetadataSpec() extends FunSuite { test("too many parameters get caught") { val metadata = - Metadata(query = Map("ts3" -> List("1", "2", "3"))) + Metadata(query = Map("ts3" -> List("1", "2", "3").map(Some(_)))) val result = Metadata.decode[Queries](metadata) val expected = MetadataError.ArityError( "ts3", diff --git a/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala b/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala index 280deda91..e35becc88 100644 --- a/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala @@ -80,7 +80,7 @@ class PathSpec() extends munit.FunSuite { val sqp = httpEndpoint.staticQueryParams val path = httpEndpoint.path - val expectedQueryMap = Map("value" -> Seq("foo"), "baz" -> Seq("bar")) + val expectedQueryMap = Map("value" -> Seq(Some("foo")), "baz" -> Seq(Some("bar"))) expect(sqp == expectedQueryMap) expect( path == diff --git a/modules/core/src/smithy4s/http/HttpEndpoint.scala b/modules/core/src/smithy4s/http/HttpEndpoint.scala index d7c42a631..7e8745226 100644 --- a/modules/core/src/smithy4s/http/HttpEndpoint.scala +++ b/modules/core/src/smithy4s/http/HttpEndpoint.scala @@ -29,7 +29,7 @@ trait HttpEndpoint[I] { def path: List[PathSegment] // Returns a map of static query parameters that are found in the uri of Http hint. - def staticQueryParams: Map[String, Seq[String]] + def staticQueryParams: Map[String, Seq[Option[String]]] def method: HttpMethod def code: Int @@ -70,7 +70,7 @@ object HttpEndpoint { } yield { new HttpEndpoint[I] { def path(input: I): List[String] = encoder.encode(input) - val staticQueryParams: Map[String, Seq[String]] = queryParams + val staticQueryParams: Map[String, Seq[Option[String]]] = queryParams val path: List[PathSegment] = httpPath.toList val method: HttpMethod = httpMethod val code: Int = http.code diff --git a/modules/core/src/smithy4s/http/HttpUri.scala b/modules/core/src/smithy4s/http/HttpUri.scala index 0c9b947de..4ddfee7bf 100644 --- a/modules/core/src/smithy4s/http/HttpUri.scala +++ b/modules/core/src/smithy4s/http/HttpUri.scala @@ -24,7 +24,7 @@ final case class HttpUri( * A sequence of URL-decoded URI segment. */ path: IndexedSeq[String], - queryParams: Map[String, Seq[String]], + queryParams: Map[String, Seq[Option[String]]], /** * Field allowing to store decoded path parameters alongside an http request, * once the routing logic has come in effect. diff --git a/modules/core/src/smithy4s/http/Metadata.scala b/modules/core/src/smithy4s/http/Metadata.scala index bc68a67e0..7cece6c0a 100644 --- a/modules/core/src/smithy4s/http/Metadata.scala +++ b/modules/core/src/smithy4s/http/Metadata.scala @@ -39,7 +39,7 @@ import smithy4s.schema.CompilationCache */ case class Metadata( path: Map[String, String] = Map.empty, - query: Map[String, Seq[String]] = Map.empty, + query: Map[String, Seq[Option[String]]] = Map.empty, headers: Map[CaseInsensitive, Seq[String]] = Map.empty, statusCode: Option[Int] = None ) { self => @@ -49,7 +49,7 @@ case class Metadata( v.map(k -> _) } - def queryFlattened: Vector[(String, String)] = query.toVector.flatMap { + def queryFlattened: Vector[(String, Option[String])] = query.toVector.flatMap { case (k, v) => v.map(k -> _) } @@ -67,12 +67,17 @@ case class Metadata( def addPathParam(key: String, value: String): Metadata = copy(path = path + (key -> value)) def addQueryParam(key: String, value: String): Metadata = + addQueryParam(key, Some(value)) + def addQueryParam(key: String, value: Option[String]): Metadata = query.get(key) match { case Some(existing) => copy(query = query + (key -> (existing :+ value))) case None => copy(query = query + (key -> List(value))) } def addQueryParamsIfNoExist(key: String, values: String*): Metadata = + addQueryParamsIfNotExist(key, values.map(Some(_)):_*) + + def addQueryParamsIfNotExist(key: String, values: Option[String]*): Metadata = query.get(key) match { case Some(_) => self case None => copy(query = query + (key -> values.toList)) @@ -91,6 +96,9 @@ case class Metadata( addMultipleHeaders(CaseInsensitive(key), value) def addMultipleQueryParams(key: String, value: List[String]): Metadata = + addMultipleQueryParamsOpt(key, value.map(Some(_))) + + def addMultipleQueryParamsOpt(key: String, value: List[Option[String]]): Metadata = query.get(key) match { case Some(existing) => copy(query = query + (key -> (existing ++ value))) @@ -119,11 +127,11 @@ case class Metadata( ) } - def find(location: HttpBinding): Option[(String, List[String])] = + def find(location: HttpBinding): Option[(Option[String], List[Option[String]])] = location match { case HttpBinding.HeaderBinding(httpName) => headers.get(httpName).flatMap { - case head :: tl => Some((head, tl)) + case head :: tl => Some((Some(head), tl.map(Some(_)))) case Nil => None } case HttpBinding.QueryBinding(httpName) => @@ -131,7 +139,7 @@ case class Metadata( case head :: tl => Some((head, tl)) case Nil => None } - case HttpBinding.PathBinding(httpName) => path.get(httpName).map(_ -> Nil) + case HttpBinding.PathBinding(httpName) => path.get(httpName).map(v => Some(v) -> Nil) case _ => None } } diff --git a/modules/core/src/smithy4s/http/internals/MetaDecode.scala b/modules/core/src/smithy4s/http/internals/MetaDecode.scala index 2424fac5f..050b2c73c 100644 --- a/modules/core/src/smithy4s/http/internals/MetaDecode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaDecode.scala @@ -78,12 +78,12 @@ private[http] sealed abstract class MetaDecode[+A] { case (QueryBinding(h), StringValueMetaDecode(f)) => lookupAndProcess(_.query, h) { (values, fieldName, putField) => if (values.size == 1) { - putField(f(values.head)) + putField(f(values.head.get)) //FIXME: denisrosca } else throw MetadataError.ArityError(fieldName, binding) } case (QueryBinding(q), StringCollectionMetaDecode(f)) => lookupAndProcess(_.query, q) { (values, fieldName, putField) => - putField(f(values.iterator)) + putField(f(values.iterator.map(_.get))) //FIXME: denisrosca } // see https://smithy.io/2.0/spec/http-bindings.html#httpqueryparams-trait // when targeting Map[String,String] we take the first value encountered @@ -92,7 +92,7 @@ private[http] sealed abstract class MetaDecode[+A] { val iter: Iterator[(FieldName, FieldName)] = metadata.query.iterator .map { case (k, values) => if (values.nonEmpty) { - k -> values.head + k -> values.head.get //FIXME: denisrosca } else throw MetadataError.NotFound(fieldName, QueryParamsBinding) } if (iter.nonEmpty) putField(f(iter)) @@ -102,7 +102,7 @@ private[http] sealed abstract class MetaDecode[+A] { (metadata, putField) => val iter = metadata.query.iterator .map { case (k, values) => - k -> values.iterator + k -> values.map(_.get).iterator //FIXME: denisrosca } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) @@ -161,6 +161,8 @@ private[http] object MetaDecode { final case class StringListMapMetaDecode[A](f: Iterator[(String, Iterator[String])] => A) extends MetaDecode[A] case object EmptyMetaDecode extends MetaDecode[Nothing] final case class StructureMetaDecode[A](f: Metadata => Either[MetadataError, A]) extends MetaDecode[A] + // final case class SparseStringCollectionMetaDecode[A](f: Iterator[Option[String]] => A) extends MetaDecode[A] + // final case class SparseStringMapMetaDecode[A](f: Iterator[(String, Option[String])]) extends MetaDecode[A] // format: on type PutField = Any => Unit diff --git a/modules/core/src/smithy4s/http/internals/package.scala b/modules/core/src/smithy4s/http/internals/package.scala index a6b03ad07..368082f9c 100644 --- a/modules/core/src/smithy4s/http/internals/package.scala +++ b/modules/core/src/smithy4s/http/internals/package.scala @@ -75,15 +75,16 @@ package object internals { private[http] def staticQueryParams( uri: String - ): Map[String, Seq[String]] = { + ): Map[String, Seq[Option[String]]] = { uri.split("\\?", 2) match { case Array(_) => Map.empty case Array(_, query) => - query.split("&").toList.foldLeft(Map.empty[String, Seq[String]]) { + query.split("&").toList.foldLeft(Map.empty[String, Seq[Option[String]]]) { case (acc, param) => val (k, v) = param.split("=", 2) match { - case Array(key) => (key, "") - case Array(key, value) => (key, value) + case Array(key) => (key, None) + case Array(key, "") => (key, None) //FIXME: denisrosca not sure about this + case Array(key, value) => (key, Some(value)) } acc.updated(k, acc.getOrElse(k, Seq.empty) :+ v) } diff --git a/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala b/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala index b9f81ff75..77e1b8f9b 100644 --- a/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala +++ b/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala @@ -99,6 +99,7 @@ package object kernel { def fromSmithy4sHttpUri(uri: Smithy4sHttpUri): Uri = { val path = Uri.Path.Root.addSegments(uri.path.map(Uri.Path.Segment(_)).toVector) val authority = uri.host.map(h => Uri.Authority(host = Uri.RegName(h), port = uri.port)) + Uri( path = path, authority = authority, @@ -107,8 +108,15 @@ package object kernel { case Smithy4sHttpUriScheme.Http => Uri.Scheme.http case Smithy4sHttpUriScheme.Https => Uri.Scheme.https } - } - ).withMultiValueQueryParams(uri.queryParams) + }, + query = Query.fromVector( + uri.queryParams + .toVector + .flatMap{ case(key, values) => + values.map(value => key -> value) + } + ) + ) } /** @@ -186,12 +194,8 @@ package object kernel { private[smithy4s] def getQueryParams[F[_]]( uri: Uri - ): Map[String, List[String]] = + ): Map[String, List[Option[String]]] = uri.query.pairs - .collect { - case (name, None) => name -> "true" - case (name, Some(value)) => name -> value - } .groupBy(_._1) .map { case (k, v) => k -> v.map(_._2).toList } From 3c8e1e1567e05cde0ed9e039f7a7067811b15b59 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Tue, 4 Jun 2024 15:19:38 +0300 Subject: [PATCH 02/11] Fix compilation errors --- .../smithy4s/http/internals/MetaDecode.scala | 27 ++++++------- .../SchemaVisitorMetadataReader.scala | 39 ++++++++++++++++--- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/modules/core/src/smithy4s/http/internals/MetaDecode.scala b/modules/core/src/smithy4s/http/internals/MetaDecode.scala index 050b2c73c..68db12123 100644 --- a/modules/core/src/smithy4s/http/internals/MetaDecode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaDecode.scala @@ -73,26 +73,29 @@ private[http] sealed abstract class MetaDecode[+A] { } case (HeaderBinding(h), StringCollectionMetaDecode(f)) => lookupAndProcess(_.headers, h) { (values, fieldName, putField) => - putField(f(values.iterator)) + putField(f(values.map(Some(_)).iterator)) } case (QueryBinding(h), StringValueMetaDecode(f)) => lookupAndProcess(_.query, h) { (values, fieldName, putField) => if (values.size == 1) { - putField(f(values.head.get)) //FIXME: denisrosca + values.head match { + case Some(value) => putField(f(value)) + case None => throw MetadataError.NotFound(fieldName, binding) + } } else throw MetadataError.ArityError(fieldName, binding) } case (QueryBinding(q), StringCollectionMetaDecode(f)) => lookupAndProcess(_.query, q) { (values, fieldName, putField) => - putField(f(values.iterator.map(_.get))) //FIXME: denisrosca + putField(f(values.iterator)) } // see https://smithy.io/2.0/spec/http-bindings.html#httpqueryparams-trait // when targeting Map[String,String] we take the first value encountered case (QueryParamsBinding, StringMapMetaDecode(f)) => { (metadata, putField) => - val iter: Iterator[(FieldName, FieldName)] = metadata.query.iterator + val iter: Iterator[(FieldName, Option[FieldName])] = metadata.query.iterator .map { case (k, values) => if (values.nonEmpty) { - k -> values.head.get //FIXME: denisrosca + k -> values.head } else throw MetadataError.NotFound(fieldName, QueryParamsBinding) } if (iter.nonEmpty) putField(f(iter)) @@ -102,7 +105,7 @@ private[http] sealed abstract class MetaDecode[+A] { (metadata, putField) => val iter = metadata.query.iterator .map { case (k, values) => - k -> values.map(_.get).iterator //FIXME: denisrosca + k -> values.iterator } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) @@ -113,7 +116,7 @@ private[http] sealed abstract class MetaDecode[+A] { .collect { case (k, values) if k.startsWith(prefix) => if (values.size == 1) { - k.toString.drop(prefix.length()) -> values.head + k.toString.drop(prefix.length()) -> Some(values.head) } else throw MetadataError.ArityError(fieldName, HeaderBinding(k)) } @@ -125,7 +128,7 @@ private[http] sealed abstract class MetaDecode[+A] { val iter = metadata.headers.iterator .collect { case (k, values) if k.startsWith(prefix) => - k.toString.drop(prefix.length()) -> values.iterator + k.toString.drop(prefix.length()) -> values.map(Some(_)).iterator } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) @@ -156,13 +159,11 @@ private[http] object MetaDecode { // format: off final case class StringValueMetaDecode[A](f: String => A) extends MetaDecode[A] - final case class StringCollectionMetaDecode[A](f: Iterator[String] => A) extends MetaDecode[A] - final case class StringMapMetaDecode[A](f: Iterator[(String, String)] => A) extends MetaDecode[A] - final case class StringListMapMetaDecode[A](f: Iterator[(String, Iterator[String])] => A) extends MetaDecode[A] + final case class StringCollectionMetaDecode[A](f: Iterator[Option[String]] => A) extends MetaDecode[A] + final case class StringMapMetaDecode[A](f: Iterator[(String, Option[String])] => A) extends MetaDecode[A] + final case class StringListMapMetaDecode[A](f: Iterator[(String, Iterator[Option[String]])] => A) extends MetaDecode[A] case object EmptyMetaDecode extends MetaDecode[Nothing] final case class StructureMetaDecode[A](f: Metadata => Either[MetadataError, A]) extends MetaDecode[A] - // final case class SparseStringCollectionMetaDecode[A](f: Iterator[Option[String]] => A) extends MetaDecode[A] - // final case class SparseStringMapMetaDecode[A](f: Iterator[(String, Option[String])]) extends MetaDecode[A] // format: on type PutField = Any => Unit diff --git a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala index 3d5c8669e..a59e9b23c 100644 --- a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala +++ b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala @@ -73,14 +73,35 @@ private[http] class SchemaVisitorMetadataReader( val isAwsHeader = hints .get(HttpBinding) .exists(_.tpe == HttpBinding.Type.HeaderType) && awsHeaderEncoding - (SchemaVisitorHeaderSplit(member), isAwsHeader) match { - case (Some(splitFunction), true) => + + val hasSparse = hints.has(smithy.api.Sparse) + + (SchemaVisitorHeaderSplit(member), isAwsHeader, hasSparse) match { + case (Some(splitFunction), true, false) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator(it.flatMap(splitFunction).map(f)) + tag.fromIterator( + it.flatMap{ + case Some(value) => splitFunction(value) + case None => throw MetadataError.ImpossibleDecoding("Collection does not accept null values") + }.map(f) + ) + } + case (Some(splitFunction), true, true) => + MetaDecode.StringCollectionMetaDecode[C[A]] { it => + tag.fromIterator( + it.flatMap{ + case Some(value) => splitFunction(value) + case None => Seq.empty + }.map(f) + ) } - case (_, _) => + case (_, _, _) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator(it.map(f)) + tag.fromIterator(it.map{ + case Some(value) => f(value) + case None if hasSparse => f("") // fixme: Denis Rosca: This is a hack to make the test pass. We should not be decoding null values. + case None => throw MetadataError.ImpossibleDecoding("Collection does not accept null values") + }) } } case _ => EmptyMetaDecode @@ -93,10 +114,16 @@ private[http] class SchemaVisitorMetadataReader( key: Schema[K], value: Schema[V] ): MetaDecode[Map[K, V]] = { + val hasSparse = hints.has(smithy.api.Sparse) + (self(key), self(value.addHints(httpHints(hints)))) match { case (StringValueMetaDecode(readK), StringValueMetaDecode(readV)) => StringMapMetaDecode[Map[K, V]](map => - map.map { case (k, v) => (readK(k), readV(v)) }.toMap + map.map { + case (k, Some(v)) => (readK(k), readV(v)) + case (k, None) if hasSparse => (readK(k), readV("")) + case _ => throw MetadataError.ImpossibleDecoding("Map does not accept null values") + }.toMap ) case (StringValueMetaDecode(readK), StringCollectionMetaDecode(readV)) => StringListMapMetaDecode[Map[K, V]](map => From cd2e12016b08b761543da80e6c85c76f053aeca0 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Tue, 4 Jun 2024 15:20:52 +0300 Subject: [PATCH 03/11] Fix formatting --- .../http/internals/MetadataSpec.scala | 8 +++-- .../smithy4s/http/internals/PathSpec.scala | 3 +- modules/core/src/smithy4s/http/Metadata.scala | 23 ++++++++----- .../smithy4s/http/internals/MetaDecode.scala | 16 +++++---- .../SchemaVisitorMetadataReader.scala | 34 ++++++++++++------- .../src/smithy4s/http/internals/package.scala | 22 +++++++----- .../src/smithy4s/http4s/kernel/package.scala | 9 +++-- 7 files changed, 71 insertions(+), 44 deletions(-) diff --git a/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala b/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala index 153f66b46..f282a27b4 100644 --- a/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala @@ -210,7 +210,9 @@ class MetadataSpec() extends FunSuite { slm = Some(Map("ts4" -> "Thu, 01 Jan 1970 00:00:00 GMT")) ) val expected = - Metadata(query = Map("ts4" -> List(Some("Thu, 01 Jan 1970 00:00:00 GMT")))) + Metadata(query = + Map("ts4" -> List(Some("Thu, 01 Jan 1970 00:00:00 GMT"))) + ) checkQueryRoundTrip(queries, expected, finished) } @@ -439,7 +441,9 @@ class MetadataSpec() extends FunSuite { test("bad data gets caught") { val metadata = - Metadata(query = Map("ts3" -> List(Some("Thu, 01 Jan 1970 00:00:00 GMT")))) + Metadata(query = + Map("ts3" -> List(Some("Thu, 01 Jan 1970 00:00:00 GMT"))) + ) val result = Metadata.decode[Queries](metadata) val expected = MetadataError.WrongType( "ts3", diff --git a/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala b/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala index e35becc88..1a53a069d 100644 --- a/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/http/internals/PathSpec.scala @@ -80,7 +80,8 @@ class PathSpec() extends munit.FunSuite { val sqp = httpEndpoint.staticQueryParams val path = httpEndpoint.path - val expectedQueryMap = Map("value" -> Seq(Some("foo")), "baz" -> Seq(Some("bar"))) + val expectedQueryMap = + Map("value" -> Seq(Some("foo")), "baz" -> Seq(Some("bar"))) expect(sqp == expectedQueryMap) expect( path == diff --git a/modules/core/src/smithy4s/http/Metadata.scala b/modules/core/src/smithy4s/http/Metadata.scala index 04a47fb5f..a88343393 100644 --- a/modules/core/src/smithy4s/http/Metadata.scala +++ b/modules/core/src/smithy4s/http/Metadata.scala @@ -49,9 +49,10 @@ case class Metadata( v.map(k -> _) } - def queryFlattened: Vector[(String, Option[String])] = query.toVector.flatMap { - case (k, v) => v.map(k -> _) - } + def queryFlattened: Vector[(String, Option[String])] = + query.toVector.flatMap { case (k, v) => + v.map(k -> _) + } def addHeader(ciKey: CaseInsensitive, value: String): Metadata = { headers.get(ciKey) match { @@ -75,7 +76,7 @@ case class Metadata( case None => copy(query = query + (key -> List(value))) } def addQueryParamsIfNoExist(key: String, values: String*): Metadata = - addQueryParamsIfNotExist(key, values.map(Some(_)):_*) + addQueryParamsIfNotExist(key, values.map(Some(_)): _*) def addQueryParamsIfNotExist(key: String, values: Option[String]*): Metadata = query.get(key) match { @@ -98,7 +99,10 @@ case class Metadata( def addMultipleQueryParams(key: String, value: List[String]): Metadata = addMultipleQueryParamsOpt(key, value.map(Some(_))) - def addMultipleQueryParamsOpt(key: String, value: List[Option[String]]): Metadata = + def addMultipleQueryParamsOpt( + key: String, + value: List[Option[String]] + ): Metadata = query.get(key) match { case Some(existing) => copy(query = query + (key -> (existing ++ value))) @@ -127,7 +131,9 @@ case class Metadata( ) } - def find(location: HttpBinding): Option[(Option[String], List[Option[String]])] = + def find( + location: HttpBinding + ): Option[(Option[String], List[Option[String]])] = location match { case HttpBinding.HeaderBinding(httpName) => headers.get(httpName).flatMap { @@ -139,8 +145,9 @@ case class Metadata( case head :: tl => Some((head, tl)) case Nil => None } - case HttpBinding.PathBinding(httpName) => path.get(httpName).map(v => Some(v) -> Nil) - case _ => None + case HttpBinding.PathBinding(httpName) => + path.get(httpName).map(v => Some(v) -> Nil) + case _ => None } } diff --git a/modules/core/src/smithy4s/http/internals/MetaDecode.scala b/modules/core/src/smithy4s/http/internals/MetaDecode.scala index 68db12123..f8eb74d08 100644 --- a/modules/core/src/smithy4s/http/internals/MetaDecode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaDecode.scala @@ -80,7 +80,7 @@ private[http] sealed abstract class MetaDecode[+A] { if (values.size == 1) { values.head match { case Some(value) => putField(f(value)) - case None => throw MetadataError.NotFound(fieldName, binding) + case None => throw MetadataError.NotFound(fieldName, binding) } } else throw MetadataError.ArityError(fieldName, binding) } @@ -92,12 +92,14 @@ private[http] sealed abstract class MetaDecode[+A] { // when targeting Map[String,String] we take the first value encountered case (QueryParamsBinding, StringMapMetaDecode(f)) => { (metadata, putField) => - val iter: Iterator[(FieldName, Option[FieldName])] = metadata.query.iterator - .map { case (k, values) => - if (values.nonEmpty) { - k -> values.head - } else throw MetadataError.NotFound(fieldName, QueryParamsBinding) - } + val iter: Iterator[(FieldName, Option[FieldName])] = + metadata.query.iterator + .map { case (k, values) => + if (values.nonEmpty) { + k -> values.head + } else + throw MetadataError.NotFound(fieldName, QueryParamsBinding) + } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) } diff --git a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala index a59e9b23c..e5a4e6beb 100644 --- a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala +++ b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala @@ -73,34 +73,41 @@ private[http] class SchemaVisitorMetadataReader( val isAwsHeader = hints .get(HttpBinding) .exists(_.tpe == HttpBinding.Type.HeaderType) && awsHeaderEncoding - + val hasSparse = hints.has(smithy.api.Sparse) - + (SchemaVisitorHeaderSplit(member), isAwsHeader, hasSparse) match { case (Some(splitFunction), true, false) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => tag.fromIterator( - it.flatMap{ + it.flatMap { case Some(value) => splitFunction(value) - case None => throw MetadataError.ImpossibleDecoding("Collection does not accept null values") + case None => + throw MetadataError.ImpossibleDecoding( + "Collection does not accept null values" + ) }.map(f) ) } case (Some(splitFunction), true, true) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => tag.fromIterator( - it.flatMap{ + it.flatMap { case Some(value) => splitFunction(value) - case None => Seq.empty + case None => Seq.empty }.map(f) ) } case (_, _, _) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator(it.map{ + tag.fromIterator(it.map { case Some(value) => f(value) - case None if hasSparse => f("") // fixme: Denis Rosca: This is a hack to make the test pass. We should not be decoding null values. - case None => throw MetadataError.ImpossibleDecoding("Collection does not accept null values") + case None if hasSparse => + f("") // fixme: Denis Rosca: This is a hack to make the test pass. We should not be decoding null values. + case None => + throw MetadataError.ImpossibleDecoding( + "Collection does not accept null values" + ) }) } } @@ -119,10 +126,13 @@ private[http] class SchemaVisitorMetadataReader( (self(key), self(value.addHints(httpHints(hints)))) match { case (StringValueMetaDecode(readK), StringValueMetaDecode(readV)) => StringMapMetaDecode[Map[K, V]](map => - map.map { - case (k, Some(v)) => (readK(k), readV(v)) + map.map { + case (k, Some(v)) => (readK(k), readV(v)) case (k, None) if hasSparse => (readK(k), readV("")) - case _ => throw MetadataError.ImpossibleDecoding("Map does not accept null values") + case _ => + throw MetadataError.ImpossibleDecoding( + "Map does not accept null values" + ) }.toMap ) case (StringValueMetaDecode(readK), StringCollectionMetaDecode(readV)) => diff --git a/modules/core/src/smithy4s/http/internals/package.scala b/modules/core/src/smithy4s/http/internals/package.scala index 368082f9c..11092806f 100644 --- a/modules/core/src/smithy4s/http/internals/package.scala +++ b/modules/core/src/smithy4s/http/internals/package.scala @@ -79,15 +79,19 @@ package object internals { uri.split("\\?", 2) match { case Array(_) => Map.empty case Array(_, query) => - query.split("&").toList.foldLeft(Map.empty[String, Seq[Option[String]]]) { - case (acc, param) => - val (k, v) = param.split("=", 2) match { - case Array(key) => (key, None) - case Array(key, "") => (key, None) //FIXME: denisrosca not sure about this - case Array(key, value) => (key, Some(value)) - } - acc.updated(k, acc.getOrElse(k, Seq.empty) :+ v) - } + query + .split("&") + .toList + .foldLeft(Map.empty[String, Seq[Option[String]]]) { + case (acc, param) => + val (k, v) = param.split("=", 2) match { + case Array(key) => (key, None) + case Array(key, "") => + (key, None) // FIXME: denisrosca not sure about this + case Array(key, value) => (key, Some(value)) + } + acc.updated(k, acc.getOrElse(k, Seq.empty) :+ v) + } } } diff --git a/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala b/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala index 77e1b8f9b..1950e3d4f 100644 --- a/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala +++ b/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala @@ -110,11 +110,10 @@ package object kernel { } }, query = Query.fromVector( - uri.queryParams - .toVector - .flatMap{ case(key, values) => - values.map(value => key -> value) - } + uri.queryParams.toVector + .flatMap { case (key, values) => + values.map(value => key -> value) + } ) ) } From 6b99b789938c7c481c3904aab29b74ee8fb5dd6c Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Tue, 4 Jun 2024 16:59:49 +0300 Subject: [PATCH 04/11] Add test --- ....example.ServiceWithSparseQueryParams.json | 59 ++++++++++++ .../ServiceWithSparseQueryParams.scala | 85 ++++++++++++++++ .../smithy4s/example/SparseFooList.scala | 18 ++++ .../smithy4s/example/SparseQueryInput.scala | 22 +++++ .../smithy4s/example/SparseQueryOutput.scala | 22 +++++ .../generated/smithy4s/example/package.scala | 3 + .../http4s/SparseQueryParametersSuite.scala | 96 +++++++++++++++++++ .../serviceWithSparseQueryParams.smithy | 35 +++++++ 8 files changed, 340 insertions(+) create mode 100644 modules/bootstrapped/resources/smithy4s.example.ServiceWithSparseQueryParams.json create mode 100644 modules/bootstrapped/src/generated/smithy4s/example/ServiceWithSparseQueryParams.scala create mode 100644 modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala create mode 100644 modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala create mode 100644 modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala create mode 100644 modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala create mode 100644 sampleSpecs/serviceWithSparseQueryParams.smithy diff --git a/modules/bootstrapped/resources/smithy4s.example.ServiceWithSparseQueryParams.json b/modules/bootstrapped/resources/smithy4s.example.ServiceWithSparseQueryParams.json new file mode 100644 index 000000000..83f290903 --- /dev/null +++ b/modules/bootstrapped/resources/smithy4s.example.ServiceWithSparseQueryParams.json @@ -0,0 +1,59 @@ +{ + "openapi": "3.0.2", + "info": { + "title": "ServiceWithSparseQueryParams", + "version": "1.0" + }, + "paths": { + "/operation/sparse-query-params": { + "get": { + "operationId": "GetOperation", + "parameters": [ + { + "name": "foo", + "in": "query", + "style": "form", + "schema": { + "type": "array", + "items": { + "type": "string" + } + }, + "explode": true, + "required": true + } + ], + "responses": { + "200": { + "description": "GetOperation 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetOperationResponseContent" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "GetOperationResponseContent": { + "type": "object", + "properties": { + "foo": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "foo" + ] + } + } + } +} \ No newline at end of file diff --git a/modules/bootstrapped/src/generated/smithy4s/example/ServiceWithSparseQueryParams.scala b/modules/bootstrapped/src/generated/smithy4s/example/ServiceWithSparseQueryParams.scala new file mode 100644 index 000000000..a671d9891 --- /dev/null +++ b/modules/bootstrapped/src/generated/smithy4s/example/ServiceWithSparseQueryParams.scala @@ -0,0 +1,85 @@ +package smithy4s.example + +import smithy4s.Endpoint +import smithy4s.Hints +import smithy4s.Schema +import smithy4s.Service +import smithy4s.ShapeId +import smithy4s.Transformation +import smithy4s.kinds.PolyFunction5 +import smithy4s.kinds.toPolyFunction5.const5 +import smithy4s.schema.OperationSchema + +trait ServiceWithSparseQueryParamsGen[F[_, _, _, _, _]] { + self => + + def getOperation(foo: List[Option[String]]): F[SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] + + def transform: Transformation.PartiallyApplied[ServiceWithSparseQueryParamsGen[F]] = Transformation.of[ServiceWithSparseQueryParamsGen[F]](this) +} + +object ServiceWithSparseQueryParamsGen extends Service.Mixin[ServiceWithSparseQueryParamsGen, ServiceWithSparseQueryParamsOperation] { + + val id: ShapeId = ShapeId("smithy4s.example", "ServiceWithSparseQueryParams") + val version: String = "1.0" + + val hints: Hints = Hints( + alloy.SimpleRestJson(), + ).lazily + + def apply[F[_]](implicit F: Impl[F]): F.type = F + + object ErrorAware { + def apply[F[_, _]](implicit F: ErrorAware[F]): F.type = F + type Default[F[+_, +_]] = Constant[smithy4s.kinds.stubs.Kind2[F]#toKind5] + } + + val endpoints: Vector[smithy4s.Endpoint[ServiceWithSparseQueryParamsOperation, _, _, _, _, _]] = Vector( + ServiceWithSparseQueryParamsOperation.GetOperation, + ) + + def input[I, E, O, SI, SO](op: ServiceWithSparseQueryParamsOperation[I, E, O, SI, SO]): I = op.input + def ordinal[I, E, O, SI, SO](op: ServiceWithSparseQueryParamsOperation[I, E, O, SI, SO]): Int = op.ordinal + override def endpoint[I, E, O, SI, SO](op: ServiceWithSparseQueryParamsOperation[I, E, O, SI, SO]) = op.endpoint + class Constant[P[-_, +_, +_, +_, +_]](value: P[Any, Nothing, Nothing, Nothing, Nothing]) extends ServiceWithSparseQueryParamsOperation.Transformed[ServiceWithSparseQueryParamsOperation, P](reified, const5(value)) + type Default[F[+_]] = Constant[smithy4s.kinds.stubs.Kind1[F]#toKind5] + def reified: ServiceWithSparseQueryParamsGen[ServiceWithSparseQueryParamsOperation] = ServiceWithSparseQueryParamsOperation.reified + def mapK5[P[_, _, _, _, _], P1[_, _, _, _, _]](alg: ServiceWithSparseQueryParamsGen[P], f: PolyFunction5[P, P1]): ServiceWithSparseQueryParamsGen[P1] = new ServiceWithSparseQueryParamsOperation.Transformed(alg, f) + def fromPolyFunction[P[_, _, _, _, _]](f: PolyFunction5[ServiceWithSparseQueryParamsOperation, P]): ServiceWithSparseQueryParamsGen[P] = new ServiceWithSparseQueryParamsOperation.Transformed(reified, f) + def toPolyFunction[P[_, _, _, _, _]](impl: ServiceWithSparseQueryParamsGen[P]): PolyFunction5[ServiceWithSparseQueryParamsOperation, P] = ServiceWithSparseQueryParamsOperation.toPolyFunction(impl) + +} + +sealed trait ServiceWithSparseQueryParamsOperation[Input, Err, Output, StreamedInput, StreamedOutput] { + def run[F[_, _, _, _, _]](impl: ServiceWithSparseQueryParamsGen[F]): F[Input, Err, Output, StreamedInput, StreamedOutput] + def ordinal: Int + def input: Input + def endpoint: Endpoint[ServiceWithSparseQueryParamsOperation, Input, Err, Output, StreamedInput, StreamedOutput] +} + +object ServiceWithSparseQueryParamsOperation { + + object reified extends ServiceWithSparseQueryParamsGen[ServiceWithSparseQueryParamsOperation] { + def getOperation(foo: List[Option[String]]): GetOperation = GetOperation(SparseQueryInput(foo)) + } + class Transformed[P[_, _, _, _, _], P1[_ ,_ ,_ ,_ ,_]](alg: ServiceWithSparseQueryParamsGen[P], f: PolyFunction5[P, P1]) extends ServiceWithSparseQueryParamsGen[P1] { + def getOperation(foo: List[Option[String]]): P1[SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] = f[SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing](alg.getOperation(foo)) + } + + def toPolyFunction[P[_, _, _, _, _]](impl: ServiceWithSparseQueryParamsGen[P]): PolyFunction5[ServiceWithSparseQueryParamsOperation, P] = new PolyFunction5[ServiceWithSparseQueryParamsOperation, P] { + def apply[I, E, O, SI, SO](op: ServiceWithSparseQueryParamsOperation[I, E, O, SI, SO]): P[I, E, O, SI, SO] = op.run(impl) + } + final case class GetOperation(input: SparseQueryInput) extends ServiceWithSparseQueryParamsOperation[SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] { + def run[F[_, _, _, _, _]](impl: ServiceWithSparseQueryParamsGen[F]): F[SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] = impl.getOperation(input.foo) + def ordinal: Int = 0 + def endpoint: smithy4s.Endpoint[ServiceWithSparseQueryParamsOperation,SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] = GetOperation + } + object GetOperation extends smithy4s.Endpoint[ServiceWithSparseQueryParamsOperation,SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] { + val schema: OperationSchema[SparseQueryInput, Nothing, SparseQueryOutput, Nothing, Nothing] = Schema.operation(ShapeId("smithy4s.example", "GetOperation")) + .withInput(SparseQueryInput.schema) + .withOutput(SparseQueryOutput.schema) + .withHints(smithy.api.Http(method = smithy.api.NonEmptyString("GET"), uri = smithy.api.NonEmptyString("/operation/sparse-query-params"), code = 200)) + def wrap(input: SparseQueryInput): GetOperation = GetOperation(input) + } +} + diff --git a/modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala new file mode 100644 index 000000000..52a3204e7 --- /dev/null +++ b/modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala @@ -0,0 +1,18 @@ +package smithy4s.example + +import smithy4s.Hints +import smithy4s.Newtype +import smithy4s.Schema +import smithy4s.ShapeId +import smithy4s.schema.Schema.bijection +import smithy4s.schema.Schema.list +import smithy4s.schema.Schema.string + +object SparseFooList extends Newtype[List[Option[String]]] { + val id: ShapeId = ShapeId("smithy4s.example", "SparseFooList") + val hints: Hints = Hints( + smithy.api.Sparse(), + ).lazily + val underlyingSchema: Schema[List[Option[String]]] = list(string.option).withId(id).addHints(hints) + implicit val schema: Schema[SparseFooList] = bijection(underlyingSchema, asBijection) +} diff --git a/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala new file mode 100644 index 000000000..35d565ee7 --- /dev/null +++ b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala @@ -0,0 +1,22 @@ +package smithy4s.example + +import smithy4s.Hints +import smithy4s.Schema +import smithy4s.ShapeId +import smithy4s.ShapeTag +import smithy4s.schema.Schema.struct + +final case class SparseQueryInput(foo: List[Option[String]]) + +object SparseQueryInput extends ShapeTag.Companion[SparseQueryInput] { + val id: ShapeId = ShapeId("smithy4s.example", "SparseQueryInput") + + val hints: Hints = Hints.empty + + // constructor using the original order from the spec + private def make(foo: List[Option[String]]): SparseQueryInput = SparseQueryInput(foo) + + implicit val schema: Schema[SparseQueryInput] = struct( + SparseFooList.underlyingSchema.required[SparseQueryInput]("foo", _.foo).addHints(smithy.api.HttpQuery("foo")), + )(make).withId(id).addHints(hints) +} diff --git a/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala new file mode 100644 index 000000000..58ef9f3fb --- /dev/null +++ b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala @@ -0,0 +1,22 @@ +package smithy4s.example + +import smithy4s.Hints +import smithy4s.Schema +import smithy4s.ShapeId +import smithy4s.ShapeTag +import smithy4s.schema.Schema.struct + +final case class SparseQueryOutput(foo: List[Option[String]]) + +object SparseQueryOutput extends ShapeTag.Companion[SparseQueryOutput] { + val id: ShapeId = ShapeId("smithy4s.example", "SparseQueryOutput") + + val hints: Hints = Hints.empty + + // constructor using the original order from the spec + private def make(foo: List[Option[String]]): SparseQueryOutput = SparseQueryOutput(foo) + + implicit val schema: Schema[SparseQueryOutput] = struct( + SparseFooList.underlyingSchema.required[SparseQueryOutput]("foo", _.foo), + )(make).withId(id).addHints(hints) +} diff --git a/modules/bootstrapped/src/generated/smithy4s/example/package.scala b/modules/bootstrapped/src/generated/smithy4s/example/package.scala index 26b3d506a..17f5d6b38 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/package.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/package.scala @@ -16,6 +16,8 @@ package object example { val PizzaAdminService = PizzaAdminServiceGen type FooService[F[_]] = smithy4s.kinds.FunctorAlgebra[FooServiceGen, F] val FooService = FooServiceGen + type ServiceWithSparseQueryParams[F[_]] = smithy4s.kinds.FunctorAlgebra[ServiceWithSparseQueryParamsGen, F] + val ServiceWithSparseQueryParams = ServiceWithSparseQueryParamsGen type KVStore[F[_]] = smithy4s.kinds.FunctorAlgebra[KVStoreGen, F] val KVStore = KVStoreGen type ObjectService[F[_]] = smithy4s.kinds.FunctorAlgebra[ObjectServiceGen, F] @@ -95,6 +97,7 @@ package object example { type SomeInt = smithy4s.example.SomeInt.Type type SomeValue = smithy4s.example.SomeValue.Type type SomeVector = smithy4s.example.SomeVector.Type + type SparseFooList = smithy4s.example.SparseFooList.Type type SparseStringList = smithy4s.example.SparseStringList.Type type SparseStringMap = smithy4s.example.SparseStringMap.Type type StreamedBlob = smithy4s.example.StreamedBlob.Type diff --git a/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala b/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala new file mode 100644 index 000000000..3204f3db8 --- /dev/null +++ b/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala @@ -0,0 +1,96 @@ +package smithy4s.http4s + +import cats.effect.kernel.Deferred +import cats.effect.IO +import io.circe.Json +import org.http4s._ +import org.http4s.circe.CirceInstances +import org.http4s.client.Client +import org.http4s.implicits._ + +import smithy4s.example.{ServiceWithSparseQueryParams, SparseQueryOutput} +import weaver._ + +object SparseQueryParametersSuite extends SimpleIOSuite with CirceInstances { + + test("Server handles sparse query parameters") { + runServerTest().map { response => + assert.same( + Json.obj("foo" -> Json.arr(Json.fromString("bar"), Json.Null, Json.fromString("baz"))), + response + ) + } + } + + test("Client handles sparse query parameters") { + runClientTest(List(Some("bar"), None, Some("baz"))).map { request => + assert.same( + Map("foo" -> List(Some("bar"), None, Some("baz"))), + request.query + ) + } + } + + private def runServerTest(): IO[Json] = { + def run( + routes: HttpRoutes[IO], + req: Request[IO] + ): IO[Json] = + routes.orNotFound.run(req).flatMap { response => + response.as[Json] + } + SimpleRestJsonBuilder + .routes(Impl) + .resource + .use { routes => + for { + result <- run( + routes, + Request[IO](method = Method.GET, uri = uri"/operation/sparse-query-params?foo=bar&foo&foo=baz") + ) + } yield result + } + } + + private def runClientTest( + input: List[Option[String]] + ): IO[TestRequest] = { + val resources = for { + promise <- Deferred[IO, Request[IO]].toResource + reqBody = Json.obj("foo" -> Json.arr(input.map(_.fold(Json.Null)(Json.fromString)): _*)).toString().getBytes() + httpClient: Client[IO] = Client(req => + req + .toStrict(None) + .flatMap(promise.complete) + .as(Response[IO](body = fs2.Stream.emits(reqBody))) + .toResource + ) + client <- SimpleRestJsonBuilder + .apply(ServiceWithSparseQueryParams) + .client(httpClient) + .resource + } yield (promise, client) + resources.use { case (promise, client) => + client.getOperation(input) >> promise.get.flatMap { req => + IO(TestRequest(queryParamsToMap(req.uri.query))) + } + } + } + + def queryParamsToMap(query: Query): Map[String, List[Option[String]]] = { + query.pairs.groupBy(_._1).map { case (k, v) => + k -> v.map(_._2).toList + } + } + + case class TestResponse(body: Json) + + case class TestRequest(query: Map[String, List[Option[String]]]) + + object Impl extends ServiceWithSparseQueryParams[IO] { + override def getOperation(foo: List[Option[String]]): IO[SparseQueryOutput] = { + IO.pure(SparseQueryOutput(foo)) + } + } + +} diff --git a/sampleSpecs/serviceWithSparseQueryParams.smithy b/sampleSpecs/serviceWithSparseQueryParams.smithy new file mode 100644 index 000000000..3c84745aa --- /dev/null +++ b/sampleSpecs/serviceWithSparseQueryParams.smithy @@ -0,0 +1,35 @@ +$version: "2" + +namespace smithy4s.example + + +use alloy#simpleRestJson + +@simpleRestJson +service ServiceWithSparseQueryParams { + version: "1.0" + operations: [GetOperation] +} + +@http(method: "GET", uri: "/operation/sparse-query-params") +operation GetOperation { + input: SparseQueryInput + + output: SparseQueryOutput +} + +structure SparseQueryInput { + @required + @httpQuery("foo") + foo: SparseFooList +} + +structure SparseQueryOutput { + @required + foo: SparseFooList +} + +@sparse +list SparseFooList { + member: String +} \ No newline at end of file From 3fb76c770a8b69a20dd1dbd239e4d0daba4bd6b1 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 6 Jun 2024 19:35:16 +0300 Subject: [PATCH 05/11] Fix metadata encoder --- .../src/smithy4s/http/HttpContractError.scala | 20 +++++- .../smithy4s/http/internals/MetaDecode.scala | 67 ++++++++++++++----- .../smithy4s/http/internals/MetaEncode.scala | 16 ++++- .../SchemaVisitorMetadataReader.scala | 63 +++++------------ .../SchemaVisitorMetadataWriter.scala | 16 ++++- .../http4s/SparseQueryParametersSuite.scala | 23 +++++-- 6 files changed, 133 insertions(+), 72 deletions(-) diff --git a/modules/core/src/smithy4s/http/HttpContractError.scala b/modules/core/src/smithy4s/http/HttpContractError.scala index 2b381a507..a9e737ea8 100644 --- a/modules/core/src/smithy4s/http/HttpContractError.scala +++ b/modules/core/src/smithy4s/http/HttpContractError.scala @@ -84,6 +84,8 @@ sealed trait MetadataError extends HttpContractError { s"Field $field, found in ${location.show}, failed constraint checks with message: $message" case ImpossibleDecoding(message) => message + case MissingValueError(field, location) => + s"Field $field, found in ${location.show}, is missing a value" } } @@ -127,6 +129,18 @@ object MetadataError { )(ArityError.apply) } + case class MissingValueError( + field: String, + location: HttpBinding + ) extends MetadataError + + object MissingValueError { + val schema = struct( + string.required[MissingValueError]("field", _.field), + HttpBinding.schema.required[MissingValueError]("location", _.location) + )(MissingValueError.apply) + } + case class FailedConstraint( field: String, location: HttpBinding, @@ -159,19 +173,23 @@ object MetadataError { FailedConstraint.schema.oneOf[MetadataError]("failedConstraint") val impossibleDecoding = ImpossibleDecoding.schema.oneOf[MetadataError]("impossibleDecoding") + val missingValueError = + MissingValueError.schema.oneOf[MetadataError]("missingValueError") union( notFound, wrongType, arityError, failedConstraint, - impossibleDecoding + impossibleDecoding, + missingValueError ) { case _: NotFound => 0 case _: WrongType => 1 case _: ArityError => 2 case _: FailedConstraint => 3 case _: ImpossibleDecoding => 4 + case _: MissingValueError => 5 } } diff --git a/modules/core/src/smithy4s/http/internals/MetaDecode.scala b/modules/core/src/smithy4s/http/internals/MetaDecode.scala index f8eb74d08..6fea4f975 100644 --- a/modules/core/src/smithy4s/http/internals/MetaDecode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaDecode.scala @@ -25,8 +25,12 @@ import HttpBinding._ private[http] sealed abstract class MetaDecode[+A] { def map[B](to: A => B): MetaDecode[B] = this match { case StringValueMetaDecode(f) => StringValueMetaDecode(f.andThen(to)) + case OptionalStringValueMetaDecode(f) => + OptionalStringValueMetaDecode(f.andThen(to)) case StringCollectionMetaDecode(f) => StringCollectionMetaDecode(f andThen to) + case SparseStringCollectionMetaDecode(f) => + SparseStringCollectionMetaDecode(f andThen to) case StringMapMetaDecode(f) => StringMapMetaDecode(f.andThen(to)) case StringListMapMetaDecode(f) => StringListMapMetaDecode(f.andThen(to)) case EmptyMetaDecode => EmptyMetaDecode @@ -73,33 +77,53 @@ private[http] sealed abstract class MetaDecode[+A] { } case (HeaderBinding(h), StringCollectionMetaDecode(f)) => lookupAndProcess(_.headers, h) { (values, fieldName, putField) => - putField(f(values.map(Some(_)).iterator)) + putField(f(values.iterator)) } case (QueryBinding(h), StringValueMetaDecode(f)) => lookupAndProcess(_.query, h) { (values, fieldName, putField) => if (values.size == 1) { values.head match { case Some(value) => putField(f(value)) - case None => throw MetadataError.NotFound(fieldName, binding) + case None => throw MetadataError.ArityError(fieldName, binding) } } else throw MetadataError.ArityError(fieldName, binding) } + case (QueryBinding(h), OptionalStringValueMetaDecode(f)) => + lookupAndProcess(_.query, h) { (values, fieldName, putField) => + putField(f(values.head)) + } case (QueryBinding(q), StringCollectionMetaDecode(f)) => lookupAndProcess(_.query, q) { (values, fieldName, putField) => - putField(f(values.iterator)) + val it = values.map { + case Some(value) => value + case None => + throw MetadataError.MissingValueError(fieldName, binding) + }.iterator + putField(f(it)) + } + case (QueryBinding(q), SparseStringCollectionMetaDecode(f)) => + lookupAndProcess(_.query, q) { (values, fieldName, putField) => + val it = values.iterator + putField(f(it)) } // see https://smithy.io/2.0/spec/http-bindings.html#httpqueryparams-trait // when targeting Map[String,String] we take the first value encountered case (QueryParamsBinding, StringMapMetaDecode(f)) => { (metadata, putField) => - val iter: Iterator[(FieldName, Option[FieldName])] = - metadata.query.iterator - .map { case (k, values) => - if (values.nonEmpty) { - k -> values.head - } else - throw MetadataError.NotFound(fieldName, QueryParamsBinding) - } + val iter: Iterator[(FieldName, FieldName)] = metadata.query.iterator + .map { case (k, values) => + if (values.nonEmpty) { + val v = values.head match { + case Some(value) => value + case None => + throw MetadataError.MissingValueError( + fieldName, + QueryParamsBinding + ) + } + k -> v + } else throw MetadataError.NotFound(fieldName, QueryParamsBinding) + } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) } @@ -107,7 +131,14 @@ private[http] sealed abstract class MetaDecode[+A] { (metadata, putField) => val iter = metadata.query.iterator .map { case (k, values) => - k -> values.iterator + k -> values.map { + case Some(value) => value + case None => + throw MetadataError.MissingValueError( + fieldName, + QueryParamsBinding + ) + }.iterator } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) @@ -118,7 +149,7 @@ private[http] sealed abstract class MetaDecode[+A] { .collect { case (k, values) if k.startsWith(prefix) => if (values.size == 1) { - k.toString.drop(prefix.length()) -> Some(values.head) + k.toString.drop(prefix.length()) -> values.head } else throw MetadataError.ArityError(fieldName, HeaderBinding(k)) } @@ -130,7 +161,7 @@ private[http] sealed abstract class MetaDecode[+A] { val iter = metadata.headers.iterator .collect { case (k, values) if k.startsWith(prefix) => - k.toString.drop(prefix.length()) -> values.map(Some(_)).iterator + k.toString.drop(prefix.length()) -> values.iterator } if (iter.nonEmpty) putField(f(iter)) else putDefault(putField) @@ -161,9 +192,11 @@ private[http] object MetaDecode { // format: off final case class StringValueMetaDecode[A](f: String => A) extends MetaDecode[A] - final case class StringCollectionMetaDecode[A](f: Iterator[Option[String]] => A) extends MetaDecode[A] - final case class StringMapMetaDecode[A](f: Iterator[(String, Option[String])] => A) extends MetaDecode[A] - final case class StringListMapMetaDecode[A](f: Iterator[(String, Iterator[Option[String]])] => A) extends MetaDecode[A] + final case class OptionalStringValueMetaDecode[A](f: Option[String] => A) extends MetaDecode[A] + final case class StringCollectionMetaDecode[A](f: Iterator[String] => A) extends MetaDecode[A] + final case class SparseStringCollectionMetaDecode[A](f: Iterator[Option[String]] => A) extends MetaDecode[A] + final case class StringMapMetaDecode[A](f: Iterator[(String, String)] => A) extends MetaDecode[A] + final case class StringListMapMetaDecode[A](f: Iterator[(String, Iterator[String])] => A) extends MetaDecode[A] case object EmptyMetaDecode extends MetaDecode[Nothing] final case class StructureMetaDecode[A](f: Metadata => Either[MetadataError, A]) extends MetaDecode[A] // format: on diff --git a/modules/core/src/smithy4s/http/internals/MetaEncode.scala b/modules/core/src/smithy4s/http/internals/MetaEncode.scala index 02e3e3922..3b65f49ae 100644 --- a/modules/core/src/smithy4s/http/internals/MetaEncode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaEncode.scala @@ -39,9 +39,15 @@ sealed trait MetaEncode[-A] { (metadata: Metadata, a: A) => metadata.addMultipleHeaders(name, f(a)) case (QueryBinding(name), StringValueMetaEncode(f)) => (metadata: Metadata, a: A) => metadata.addQueryParam(name, f(a)) + case (QueryBinding(name), OptionalStringValueMetaEncode(f)) => + (metadata: Metadata, a: A) => + f(a).fold(metadata)(metadata.addQueryParam(name, _)) case (QueryBinding(name), StringListMetaEncode(f)) => (metadata: Metadata, a: A) => metadata.addMultipleQueryParams(name, f(a)) + case (QueryBinding(name), SparseStringListMetaEncode(f)) => + (metadata: Metadata, a: A) => + metadata.addMultipleQueryParamsOpt(name, f(a)) case (QueryParamsBinding, StringMapMetaEncode(f)) => (metadata: Metadata, a: A) => f(a).foldLeft(metadata) { case (m, (k, v)) => @@ -66,8 +72,12 @@ sealed trait MetaEncode[-A] { } def contramap[B](from: B => A): MetaEncode[B] = this match { - case StringValueMetaEncode(f) => StringValueMetaEncode(from andThen f) - case StringListMetaEncode(f) => StringListMetaEncode(from andThen f) + case StringValueMetaEncode(f) => StringValueMetaEncode(from andThen f) + case OptionalStringValueMetaEncode(f) => + OptionalStringValueMetaEncode(from andThen f) + case StringListMetaEncode(f) => StringListMetaEncode(from andThen f) + case SparseStringListMetaEncode(f) => + SparseStringListMetaEncode(from andThen f) case StringMapMetaEncode(f) => StringMapMetaEncode(from andThen f) case StringListMapMetaEncode(f) => StringListMapMetaEncode(from andThen f) case EmptyMetaEncode => EmptyMetaEncode @@ -84,7 +94,9 @@ object MetaEncode { // format: off case class StringValueMetaEncode[A](f: A => String) extends MetaEncode[A] + case class OptionalStringValueMetaEncode[A](f: A => Option[String]) extends MetaEncode[A] case class StringListMetaEncode[A](f: A => List[String]) extends MetaEncode[A] + case class SparseStringListMetaEncode[A](f: A => List[Option[String]]) extends MetaEncode[A] case class StringMapMetaEncode[A](f: A => Map[String, String]) extends MetaEncode[A] case class StringListMapMetaEncode[A](f: A => Map[String, List[String]]) extends MetaEncode[A] case object EmptyMetaEncode extends MetaEncode[Any] diff --git a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala index e5a4e6beb..0d71ecaa2 100644 --- a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala +++ b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala @@ -24,6 +24,7 @@ import smithy4s.http.internals.MetaDecode.StringCollectionMetaDecode import smithy4s.http.internals.MetaDecode.StringListMapMetaDecode import smithy4s.http.internals.MetaDecode.StringMapMetaDecode import smithy4s.http.internals.MetaDecode.StringValueMetaDecode +import smithy4s.http.internals.MetaDecode.OptionalStringValueMetaDecode import smithy4s.http.internals.MetaDecode.StructureMetaDecode import smithy4s.internals.SchemaDescription import smithy4s.schema._ @@ -69,48 +70,24 @@ private[http] class SchemaVisitorMetadataReader( ): MetaDecode[C[A]] = { val amendedMember = member.addHints(httpHints(hints)) self(amendedMember) match { - case MetaDecode.StringValueMetaDecode(f) => + case MetaDecode.StringValueMetaDecode(decode) => val isAwsHeader = hints .get(HttpBinding) .exists(_.tpe == HttpBinding.Type.HeaderType) && awsHeaderEncoding - - val hasSparse = hints.has(smithy.api.Sparse) - - (SchemaVisitorHeaderSplit(member), isAwsHeader, hasSparse) match { - case (Some(splitFunction), true, false) => - MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator( - it.flatMap { - case Some(value) => splitFunction(value) - case None => - throw MetadataError.ImpossibleDecoding( - "Collection does not accept null values" - ) - }.map(f) - ) - } - case (Some(splitFunction), true, true) => + (SchemaVisitorHeaderSplit(member), isAwsHeader) match { + case (Some(splitFunction), true) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator( - it.flatMap { - case Some(value) => splitFunction(value) - case None => Seq.empty - }.map(f) - ) + tag.fromIterator(it.flatMap(splitFunction).map(decode)) } - case (_, _, _) => + case (_, _) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator(it.map { - case Some(value) => f(value) - case None if hasSparse => - f("") // fixme: Denis Rosca: This is a hack to make the test pass. We should not be decoding null values. - case None => - throw MetadataError.ImpossibleDecoding( - "Collection does not accept null values" - ) - }) + tag.fromIterator(it.map(decode)) } } + case MetaDecode.OptionalStringValueMetaDecode(decode) => + MetaDecode.SparseStringCollectionMetaDecode[C[A]] { it => + tag.fromIterator(it.map(decode)) + } case _ => EmptyMetaDecode } } @@ -121,19 +98,10 @@ private[http] class SchemaVisitorMetadataReader( key: Schema[K], value: Schema[V] ): MetaDecode[Map[K, V]] = { - val hasSparse = hints.has(smithy.api.Sparse) - (self(key), self(value.addHints(httpHints(hints)))) match { case (StringValueMetaDecode(readK), StringValueMetaDecode(readV)) => StringMapMetaDecode[Map[K, V]](map => - map.map { - case (k, Some(v)) => (readK(k), readV(v)) - case (k, None) if hasSparse => (readK(k), readV("")) - case _ => - throw MetadataError.ImpossibleDecoding( - "Map does not accept null values" - ) - }.toMap + map.map { case (k, v) => (readK(k), readV(v)) }.toMap ) case (StringValueMetaDecode(readK), StringCollectionMetaDecode(readV)) => StringListMapMetaDecode[Map[K, V]](map => @@ -263,5 +231,10 @@ private[http] class SchemaVisitorMetadataReader( EmptyMetaDecode override def option[A](schema: Schema[A]): MetaDecode[Option[A]] = - self(schema).map(Some(_)) + self(schema) match { + case StringValueMetaDecode(f) => + OptionalStringValueMetaDecode[Option[A]](_.map(f)) + case metaDecode => + metaDecode.map(Some(_)) + } } diff --git a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataWriter.scala b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataWriter.scala index b6f5f15ec..56259c2bd 100644 --- a/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataWriter.scala +++ b/modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataWriter.scala @@ -84,6 +84,8 @@ class SchemaVisitorMetadataWriter( self(amendedMember) match { case StringValueMetaEncode(f) => StringListMetaEncode[C[A]](c => tag.iterator(c).map(f).toList) + case OptionalStringValueMetaEncode(f) => + SparseStringListMetaEncode[C[A]](c => tag.iterator(c).map(f).toList) case _ => MetaEncode.empty } } @@ -93,15 +95,25 @@ class SchemaVisitorMetadataWriter( override def option[A](schema: Schema[A]): MetaEncode[Option[A]] = self(schema) match { case StringValueMetaEncode(f) => - StringValueMetaEncode { + OptionalStringValueMetaEncode { + case Some(value) => Some(f(value)) + case None => None + } + case OptionalStringValueMetaEncode(f) => + OptionalStringValueMetaEncode { case Some(value) => f(value) - case None => "" + case None => None } case StringListMetaEncode(f) => StringListMetaEncode { case Some(value) => f(value) case None => List.empty } + case SparseStringListMetaEncode(f) => + SparseStringListMetaEncode { + case Some(value) => f(value) + case None => List.empty + } case StringMapMetaEncode(f) => StringMapMetaEncode { case Some(value) => f(value) diff --git a/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala b/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala index 3204f3db8..8340a117b 100644 --- a/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala +++ b/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala @@ -12,11 +12,14 @@ import smithy4s.example.{ServiceWithSparseQueryParams, SparseQueryOutput} import weaver._ object SparseQueryParametersSuite extends SimpleIOSuite with CirceInstances { - + test("Server handles sparse query parameters") { runServerTest().map { response => assert.same( - Json.obj("foo" -> Json.arr(Json.fromString("bar"), Json.Null, Json.fromString("baz"))), + Json.obj( + "foo" -> Json + .arr(Json.fromString("bar"), Json.Null, Json.fromString("baz")) + ), response ) } @@ -46,7 +49,10 @@ object SparseQueryParametersSuite extends SimpleIOSuite with CirceInstances { for { result <- run( routes, - Request[IO](method = Method.GET, uri = uri"/operation/sparse-query-params?foo=bar&foo&foo=baz") + Request[IO]( + method = Method.GET, + uri = uri"/operation/sparse-query-params?foo=bar&foo&foo=baz" + ) ) } yield result } @@ -57,7 +63,12 @@ object SparseQueryParametersSuite extends SimpleIOSuite with CirceInstances { ): IO[TestRequest] = { val resources = for { promise <- Deferred[IO, Request[IO]].toResource - reqBody = Json.obj("foo" -> Json.arr(input.map(_.fold(Json.Null)(Json.fromString)): _*)).toString().getBytes() + reqBody = Json + .obj( + "foo" -> Json.arr(input.map(_.fold(Json.Null)(Json.fromString)): _*) + ) + .toString() + .getBytes() httpClient: Client[IO] = Client(req => req .toStrict(None) @@ -88,7 +99,9 @@ object SparseQueryParametersSuite extends SimpleIOSuite with CirceInstances { case class TestRequest(query: Map[String, List[Option[String]]]) object Impl extends ServiceWithSparseQueryParams[IO] { - override def getOperation(foo: List[Option[String]]): IO[SparseQueryOutput] = { + override def getOperation( + foo: List[Option[String]] + ): IO[SparseQueryOutput] = { IO.pure(SparseQueryOutput(foo)) } } From 37c3555b4eb032af650010703ef9d1c070fcc44d Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 6 Jun 2024 21:09:12 +0300 Subject: [PATCH 06/11] Add missing cases and fix test --- .../smithy4s/http/internals/MetaDecode.scala | 19 ++++++++++++++++++- .../smithy4s/http/internals/MetaEncode.scala | 6 ++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/modules/core/src/smithy4s/http/internals/MetaDecode.scala b/modules/core/src/smithy4s/http/internals/MetaDecode.scala index 6fea4f975..f7ac004c7 100644 --- a/modules/core/src/smithy4s/http/internals/MetaDecode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaDecode.scala @@ -75,6 +75,10 @@ private[http] sealed abstract class MetaDecode[+A] { putField(f(values.head)) } else throw MetadataError.ArityError(fieldName, binding) } + case (HeaderBinding(h), OptionalStringValueMetaDecode(f)) => + lookupAndProcess(_.headers, h) { (values, fieldName, putField) => + putField(f(Some(values.head))) + } case (HeaderBinding(h), StringCollectionMetaDecode(f)) => lookupAndProcess(_.headers, h) { (values, fieldName, putField) => putField(f(values.iterator)) @@ -90,7 +94,9 @@ private[http] sealed abstract class MetaDecode[+A] { } case (QueryBinding(h), OptionalStringValueMetaDecode(f)) => lookupAndProcess(_.query, h) { (values, fieldName, putField) => - putField(f(values.head)) + if (values.size == 1) { + putField(f(values.head)) + } else throw MetadataError.ArityError(fieldName, binding) } case (QueryBinding(q), StringCollectionMetaDecode(f)) => lookupAndProcess(_.query, q) { (values, fieldName, putField) => @@ -177,6 +183,17 @@ private[http] sealed abstract class MetaDecode[+A] { // TODO add a specialised case for this putField(f(statusCode.toString)) } + case (StatusCodeBinding, OptionalStringValueMetaDecode(f)) => + (metadata, putField) => + metadata.statusCode match { + case None => + sys.error( + "Status code is not available and field needs it." + ) + case Some(statusCode) => + // TODO add a specialised case for this + putField(f(Some(statusCode.toString))) + } case _ => (metadata: Metadata, buffer) => () } } diff --git a/modules/core/src/smithy4s/http/internals/MetaEncode.scala b/modules/core/src/smithy4s/http/internals/MetaEncode.scala index 3b65f49ae..443adb510 100644 --- a/modules/core/src/smithy4s/http/internals/MetaEncode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaEncode.scala @@ -35,13 +35,15 @@ sealed trait MetaEncode[-A] { (metadata: Metadata, a: A) => metadata.addPathParam(path, f(a)) case (HeaderBinding(name), StringValueMetaEncode(f)) => (metadata: Metadata, a: A) => metadata.addHeader(name, f(a)) + case (HeaderBinding(name), OptionalStringValueMetaEncode(f)) => + (metadata: Metadata, a: A) => + f(a).fold(metadata)(metadata.addHeader(name, _)) case (HeaderBinding(name), StringListMetaEncode(f)) => (metadata: Metadata, a: A) => metadata.addMultipleHeaders(name, f(a)) case (QueryBinding(name), StringValueMetaEncode(f)) => (metadata: Metadata, a: A) => metadata.addQueryParam(name, f(a)) case (QueryBinding(name), OptionalStringValueMetaEncode(f)) => - (metadata: Metadata, a: A) => - f(a).fold(metadata)(metadata.addQueryParam(name, _)) + (metadata: Metadata, a: A) => metadata.addQueryParam(name, f(a)) case (QueryBinding(name), StringListMetaEncode(f)) => (metadata: Metadata, a: A) => metadata.addMultipleQueryParams(name, f(a)) From 4e91fb6126aa74e3b72485e7a50596a544fe1148 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 6 Jun 2024 21:51:53 +0300 Subject: [PATCH 07/11] Use existing sparse list structure --- .../smithy4s/example/SparseFooList.scala | 18 ------------------ .../smithy4s/example/SparseQueryInput.scala | 2 +- .../smithy4s/example/SparseQueryOutput.scala | 2 +- .../generated/smithy4s/example/package.scala | 1 - .../serviceWithSparseQueryParams.smithy | 9 ++------- 5 files changed, 4 insertions(+), 28 deletions(-) delete mode 100644 modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala diff --git a/modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala deleted file mode 100644 index 52a3204e7..000000000 --- a/modules/bootstrapped/src/generated/smithy4s/example/SparseFooList.scala +++ /dev/null @@ -1,18 +0,0 @@ -package smithy4s.example - -import smithy4s.Hints -import smithy4s.Newtype -import smithy4s.Schema -import smithy4s.ShapeId -import smithy4s.schema.Schema.bijection -import smithy4s.schema.Schema.list -import smithy4s.schema.Schema.string - -object SparseFooList extends Newtype[List[Option[String]]] { - val id: ShapeId = ShapeId("smithy4s.example", "SparseFooList") - val hints: Hints = Hints( - smithy.api.Sparse(), - ).lazily - val underlyingSchema: Schema[List[Option[String]]] = list(string.option).withId(id).addHints(hints) - implicit val schema: Schema[SparseFooList] = bijection(underlyingSchema, asBijection) -} diff --git a/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala index 35d565ee7..b46d4f1ea 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala @@ -17,6 +17,6 @@ object SparseQueryInput extends ShapeTag.Companion[SparseQueryInput] { private def make(foo: List[Option[String]]): SparseQueryInput = SparseQueryInput(foo) implicit val schema: Schema[SparseQueryInput] = struct( - SparseFooList.underlyingSchema.required[SparseQueryInput]("foo", _.foo).addHints(smithy.api.HttpQuery("foo")), + SparseStringList.underlyingSchema.required[SparseQueryInput]("foo", _.foo).addHints(smithy.api.HttpQuery("foo")), )(make).withId(id).addHints(hints) } diff --git a/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala index 58ef9f3fb..6dafe7aea 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryOutput.scala @@ -17,6 +17,6 @@ object SparseQueryOutput extends ShapeTag.Companion[SparseQueryOutput] { private def make(foo: List[Option[String]]): SparseQueryOutput = SparseQueryOutput(foo) implicit val schema: Schema[SparseQueryOutput] = struct( - SparseFooList.underlyingSchema.required[SparseQueryOutput]("foo", _.foo), + SparseStringList.underlyingSchema.required[SparseQueryOutput]("foo", _.foo), )(make).withId(id).addHints(hints) } diff --git a/modules/bootstrapped/src/generated/smithy4s/example/package.scala b/modules/bootstrapped/src/generated/smithy4s/example/package.scala index 17f5d6b38..f69c42229 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/package.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/package.scala @@ -97,7 +97,6 @@ package object example { type SomeInt = smithy4s.example.SomeInt.Type type SomeValue = smithy4s.example.SomeValue.Type type SomeVector = smithy4s.example.SomeVector.Type - type SparseFooList = smithy4s.example.SparseFooList.Type type SparseStringList = smithy4s.example.SparseStringList.Type type SparseStringMap = smithy4s.example.SparseStringMap.Type type StreamedBlob = smithy4s.example.StreamedBlob.Type diff --git a/sampleSpecs/serviceWithSparseQueryParams.smithy b/sampleSpecs/serviceWithSparseQueryParams.smithy index 3c84745aa..251a68fce 100644 --- a/sampleSpecs/serviceWithSparseQueryParams.smithy +++ b/sampleSpecs/serviceWithSparseQueryParams.smithy @@ -21,15 +21,10 @@ operation GetOperation { structure SparseQueryInput { @required @httpQuery("foo") - foo: SparseFooList + foo: SparseStringList } structure SparseQueryOutput { @required - foo: SparseFooList + foo: SparseStringList } - -@sparse -list SparseFooList { - member: String -} \ No newline at end of file From b9803da1b34d9063a3817cb91e752baef3c9ef04 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 6 Jun 2024 21:59:05 +0300 Subject: [PATCH 08/11] Add changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0022b9f3a..426e8e062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ Thank you! # 0.19.0 +## Update HttpUri model to allow optional query parameters in [1513](https://github.com/disneystreaming/smithy4s/pull/1513) + +Change the type of query parameters in `HttpUri` from `queryParams: Map[String, Seq[String]]` to `queryParams: Map[String, Seq[Option[String]]]` to allow +modeling of sparse collections. + ## Documentation fix Prevent documentation from being generated for case class when the field are not generated because they're annotated with `@streaming` From 3eded4a595376384da10c5821f61b64ef1903d54 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 13 Jun 2024 16:46:08 +0300 Subject: [PATCH 09/11] Change HttpUri queryParams type to IndexedSeq --- .../aws-http4s/src/smithy4s/aws/AwsClient.scala | 2 +- .../test/src/smithy4s/http/HttpRequestSpec.scala | 2 +- modules/core/src/smithy4s/http/HttpRequest.scala | 14 ++++++++++---- modules/core/src/smithy4s/http/HttpUri.scala | 2 +- .../src/smithy4s/http/internals/MetaDecode.scala | 5 +++-- .../core/src/smithy4s/http/internals/package.scala | 2 -- .../src/smithy4s/http4s/kernel/package.scala | 12 +----------- .../http4s/kernel/Http4sConversionSpec.scala | 2 +- 8 files changed, 18 insertions(+), 23 deletions(-) diff --git a/modules/aws-http4s/src/smithy4s/aws/AwsClient.scala b/modules/aws-http4s/src/smithy4s/aws/AwsClient.scala index d2b6a4365..a6212f024 100644 --- a/modules/aws-http4s/src/smithy4s/aws/AwsClient.scala +++ b/modules/aws-http4s/src/smithy4s/aws/AwsClient.scala @@ -77,7 +77,7 @@ object AwsClient { host = Some(s"$endpointPrefix.$region.amazonaws.com"), port = None, path = IndexedSeq.empty, - queryParams = Map.empty, + queryParams = IndexedSeq.empty, pathParams = None ) // Uri.unsafeFromString(s"https://$endpointPrefix.$region.amazonaws.com/") diff --git a/modules/bootstrapped/test/src/smithy4s/http/HttpRequestSpec.scala b/modules/bootstrapped/test/src/smithy4s/http/HttpRequestSpec.scala index 0e64c803e..7a8f85636 100644 --- a/modules/bootstrapped/test/src/smithy4s/http/HttpRequestSpec.scala +++ b/modules/bootstrapped/test/src/smithy4s/http/HttpRequestSpec.scala @@ -31,7 +31,7 @@ final class HttpRequestSpec extends FunSuite { Some("example.com"), None, IndexedSeq.empty, - Map.empty, + IndexedSeq.empty, None ) val request = HttpRequest(HttpMethod.GET, uri, Map.empty, "") diff --git a/modules/core/src/smithy4s/http/HttpRequest.scala b/modules/core/src/smithy4s/http/HttpRequest.scala index 502ccffb3..24ee1d5fa 100644 --- a/modules/core/src/smithy4s/http/HttpRequest.scala +++ b/modules/core/src/smithy4s/http/HttpRequest.scala @@ -33,7 +33,7 @@ final case class HttpRequest[+A]( def toMetadata: Metadata = Metadata( path = uri.pathParams.getOrElse(Map.empty), - query = uri.queryParams, + query = uri.queryParams.groupBy(_._1).view.mapValues(_.map(_._2)).toMap, headers = headers ) @@ -79,10 +79,12 @@ object HttpRequest { ): Writer[Body, I] = new Writer[Body, I] { def write(request: HttpRequest[Body], input: I): HttpRequest[Body] = { val path = httpEndpoint.path(input) - val staticQueries = httpEndpoint.staticQueryParams val oldUri = request.uri val newUri = - oldUri.copy(path = oldUri.path ++ path, queryParams = staticQueries) + oldUri.copy( + path = oldUri.path ++ path, + queryParams = mapToIndexedSeq(httpEndpoint.staticQueryParams) + ) val method = httpEndpoint.method request.copy(method = method, uri = newUri) } @@ -92,7 +94,9 @@ object HttpRequest { (req: HttpRequest[Body], meta: Metadata) => val oldUri = req.uri val newUri = - oldUri.copy(queryParams = oldUri.queryParams ++ meta.query) + oldUri.copy(queryParams = + oldUri.queryParams ++ mapToIndexedSeq(meta.query) + ) req.addHeaders(meta.headers).copy(uri = newUri) } @@ -118,6 +122,8 @@ object HttpRequest { } } + private def mapToIndexedSeq[A, B](m: Map[A, Seq[B]]): IndexedSeq[(A, B)] = + m.toIndexedSeq.flatMap { case (k, v) => v.map(k -> _) } } object Decoder { diff --git a/modules/core/src/smithy4s/http/HttpUri.scala b/modules/core/src/smithy4s/http/HttpUri.scala index 4ddfee7bf..e7d39102b 100644 --- a/modules/core/src/smithy4s/http/HttpUri.scala +++ b/modules/core/src/smithy4s/http/HttpUri.scala @@ -24,7 +24,7 @@ final case class HttpUri( * A sequence of URL-decoded URI segment. */ path: IndexedSeq[String], - queryParams: Map[String, Seq[Option[String]]], + queryParams: IndexedSeq[(String, Option[String])], /** * Field allowing to store decoded path parameters alongside an http request, * once the routing logic has come in effect. diff --git a/modules/core/src/smithy4s/http/internals/MetaDecode.scala b/modules/core/src/smithy4s/http/internals/MetaDecode.scala index f7ac004c7..734cd6a28 100644 --- a/modules/core/src/smithy4s/http/internals/MetaDecode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaDecode.scala @@ -187,8 +187,9 @@ private[http] sealed abstract class MetaDecode[+A] { (metadata, putField) => metadata.statusCode match { case None => - sys.error( - "Status code is not available and field needs it." + throw new MetadataError.MissingValueError( + fieldName, + StatusCodeBinding ) case Some(statusCode) => // TODO add a specialised case for this diff --git a/modules/core/src/smithy4s/http/internals/package.scala b/modules/core/src/smithy4s/http/internals/package.scala index 11092806f..089e89ee6 100644 --- a/modules/core/src/smithy4s/http/internals/package.scala +++ b/modules/core/src/smithy4s/http/internals/package.scala @@ -86,8 +86,6 @@ package object internals { case (acc, param) => val (k, v) = param.split("=", 2) match { case Array(key) => (key, None) - case Array(key, "") => - (key, None) // FIXME: denisrosca not sure about this case Array(key, value) => (key, Some(value)) } acc.updated(k, acc.getOrElse(k, Seq.empty) :+ v) diff --git a/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala b/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala index 1950e3d4f..345aa32de 100644 --- a/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala +++ b/modules/http4s-kernel/src/smithy4s/http4s/kernel/package.scala @@ -68,7 +68,7 @@ package object kernel { uri.host.map(_.renderString), uri.port, uri.path.segments.map(_.decoded()), - getQueryParams(uri), + uri.query.pairs, pathParams ) } @@ -111,9 +111,6 @@ package object kernel { }, query = Query.fromVector( uri.queryParams.toVector - .flatMap { case (key, values) => - values.map(value => key -> value) - } ) ) } @@ -191,13 +188,6 @@ package object kernel { (CaseInsensitive(k.toString), v.map(_.value)) } - private[smithy4s] def getQueryParams[F[_]]( - uri: Uri - ): Map[String, List[Option[String]]] = - uri.query.pairs - .groupBy(_._1) - .map { case (k, v) => k -> v.map(_._2).toList } - private def collectBytes[F[_]: Concurrent]( stream: fs2.Stream[F, Byte] ): F[Blob] = stream.chunks.compile diff --git a/modules/http4s-kernel/test/src/smithy4s/http4s/kernel/Http4sConversionSpec.scala b/modules/http4s-kernel/test/src/smithy4s/http4s/kernel/Http4sConversionSpec.scala index 464a96e67..ff216e62f 100644 --- a/modules/http4s-kernel/test/src/smithy4s/http4s/kernel/Http4sConversionSpec.scala +++ b/modules/http4s-kernel/test/src/smithy4s/http4s/kernel/Http4sConversionSpec.scala @@ -121,7 +121,7 @@ object Http4sConversionSpec extends SimpleIOSuite { host = None, port = None, path = IndexedSeq.empty, - queryParams = Map.empty, + queryParams = IndexedSeq.empty, pathParams = None ) } From 8f9abc7fce96a141fa9a79b1a1ef7562aa47f364 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Thu, 13 Jun 2024 17:41:47 +0300 Subject: [PATCH 10/11] Fix compilation issue on 2.12 --- modules/core/src/smithy4s/http/HttpRequest.scala | 6 +++++- modules/core/src/smithy4s/http/internals/package.scala | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/modules/core/src/smithy4s/http/HttpRequest.scala b/modules/core/src/smithy4s/http/HttpRequest.scala index 24ee1d5fa..ad1f82da5 100644 --- a/modules/core/src/smithy4s/http/HttpRequest.scala +++ b/modules/core/src/smithy4s/http/HttpRequest.scala @@ -33,7 +33,11 @@ final case class HttpRequest[+A]( def toMetadata: Metadata = Metadata( path = uri.pathParams.getOrElse(Map.empty), - query = uri.queryParams.groupBy(_._1).view.mapValues(_.map(_._2)).toMap, + query = uri.queryParams + .groupBy(_._1) + .view + .map { case (key, values) => key -> values.map(_._2).toIndexedSeq } + .toMap, headers = headers ) diff --git a/modules/core/src/smithy4s/http/internals/package.scala b/modules/core/src/smithy4s/http/internals/package.scala index 089e89ee6..6e7d9be71 100644 --- a/modules/core/src/smithy4s/http/internals/package.scala +++ b/modules/core/src/smithy4s/http/internals/package.scala @@ -85,7 +85,7 @@ package object internals { .foldLeft(Map.empty[String, Seq[Option[String]]]) { case (acc, param) => val (k, v) = param.split("=", 2) match { - case Array(key) => (key, None) + case Array(key) => (key, None) case Array(key, value) => (key, Some(value)) } acc.updated(k, acc.getOrElse(k, Seq.empty) :+ v) From 4414abb235ff5b1540713e8989d65c33fe292634 Mon Sep 17 00:00:00 2001 From: Denis Rosca Date: Tue, 18 Jun 2024 19:39:28 +0300 Subject: [PATCH 11/11] Change compliance tests to accept sparse arguments --- .../internals/Assertions.scala | 6 ++-- .../ClientHttpComplianceTestCase.scala | 10 ++++--- .../ServerHttpComplianceTestCase.scala | 28 +++++++++++++++++-- .../compliancetests/internals/package.scala | 27 +++++++----------- 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/modules/compliance-tests/src/smithy4s/compliancetests/internals/Assertions.scala b/modules/compliance-tests/src/smithy4s/compliancetests/internals/Assertions.scala index 7918004c4..95d92f622 100644 --- a/modules/compliance-tests/src/smithy4s/compliancetests/internals/Assertions.scala +++ b/modules/compliance-tests/src/smithy4s/compliancetests/internals/Assertions.scala @@ -146,7 +146,7 @@ private[internals] object assert { } private def queryParamsExistenceCheck( - queryParameters: Map[String, Seq[String]], + queryParameters: Map[String, Seq[Option[String]]], requiredParameters: Option[List[String]], forbiddenParameters: Option[List[String]] ) = { @@ -172,7 +172,7 @@ private[internals] object assert { } private def queryParamValuesCheck( - queryParameters: Map[String, Seq[String]], + queryParameters: Map[String, Seq[Option[String]]], testCase: Option[List[String]] ) = { testCase.toList.flatten @@ -242,7 +242,7 @@ private[internals] object assert { def checkQueryParameters( tc: HttpRequestTestCase, - queryParameters: Map[String, Seq[String]] + queryParameters: Map[String, Seq[Option[String]]] ): ComplianceResult = { val existenceChecks = assert.queryParamsExistenceCheck( queryParameters = queryParameters, diff --git a/modules/compliance-tests/src/smithy4s/compliancetests/internals/ClientHttpComplianceTestCase.scala b/modules/compliance-tests/src/smithy4s/compliancetests/internals/ClientHttpComplianceTestCase.scala index cb6855921..b1c29aaa2 100644 --- a/modules/compliance-tests/src/smithy4s/compliancetests/internals/ClientHttpComplianceTestCase.scala +++ b/modules/compliance-tests/src/smithy4s/compliancetests/internals/ClientHttpComplianceTestCase.scala @@ -22,6 +22,7 @@ import cats.effect.Async import cats.effect.syntax.all._ import org.http4s.HttpApp import org.http4s.Headers +import org.http4s.Query import org.http4s.Request import org.http4s.Response import org.http4s.Status @@ -82,9 +83,8 @@ private[compliancetests] class ClientHttpComplianceTestCase[ val expectedUri = baseUri .withPath(Uri.Path.unsafeFromString(testCase.uri)) - .withMultiValueQueryParams( - parseQueryParams(testCase.queryParams) - ) + .copy(query = Query.fromVector(parseQueryParams(testCase.queryParams))) + val pathAssert = assert.eql( receivedPathSegments, @@ -93,7 +93,9 @@ private[compliancetests] class ClientHttpComplianceTestCase[ ) val queryAssert = assert.testCase.checkQueryParameters( testCase, - expectedUri.query.multiParams + expectedUri.query.pairs.groupBy(_._1).map { case (k, v) => + k -> v.map(_._2).toList + } ) val methodAssert = assert.eql( request.method.name.toLowerCase(), diff --git a/modules/compliance-tests/src/smithy4s/compliancetests/internals/ServerHttpComplianceTestCase.scala b/modules/compliance-tests/src/smithy4s/compliancetests/internals/ServerHttpComplianceTestCase.scala index 47482ae5e..7d6fde644 100644 --- a/modules/compliance-tests/src/smithy4s/compliancetests/internals/ServerHttpComplianceTestCase.scala +++ b/modules/compliance-tests/src/smithy4s/compliancetests/internals/ServerHttpComplianceTestCase.scala @@ -33,6 +33,8 @@ import smithy4s.compliancetests.internals.eq.EqSchemaVisitor import smithy4s.compliancetests.TestConfig._ import cats.MonadThrow import java.util.concurrent.TimeoutException +import scala.collection.immutable.ListMap + private[compliancetests] class ServerHttpComplianceTestCase[ F[_], Alg[_[_, _, _, _, _]] @@ -66,13 +68,33 @@ private[compliancetests] class ServerHttpComplianceTestCase[ .fromString(testCase.method) .getOrElse(sys.error("Invalid method")) + val expectedQueryParams: Vector[(String, Option[String])] = + parseQueryParams(testCase.queryParams) + .foldLeft[ListMap[String, Vector[Option[String]]]](ListMap.empty) { + case (acc, (k, v)) => + acc.get(k) match { + case Some(value) => acc + (k -> (value :+ v)) + case None => acc + (k -> Vector(v)) + } + } + .map { + // FIXME: replacing single query parameter without value with empty string + // to make sure that https://github.com/smithy-lang/smithy/blob/6c42bc9d60a681e63c66f4cde33d7a189a1ff9a6/smithy-aws-protocol-tests/model/restJson1/http-query.smithy#L476-L489 + // passes. + // Previously this was done in `parseQueryParams`. + case (k, Vector(None)) => k -> Vector(Some("")) + case kv => kv + } + .toVector + .flatMap { case (k, v) => + v.map(k -> _) + } + val expectedUri = baseUri .withPath( Uri.Path.unsafeFromString(testCase.uri).addEndsWithSlash ) - .withMultiValueQueryParams( - parseQueryParams(testCase.queryParams) - ) + .copy(query = Query.fromVector(expectedQueryParams)) val body = testCase.body diff --git a/modules/compliance-tests/src/smithy4s/compliancetests/internals/package.scala b/modules/compliance-tests/src/smithy4s/compliancetests/internals/package.scala index b5bb11be1..e089d8f7b 100644 --- a/modules/compliance-tests/src/smithy4s/compliancetests/internals/package.scala +++ b/modules/compliance-tests/src/smithy4s/compliancetests/internals/package.scala @@ -18,42 +18,35 @@ package smithy4s package compliancetests import org.http4s.{Header, Headers, Uri} -import cats.implicits._ import org.typelevel.ci.CIString import java.nio.charset.StandardCharsets -import scala.collection.immutable.ListMap package object internals { private[compliancetests] def splitQuery( queryString: String - ): (String, String) = { + ): (String, Option[String]) = { queryString.split("=", 2) match { case Array(k, v) => ( k, - Uri.decode( - toDecode = v, - charset = StandardCharsets.UTF_8, - plusIsSpace = true + Some( + Uri.decode( + toDecode = v, + charset = StandardCharsets.UTF_8, + plusIsSpace = true + ) ) ) - case Array(k) => (k, "") + case Array(k) => (k, None) } } private[compliancetests] def parseQueryParams( queryParams: Option[List[String]] - ): ListMap[String, List[String]] = { - queryParams.combineAll + ): Vector[(String, Option[String])] = { + queryParams.toVector.flatten .map(splitQuery) - .foldLeft[ListMap[String, List[String]]](ListMap.empty) { - case (acc, (k, v)) => - acc.get(k) match { - case Some(value) => acc + (k -> (value :+ v)) - case None => acc + (k -> List(v)) - } - } } private def escape(str: String): String = {