diff --git a/CHANGELOG.md b/CHANGELOG.md index 61e644bb3..e907c2d51 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` 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/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/SparseQueryInput.scala b/modules/bootstrapped/src/generated/smithy4s/example/SparseQueryInput.scala new file mode 100644 index 000000000..b46d4f1ea --- /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( + 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 new file mode 100644 index 000000000..6dafe7aea --- /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( + 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 a914f9cf4..c9e529d23 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] 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/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala b/modules/bootstrapped/test/src/smithy4s/http/internals/MetadataSpec.scala index 43e55ebd3..f282a27b4 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,9 @@ 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 +220,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 +228,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 +239,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 +250,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 +261,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 +273,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 +284,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 +441,9 @@ 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 +466,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..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("foo"), "baz" -> Seq("bar")) + val expectedQueryMap = + Map("value" -> Seq(Some("foo")), "baz" -> Seq(Some("bar"))) expect(sqp == expectedQueryMap) expect( path == 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 = { 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/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/HttpRequest.scala b/modules/core/src/smithy4s/http/HttpRequest.scala index 502ccffb3..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, + query = uri.queryParams + .groupBy(_._1) + .view + .map { case (key, values) => key -> values.map(_._2).toIndexedSeq } + .toMap, headers = headers ) @@ -79,10 +83,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 +98,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 +126,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 0c9b947de..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[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/Metadata.scala b/modules/core/src/smithy4s/http/Metadata.scala index 4f026d232..a88343393 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,9 +49,10 @@ case class Metadata( v.map(k -> _) } - def queryFlattened: Vector[(String, 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 { @@ -67,12 +68,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 +97,12 @@ 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 +131,13 @@ 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,8 +145,9 @@ case class Metadata( case head :: tl => Some((head, tl)) case Nil => None } - case HttpBinding.PathBinding(httpName) => path.get(httpName).map(_ -> 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 2424fac5f..734cd6a28 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 @@ -71,11 +75,24 @@ 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)) } 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.ArityError(fieldName, binding) + } + } else throw MetadataError.ArityError(fieldName, binding) + } + case (QueryBinding(h), OptionalStringValueMetaDecode(f)) => lookupAndProcess(_.query, h) { (values, fieldName, putField) => if (values.size == 1) { putField(f(values.head)) @@ -83,7 +100,17 @@ private[http] sealed abstract class MetaDecode[+A] { } 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 @@ -92,7 +119,15 @@ 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 + 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)) @@ -102,7 +137,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) @@ -141,6 +183,18 @@ 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 => + throw new MetadataError.MissingValueError( + fieldName, + StatusCodeBinding + ) + case Some(statusCode) => + // TODO add a specialised case for this + putField(f(Some(statusCode.toString))) + } case _ => (metadata: Metadata, buffer) => () } } @@ -156,7 +210,9 @@ private[http] object MetaDecode { // format: off final case class StringValueMetaDecode[A](f: 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] diff --git a/modules/core/src/smithy4s/http/internals/MetaEncode.scala b/modules/core/src/smithy4s/http/internals/MetaEncode.scala index 02e3e3922..443adb510 100644 --- a/modules/core/src/smithy4s/http/internals/MetaEncode.scala +++ b/modules/core/src/smithy4s/http/internals/MetaEncode.scala @@ -35,13 +35,21 @@ 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) => metadata.addQueryParam(name, f(a)) 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 +74,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 +96,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 3d5c8669e..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,20 +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 (SchemaVisitorHeaderSplit(member), isAwsHeader) match { case (Some(splitFunction), true) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator(it.flatMap(splitFunction).map(f)) + tag.fromIterator(it.flatMap(splitFunction).map(decode)) } case (_, _) => MetaDecode.StringCollectionMetaDecode[C[A]] { it => - tag.fromIterator(it.map(f)) + tag.fromIterator(it.map(decode)) } } + case MetaDecode.OptionalStringValueMetaDecode(decode) => + MetaDecode.SparseStringCollectionMetaDecode[C[A]] { it => + tag.fromIterator(it.map(decode)) + } case _ => EmptyMetaDecode } } @@ -226,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/core/src/smithy4s/http/internals/package.scala b/modules/core/src/smithy4s/http/internals/package.scala index a6b03ad07..6e7d9be71 100644 --- a/modules/core/src/smithy4s/http/internals/package.scala +++ b/modules/core/src/smithy4s/http/internals/package.scala @@ -75,18 +75,21 @@ 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]]) { - case (acc, param) => - val (k, v) = param.split("=", 2) match { - case Array(key) => (key, "") - case Array(key, value) => (key, 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, 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..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 ) } @@ -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,11 @@ package object kernel { case Smithy4sHttpUriScheme.Http => Uri.Scheme.http case Smithy4sHttpUriScheme.Https => Uri.Scheme.https } - } - ).withMultiValueQueryParams(uri.queryParams) + }, + query = Query.fromVector( + uri.queryParams.toVector + ) + ) } /** @@ -184,17 +188,6 @@ package object kernel { (CaseInsensitive(k.toString), v.map(_.value)) } - private[smithy4s] def getQueryParams[F[_]]( - uri: Uri - ): Map[String, List[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 } - 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 ) } 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..8340a117b --- /dev/null +++ b/modules/http4s/test/src/smithy4s/http4s/SparseQueryParametersSuite.scala @@ -0,0 +1,109 @@ +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..251a68fce --- /dev/null +++ b/sampleSpecs/serviceWithSparseQueryParams.smithy @@ -0,0 +1,30 @@ +$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: SparseStringList +} + +structure SparseQueryOutput { + @required + foo: SparseStringList +}