-
Notifications
You must be signed in to change notification settings - Fork 967
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
Add multipart/form-data response builders to axum-extra #2654
base: main
Are you sure you want to change the base?
Conversation
I may need some help fixing those CI errors, I am unsure how to address them |
Ok, I managed to fix all of those CI errors, thank you for your patience |
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.
Thanks, for the PR I think the functionality looks good. I have not personally worked with multipart on this level so I don't know much of the technical details but I did comment on some of the other stuff.
axum-extra/src/multipart_builder.rs
Outdated
/// .part(Part::text("other_field_name", "def")) | ||
/// .part(Part::file("file", "file.txt", vec![0x68, 0x68, 0x20, 0x6d, 0x6f, 0x6d])); | ||
/// ``` | ||
pub fn part(&mut self, part: Part) -> &mut Self { |
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.
I think this should be named so it's clearer what it does, e.g. add_part
or push_part
.
Personally, I'm not also sure this API is that useful,wouldn't it be better to just have something like parts_mut
which would return a mutable reference to the parts vector and then the user can just push there directly, but also remove stuff, clear it, or any other operation they might need to take.
On the other hand they can just build the vector beforehand and then call from_parts
just before returning the response.
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.
You're right, it did feel kind of odd, I was trying to create something similar to Reqwest's implementation (.part()
) chained calls, but with .with_parts()
it feels redundant, so I removed it. I can add parts_mut
if you feel it would prudent.
@@ -83,6 +83,9 @@ pub mod routing; | |||
#[cfg(feature = "json-lines")] | |||
pub mod json_lines; | |||
|
|||
#[cfg(feature = "multipart")] | |||
pub mod multipart_builder; |
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.
Why "builder"?
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.
There's already a Multipart
extractor, so I wanted to clarify between that and the functionality for creating forms.
I have commit ready to fix some of those minor issues, I just need to figure out how to handle mime type stuff from a public api level |
Closes #2624 (mine)
This pull request provides builders that enable
multipart/form-data
forms to be returned from handlers.Motivation
multipart/form-data
forms provide a way to include multiple types of data in a single response as key value pairs. You could include an image and a JSON, or any other combination of data.As an example, this could be used for a pastebin, so that a single request to an axum backend could return metadata about a paste in a json, and the paste itself. The paste wouldn't need to conform to UTF-8, and could be sent separately from the json.
Solution
I added a
MultipartForm
struct that implementsIntoResponse
, and implementers can add parts to that form by definingPart
s. I did not implementIntoResponse
on theMultipart
extractor, as advised in the original issue.One dependency was added if the
multipart
feature is enabled.fastrand
is used to generate boundaries, mirroring Reqwest's implementation.Thank you for your time :)