HV-1328 Add an option to validate class-level constraints only if all property constraints are valid

[marko-bekhta]

• https://hibernate.atlassian.net/browse/HV-1328

First commit has a change for what looked like a strange behavior. the value only for the last group was considered....

Any ideas on naming of the "mode" is also welcomed. For now PR is missing the doc.

[marko-bekhta]

And here's the benchmark results that I've got from yesterday:

6.1
Benchmark                                   Mode  Cnt     Score     Error   Units
SimpleValidation.testSimpleBeanValidation  thrpt   20  1595.544 ± 127.297  ops/ms

CascadedValidation.testCascadedValidation                                                  thrpt   20   377.533 ±  6.882  ops/ms
CascadedWithLotsOfItemsAndMoreConstraintsValidation.testCascadedValidationWithLotsOfItems  thrpt   20  1012.516 ± 12.189   ops/s
CascadedWithLotsOfItemsValidation.testCascadedValidationWithLotsOfItems                    thrpt   20  1147.559 ± 13.166   ops/s

current:
Benchmark                                   Mode  Cnt     Score     Error   Units
SimpleValidation.testSimpleBeanValidation  thrpt   20  1628.147 ± 185.557  ops/ms

CascadedValidation.testCascadedValidation                                                  thrpt   20   332.512 ±  5.129  ops/ms
CascadedWithLotsOfItemsAndMoreConstraintsValidation.testCascadedValidationWithLotsOfItems  thrpt   20  1039.538 ± 12.060   ops/s
CascadedWithLotsOfItemsValidation.testCascadedValidationWithLotsOfItems                    thrpt   20  1127.804 ± 13.872   ops/s

[gsmet]

@marko-bekhta do we have a benchmark with property constraints + class level constraints? Because that would be an interesting one :).

I'm not sure the others are significant in this case. Although we should probably have a closer look to CascadedValidation.testCascadedValidation.

[marko-bekhta]

jenkins retest this please

[marko-bekhta]

I've added another benchmark that has both class and property level constraints. The results are: no changes:

Benchmark                                                Mode  Cnt     Score    Error   Units
SimpleClassPropertyValidation.testSimpleBeanValidation  thrpt   20  1452.835 ± 21.071  ops/ms

with changes:

Benchmark                                                Mode  Cnt     Score    Error   Units
SimpleClassPropertyValidation.testSimpleBeanValidation  thrpt   20  1438.992 ± 216.247  ops/ms

I also tried to run the cascading benchmarks again. The results are all around the place :roll_eyes: . I'll try to run these on my laptop later today to see how they would behave there.

[marko-bekhta]

While looking through the PRs I've realized that I had not added those benchmarks.... So here they are:

#nochange (current)
Benchmark                                                Mode  Cnt     Score    Error   Units
SimpleClassPropertyValidation.testSimpleBeanValidation  thrpt   20  1694.371 ± 22.571  ops/ms

#chagne (changes in this PR)
Benchmark                                                Mode  Cnt     Score    Error   Units
SimpleClassPropertyValidation.testSimpleBeanValidation  thrpt   20  1518.953 ± 29.393  ops/ms

# update (not in this PR, additional if to check if the option is enabled and if not use the "current" all constraints collections)
Benchmark                                                Mode  Cnt     Score    Error   Units
SimpleClassPropertyValidation.testSimpleBeanValidation  thrpt   20  1679.028 ± 33.595  ops/ms

[gsmet]

@marko-bekhta so I think it makes it clear we need the additional test. Any chance you could push this to the PR (your report above says it wasn't pushed)?

[gsmet]

I need to do a quick release so I'm merging the first commit to master and 6.0 as it looks like a bug to me.

[marko-bekhta]

@gsmet yes, it totally make sense to break this into two pieces. I'll look for that additional commit and push it here when I find it :)

[gsmet]

@marko-bekhta hey Marko. I tried to contact you by email but I didn't hear from you? Did you get my email from December?