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) #4738 #4922

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jfiala
Copy link
Contributor

@jfiala jfiala commented Mar 4, 2017

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch 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)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Implemented #4738 for CXF

Open issues:
@Valid is currently also generated for properties of type (isPrimitiveType/^isContainer does not cover these):

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

Please advise how we can best check for model-specific pojos?

In fact, there is only one valid @Valid-reference in Pet.java:

@Valid
private Category category = null;

@jfiala jfiala changed the title [Jaxrs-cxf] Add cascaded beanvalidation @Valid for pojos #4738 [Jaxrs-cxf] Add cascaded beanvalidation for pojos (@Valid) #4738 Mar 4, 2017
@jfiala jfiala changed the title [Jaxrs-cxf] Add cascaded beanvalidation for pojos (@Valid) #4738 [Jaxrs-cxf] Add bean-level cascaded beanvalidation for pojos (@Valid) #4738 Mar 4, 2017
@jfiala
Copy link
Contributor Author

jfiala commented Mar 9, 2017

@wing328 can you pls advise regarding the open issues above?
The question is how to best scope @Valid to be generated only for classes of the model, not for other classes.

@wing328
Copy link
Contributor

wing328 commented Mar 12, 2017

@jfiala My take is that {{#isPrimitive}} should cover the following as well

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

so that the following should imply models:

{{^isContainer}}{{^isPrimitiveType}} ... {{/isPrimitiveType}}{{/isContainer}}

@jfiala
Copy link
Contributor Author

jfiala commented Mar 12, 2017

This is exactly what I am using at pojo.mustache:

{{^isPrimitiveType}}{{^isContainer}}{{#useBeanValidation}}
+  @Valid{{/useBeanValidation}}{{/isContainer}}{{/isPrimitiveType}}

PrimitiveType covers only the real primitive types and container the List/Map items.
So I think we need a new flag to check for isModelType?

@fehguy
Copy link
Contributor

fehguy commented Mar 12, 2017

@jfiala shouldn't primitive or not primitive cover it?

@jfiala
Copy link
Contributor Author

jfiala commented Mar 12, 2017

@fehguy unfortunately not, see above listed mustache snippet, I actually used ^isPrimitiveType/^isContainer.
The problem is that the above datatypes (BigDecimal, UUID etc.) are not considered primitives.
On the other hand, probably it would be better to introduce a new flag to check if a datatype is a pojo of the model?

@wing328
Copy link
Contributor

wing328 commented Mar 13, 2017

So I think we need a new flag to check for isModelType?

I would rather update "isPrimitiveType" to include the additional items (e.g. uuid) you've listed.

Ref: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types

@olivierpaquet
Copy link

@wing328 any news about this PR? We also need this feature but seems that the PR is not yet merged / final.

@jfiala
Copy link
Contributor Author

jfiala commented Jan 31, 2018

@olivierpaquet Thx for asking, but currently I'm too busy to rebase, if you like you can rebase and re-provide the PR.

@mohangadm
Copy link

mohangadm commented Jan 29, 2020

We were trying to use this plugin for open api 3, and the validation of cascading beans is failing as @Valid annotation is not added on cascaded pojos.
Is this pull req merged to 3.X version for open api-3?

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.

6 participants