-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Hibernate Validator - Allow customization of the ValidatorFactory #26658
Conversation
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very interesting!
I think we could even make the mechanism more general given you can do whatever you want with the configuration and we could follow the pattern used for ObjectMapperCustomizer
.
In this pattern, we only consider the ObjectMapperCustomizer
that have been made CDI beans by the user so we should probably do the very same here (see comment inline about AdditionalBeanBuildItem
).
How we handle the priority is an open subject, maybe don't focus on that for now but you can adjust the rest IMO.
...ent/src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java
Outdated
Show resolved
Hide resolved
...ent/src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java
Show resolved
Hide resolved
.../runtime/src/main/java/io/quarkus/hibernate/validator/runtime/ConstraintMappingProvider.java
Outdated
Show resolved
Hide resolved
.../runtime/src/main/java/io/quarkus/hibernate/validator/runtime/ConstraintMappingProvider.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/io/quarkus/hibernate/validator/runtime/HibernateValidatorRecorder.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this is mostly good now. I added a few suggestions for the Javadoc and the interface needs to be in the API package.
Once this is done, it's good to go!
...rc/main/java/io/quarkus/hibernate/validator/runtime/HibernateValidatorFactoryCustomizer.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/quarkus/hibernate/validator/runtime/HibernateValidatorFactoryCustomizer.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/quarkus/hibernate/validator/runtime/HibernateValidatorFactoryCustomizer.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, very nice to have this in.
Signed-off-by: Theodor Mihalache <[email protected]>
I rebased and squashed the commits. I will merge as soon as CI is green. |
*/ | ||
public interface HibernateValidatorFactoryCustomizer { | ||
|
||
<T extends BaseHibernateValidatorConfiguration<T>> void customize(T configuration); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit late to the party, but... These generics are not useful at all, as far as I can see you could just do this.
@gsmet Should I send a PR?
<T extends BaseHibernateValidatorConfiguration<T>> void customize(T configuration); | |
void customize(BaseHibernateValidatorConfiguration<?> configuration); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sure.
Also, maybe we should add some documentation for this feature? Or is the javadoc considered enough? |
See quarkusio#26658 (comment) (cherry picked from commit c6d89f5)
Fixes #23600
Added ability to add constraint definitions programmatically
Signed-off-by: Theodor Mihalache [email protected]