Normalize CDIWrappers to an interface that they implement for Validator #42654
Conversation
| public interface TestInterfaceRestClient { | ||
| @POST | ||
| @Consumes({ "application/json" }) | ||
| Response doSomething(@Valid @ConvertGroup(to = Error.class) @NotNull String myParameter); |
There was a problem hiding this comment.
Should we have an actual test to make sure the constraint are validated? Not necessarily with this example.
There was a problem hiding this comment.
yeah, I'll try adding one, having this interface only tells us that the build will not fail if it is a part of the app 🙈 😃
There was a problem hiding this comment.
OK, I've pushed the test, but this actually won't work 😖
if we normalize the $$CDIWrapper class and end up with an interface -- the bean metadata collection is ignored on the HV side. (somewhat related to this PR hibernate/hibernate-validator#1399) the class hierarchy for the interface is empty, and as a result nothing gets collected in this block
https://github.com/hibernate/hibernate-validator/blob/d2694eced589af7a279e366984e797fc026491e9/engine/src/main/java/org/hibernate/validator/internal/metadata/PredefinedScopeBeanMetaDataManager.java#L88-L103
At this point, it seems that it would be better to do this hibernate/hibernate-validator#1428 on the HV side and keep things as they are in Quarkus or figure out how to break the cycle and skip copying the bean validation annotations to $$CDIWrapper classes.
There was a problem hiding this comment.
Yeah, I don't see an easy way to break the cycle without it being brittle.
So, yeah, let's fix it on the HV side, I suppose.
There was a problem hiding this comment.
ok 🤝 I'll keep this draft open then and once we upgrade HV with #42292 I'll double check that the test in this PR passes...
There was a problem hiding this comment.
The upgrade #42292 should fix it. Moved the test there. I am going to close this PR.
marko-bekhta
force-pushed
the
fix/i35144-group-convert
branch
from
930f6c9
to
60a3b96
Compare
see #35144 (comment)