Skip to content

Conversation

marko-bekhta
Copy link
Contributor

@quarkus-bot quarkus-bot bot added the area/hibernate-validator Hibernate Validator label Aug 20, 2024
public interface TestInterfaceRestClient {
@POST
@Consumes({ "application/json" })
Response doSomething(@Valid @ConvertGroup(to = Error.class) @NotNull String myParameter);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have an actual test to make sure the constraint are validated? Not necessarily with this example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙈 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upgrade #42292 should fix it. Moved the test there. I am going to close this PR.

@marko-bekhta marko-bekhta force-pushed the fix/i35144-group-convert branch from 930f6c9 to 60a3b96 Compare August 21, 2024 08:45
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/hibernate-validator Hibernate Validator triage/invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants