From db37dc4302891304a5c2dfa2f7b3d08584e23570 Mon Sep 17 00:00:00 2001 From: Nabil Abdel-Hafeez <7283535+987Nabil@users.noreply.github.com> Date: Thu, 12 Sep 2024 23:16:05 +0200 Subject: [PATCH] Encode opt query params correctly in open api (#3127) Add tests for optional headers and payload as well --- .../endpoint/openapi/OpenAPIGenSpec.scala | 356 ++++++++++++++++++ .../http/endpoint/openapi/OpenAPIGen.scala | 17 +- 2 files changed, 367 insertions(+), 6 deletions(-) diff --git a/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala b/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala index 99faac7f3d..56978161e6 100644 --- a/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala +++ b/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala @@ -180,6 +180,13 @@ object OpenAPIGenSpec extends ZIOSpecDefault { .out[SimpleOutputBody] .outError[NotFoundError](Status.NotFound) + private val queryParamOptEndpoint = + Endpoint(GET / "withQuery") + .in[SimpleInputBody] + .query(HttpCodec.query[String]("query").optional) + .out[SimpleOutputBody] + .outError[NotFoundError](Status.NotFound) + private val queryParamCollectionEndpoint = Endpoint(GET / "withQuery") .in[SimpleInputBody] @@ -194,6 +201,19 @@ object OpenAPIGenSpec extends ZIOSpecDefault { .out[SimpleOutputBody] .outError[NotFoundError](Status.NotFound) + private val optionalHeaderEndpoint = + Endpoint(GET / "withHeader") + .in[SimpleInputBody] + .header(HttpCodec.contentType.optional) + .out[SimpleOutputBody] + .outError[NotFoundError](Status.NotFound) + + private val optionalPayloadEndpoint = + Endpoint(GET / "withPayload") + .inCodec(HttpCodec.content[Payload].optional) + .out[SimpleOutputBody] + .outError[NotFoundError](Status.NotFound) + private val alternativeInputEndpoint = Endpoint(GET / "inputAlternative") .inCodec( @@ -474,6 +494,134 @@ object OpenAPIGenSpec extends ZIOSpecDefault { |}""".stripMargin assertTrue(json == toJsonAst(expectedJson)) }, + test("with optional query parameter") { + val generated = OpenAPIGen.fromEndpoints("Simple Endpoint", "1.0", queryParamOptEndpoint) + val json = toJsonAst(generated) + val expectedJson = """{ + | "openapi" : "3.1.0", + | "info" : { + | "title" : "Simple Endpoint", + | "version" : "1.0" + | }, + | "paths" : { + | "/withQuery" : { + | "get" : { + | "parameters" : [ + | + | { + | "name" : "query", + | "in" : "query", + | "schema" : + | { + | "type" :[ + | "string", + | "null" + | ] + | }, + | "allowReserved" : false, + | "style" : "form" + | } + | ], + | "requestBody" : + | { + | "content" : { + | "application/json" : { + | "schema" : + | { + | "$ref" : "#/components/schemas/SimpleInputBody" + | } + | } + | }, + | "required" : true + | }, + | "responses" : { + | "200" : + | { + | "content" : { + | "application/json" : { + | "schema" : + | { + | "$ref" : "#/components/schemas/SimpleOutputBody" + | } + | } + | } + | }, + | "404" : + | { + | "content" : { + | "application/json" : { + | "schema" : + | { + | "$ref" : "#/components/schemas/NotFoundError" + | } + | } + | } + | } + | } + | } + | } + | }, + | "components" : { + | "schemas" : { + | "NotFoundError" : + | { + | "type" : + | "object", + | "properties" : { + | "message" : { + | "type" : + | "string" + | } + | }, + | "required" : [ + | "message" + | ] + | }, + | "SimpleInputBody" : + | { + | "type" : + | "object", + | "properties" : { + | "name" : { + | "type" : + | "string" + | }, + | "age" : { + | "type" : + | "integer", + | "format" : "int32" + | } + | }, + | "required" : [ + | "name", + | "age" + | ] + | }, + | "SimpleOutputBody" : + | { + | "type" : + | "object", + | "properties" : { + | "userName" : { + | "type" : + | "string" + | }, + | "score" : { + | "type" : + | "integer", + | "format" : "int32" + | } + | }, + | "required" : [ + | "userName", + | "score" + | ] + | } + | } + | } + |}""".stripMargin + assertTrue(json == toJsonAst(expectedJson)) + }, test("with query parameter with multiple values") { val generated = OpenAPIGen.fromEndpoints("Simple Endpoint", "1.0", queryParamCollectionEndpoint) val json = toJsonAst(generated) @@ -730,6 +878,214 @@ object OpenAPIGenSpec extends ZIOSpecDefault { |}""".stripMargin assertTrue(json == toJsonAst(expectedJson)) }, + test("optional header") { + val generated = OpenAPIGen.fromEndpoints("Simple Endpoint", "1.0", optionalHeaderEndpoint) + val json = toJsonAst(generated) + val expectedJson = """{ + | "openapi" : "3.1.0", + | "info" : { + | "title" : "Simple Endpoint", + | "version" : "1.0" + | }, + | "paths" : { + | "/withHeader" : { + | "get" : { + | "parameters" : [ + | { + | "name" : "content-type", + | "in" : "header", + | "schema" : { + | "type" : [ + | "string", + | "null" + | ] + | }, + | "style" : "simple" + | } + | ], + | "requestBody" : { + | "content" : { + | "application/json" : { + | "schema" : { + | "$ref" : "#/components/schemas/SimpleInputBody", + | "description" : "" + | } + | } + | }, + | "required" : true + | }, + | "responses" : { + | "200" : { + | "content" : { + | "application/json" : { + | "schema" : { + | "$ref" : "#/components/schemas/SimpleOutputBody" + | } + | } + | } + | }, + | "404" : { + | "content" : { + | "application/json" : { + | "schema" : { + | "$ref" : "#/components/schemas/NotFoundError" + | } + | } + | } + | } + | } + | } + | } + | }, + | "components" : { + | "schemas" : { + | "NotFoundError" : { + | "type" : "object", + | "properties" : { + | "message" : { + | "type" : "string" + | } + | }, + | "required" : [ + | "message" + | ] + | }, + | "SimpleInputBody" : { + | "type" : "object", + | "properties" : { + | "name" : { + | "type" : "string" + | }, + | "age" : { + | "type" : "integer", + | "format" : "int32" + | } + | }, + | "required" : [ + | "name", + | "age" + | ] + | }, + | "SimpleOutputBody" : { + | "type" : "object", + | "properties" : { + | "userName" : { + | "type" : "string" + | }, + | "score" : { + | "type" : "integer", + | "format" : "int32" + | } + | }, + | "required" : [ + | "userName", + | "score" + | ] + | } + | } + | } + |}""".stripMargin + assertTrue(json == toJsonAst(expectedJson)) + }, + test("optional payload") { + val generated = OpenAPIGen.fromEndpoints("Simple Endpoint", "1.0", optionalPayloadEndpoint) + val json = toJsonAst(generated) + val expected = """{ + | "openapi" : "3.1.0", + | "info" : { + | "title" : "Simple Endpoint", + | "version" : "1.0" + | }, + | "paths" : { + | "/withPayload" : { + | "get" : { + | "requestBody" : { + | "content" : { + | "application/json" : { + | "schema" : { + | "anyOf" : [ + | { + | "type" : "null" + | }, + | { + | "$ref" : "#/components/schemas/Payload" + | } + | ], + | "description" : "" + | } + | } + | }, + | "required" : false + | }, + | "responses" : { + | "200" : { + | "content" : { + | "application/json" : { + | "schema" : { + | "$ref" : "#/components/schemas/SimpleOutputBody" + | } + | } + | } + | }, + | "404" : { + | "content" : { + | "application/json" : { + | "schema" : { + | "$ref" : "#/components/schemas/NotFoundError" + | } + | } + | } + | } + | } + | } + | } + | }, + | "components" : { + | "schemas" : { + | "NotFoundError" : { + | "type" : "object", + | "properties" : { + | "message" : { + | "type" : "string" + | } + | }, + | "required" : [ + | "message" + | ] + | }, + | "Payload" : { + | "type" : "object", + | "properties" : { + | "content" : { + | "type" : "string" + | } + | }, + | "required" : [ + | "content" + | ], + | "description" : "A simple payload" + | }, + | "SimpleOutputBody" : { + | "type" : "object", + | "properties" : { + | "userName" : { + | "type" : "string" + | }, + | "score" : { + | "type" : "integer", + | "format" : "int32" + | } + | }, + | "required" : [ + | "userName", + | "score" + | ] + | } + | } + | } + |}""".stripMargin + assertTrue(json == toJsonAst(expected)) + }, test("alternative input") { val generated = OpenAPIGen.fromEndpoints("Simple Endpoint", "1.0", alternativeInputEndpoint) val json = toJsonAst(generated) diff --git a/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala b/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala index 8506980560..0554628248 100644 --- a/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala +++ b/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala @@ -618,7 +618,7 @@ object OpenAPIGen { def queryParams: Set[OpenAPI.ReferenceOr[OpenAPI.Parameter]] = { inAtoms.query.collect { - case mc @ MetaCodec(HttpCodec.Query(HttpCodec.Query.QueryType.Primitive(name, codec), _), _) => + case mc @ MetaCodec(q @ HttpCodec.Query(HttpCodec.Query.QueryType.Primitive(name, codec), _), _) => OpenAPI.ReferenceOr.Or( OpenAPI.Parameter.queryParameter( name = name, @@ -631,10 +631,10 @@ object OpenAPIGen { examples = mc.examples.map { case (name, value) => name -> OpenAPI.ReferenceOr.Or(OpenAPI.Example(value = Json.Str(value.toString))) }, - required = mc.required, + required = mc.required && !q.isOptional, ), ) :: Nil - case mc @ MetaCodec(HttpCodec.Query(record @ HttpCodec.Query.QueryType.Record(schema), _), _) => + case mc @ MetaCodec(HttpCodec.Query(record @ HttpCodec.Query.QueryType.Record(schema), _), _) => val recordSchema = (schema match { case schema if schema.isInstanceOf[Schema.Optional[_]] => schema.asInstanceOf[Schema.Optional[_]].schema case _ => schema @@ -692,7 +692,7 @@ object OpenAPIGen { examples = mc.examples.map { case (exName, value) => exName -> OpenAPI.ReferenceOr.Or(OpenAPI.Example(value = Json.Str(value.toString))) }, - required = !optional, + required = mc.required && !optional, ), ) :: Nil } @@ -723,7 +723,8 @@ object OpenAPIGen { OpenAPI.Parameter.headerParameter( name = mc.name.getOrElse(codec.name), description = mc.docsOpt, - definition = Some(OpenAPI.ReferenceOr.Or(JsonSchema.fromTextCodec(codec.textCodec))), + definition = + Some(OpenAPI.ReferenceOr.Or(JsonSchema.fromTextCodec(codec.textCodec).nullable(!mc.required))), deprecated = mc.deprecated, examples = mc.examples.map { case (name, value) => name -> OpenAPI.ReferenceOr.Or(OpenAPI.Example(codec.textCodec.encode(value).toJsonAST.toOption.get)) @@ -923,9 +924,13 @@ object OpenAPIGen { case (mediaType, values) => val combinedAtomized: AtomizedMetaCodecs = values.map(_._1).reduce(_ ++ _) val combinedContentDoc = combinedAtomized.contentDocs.toCommonMark + val vals = + if (values.forall(v => v._2.isNullable || v._2 == JsonSchema.Null)) + values.map(_._2).filter(_ != JsonSchema.Null) + else values.map(_._2) val alternativesSchema = { JsonSchema - .AnyOfSchema(values.map { case (_, schema) => + .AnyOfSchema(vals.map { schema => schema.description match { case Some(value) => schema.description(value.replace(combinedContentDoc, "")) case None => schema