Skip to content
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 enums in paths #3889

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Jul 1, 2024

Support enums in path params. Most of the diff here is just renaming variables and types to reflect that they no longer pertain only to query params, but also to path params; nearly all of the implementation is supported just by reusing those existing mechanisms

@adamw adamw requested a review from kciesielski July 1, 2024 08:47
@@ -169,10 +172,6 @@ object BasicGenerator {
| case Some(values) => DecodeResult.sequence(values.split(',').toSeq.map(e => support.rawDecode(List(e)))).map(r => Some(CommaSeparatedValues(r.toList)))
| }(_.map(_.values.map(support.encode).mkString(",")))
|}
|implicit def makeExplodedQuerySeqCodecFromSupport[T](implicit support: QueryParamSupport[T]): sttp.tapir.Codec[List[String], ExplodedValues[T], sttp.tapir.CodecFormat.TextPlain] = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now provide a Codec[String, T, sttp.tapir.CodecFormat.TextPlain], from which we can derive a Codec[List[String], List[T], sttp.tapir.CodecFormat.TextPlain], the final implicit derivation is provided by makeExplodedQuerySeqCodecFromListSeq and this is now redundant (it also causes explicit resolution conflicts in scala 3 if left in, although scala 2 seems happy enough with the ambiguity for some reason).

.leftMap(err => err: Error)
.flatMap(_.as[OpenapiDocument]) shouldBe Right(
TestHelpers.enumQueryParamDocs
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was previously missing

Copy link
Member

@kciesielski kciesielski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kciesielski kciesielski merged commit ae26fc7 into softwaremill:master Jul 2, 2024
21 checks passed
@kciesielski
Copy link
Member

@hughsimpson I'd like to release v1.10.11 with your recent codegen enhancements, unless there's something extra you'd like to include before that?

@hughsimpson
Copy link
Contributor Author

@kciesielski I've got nothing else I need, would love a 1.10.11! 🎉

@hughsimpson hughsimpson deleted the path_enums branch July 2, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants