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

fix: don’t leakage Micronaut Validation #1267

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Conversation

sdelamo
Copy link
Contributor

@sdelamo sdelamo commented Feb 12, 2024

Close: #1266

@sdelamo sdelamo added the type: bug Something isn't working label Feb 12, 2024
@sdelamo sdelamo requested a review from radovanradic February 12, 2024 10:36
@@ -7,7 +7,7 @@ dependencies {

api(libs.managed.hibernate.core)
api(libs.managed.jakarta.transaction.api)
api(mnValidation.micronaut.validation)
implementation(mnValidation.validation) // jakarta.validation:jakarta.validation-api
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to include the validation anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will fix the issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dstepanov We need the dependency to jakarta.validation:jakarta.validation-api because of ValidatorFactorySettingSupplier.

@graemerocher

I don't think this will fix the issue

In this PR for hibernate-jpabuild I included:

testImplementation(mnHibernateValidator.micronaut.hibernate.validator)

It seems to get rid of the Multiple possible bean candidates found: [DefaultInternalConstraintValidatorFactory, DefaultConstraintValidatorFactory] error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Put it compile only

Copy link
Contributor Author

@sdelamo sdelamo Feb 12, 2024

Choose a reason for hiding this comment

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

@dstepanov

Put it compile only

done 09609a2

@sdelamo sdelamo marked this pull request as ready for review February 12, 2024 11:46
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@graemerocher
Copy link
Contributor

Regarding this, it is a patch/workaround to the actual issue. IMO the Hibernate Validator module should be updated to allow it to live side by side with the Micronaut validation module

@dstepanov
Copy link
Contributor

The changes in Micronaut Serialization actually broke it, but I think the combination of two shouldn’t be allowed anyway

@sdelamo sdelamo merged commit 2dcf574 into master Feb 13, 2024
39 checks passed
@sdelamo sdelamo deleted the validation-leakage branch February 13, 2024 05:41
@radovanradic
Copy link
Contributor

radovanradic commented Feb 26, 2024

@sdelamo Is this going to be reverted maybe after this PR micronaut-projects/micronaut-hibernate-validator#370 ?
Asking because it seems it caused issue in micronaut-data micronaut-projects/micronaut-data#2808
Validation is removed and then Hibernate behaves differently, it validates nullability before listeners are fired and created date populated. It was talked about nullability validation here https://stackoverflow.com/questions/66524227/columnnullable-false-validating-the-null-check-at-application-level
Cc @dstepanov

@dstepanov
Copy link
Contributor

We might need to make remove that second bean from Micronaut Validation, like it was before

@sdelamo
Copy link
Contributor Author

sdelamo commented Feb 28, 2024

We might need to make remove that second bean from Micronaut Validation, like it was before

@dstepanov I don't follow what you suggest.

@dstepanov
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
No open projects
Status: Done
4 participants