-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
#533: Add Validation for Discriminator with OneOf and Mapping. #536
#533: Add Validation for Discriminator with OneOf and Mapping. #536
Conversation
WalkthroughThe changes enhance the schema validation framework by introducing a new Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- validate.go (3 hunks)
- validate_test.go (3 hunks)
- validation/messages.go (1 hunks)
Additional comments not posted (15)
validation/messages.go (3)
20-20
: Renamed constant is appropriate.The new name
MsgExpectedMatchAtLeastOneSchema
accurately reflects its purpose and is consistent with the other message constants.
21-21
: New constant is appropriate.The new constant
MsgExpectedMatchExactlyOneSchema
is correctly defined and appropriately named.
23-23
: New constant is appropriate.The new constant
MsgExpectedPropertyNameInObject
is correctly defined and appropriately named.validate.go (6)
278-278
: Updated error message is appropriate.The new message constant
validation.MsgExpectedMatchExactlyOneSchema
is correctly used.
294-294
: Updated error message is appropriate.The new message constant
validation.MsgExpectedMatchAtLeastOneSchema
is correctly used.
298-324
: New functionvalidateDiscriminator
is well-implemented.The function correctly handles the validation logic for discriminators, ensuring the presence and type of the discriminator property, and validating against the appropriate schema based on the discriminator mapping.
349-353
: Integration ofvalidateDiscriminator
is appropriate.The
Validate
function correctly callsvalidateDiscriminator
if a discriminator is present, ensuring the new validation logic is utilized.
350-350
: Integration ofvalidateDiscriminator
is appropriate.The
Validate
function correctly callsvalidateDiscriminator
if a discriminator is present, ensuring the new validation logic is utilized.
352-352
: Integration ofvalidateOneOf
is appropriate.The
Validate
function correctly callsvalidateOneOf
if no discriminator is present, ensuring the existing validation logic is utilized.validate_test.go (6)
12-12
: Addition ofrequire
package is appropriate.The
require
package is correctly imported for assertions.
1259-1323
: New test cases for schemas with discriminators are well-implemented.The test cases correctly validate the behavior of schemas with discriminators, ensuring the new validation logic is thoroughly tested.
1460-1463
: New typeCat
is appropriately defined.The
Cat
type is correctly defined with validation rules for thename
andkind
properties.
1465-1468
: New typeDog
is appropriately defined.The
Dog
type is correctly defined with validation rules for thecolor
andkind
properties.
1470-1562
: New test functionTest_validateWithDiscriminator
is well-implemented.The test function correctly tests the validation logic for schemas with discriminators, ensuring the new validation logic is thoroughly validated.
1470-1470
: New test functionTest_validateWithDiscriminator
is well-implemented.The test function correctly tests the validation logic for schemas with discriminators, ensuring the new validation logic is thoroughly validated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #536 +/- ##
==========================================
+ Coverage 92.69% 92.74% +0.05%
==========================================
Files 22 22
Lines 3845 3872 +27
==========================================
+ Hits 3564 3591 +27
Misses 236 236
Partials 45 45 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- validate.go (3 hunks)
- validate_test.go (3 hunks)
Additional comments not posted (7)
validate.go (3)
278-278
: Improved error message clarity.The updated error message in
validateOneOf
enhances clarity, aligning with the goal of providing more precise validation feedback.
294-294
: Improved error message clarity.The updated error message in
validateAnyOf
enhances clarity, aligning with the goal of providing more precise validation feedback.
298-335
: New functionvalidateDiscriminator
added.The function effectively handles validation with discriminators, improving schema validation logic. The integration with existing validation functions appears seamless.
Ensure that this function is invoked correctly in all relevant scenarios.
validate_test.go (4)
12-12
: Addition ofrequire
package for assertions.The use of the
require
package improves the reliability of test assertions, ensuring that tests fail immediately on errors.
Line range hint
477-485
: New structsCat
andDog
added for testing.The addition of these structs with specific validation rules enhances the test coverage for discriminator functionality.
1259-1340
: Comprehensive test cases for discriminator validation.The new test cases effectively cover a range of scenarios for schemas with discriminators, ensuring robust validation logic.
1477-1597
: Comprehensive test suite forvalidateWithDiscriminator
.The test suite covers various scenarios, ensuring that the discriminator logic is thoroughly validated. The use of
require
for assertions strengthens the test reliability.
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.
@superstas nice work! I hope you don't mind I added a small commit to support map[any]any
instead of just map[string]any
because some formats (CBOR/YAML/MsgPack/Protobuf/etc) support keys of any
type and since Huma supports custom formats with client-driven content negotiation we should support them, but otherwise this looks great!
@danielgtaylor sure, thank you for the addition and the quick merge. |
It fixes #533.
@danielgtaylor, since it quite extends the current validation logic, I'm open to your feedback and requests for changes.
Thank you.
Summary by CodeRabbit
New Features
Cat
andDog
) for improved validation scenarios.Bug Fixes
Tests