fix: recompute property names after schema transform #611
Conversation
WalkthroughThe changes made in this pull request enhance the testing and schema handling capabilities of the Huma API framework. Updates include the addition of new test cases in Changes
Assessment against linked issues
Possibly related PRs
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #611 +/- ##
=======================================
Coverage 92.83% 92.83%
=======================================
Files 22 22
Lines 3923 3923
=======================================
Hits 3642 3642
Misses 236 236
Partials 45 45 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- huma_test.go (1 hunks)
- schema.go (2 hunks)
- validate_test.go (2 hunks)
🧰 Additional context used
🔇 Additional comments (4)
schema.go (3)
293-301: Improved initialization and sorting of schema propertiesThe changes in the
PrecomputeMessagesmethod enhance the clarity and efficiency of the schema handling logic:
propertyNamesis now initialized with a known capacity, which is more memory-efficient.- Sorting
propertyNamesensures a consistent order, beneficial for testing and deterministic behavior.requiredMapis now initialized directly, simplifying the code.These improvements contribute to better performance and maintainability of the schema handling process.
672-675: Enhanced schema customization with SchemaTransformerThe addition of the
SchemaTransformerinterface support inSchemaFromTypeis a valuable improvement:
- It allows types to implement custom schema transformations, providing greater flexibility in schema generation.
- The
TransformSchemamethod enables fine-grained control over the generated schema.- Recomputing error messages after transformation ensures consistency between the modified schema and its validation messages.
This feature enhances the extensibility of the schema generation process while maintaining the integrity of the validation system.
293-301: Summary: Effective improvements in schema handling and transformationThe changes in
schema.gosuccessfully address the objectives outlined in the PR:
- The
PrecomputeMessagesmethod now ensures consistent and efficient initialization ofpropertyNamesandrequiredMap.- The introduction of
SchemaTransformersupport inSchemaFromTypeallows for flexible schema customization.- Recomputing messages after schema transformation maintains the integrity of the validation system.
These improvements effectively resolve the inconsistencies mentioned in issue #607 by ensuring that schema transformations are properly handled and reflected in the final schema representation. The changes enhance the overall reliability and flexibility of the Huma framework's schema handling capabilities.
Also applies to: 672-675
huma_test.go (1)
1957-1960: LGTM!The added error message assertions accurately reflect the expected length validation errors in the test case. This enhances the comprehensiveness of the test coverage.
| func TestValidateSchemaTransformerDeleteField(t *testing.T) { | ||
| registry := huma.NewMapRegistry("#/components/schemas/", huma.DefaultSchemaNamer) | ||
| s := registry.Schema(reflect.TypeOf(&TransformDeleteField{}), true, "TestInput") | ||
| pb := huma.NewPathBuffer([]byte(""), 0) | ||
| res := &huma.ValidateResult{} | ||
|
|
||
| huma.Validate(registry, s, pb, huma.ModeReadFromServer, map[string]any{"field1": "value1"}, res) | ||
| // We should have no errors and no panics. | ||
| assert.Empty(t, res.Errors) | ||
| } |
There was a problem hiding this comment.
Ensure the schema transformation is correctly validated
In the TestValidateSchemaTransformerDeleteField test function, consider adding assertions to verify that field2 has been removed from the schema. This will confirm that the schema transformation behaves as expected and that the validation process is accurate.
You can add an assertion like:
assert.NotContains(t, s.Properties, "field2")There was a problem hiding this comment.
@danielgtaylor You're welcome! Glad it was helpful!
((_/)
( •_•)
❤️
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
superstas
left a comment
There was a problem hiding this comment.
@danielgtaylor Looks good to me, thanks!
This PR makes a few small but important changes:
schema.PrecomputeMessages()after a call to ahuma.SchemaTransformeras it may have modified the schema.PrecomputeMessages()recreate the property names, rather than only setting them if they don't already exist.Fixes #607.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation