Skip to content

Commit f5b9eae

Browse files
hughsimpsonhughsimpson
authored
openapi: Fix mutual-recursion bug in scala 2.13 (issue 4911) (#4916)
Contains the fix for a few disparate things: - fix runtime stackoverflow for mutually-recursive scheme definitions in scala 2.13 ([issue](#4911)) - permit defining schema aliases for simple types (i.e ```yaml ClientIdParameterStr: type: string ``` becomes `type ClientIdParameterStr = String`) - handle missing descriptions on responses (these are technically required by the spec, but often absent IRL) - handle non-200 streaming responses with descriptions - handle optional streaming responses with empty alternatives defined for other status codes - produce io.circe.JsonObject for [free-form object](https://swagger.io/docs/specification/v3_0/data-models/data-types/#free-form-object) types, rather than empty case classes (note that old behaviour can be achieved by adding an empty `properties` map to the definition; but that an empty case class was never an appropriate model to generate for a free form object). This is not always going to be the correct model -- arguably, for any non-json endpoint, it's probably not ideal -- but it's the most generic model that's known to be available, and which can be generally mapped to object schema definitions. - handle inline enum definitions in paths - default `*/*` to strict for convenience (this behaviour can be overridden by the appropriate directives, but seems like it's _probably_ a saner default) Includes the swagger from https://nifi.apache.org/nifi-docs/swagger.yaml, which prompted these fixes - since it's an OSS apache project, that seems legit - along with a few modifications to it (mostly commenting out some security declarations, although also removes some 'object' wrappers around what would otherwise be primitive schema declarations, fudges the wrapper around `BulletinBoardPatternParameter` a bit since doing otherwise would require supporting objects in query params, and adds a `flows` field to the `HTTPBearerJWT` decl. Doesn't address the fact that some query params look like they should probably be arrays judging by the description, nor does it try to make sense of the security declarations since they don't match up with any openapi conventions I'm aware of). This is a pretty chunky spec and adds a couple of mins to the openapi test duration, but I think it's worth it to get the extra coverage... Given the nature of the bug, we not only run the check to validate expected scala output and that the code compiles, but also explicitly check that the `controllerServiceReferencingComponentDTOTapirSchema` schema doesn't throw a `StackOverflow` at runtime.
1 parent a583cd0 commit f5b9eae

File tree

21 files changed

+33646
-75
lines changed

21 files changed

+33646
-75
lines changed

‎openapi-codegen/core/src/main/scala/sttp/tapir/codegen/ClassDefinitionGenerator.scala‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ class ClassDefinitionGenerator {
8585
case (name, OpenapiSchemaMap(valueSchema, _, _)) => generateMap(name, valueSchema)
8686
case (name, OpenapiSchemaArray(valueSchema, _, _, rs)) => generateArray(name, valueSchema, rs)
8787
case (_, _: OpenapiSchemaOneOf) => Nil
88+
case (name, r: OpenapiSchemaSimpleType) => generateAlias(name, r)
8889
case (n, x) => throw new NotImplementedError(s"Only objects, enums and maps supported! (for $n found ${x})")
8990
})
9091
.map(_.mkString("\n"))
@@ -196,6 +197,13 @@ class ClassDefinitionGenerator {
196197
else Seq(s"""type $name = List[$valueSchemaName]""")
197198
}
198199

200+
private[codegen] def generateAlias(name: String, valueSchema: OpenapiSchemaSimpleType): Seq[String] = valueSchema match {
201+
case r: OpenapiSchemaRef => Seq(s"""type $name = ${r.stripped}""")
202+
case r: OpenapiSchemaSimpleType =>
203+
val simpleType = mapSchemaSimpleTypeToType(r)._1
204+
Seq(s"""type $name = $simpleType""")
205+
}
206+
199207
private[codegen] def generateClass(
200208
allSchemas: Map[String, OpenapiSchemaType],
201209
name: String,

‎openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EndpointGenerator.scala‎

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import sttp.tapir.codegen.openapi.models.OpenapiModels.{
1313
OpenapiResponseDef
1414
}
1515
import sttp.tapir.codegen.openapi.models.OpenapiSchemaType.{
16+
AnyType,
1617
OpenapiSchemaAny,
1718
OpenapiSchemaArray,
1819
OpenapiSchemaBinary,
@@ -99,6 +100,13 @@ case class EndpointDefs(
99100
details: EndpointDetails
100101
)
101102

103+
case class UrlMapResponse(
104+
pathDecl: String,
105+
pathTypes: Seq[String],
106+
maybeSecurityPath: Option[(String, Seq[String])],
107+
inlineUrlDefs: Seq[String]
108+
)
109+
102110
class EndpointGenerator {
103111
private def combine(inlineDefn1: Option[String], inlineDefn2: Option[String], separator: String = "\n") =
104112
(inlineDefn1, inlineDefn2) match {
@@ -119,6 +127,8 @@ class EndpointGenerator {
119127
case FS2(effectType) => s"fs2.Stream[$effectType, Byte]"
120128
case x => s"${capabilityImpl(x)}.BinaryStream"
121129
}
130+
// These types all use 'eager' schemas, except for '*/*', which we default to eager for convenience but which has no schema mappings
131+
private val eagerTypes = Set("application/json", "application/xml", "text/plain", "text/html", "multipart/form-data", "*/*")
122132

123133
def endpointDefs(
124134
doc: OpenapiDocument,
@@ -211,8 +221,11 @@ class EndpointGenerator {
211221

212222
val name = m.name(p.url)
213223
val maybeName = m.operationId.map(n => s"""\n .name("${JavaEscape.escapeString(n)}")""").getOrElse("")
214-
val (pathDecl, pathTypes, maybeSecurityPath) = urlMapper(
224+
val UrlMapResponse(pathDecl, pathTypes, maybeSecurityPath, inlineUrlDefs) = urlMapper(
215225
p.url,
226+
name,
227+
targetScala3,
228+
jsonSerdeLib,
216229
m.resolvedParameters,
217230
doc.pathsExtensions.get(securityPrefixKey).flatMap(_.asArray).toSeq.flatMap(_.flatMap(_.asString)),
218231
doc,
@@ -253,7 +266,8 @@ class EndpointGenerator {
253266
errTypes.toSeq,
254267
outTypes.toSeq
255268
)
256-
val inlineDefn = combine(inlineInDefns, inlineDefns)
269+
val maybeInlineUrlDefs = if (inlineUrlDefs.isEmpty) None else Some(inlineUrlDefs.mkString("\n"))
270+
val inlineDefn = combine(maybeInlineUrlDefs, combine(inlineInDefns, inlineDefns))
257271
val sec = securityDecl.map(indent(2)(_) + "\n").getOrElse("")
258272
val securityPathDecl = maybeSecurityPath.map("\n " + _._1).getOrElse("")
259273
val definition =
@@ -280,13 +294,13 @@ class EndpointGenerator {
280294
.collect { case ref: OpenapiSchemaRef if ref.isSchema => ref.stripped }
281295
val jsonParamRefs = (m.requestBody.toSeq.flatMap(_.resolve(doc).content.map(c => (c.contentType, c.schema))) ++
282296
m.responses.flatMap(_.resolve(doc).content.map(c => (c.contentType, c.schema))))
297+
.filterNot(_ => m.tapirCodegenDirectives.contains(jsonBodyAsString))
283298
.collect { case (contentType, schema) if contentType == "application/json" => schema }
284299
.collect {
285300
case ref: OpenapiSchemaRef if ref.isSchema => ref.stripped
286301
case OpenapiSchemaArray(ref: OpenapiSchemaRef, _, _, _) if ref.isSchema => ref.stripped
287-
case OpenapiSchemaArray(OpenapiSchemaAny(_), _, _, _) =>
288-
bail("Cannot generate schema for 'Any' with jsoniter library")
289-
case OpenapiSchemaArray(simple: OpenapiSchemaSimpleType, _, _, _) =>
302+
case OpenapiSchemaArray(OpenapiSchemaAny(_, tpe), _, _, _) => AnyType.toCirceTpe(tpe)
303+
case OpenapiSchemaArray(simple: OpenapiSchemaSimpleType, _, _, _) =>
290304
val name = RootGenerator.mapSchemaSimpleTypeToType(simple)._1
291305
s"List[$name]"
292306
case simple: OpenapiSchemaSimpleType =>
@@ -328,18 +342,21 @@ class EndpointGenerator {
328342

329343
private def urlMapper(
330344
url: String,
345+
endpointName: String,
346+
targetScala3: Boolean,
347+
jsonSerdeLib: JsonSerdeLib,
331348
parameters: Seq[OpenapiParameter],
332349
securityPathPrefixes: Seq[String],
333350
doc: OpenapiDocument,
334351
generateValidators: Boolean
335352
)(implicit
336353
location: Location
337-
): (String, Seq[String], Option[(String, Seq[String])]) = {
354+
): UrlMapResponse = {
338355
val securityPrefixes = securityPathPrefixes.filter(url.startsWith)
339356
// .in(("books" / path[String]("genre") / path[Int]("year")).mapTo[BooksFromYear])
340357
val maxSecurityPrefix = if (securityPrefixes.nonEmpty) Some(securityPrefixes.maxBy(_.length)) else None
341358
val inUrl = maxSecurityPrefix.fold(url)(url.stripPrefix)
342-
def toPathDecl(url: String) = url
359+
def toPathDecl(url: String): (Array[String], Array[Option[String]], Array[Option[String]]) = url
343360
.split('/')
344361
.filter(_.nonEmpty)
345362
.map { segment =>
@@ -352,20 +369,26 @@ class EndpointGenerator {
352369
val (t, _) = mapSchemaSimpleTypeToType(st)
353370
val desc = p.description.fold("")(d => s""".description("$d")""")
354371
val validations = if (generateValidators) ValidationGenerator.mkValidations(doc, st, true) else ""
355-
s"""path[$t]("$name")$validations$desc""" -> Some(t)
372+
(s"""path[$t]("$name")$validations$desc""", Some(t), None)
373+
case e: OpenapiSchemaEnum =>
374+
val (param, inlineDefn, tpe) = getEnumParamDefn(endpointName, targetScala3, jsonSerdeLib, p, e, false)
375+
(param, Some(tpe), inlineDefn.map(_.mkString("\n")))
356376
case _ => bail("Can't create non-simple params to url yet")
357377
}
358378
}
359379
} else {
360-
'"' + segment + '"' -> None
380+
('"' + segment + '"', None, None)
361381
}
362382
}
363-
.unzip
364-
val (inPath, tpes) = toPathDecl(inUrl)
383+
.unzip3
384+
val (inPath, tpes, inlineDefns) = toPathDecl(inUrl)
365385
val inPathDecl = if (inPath.nonEmpty) ".in((" + inPath.mkString(" / ") + "))" else ""
366-
val secPathDecl =
367-
maxSecurityPrefix.map(toPathDecl).map { case (ds, ts) => ".prependSecurityIn(" + ds.mkString(" / ") + ")" -> ts.toSeq.flatten }
368-
(inPathDecl, tpes.toSeq.flatten, secPathDecl)
386+
val (secPathDecl, secInlineDefs) =
387+
maxSecurityPrefix
388+
.map(toPathDecl)
389+
.map { case (ds, ts, inline) => (".prependSecurityIn(" + ds.mkString(" / ") + ")" -> ts.toSeq.flatten) -> inline }
390+
.unzip
391+
UrlMapResponse(inPathDecl, tpes.toSeq.flatten, secPathDecl.headOption, secInlineDefs.flatten.flatten.toSeq ++ inlineDefns.toSeq.flatten)
369392
}
370393

371394
private def toOutType(baseType: String, isArray: Boolean, noOptionWrapper: Boolean) = (isArray, noOptionWrapper) match {
@@ -382,7 +405,7 @@ class EndpointGenerator {
382405
param: OpenapiParameter,
383406
e: OpenapiSchemaEnum,
384407
isArray: Boolean
385-
) = {
408+
): (String, Some[Seq[String]], String) = {
386409
val enumName = endpointName.capitalize + strippedToCamelCase(param.name).capitalize
387410
val enumParamRefs = if (param.in == "query" || param.in == "path") Set(enumName) else Set.empty[String]
388411
val enumDefn = EnumGenerator.generateEnum(
@@ -505,8 +528,7 @@ class EndpointGenerator {
505528
val d = b.description.map(s => s""".description("${JavaEscape.escapeString(s)}")""").getOrElse("")
506529
Some((s".in($decl$d)", tpe, maybeInlineDefn))
507530
} else {
508-
// These types all use 'eager' schemas; we cannot mix eager and streaming types when using oneOfBody
509-
val eagerTypes = Set("application/json", "application/xml", "text/plain", "text/html", "multipart/form-data")
531+
// We cannot mix eager and streaming types when using oneOfBody
510532
val preferEager = b.content.exists(c => eagerTypes.contains(c.contentType))
511533
val mapped = b.content.map(mapContent(_, b.required, preferEager))
512534
val (decls, tpes, maybeInlineDefns) = mapped.unzip3
@@ -651,8 +673,7 @@ class EndpointGenerator {
651673
val d = s""".description("${JavaEscape.escapeString(resp.description)}")"""
652674
(s"$decl$d", Some(tpe), maybeInlineDefn)
653675
case seq =>
654-
// These types all use 'eager' schemas; we cannot mix eager and streaming types when using oneOfBody
655-
val eagerTypes = Set("application/json", "application/xml", "text/plain", "text/html", "multipart/form-data")
676+
// We cannot mix eager and streaming types when using oneOfBody
656677
val preferEager = seq.exists(c => eagerTypes.contains(c.contentType))
657678
val (decls, tpes, maybeInlineDefns) = seq.map(wrapContent(_, preferEager)).unzip3
658679
val distinctTypes = tpes.distinct
@@ -770,11 +791,13 @@ class EndpointGenerator {
770791
if (outHeaderTypes.isEmpty) maybeBodyType
771792
else if (maybeBodyType.isEmpty) ht()
772793
else maybeBodyType.map(t => s"($t, ${ht(false).get})")
794+
val tpeIsBin = maybeBodyType.exists(t => t.contains("BinaryStream") || t.contains("fs2.Stream"))
773795
(
774796
Some(resp.code match {
775-
case "200" | "default" => s"$decl$hs"
776-
case okStatus(s) => s"$decl$hs.and(statusCode(sttp.model.StatusCode($s)))"
777-
case errorStatus(s) => s"$decl$hs.and(statusCode(sttp.model.StatusCode($s)))"
797+
case "200" | "default" => s"$decl$hs"
798+
case okStatus(s) if tpeIsBin => s"$decl.toEndpointIO$hs.and(statusCode(sttp.model.StatusCode($s)))"
799+
case okStatus(s) => s"$decl$hs.and(statusCode(sttp.model.StatusCode($s)))"
800+
case errorStatus(s) => s"$decl$hs.and(statusCode(sttp.model.StatusCode($s)))"
778801
}),
779802
tpe,
780803
inlineDefn.map(_ ++ inlineHeaderEnumDefns.getOrElse("")).orElse(inlineHeaderEnumDefns)
@@ -834,18 +857,20 @@ class EndpointGenerator {
834857
} else {
835858
def withHeaderTypes(t: String): String = if (commonResponseHeaders.isEmpty) t else s"($t, ${ht(false).get})"
836859
def withUnderscores(t: String): String = if (commonResponseHeaders.isEmpty) t else s"($t, $underscores)"
860+
val tpeIsBin = maybeBodyType.exists(t => t.contains("BinaryStream") || t.contains("fs2.Stream"))
861+
val maybeStrict = if (tpeIsBin) ".toEndpointIO" else ""
837862
if (contentCanBeEmpty) {
838863
val (_, nonOptionalType, _) = bodyFmt(m, isErrorPosition)
839-
val maybeMap = if (m.content.size > 1) ".map(Some(_))(_.orNull)" else ""
864+
val maybeMap = if (m.content.size > 1 || tpeIsBin) ".map(Some(_))(_.orNull)" else ""
840865
val someType = nonOptionalType.map(": " + _.replaceAll("^Option\\[(.+)]$", "$1")).getOrElse("")
841866
(
842-
s"oneOfVariantValueMatcher(sttp.model.StatusCode(${code}), $decl$maybeMap$hs){ case ${withUnderscores(s"Some(_$someType)")} => true }",
867+
s"oneOfVariantValueMatcher(sttp.model.StatusCode(${code}), $decl$maybeStrict$maybeMap$hs){ case ${withUnderscores(s"Some(_$someType)")} => true }",
843868
maybeBodyType,
844869
inlineDefn1
845870
)
846871
} else
847872
(
848-
s"oneOfVariant${maybeBodyType.map(s => s"[${withHeaderTypes(s)}]").getOrElse("")}(sttp.model.StatusCode(${code}), $decl$hs)",
873+
s"oneOfVariant${maybeBodyType.map(s => s"[${withHeaderTypes(s)}]").getOrElse("")}(sttp.model.StatusCode(${code}), $decl$maybeStrict$hs)",
849874
maybeBodyType,
850875
inlineDefn1
851876
)

‎openapi-codegen/core/src/main/scala/sttp/tapir/codegen/JsonSerdeGenerator.scala‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import sttp.tapir.codegen.openapi.models.OpenapiSchemaType.{
1414
OpenapiSchemaObject,
1515
OpenapiSchemaOneOf,
1616
OpenapiSchemaRef,
17+
OpenapiSchemaSimpleType,
1718
OpenapiSchemaString,
1819
OpenapiSchemaStringType
1920
}
@@ -152,6 +153,7 @@ object JsonSerdeGenerator {
152153
Some(genCirceAdtSerde(allSchemas, schema, name, validateNonDiscriminatedOneOfs))
153154
case (_, _: OpenapiSchemaObject | _: OpenapiSchemaMap | _: OpenapiSchemaEnum | _: OpenapiSchemaOneOf | _: OpenapiSchemaAny, _) =>
154155
None
156+
case (_, t: OpenapiSchemaSimpleType, _) if !t.isInstanceOf[OpenapiSchemaRef] => None
155157
case (n, x, _) => throw new NotImplementedError(s"Only objects, enums, maps, arrays and oneOf supported! (for $n found ${x})")
156158
}
157159
.foldLeft(Option.empty[String]) {
@@ -276,7 +278,13 @@ object JsonSerdeGenerator {
276278
val maybeAnySerde =
277279
if (schemasContainAny)
278280
Some(
279-
"implicit lazy val anyJsonSupport: com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec[io.circe.Json] = com.github.plokhotnyuk.jsoniter_scala.circe.JsoniterScalaCodec.jsonCodec()"
281+
"""implicit lazy val anyJsonSupport: com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec[io.circe.Json] = com.github.plokhotnyuk.jsoniter_scala.circe.JsoniterScalaCodec.jsonCodec()
282+
|implicit lazy val anyObjJsonSupport: com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec[io.circe.JsonObject] = new com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec[io.circe.JsonObject] {
283+
| import com.github.plokhotnyuk.jsoniter_scala.core.{JsonReader, JsonWriter}
284+
| override def decodeValue(in: JsonReader, default: io.circe.JsonObject): io.circe.JsonObject = anyJsonSupport.decodeValue(in, io.circe.Json.fromJsonObject(default)).asObject.getOrElse(throw new RuntimeException(s"${default.getClass.getSimpleName} is not an object"))
285+
| override def encodeValue(x: io.circe.JsonObject, out: JsonWriter): Unit = anyJsonSupport.encodeValue(io.circe.Json.fromJsonObject(x), out)
286+
| override def nullValue: io.circe.JsonObject = null
287+
|}""".stripMargin
280288
)
281289
else None
282290
// For jsoniter-scala, we define explicit serdes for any 'primitive' params (e.g. List[java.util.UUID]) that we reference.
@@ -333,6 +341,7 @@ object JsonSerdeGenerator {
333341
_
334342
) =>
335343
Nil
344+
case (_, t: OpenapiSchemaSimpleType, _) if !t.isInstanceOf[OpenapiSchemaRef] => Nil
336345
case (n, x, _) => throw new NotImplementedError(s"Only objects, enums, maps, arrays and oneOf supported! (for $n found ${x})")
337346
}
338347
(docSchemas.map { case (n, t) => (n, t, false) } ++ pathSchemas)
@@ -492,6 +501,7 @@ object JsonSerdeGenerator {
492501
Some(genZioAdtSerde(allSchemas, schema, name, validateNonDiscriminatedOneOfs))
493502
case (_, _: OpenapiSchemaObject | _: OpenapiSchemaMap | _: OpenapiSchemaArray | _: OpenapiSchemaEnum | _: OpenapiSchemaOneOf, _) =>
494503
None
504+
case (_, t: OpenapiSchemaSimpleType, _) if !t.isInstanceOf[OpenapiSchemaRef] => None
495505
case (n, x, _) => throw new NotImplementedError(s"Only objects, enums, maps, arrays and oneOf supported! (for $n found ${x})")
496506
}
497507
.foldLeft(Option.empty[String]) {

‎openapi-codegen/core/src/main/scala/sttp/tapir/codegen/RootGenerator.scala‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package sttp.tapir.codegen
22

33
import sttp.tapir.codegen.openapi.models.OpenapiModels.OpenapiDocument
44
import sttp.tapir.codegen.openapi.models.OpenapiSchemaType.{
5+
AnyType,
56
OpenapiSchemaAny,
67
OpenapiSchemaBinary,
78
OpenapiSchemaBoolean,
@@ -360,8 +361,8 @@ object RootGenerator {
360361
("Array[Byte]", nb)
361362
case OpenapiSchemaByte(nb) =>
362363
("ByteString", nb)
363-
case OpenapiSchemaAny(nb) =>
364-
("io.circe.Json", nb)
364+
case OpenapiSchemaAny(nb, t) =>
365+
(AnyType.toCirceTpe(t), nb)
365366
case OpenapiSchemaRef(t) =>
366367
(t.split('/').last, false)
367368
case x => throw new NotImplementedError(s"Not all simple types supported! Found $x")

0 commit comments

Comments
 (0)