-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove request body type param #1154
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,10 @@ | |
use crate::{BoxError, Error}; | ||
use bytes::Bytes; | ||
use bytes::{Buf, BufMut}; | ||
use http_body::Body; | ||
use http_body::Body as _; | ||
|
||
#[doc(no_inline)] | ||
pub use hyper::Body; | ||
|
||
/// A boxed [`Body`] trait object. | ||
/// | ||
|
@@ -55,7 +58,7 @@ where | |
// THE SOFTWARE. | ||
pub(crate) async fn to_bytes<T>(body: T) -> Result<Bytes, T::Error> | ||
where | ||
T: Body, | ||
T: http_body::Body, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: Wo can remove this method and just use the one in hyper. |
||
{ | ||
futures_util::pin_mut!(body); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,9 @@ | |
//! [`axum::extract`]: https://docs.rs/axum/latest/axum/extract/index.html | ||
|
||
use self::rejection::*; | ||
use crate::response::IntoResponse; | ||
use crate::{body::Body, response::IntoResponse, BoxError}; | ||
use async_trait::async_trait; | ||
use bytes::Bytes; | ||
use http::{Extensions, HeaderMap, Method, Request, Uri, Version}; | ||
use std::convert::Infallible; | ||
|
||
|
@@ -60,37 +61,41 @@ mod tuple; | |
/// [`http::Request<B>`]: http::Request | ||
/// [`axum::extract`]: https://docs.rs/axum/latest/axum/extract/index.html | ||
#[async_trait] | ||
pub trait FromRequest<B>: Sized { | ||
pub trait FromRequest: Sized { | ||
/// If the extractor fails it'll use this "rejection" type. A rejection is | ||
/// a kind of error that can be converted into a response. | ||
type Rejection: IntoResponse; | ||
|
||
/// Perform the extraction. | ||
async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection>; | ||
async fn from_request(req: &mut RequestParts) -> Result<Self, Self::Rejection>; | ||
} | ||
|
||
/// The type used with [`FromRequest`] to extract data from requests. | ||
/// | ||
/// Has several convenience methods for getting owned parts of the request. | ||
#[derive(Debug)] | ||
pub struct RequestParts<B> { | ||
pub struct RequestParts { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Err, why is there a |
||
method: Method, | ||
uri: Uri, | ||
version: Version, | ||
headers: HeaderMap, | ||
extensions: Extensions, | ||
body: Option<B>, | ||
body: Option<Body>, | ||
} | ||
|
||
impl<B> RequestParts<B> { | ||
impl RequestParts { | ||
/// Create a new `RequestParts`. | ||
/// | ||
/// You generally shouldn't need to construct this type yourself, unless | ||
/// using extractors outside of axum for example to implement a | ||
/// [`tower::Service`]. | ||
/// | ||
/// [`tower::Service`]: https://docs.rs/tower/lastest/tower/trait.Service.html | ||
pub fn new(req: Request<B>) -> Self { | ||
pub fn new<B>(req: Request<B>) -> Self | ||
where | ||
B: http_body::Body<Data = Bytes> + Send + 'static, | ||
B::Error: Into<BoxError>, | ||
{ | ||
let ( | ||
http::request::Parts { | ||
method, | ||
|
@@ -109,7 +114,7 @@ impl<B> RequestParts<B> { | |
version, | ||
headers, | ||
extensions, | ||
body: Some(body), | ||
body: Some(Body::wrap_body(body)), | ||
} | ||
} | ||
|
||
|
@@ -141,7 +146,10 @@ impl<B> RequestParts<B> { | |
/// } | ||
/// } | ||
/// ``` | ||
pub async fn extract<E: FromRequest<B>>(&mut self) -> Result<E, E::Rejection> { | ||
pub async fn extract<E>(&mut self) -> Result<E, E::Rejection> | ||
where | ||
E: FromRequest, | ||
{ | ||
E::from_request(self).await | ||
} | ||
|
||
|
@@ -151,7 +159,7 @@ impl<B> RequestParts<B> { | |
/// been called. | ||
/// | ||
/// [`take_body`]: RequestParts::take_body | ||
pub fn try_into_request(self) -> Result<Request<B>, BodyAlreadyExtracted> { | ||
pub fn try_into_request(self) -> Result<Request<Body>, BodyAlreadyExtracted> { | ||
let Self { | ||
method, | ||
uri, | ||
|
@@ -229,46 +237,44 @@ impl<B> RequestParts<B> { | |
/// Gets a reference to the request body. | ||
/// | ||
/// Returns `None` if the body has been taken by another extractor. | ||
pub fn body(&self) -> Option<&B> { | ||
pub fn body(&self) -> Option<&Body> { | ||
self.body.as_ref() | ||
} | ||
|
||
/// Gets a mutable reference to the request body. | ||
/// | ||
/// Returns `None` if the body has been taken by another extractor. | ||
// this returns `&mut Option<B>` rather than `Option<&mut B>` such that users can use it to set the body. | ||
pub fn body_mut(&mut self) -> &mut Option<B> { | ||
pub fn body_mut(&mut self) -> &mut Option<Body> { | ||
&mut self.body | ||
} | ||
|
||
/// Takes the body out of the request, leaving a `None` in its place. | ||
pub fn take_body(&mut self) -> Option<B> { | ||
pub fn take_body(&mut self) -> Option<Body> { | ||
self.body.take() | ||
} | ||
} | ||
|
||
#[async_trait] | ||
impl<T, B> FromRequest<B> for Option<T> | ||
impl<T> FromRequest for Option<T> | ||
where | ||
T: FromRequest<B>, | ||
B: Send, | ||
T: FromRequest, | ||
{ | ||
type Rejection = Infallible; | ||
|
||
async fn from_request(req: &mut RequestParts<B>) -> Result<Option<T>, Self::Rejection> { | ||
async fn from_request(req: &mut RequestParts) -> Result<Option<T>, Self::Rejection> { | ||
Ok(T::from_request(req).await.ok()) | ||
} | ||
} | ||
|
||
#[async_trait] | ||
impl<T, B> FromRequest<B> for Result<T, T::Rejection> | ||
impl<T> FromRequest for Result<T, T::Rejection> | ||
where | ||
T: FromRequest<B>, | ||
B: Send, | ||
T: FromRequest, | ||
{ | ||
type Rejection = Infallible; | ||
|
||
async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> { | ||
async fn from_request(req: &mut RequestParts) -> Result<Self, Self::Rejection> { | ||
Ok(T::from_request(req).await) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Note this makes hyper a public dependency of axum-core. I think thats a little unfortunate but its required to make
impl FromRequest for Request<hyper::Body>
work. I think thats an okay trade-off.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.
It also does mean that any crate that depends on axum-core now also depends on hyper. Thats pretty unfortunate 🤔 But yeah not sure there is a way around it.
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.
One solution would be to keep the type param on
FromRequest
andRequestParts
but that would mean they get three type params in 0.6 😬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.
Perhaps its also worth considering adding our own body type to
axum-core
, to avoid the dependency on hyper. I believe hyper'sBody
will be moved into hyper-utils for 1.0 and we don't wanna have a public dependency on hyper-utils. So we'll probably need our own body type eventually.Though we have to consider compatibility with things like reqwest. I probably should be possible to extract the body with axum for send it somewhere else with reqwest or a
hyper::Client
.