-
Notifications
You must be signed in to change notification settings - Fork 422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
codegen: support enum query params #3602
codegen: support enum query params #3602
Conversation
I think the failure was due to an unrelated flaky test rather than an issue with the code. Would someone mind kicking the test? |
Sure, tests kicked and now all pass. I'll do a review soon. |
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/ClassDefinitionGenerator.scala
Outdated
Show resolved
Hide resolved
6709a66
to
d1bbdd7
Compare
openapi-codegen/core/src/test/scala/sttp/tapir/codegen/TestHelpers.scala
Show resolved
Hide resolved
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EndpointGenerator.scala
Outdated
Show resolved
Hide resolved
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/ClassDefinitionGenerator.scala
Outdated
Show resolved
Hide resolved
I managed to hack a working solution for Scala 3: package sttp.tapir.generated
object TapirGeneratedEndpoints {
import sttp.tapir._
import sttp.tapir.model._
import sttp.tapir.json.circe._
import sttp.tapir.generic.auto._
import io.circe.generic.auto._
import scala.compiletime.{erasedValue, summonInline}
import scala.deriving.Mirror
import enumextensions.EnumMirror
def enumMap[E: EnumMirror]: Map[String, E] =
Map.from(
for e <- EnumMirror[E].values yield e.name.toUpperCase -> e
)
def makeQueryCodecForEnum3[T: EnumMirror]: sttp.tapir.Codec[List[String], T, sttp.tapir.CodecFormat.TextPlain] =
sttp.tapir.Codec
.listHead[String, String, sttp.tapir.CodecFormat.TextPlain]
.mapDecode(s =>
// Case-insensitive mapping
scala.util
.Try(enumMap[T](using EnumMirror[T])(s.toUpperCase)) // TODO a nicer error on the stack trace instead of java.util.NoSuchElementException: key not found
.fold(sttp.tapir.DecodeResult.Error(s, _), sttp.tapir.DecodeResult.Value(_))
)(_.name)
enum MyEnumKc derives org.latestbit.circe.adt.codec.JsonTaggedAdt.PureCodec, EnumMirror {
case paperback, hardback
}
object MyEnumKc {
given codecTapir: sttp.tapir.Codec[List[String], MyEnumKc, sttp.tapir.CodecFormat.TextPlain] =
makeQueryCodecForEnum3[MyEnumKc]
}
import cats.effect.IO
lazy val postQueryTest =
endpoint.post
.in(("queryTest"))
.in(query[MyEnumKc]("test"))
.out(statusCode(sttp.model.StatusCode.NoContent))
.serverLogic[IO] { (test: MyEnumKc) =>
// Some server logic
IO.println(s"Received: $test").map(Right(_))
}
lazy val generatedEndpoints = List(postQueryTest)
} It requires an additional dependency on What do you think @hughsimpson? |
Nice! I was a bit uncomfortable about not having scala 3 support. Let's include that in this pr |
Pushed up. Shame we can't directly check the scala 3 support in the test 😪 but it looks right to me... |
Thanks! I checked Scala 3 in my side project and it works, I also added a small improvement to the error message. Could you update the description to reflect Scala 3 support? Feel free to merge after that (if you have permissions, I'm not sure about that :) |
I don't have permissions to merge, sadly, and the test makes some rather bold assertions about the precise code generated for scala 3, so I think that will need updating anyway. End of my day now but I'll push up a fix first thing tomorrow morning |
…at the enumeratum.Enum have a Circe encoder
Pushed a fix for the test; also updated the scala 2 decoding to provide the same nicer error message as for scala 3 |
Thanks for your help with this, @kciesielski! I'm hoping to be able to open a pr with support for jsoniter later on today once I've tidied up a bit, and it builds off this branch (because it turns out that tracking which positions the models are used in is very useful for that 😄) |
Thank you for these great contributions! Keep'em coming 😅 |
Support for enum query params in codegen. I've gone for case-insensitive decoding here, but arguably case-sensitive would be the better option; it should be very easy to change or make configurable later on.