feat: case-insensitive field matching for JSON/CBOR #629
Conversation
WalkthroughThe changes introduce a new global variable Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
validate.go (1)
38-43: Consider thread-safety implications of the global variable.While the documentation clearly explains the purpose and default behavior, using a global variable for validation configuration could lead to race conditions in concurrent scenarios. Consider one of these alternatives:
- Make it a field in the
ValidateResultstruct- Add it as a parameter to the
Validatefunctionvalidate_test.go (1)
923-950: Consider grouping case sensitivity test cases.While the test cases comprehensively cover the case-insensitive functionality, consider grouping them together using a descriptive prefix like
TestCaseSensitivity_for better organization and readability.Apply this diff to improve test organization:
- name: "case-insensive success", + name: "TestCaseSensitivity_InsensitiveSuccess", typ: reflect.TypeOf(struct { Value string `json:"value"` }{}), input: map[string]any{"VaLuE": "works"}, }, { - name: "case-insensive fail", + name: "TestCaseSensitivity_InsensitiveFail", typ: reflect.TypeOf(struct { Value string `json:"value" maxLength:"3"` }{}), input: map[string]any{"VaLuE": "fails"}, errs: []string{"expected length <= 3"}, }, { - name: "case-sensive fail", + name: "TestCaseSensitivity_StrictFail", before: func() { huma.ValidateStrictCasing = true }, cleanup: func() { huma.ValidateStrictCasing = false },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- validate.go (4 hunks)
- validate_test.go (4 hunks)
🔇 Additional comments (4)
validate.go (2)
Line range hint
619-662: LGTM! Clean implementation of case-insensitive matching.The implementation correctly:
- Preserves the original casing from the input
- Uses
strings.EqualFoldfor proper case-insensitive comparison- Maintains validation of required fields and dependencies
667-679: LGTM! Well-structured additional properties validation.The implementation:
- Correctly handles case-insensitive matching for additional properties
- Uses a clean labeled break pattern for control flow
- Maintains clear error messaging
validate_test.go (2)
31-39: LGTM: Well-structured test case setup.The addition of
beforeandcleanupfields follows good testing practices by providing clear setup and teardown capabilities for each test case.
1401-1407: LGTM: Proper test setup and cleanup handling.The implementation correctly handles test setup and cleanup, using defer for cleanup functions to ensure they run even if the test panics.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #629 +/- ##
==========================================
+ Coverage 92.88% 92.90% +0.02%
==========================================
Files 22 22
Lines 4834 4848 +14
==========================================
+ Hits 4490 4504 +14
Misses 299 299
Partials 45 45 ☔ View full report in Codecov by Sentry. |
|
Thanks for the change. |
This PR updates the field validation to use case-insensitive matches when validating input objects. This is done because:
Fixes #577.
Summary by CodeRabbit
New Features
Bug Fixes
Tests