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

hibernate-validator: better support for custom ConstraintValidatorFactory #3596

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

jknack
Copy link
Member

@jknack jknack commented Dec 8, 2024

  • simplify custom ConstraintValidatorFactory based on DI as well as manually created factories
  • integrates Validator factory into hibernate

@jknack
Copy link
Member Author

jknack commented Dec 8, 2024

@kliushnichenko please check (ignore the Jetty change)

@jknack jknack added this to the 3.5.5 milestone Dec 8, 2024
Copy link
Contributor

@kliushnichenko kliushnichenko left a comment

Choose a reason for hiding this comment

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

Cool! Added my 2 cents to it

@@ -281,72 +291,25 @@ As you know, `Hibernate Validator` allows you to build fully custom `ConstraintV
In some scenarios, you may need access not only to the bean but also to services, repositories, or other resources
to perform more complex validations required by business rules.

In this case you need to implement a custom `ConstraintValidatorFactory` that will rely on your DI framework
In this case you need to implement a custom `ConstraintValidator` that will rely on your DI framework
instantiating your custom `ConstraintValidator`

1) Implement custom `ConstraintValidatorFactory`:
Copy link
Contributor

Choose a reason for hiding this comment

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

ConstraintValidatorFactory, guess we can delete the whole line, it's one step only now

3) Implement your custom `ConstraintValidator`

[source, java]
----
public class MyCustomValidator implements ConstraintValidator<MyCustomAnnotation, Bean> {
Copy link
Contributor

Choose a reason for hiding this comment

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

for avaje it's important to add @Singleton annotation, otherwise, the instantiation will fail, and the error message is not clear - it hides the real reason related to DI.

@@ -58,6 +59,7 @@ public class HibernateValidatorModule implements Extension {
private String title = "Validation failed";
private boolean disableDefaultViolationHandler = false;
private boolean logException = false;
private List<ConstraintValidatorFactory> factories;
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, do you have real use cases in mind where you need multiple factories?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't need it if we rely on DI. So this is for people who doesn't like DI (reflective or not), so they can just do:

 getInstance(Class type) {
    if (type == Validator1) {..}
    else if (type== Validator2) {...}
    else return null
  }

@Override
public <T extends ConstraintValidator<?, ?>> T getInstance(Class<T> key) {
try {
return registry.require(key);
Copy link
Contributor

Choose a reason for hiding this comment

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

should work for Avaje/Guice, but not for Dagger. A note in docs that, for the Dagger, the factory needs to be implemented manually, would be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if possible to implement a registry for Dagger, will add a note.

…tory

- simplify custom ConstraintValidatorFactory based on DI as well as manually created factories
- integrates Validator factory into hibernate

- fix #3595
@jknack jknack merged commit 13e1c7c into 3.x Dec 9, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants