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

HV-1494 Hibernate Validator specific @NotEmpty used on return type throws an exception #863

Closed
wants to merge 1 commit into from

Conversation

marko-bekhta
Copy link
Member

I found that only NotEmpty Range and TituloEleitoral were the composite constraints without validators. So I've added SupportedValidationTarget for them.

@gsmet
Copy link
Member

gsmet commented Oct 18, 2017

@marko-bekhta you removed the rest of your changes? How does it work for custom composite constraints?

Note that we can require that users define SupportedValidationTarget on them.

What you did in this PR is what Gunnar originally had in mind but I'm curious on why you removed the rest of your changes?

@marko-bekhta
Copy link
Member Author

@gsmet well they are in that other branch (in the closed PR, I left them there if you'd like to check those, as otherwise they wouldn't be visible if I update the commit and force push the changes right?)
And this one is a clean one. With just what Gunnar was suggesting.

I just thought that it was an over-complicated solution (because I didn't know about SupportedValidationTarget). For the custom ones - there's one in the test. If SupportedValidationTarget is placed on it - all works fine.

So I thought if there's an existing solution to the problem - it'll be better to use it rather than make the ConstraintDescriptorImpl#determineConstraintType more complicated ...

@gsmet
Copy link
Member

gsmet commented Oct 18, 2017

@marko-bekhta sure. And this one will be easily backportable too. Thanks for that!

@gunnarmorling
Copy link
Member

That looks good +1. We can still go for the more automated route in a next release, only see my comment about combining the annotation tree traversals if possible in the closed PR.

@gsmet
Copy link
Member

gsmet commented Oct 18, 2017

Merged with a few minor changes I included in your commit.

@gsmet gsmet closed this Oct 18, 2017
@gsmet
Copy link
Member

gsmet commented Oct 18, 2017

Backported to 5.4 and 5.3.

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