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

Replace strings with schema type resolvers #1407

Conversation

blast-hardcheese
Copy link
Member

Continuing the work started in #1359, push the resolvers down and adding more structure (ReifiedRawType hierarchy for influencing generators)

  • Pushing Schema[_] further down instead of just the (rawType: Option[String], rawFormat: Option[String]) pair
  • Using Schema[_] for getDefault
  • Adding (Schema[_], Components) => Schema[_] for dereferencing $ref

@github-actions github-actions bot added core Pertains to guardrail-core guardrail Pertains to guardrail java-support Pertains to guardrail-java-support labels Feb 20, 2022
@blast-hardcheese blast-hardcheese force-pushed the replace-strings-with-schema-type-resolvers branch from d3d85f9 to 05904b3 Compare February 20, 2022 04:20
@blast-hardcheese blast-hardcheese added the minor Bump minor version label Feb 20, 2022
@codecov
Copy link

codecov bot commented Feb 20, 2022

Codecov Report

Merging #1407 (5d87508) into master (d4e6652) will increase coverage by 0.08%.
The diff coverage is 87.43%.

@@            Coverage Diff             @@
##           master    #1407      +/-   ##
==========================================
+ Coverage   83.19%   83.28%   +0.08%     
==========================================
  Files          88       88              
  Lines        5464     5510      +46     
  Branches      151      142       -9     
==========================================
+ Hits         4546     4589      +43     
- Misses        918      921       +3     
Impacted Files Coverage Δ
...ators/scala/akkaHttp/AkkaHttpServerGenerator.scala 91.14% <ø> (+0.36%) ⬆️
...enerators/scala/http4s/Http4sServerGenerator.scala 96.34% <ø> (ø)
...c/main/scala/dev/guardrail/core/ResolvedType.scala 75.00% <53.84%> (+5.00%) ⬆️
...la/dev/guardrail/generators/SwaggerGenerator.scala 70.54% <63.63%> (-2.97%) ⬇️
...ore/src/main/scala/dev/guardrail/SwaggerUtil.scala 81.68% <89.24%> (+2.89%) ⬆️
...a/dev/guardrail/generators/ProtocolGenerator.scala 90.23% <96.55%> (-0.07%) ⬇️
...les/core/src/main/scala/dev/guardrail/Common.scala 98.93% <100.00%> (+0.01%) ⬆️
...ala/dev/guardrail/generators/ClientGenerator.scala 100.00% <100.00%> (ø)
...a/dev/guardrail/generators/LanguageParameter.scala 89.36% <100.00%> (+1.99%) ⬆️
...ala/dev/guardrail/generators/ServerGenerator.scala 100.00% <100.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4e6652...5d87508. Read the comment docs.

@blast-hardcheese
Copy link
Member Author

The one caveat to this approach is that it (finally?) unifies the Scala server generators (akka-http and http4s) under Vector[_] instead of Iterable[_] for all route types.

This is a pretty significant breaking change, but hopefully a desirable one from the perspective of both consistency and speed, as well as by using a distinct collection instead of an interface, we now get typeclass instances from cats.

Due to the fairly idiosyncratic respond parameter, it should actually be possible to write a scalafix rule that takes care of at least some of these changes, but due to the fact that bugs like #190 have existed for some time, this won't be able to catch all breaking changes.

@blast-hardcheese
Copy link
Member Author

Also resolves #738

@blast-hardcheese blast-hardcheese force-pushed the replace-strings-with-schema-type-resolvers branch from 05904b3 to 4a550ae Compare February 26, 2022 17:30
@blast-hardcheese
Copy link
Member Author

This is blocked by actually writing the scalafix rule to replace Iterator[A] with Vector[A] in route parameters, at the very least.

@blast-hardcheese blast-hardcheese force-pushed the replace-strings-with-schema-type-resolvers branch from 4a550ae to f5cefbf Compare May 11, 2022 01:06
@blast-hardcheese blast-hardcheese force-pushed the replace-strings-with-schema-type-resolvers branch from f5cefbf to 5d87508 Compare May 22, 2022 20:28
@blast-hardcheese
Copy link
Member Author

Alright, scalafix rule was submitted: scala-steward-org/scala-steward#2612

@blast-hardcheese blast-hardcheese merged commit 58105d3 into guardrail-dev:master May 22, 2022
@blast-hardcheese blast-hardcheese deleted the replace-strings-with-schema-type-resolvers branch May 22, 2022 20:46
@blast-hardcheese blast-hardcheese added the enhancement Functionality that has never existed in guardrail label May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Pertains to guardrail-core enhancement Functionality that has never existed in guardrail guardrail Pertains to guardrail java-support Pertains to guardrail-java-support minor Bump minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant