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

[Jaxrs-cxf] Add bean-level cascaded beanvalidation for pojos (@Valid) fix #4738 #7807

Merged
merged 6 commits into from
Apr 7, 2018

Conversation

fabian-braun
Copy link
Contributor

@fabian-braun fabian-braun commented Mar 9, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

fix #4738 . Followup on PR #4922 .

This PR allows to use cascaded bean validation for jaxrs-cxf.
In short: If a POJO contain other POJO fields, these can be validated recursively by CXF if they are annotated with @Valid.

In #4922 one open issue was mentioned:

@Valid is currently also generated for properties of type:

  • java.lang.BigDecimal
  • javax.xml.datatype.XMLGregorianCalendar
  • org.joda.time.LocalDate
  • java.util.UUID

With this new PR the behavior changes:

  • java.lang.BigDecimal - @Valid still generated
  • javax.xml.datatype.XMLGregorianCalendar - untested
  • org.joda.time.LocalDate - fixed (no @Valid generated)
  • java.util.UUID - fixed (no @Valid generated)

However IMO it is not a problem that some non-applicable types are still annotated with @Valid. The validation provider will skip validation as soon as it notices that no further validation annotations are present on these types.

One additional change to the original PR is, that also containers are annotated with @Valid
According to http://beanvalidation.org/2.0/spec/:

Collection-valued, array-valued and generally Iterable fields and properties may also be decorated with the @Valid annotation. This causes the contents of the iterator to be validated. Any object implementing java.lang.Iterable is supported.

This feature is already present since beanvalidation 1.0

Some additional remarks because this is my first contribution:

  • I have added a new windows-bat file swagger-codegen\bin\windows\jaxrs-cxf-petstore-server.bat to the project, because it was missing (I created it based on the corresponding .sh file).
  • When I checked out the current master and generated the model it was already not in line with what was committed. In the contribution guide it says that this can happen, so I just committed those files in the last commit of this PR.

@jfiala : would you be so kind to review the PR

@fabian-braun fabian-braun changed the title Cxf beanval valid pojo [Jaxrs-cxf] Add bean-level cascaded beanvalidation for pojos (@Valid) fix #4738 Mar 9, 2018
@wing328
Copy link
Contributor

wing328 commented Apr 6, 2018

@fabian-braun thanks for the PR. I did more tests and the result is good (https://circleci.com/gh/swagger-api/swagger-codegen/4419)

@wing328
Copy link
Contributor

wing328 commented Apr 6, 2018

@fabian-braun I'll merge it this weekend if there's no further question/feedback from the community.

@fabian-braun
Copy link
Contributor Author

@wing328 Thanks for your initiative and feedback!

@wing328 wing328 merged commit ce930e7 into swagger-api:master Apr 7, 2018
@wing328
Copy link
Contributor

wing328 commented Apr 7, 2018

@fabian-braun PR merged into master. Have a nice weekend.

@filbert3
Copy link

Hi @wing328, is the fixed included in swagger codegen v3? What else do I need to do except set true in ? I don't see nested @Valid in my generated code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for BeanValidation @Valid annotation on nested attributes
4 participants