-
Notifications
You must be signed in to change notification settings - Fork 207
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
Validate plugin configuration objects using JSR-303 validation #826
Validate plugin configuration objects using JSR-303 validation #826
Conversation
…ibernate Validator provides this validation, removing the need for bval. Signed-off-by: David Venable <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #826 +/- ##
============================================
+ Coverage 91.37% 91.61% +0.23%
- Complexity 637 641 +4
============================================
Files 75 76 +1
Lines 1902 1919 +17
Branches 161 162 +1
============================================
+ Hits 1738 1758 +20
+ Misses 124 121 -3
Partials 40 40
Continue to review full report at Codecov.
|
implementation "org.reflections:reflections:0.10.2" | ||
implementation 'io.micrometer:micrometer-core' | ||
implementation 'io.micrometer:micrometer-registry-prometheus' | ||
implementation 'io.micrometer:micrometer-registry-cloudwatch2' | ||
implementation 'software.amazon.awssdk:cloudwatch' | ||
implementation 'org.hibernate.validator:hibernate-validator:7.0.2.Final' | ||
implementation 'org.glassfish:jakarta.el:4.0.1' |
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.
Just curious since we were talking about these dependencies recently. Originally you had thought that the Glassfish dependency was unnecessary because we just needed the built in JSR-303 annotations. What was your finding exactly that showed a need for the glassfish dependency?
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.
Great question. It turns out that the documentation states that we need these dependencies since we are not running in an environment which includes them. Indeed, it won't run without this.
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.PropertyNamingStrategies; | ||
import jakarta.validation.ConstraintViolation; | ||
import jakarta.validation.Validation; |
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.
How is the validation import used ?
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.
Line 33:
final ValidatorFactory validationFactory = Validation.buildDefaultValidatorFactory();
Description
This change validates plugin configuration objects using JSR-303 bean validation. It only supports the
jakarta.validation
package which supersedes thejavax.validation
package.The implementation uses Hibernate Validator which is the primary reference implementation.
Additionally, I added an integration test for the plugin framework to test the components together, including one which validates actual plugin configurations.
Issues Resolved
Resolves #709
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.