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

Clarify 'required: false' and 'default: ...' both being present #1534

Merged
merged 11 commits into from
Jul 10, 2022

Conversation

blast-hardcheese
Copy link
Member

@lolgab correctly pointed out this part of the OpenAPI docs:

Using default with required parameters or properties, for example, with path parameters. This does not make sense – if a value is required, the client must always send it, and the default value is never used.

This PR alters the handling for parameters and properties in the case where getRequired does not return true, and that there is a default present.

Previously, we would liftOptionalTerm and liftOptionalType based purely on getRequired() == true, but now if there is a default value present we leave the values as non-Option-al types and terms.

Practically, that means that if we have a

        - name: p1
          in: query
          required: false
          schema:
            type: integer
            format: int64
            default: 300

we used to get a function like:

def doFoo(p1: Option[Long] = Some(300L)): ...

but now we will get

def doFoo(p1: Long = 300L): ...

This is because the spec mandates that clients always send values in this case, it is invalid to override the default Some(300L) with a None.

@blast-hardcheese blast-hardcheese added bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past scala Broadly concerning Scala code generation or the generated Scala code core Pertains to guardrail-core minor Bump minor version labels Jul 9, 2022
@codecov
Copy link

codecov bot commented Jul 9, 2022

Codecov Report

Merging #1534 (83f3602) into master (c107e05) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1534   +/-   ##
=======================================
  Coverage   84.80%   84.81%           
=======================================
  Files          95       95           
  Lines        5707     5702    -5     
  Branches      141      152   +11     
=======================================
- Hits         4840     4836    -4     
+ Misses        867      866    -1     
Impacted Files Coverage Δ
...ore/src/main/scala/dev/guardrail/SwaggerUtil.scala 90.43% <100.00%> (ø)
...a/dev/guardrail/generators/LanguageParameter.scala 100.00% <100.00%> (ø)
...ail/generators/java/jackson/JacksonGenerator.scala 95.64% <100.00%> (-0.28%) ⬇️
...enerators/scala/circe/CirceProtocolGenerator.scala 92.70% <100.00%> (ø)
...l/generators/scala/ScalaCollectionsGenerator.scala 65.21% <0.00%> (-4.35%) ⬇️
...yncHttpClient/AsyncHttpClientClientGenerator.scala 94.42% <0.00%> (+0.15%) ⬆️
...la/dev/guardrail/generators/java/syntax/Java.scala 78.62% <0.00%> (+0.68%) ⬆️
...s/scala/dropwizard/DropwizardServerGenerator.scala 84.25% <0.00%> (+0.92%) ⬆️

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 c107e05...83f3602. Read the comment docs.

@blast-hardcheese blast-hardcheese removed bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past scala Broadly concerning Scala code generation or the generated Scala code core Pertains to guardrail-core minor Bump minor version labels Jul 10, 2022
@github-actions github-actions bot added core Pertains to guardrail-core java-support Pertains to guardrail-java-support scala-akka-http Pertains to guardrail-scala-akka-http scala-http4s Pertains to guardrail-scala-http4s scala-support Pertains to guardrail-scala-support labels Jul 10, 2022
@blast-hardcheese blast-hardcheese added bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past scala Broadly concerning Scala code generation or the generated Scala code minor Bump minor version and removed scala-akka-http Pertains to guardrail-scala-akka-http scala-http4s Pertains to guardrail-scala-http4s labels Jul 10, 2022
@blast-hardcheese blast-hardcheese merged commit 0ffaa26 into guardrail-dev:master Jul 10, 2022
@blast-hardcheese blast-hardcheese deleted the resolve-1532 branch July 10, 2022 00:10
@blast-hardcheese blast-hardcheese restored the resolve-1532 branch July 17, 2022 04:14
@blast-hardcheese blast-hardcheese deleted the resolve-1532 branch July 17, 2022 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past core Pertains to guardrail-core java-support Pertains to guardrail-java-support minor Bump minor version scala Broadly concerning Scala code generation or the generated Scala code scala-support Pertains to guardrail-scala-support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant