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

Fix behaviuor of required flag for schema class fields. #2185

Closed
mikehalmamoj opened this issue Mar 30, 2023 · 4 comments
Closed

Fix behaviuor of required flag for schema class fields. #2185

mikehalmamoj opened this issue Mar 30, 2023 · 4 comments

Comments

@mikehalmamoj
Copy link

mikehalmamoj commented Mar 30, 2023

Describe the bug

In schema generated from Kotlin classes the required flag defaults to false even if the type is non-nullable. This contradicts the openapi-generator which uses the required flag to determine whether a generated property is nullable.

To Reproduce
Steps to reproduce the behavior:

  @Schema
  val id: Long,

This will generate a field that is not required. It should be required as it is non-nullable.

  • What version of spring-boot you are using?
    3.0.5
  • What modules and versions of springdoc-openapi are you using?
    org.springdoc:springdoc-openapi-starter-webmvc-ui:2.0.4

Expected behavior
When deciding if a field is required we need to consider the Schema annotation required attribute, the type of the field and whether there is a default value.

If there is a schema required value then respect that.
If not then required is true for non-nullable and false for nullable.
Except non-nullable with a default value for which required = false.

I would suggest the following test cases:

required? true true true true false false false false false false false true
schema required value true true true true false false false false none none none none
nullable true true false false true true false false true true false false
default value true false true false true false true false true false true false
@bnasslahsen
Copy link
Contributor

@mikehalmamoj,

Would you be able to propose a PR respecting the test cases you mentioned ?

@mikehalmamoj
Copy link
Author

Fair point, I'll see what I can do.

@bnasslahsen
Copy link
Contributor

@mikehalmamoj ,

I have added the requested fix. Let me know if that works for you.

You can test it with the latest SNAPSHOT and let us know.

@mikehalmamoj
Copy link
Author

Yes that looks spot on. Thanks for the quick turn around! 🙌

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

No branches or pull requests

2 participants