14110: add validation service #14388
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
Integration Test Results 62 files 62 suites 33m 35s ⏱️ Results for commit 70d7c6b. ♻️ This comment has been updated with latest results. |
| ConfigurationValueValidationServiceImpl(), | ||
| ) : ConfigurationValidationService { | ||
|
|
||
| override fun <T> validateYAML(configType: ConfigurationType<T>, file: File): ConfigurationValidationResult<T> { |
There was a problem hiding this comment.
Might answer this question myself by the time I'm done looking at the code: Why do we need to overload the validateYAML to take in different input types (same goes for the other files with similar method overloading)? If we're controlling the validation point shouldn't we just need one?
If we do need them, it would be good to have some unit tests for them too.
There was a problem hiding this comment.
I overloaded it because I wanted to use some simple strings in the tests.
I also wanted to use some shared logic between both files and strings and found that input streams would let me handle both the same way.
| /** | ||
| * A configration type containing all necessary data to parse and validate a YAML file | ||
| */ | ||
| sealed class ConfigurationType<T> { |
There was a problem hiding this comment.
First time I've come across a "sealed" class. I read this but if you have a second to huddle I'd be interested in hearing more about it.
There was a problem hiding this comment.
sealed classes and interfaces are great when you want to know all your implementing/subclasses at runtime. Its useful in this case because I want to define the possible configuration types and include a generic type, vals, and functions along with each configuration type. I'm using it in a very "enum" way.
| ConfigurationValidationFailure(schemaErrors) | ||
| } | ||
| } catch (ex: Exception) { | ||
| ConfigurationValidationFailure(schemaErrors, ex) |
There was a problem hiding this comment.
When these ConfigurationValidationFailure data classes are being created, is anything happening to them? Or is all the logging/output going to happen in a later PR?
There was a problem hiding this comment.
Right now this isn't hooked up to anything (besides unit tests). I'll be using it as a part of 14158 (CLI task).
| val jsonSchemaValidation = jsonSchemaValidationService.validateYAMLStructure(configType, inputStream) | ||
| ) { | ||
| is ConfigurationValidationSuccess -> { | ||
| configurationValueValidationService.validate(configType, jsonSchemaValidation.parsed) |
There was a problem hiding this comment.
Just want to make sure I understand the workflow happening here.
First the ConfigurationValidationService is called.
This calls the JsonSchemaValidationService, which validates the structure of the YAML file?
And if that passes it then calls the ConfigurationValueValidationService, which validates that the filters are valid?
There was a problem hiding this comment.
Yup thats correct
- validate against json schema
- parse with jackson into class
- validate with konform
- return success/failure
There was a problem hiding this comment.
side note: if step 1 fails it will short circuit to a failure (since we can't validate values if we cant even parse something into a type)
| } | ||
|
|
||
| @Test | ||
| fun organizations() { |
There was a problem hiding this comment.
Love the idea of testing this in the unit tests
|
Forgot to add that I did pull the branch down and tests passed locally for me 👍 |
|
This PR adds a validation service that handles both json schema structure validation as well as value validation for filter definitions.
Test Steps:
Changes
Checklist
Testing
./prime testor./gradlew testSmokeagainst local Docker ReportStream container?Linked Issues