Repository navigation
codegen: Fix code generation for some unsupported response combinations - #5592
Conversation
| } | ||
| case class PostJsonStringJsonBodyBodyOutFull ( | ||
| override val `application/json`: () => Option[String], | ||
| override val `application/json`: () => String, |
There was a problem hiding this comment.
This shouldn't really have been wrapped before -- the declaration is not for a nullable schema (the None case is on a separate status code with no declared content) so it made no sense to do it (just made it possible to represent invalid forms)
| } | ||
| case class PostJsonStringJsonBodyBodyOption_String_Out(value: Option[String]) extends PostJsonStringJsonBodyBodyOut{ | ||
| override def `application/json`: () => Option[String] = () => value | ||
| case class PostJsonStringJsonBodyBodyStringOut(value: String) extends PostJsonStringJsonBodyBodyOut{ |
There was a problem hiding this comment.
ditto (shouldn't've been wrapped at this level -- is handled by an outer oneOfVariantValueMatcher)
| .in(("headers" / "tests")) | ||
| .securityIn(auth.oauth2.implicitFlow("https://petstore3.swagger.io/oauth/authorize", None)) | ||
| .out(oneOf[(Option[String], PutHeadersTestsResponseHeader)]( | ||
| oneOfVariantValueMatcher(sttp.model.StatusCode(200), stringBody.map(Option(_))(_.orNull).description("Strict text response with a header").and(header[String]("Content-Disposition").description("").map(PutHeadersTestsResponseHeader200(_))(_.`Content-Disposition`))){ case (Some(_: String), (_: PutHeadersTestsResponseHeader200)) => true }, |
There was a problem hiding this comment.
The generated code didn't compile before, because we were generating the outer oneOfVariantValueMatcher but the inner value didn't have an option type because it was missing the map. That new map should be safe given the matcher.
| val body = if (ContentTypes.isSuffixed(ct)) suffixedBody("sttp.tapir.Codec.string", ct) else "stringJsonBody" | ||
| // this doesn't use the wrap helpers to preserve what was going on on requests before it was implemented; may be buggy for those cases. | ||
| if (required) MappedContentType(body, "String", None) | ||
| else MappedContentType(s"$body.map(Option(_))(_.orNull)", "Option[String]", None) |
There was a problem hiding this comment.
I would've messed around with this more but for the fact that I couldn't figure out a good way to handle non-required request bodies in the general case. It'll want looking at again but I think it can wait for now.
Comment left in as a hint...
|
Thank you - as always 👌 |
2 participants
@uncheckedannotation on response check for streaming types (since generic param otherwise causes scala compiler to raise a warning which can be hard to silence)I was initially trying to improve handling of non-required inputs too, but ran into a whole host of issues, so I've stripped this back to be more focused than it was before... There are almost certainly output cases that this will still not work for as well, but I think this pretty clearly increases the set of supported combinations.