feat: allow scalar pointers with defaults

danielgtaylor commented 3 weeks ago

This PR is an attempt to fix a long-standing issue where defaults would override explicitly sent zero values from a client if the zero value has semantic meaning for an operation handler.

It implements a new feature that allows the use of scalar pointers with defaults to match the existing behavior of the standard library JSON and other marshaling packages like https://github.com/fxamacker/cbor, which is to use a scalar pointer to determine whether a value has been set by a client or not. You can now do something like this:

type MyInput struct {
    Body struct {
        Enabled *bool `json:"enabled,omitempty" default:"true"`
    }
}

The behavior is:

Client sends	Handler sees
`true`	`true`
`false`	`false`
`null` / `undefined`	`true`

This implementation does not require any additional marshal/unmarshal steps or impact the performance of the running service. It does require some additional reflection & conversions on service startup, but that's a one-time cost and the additional code complexity is not too bad.

Just to note, you still cannot provide defaults for pointers to structs. It would be possible to support this but requires different unmarshaling code in jsonTagValue so I'm not going to implement it unless there is a good reason to do so.

FYI @Grumpfy, @robsonpeixoto, @barata0 - what do you think?

Fixes #597, #627, #628

Summary by CodeRabbit

Documentation
- Expanded and clarified request validation documentation, including details on struct tags, optional/required fields, nullable fields, and validation tags.
- Added sections on defaults, read/write-only fields, strict vs. loose validation, and advanced validation rules.
Bug Fixes
- Improved error handling and parameter processing in the API framework, enhancing clarity and robustness.
Tests
- Enhanced middleware and request handling tests, including scenarios for cookie handling and request bodies with pointers.
- Added tests for various error conditions and improved OpenAPI documentation generation.

coderabbitai[bot] commented 3 weeks ago

Walkthrough

The pull request introduces significant updates to the documentation and implementation of request validation within the API framework. Key changes include expanded explanations of Go struct tags for JSON Schema generation, detailed handling of optional and required fields, and new sections addressing nullable fields, defaults, and strict versus loose validation. Additionally, the codebase sees improvements in error handling and parameter processing, particularly in the huma.go file. Enhanced test cases in huma_test.go ensure robust validation and error management across various scenarios.

Changes

File	Change Summary
docs/docs/features/request-validation.md	Expanded documentation on request validation, including sections on defaults, read/write-only fields, strict vs. loose validation, and advanced validation.
huma.go	Refined error handling in `findDefaults`, `every`, and `transformAndWrite` functions; improved parameter processing in `Register`.
huma_test.go	Added logging for panic messages, expanded test cases for middleware and request handling, and improved OpenAPI documentation generation.
schema.go	Enhanced `convertType` function for better handling of type conversion for slices, especially with pointers and interfaces.

Assessment against linked issues

Objective	Addressed	Explanation
Improved handling of default values for input fields (#597)	✅
Enhanced middleware functionality for cookie handling (#597)	✅
Restored input values that were incorrectly overridden (#597)	❌	No specific restoration of overridden input values.
Expanded test coverage for middleware and request parameters (#597)	✅

Possibly related PRs

#608: Enhances documentation for request validation, particularly on nullable fields, aligning with the main PR's focus.
#609: Adds notes on the behavior of write-only fields, relevant to updates on request validation documentation.
#612: Introduces a configurable option for nullable arrays, relating to documentation updates on nullable fields.

Poem

In the land of code where rabbits play,
We've spruced up docs in a grand way!
With tags and fields, both strict and loose,
Our validation's now a mighty moose!
So hop along, dear coder friends,
With clearer paths, our joy transcends! 🐇✨

📜 Recent review details

**Configuration used: CodeRabbit UI** **Review profile: CHILL**

📥 Commits

Reviewing files that changed from the base of the PR and between 829f7202b38c2545840d9e7206f4de1f135c82b4 and 9f1ee235e7df2aeb6bc46d37f382a03b893e3058.

📒 Files selected for processing (1)

* `huma_test.go` (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

* huma_test.go

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?

❤️ Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

🪧 Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit , please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` 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) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 72.88136% with 16 lines in your changes missing coverage. Please review.

Project coverage is 92.70%. Comparing base (d67ab01) to head (9f1ee23). Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
huma.go	66.66%	10 Missing and 4 partials :warning:
schema.go	88.23%	1 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #633 +/- ## ========================================== - Coverage 92.90% 92.70% -0.21% ========================================== Files 22 22 Lines 4848 4879 +31 ========================================== + Hits 4504 4523 +19 - Misses 299 308 +9 - Partials 45 48 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

barata0 commented 3 weeks ago

@danielgtaylor Do you think omitting a field and explcity assigning a null value should have different ouputs?
eg. Body: `{"items": [{"id": 1, verified: null}]}`, and Body: `{"items": [{"id": 1}]}`, ?

danielgtaylor commented 3 weeks ago

Do you think omitting a field and explcity assigning a null value should have different ouputs?

@barata0 since Go's JSON marshaler and the CBOR marshaler we use don't differentiate between those I don't think it it makes sense at the parsing layer. Sending null for something with a default will be just like explicitly saying you want the default. I think this is just a limitation of Go we have to deal with, though I'm open to ideas!

Edit: you can always not add the default tag and use custom unmarshaling on your struct (and provide a custom schema) and then you can handle it however you like, for example https://github.com/danielgtaylor/huma/blob/main/examples/omit/main.go#L41.

barata0 commented 3 weeks ago

I agree. Cool. I don´t think I can technically review your PR because I'm new in GO and u r using a lot of reflection (I didn't get there yet) But it looks ok to me!

danielgtaylor / huma