Skip to content
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

Multipart Form RawBody Type Support #324

Merged
merged 6 commits into from
Mar 20, 2024

Conversation

jamelt
Copy link
Contributor

@jamelt jamelt commented Mar 19, 2024

I'm not entirely sure if this is the direction you want to go in. But this makes adding simple upload endpoints more approachable.

Copy link
Owner

@danielgtaylor danielgtaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay as I wasn't 100% sure what to do about multipart forms and how best to handle them. A couple thoughts:

  1. This PR is great, thank you! I think allowing raw access to the form as a convenience is a good idea and will merge it in.
  2. I think in the future it would be great to have a huma.Format that can unmarshal multipart forms into user-defined structs just like you unmarshal JSON, but that's for another PR. For now raw access to the form is a step in the right direction.

Thanks! 🚀

@danielgtaylor danielgtaylor linked an issue Mar 20, 2024 that may be closed by this pull request
Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 87.12871% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 95.28%. Comparing base (c312351) to head (fbac026).
Report is 4 commits behind head on main.

Files Patch % Lines
huma.go 87.12% 11 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
+ Coverage   95.14%   95.28%   +0.14%     
==========================================
  Files          19       19              
  Lines        2782     2823      +41     
==========================================
+ Hits         2647     2690      +43     
+ Misses         98       97       -1     
+ Partials       37       36       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielgtaylor danielgtaylor merged commit 53aac0f into danielgtaylor:main Mar 20, 2024
2 of 3 checks passed
@jamelt
Copy link
Contributor Author

jamelt commented Mar 21, 2024

@danielgtaylor Hey, no worries. I know that life can get busy. I just wanted to put something in there. I thought I saw you put a comment in a ticket that you were open to folks contributing multipart form support.

So, I tried to stick it in there while fitting nicely into the design spirit of what you had there.

But your suggestion for number two is fantastic. I had that thought as well. But considering my requirement was uploading files, it seemed like quite a bit of a stretch to try to support a bunch of generic parameters and unmarshaling those into JSON.

Also, the structure of the possibilities of the multipart form's key values seems quite flexible to the extent that you might have to make some choices on how the JSON mapping will work.

But it would help because you could then get your open API spec to document what properties are available in these multipart form endpoints.

Thanks again!

@JanRuettinger
Copy link

With this PR the docs now include the following example:

type MyInput struct {
	ID      string `path:"id"`
	Detail  bool   `query:"detail" doc:"Show full details"`
	Auth    string `header:"Authorization"`
	Body    MyBody
	RawBody []byte
}

That works. However, ideally the RawBody can also be of the type multipart.Form. If I change the input to the following:

type MyInput struct {
	Auth    string `header:"Authorization"`
	Body    MyBody
	RawBody multipart.Form
}

I get the warning: WARN http: superfluous response.WriteHeader call from github.com/danielgtaylor/huma/v2/adapters/humachi.(*chiContext).SetStatus (humachi.go:86)

Is there a way to use a multipart.Form RawBody and a structured Body at the same time?

@minpeter
Copy link

I am in exactly the same situation.
i need help..

@danielgtaylor
Copy link
Owner

@JanRuettinger @minpeter good catch. These should be fixed by #329.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom resolver for multipart form file upload
4 participants