From 4bb17cbc2da9a1ccad4303f732ae1f3f80d59671 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 7 Aug 2021 20:24:13 +0200 Subject: [PATCH] Remove `take_*` methods from RequestParts for Version, Method, and Uri (#151) * Remove `RequestParts::take_method` * Remove `RequestParts::take_uri` * Remove `RequestParts::take_version` --- CHANGELOG.md | 13 +++++- src/extract/form.rs | 8 +--- src/extract/mod.rs | 81 ++++++++++-------------------------- src/extract/query.rs | 6 +-- src/extract/raw_query.rs | 5 +-- src/extract/rejection.rs | 24 ----------- src/extract/request_parts.rs | 27 +++++------- src/extract/ws.rs | 3 +- 8 files changed, 51 insertions(+), 116 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index def255bf..366306a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 required for returning responses with bodies other than `hyper::Body` from handlers. See the docs for advice on how to implement `IntoResponse` ([#86](https://github.com/tokio-rs/axum/pull/86)) - Change WebSocket API to use an extractor ([#121](https://github.com/tokio-rs/axum/pull/121)) -- Make WebSocket Message an enum ([#116](https://github.com/tokio-rs/axum/pull/116)) +- Make WebSocket `Message` an enum ([#116](https://github.com/tokio-rs/axum/pull/116)) - Add `RoutingDsl::or` for combining routes. ([#108](https://github.com/tokio-rs/axum/pull/108)) - Ensure a `HandleError` service created from `axum::ServiceExt::handle_error` _does not_ implement `RoutingDsl` as that could lead to confusing routing @@ -33,6 +33,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `ExtractorMiddlewareLayer` - Replace `axum::body::BoxStdError` with `axum::Error`, which supports downcasting ([#150](https://github.com/tokio-rs/axum/pull/150)) - `WebSocket` now uses `axum::Error` as its error type ([#150](https://github.com/tokio-rs/axum/pull/150)) +- `RequestParts` changes: + - `method` new returns an `&http::Method` + - `method_mut` new returns an `&mut http::Method` + - `take_method` has been removed + - `uri` new returns an `&http::Uri` + - `uri_mut` new returns an `&mut http::Uri` + - `take_uri` has been removed +- These rejections have been removed as they're no longer used + - `MethodAlreadyExtracted` + - `UriAlreadyExtracted` + - `VersionAlreadyExtracted` # 0.1.3 (06. August, 2021) diff --git a/src/extract/form.rs b/src/extract/form.rs index 1dc93fa2..0dea3670 100644 --- a/src/extract/form.rs +++ b/src/extract/form.rs @@ -51,12 +51,8 @@ where #[allow(warnings)] async fn from_request(req: &mut RequestParts) -> Result { - if req.method().ok_or(MethodAlreadyExtracted)? == Method::GET { - let query = req - .uri() - .ok_or(UriAlreadyExtracted)? - .query() - .unwrap_or_default(); + if req.method() == Method::GET { + let query = req.uri().query().unwrap_or_default(); let value = serde_urlencoded::from_str(query) .map_err(FailedToDeserializeQueryString::new::)?; Ok(Form(value)) diff --git a/src/extract/mod.rs b/src/extract/mod.rs index c0e760ec..1ba4015d 100644 --- a/src/extract/mod.rs +++ b/src/extract/mod.rs @@ -368,9 +368,9 @@ pub trait FromRequest: Sized { /// Has several convenience methods for getting owned parts of the request. #[derive(Debug)] pub struct RequestParts { - method: Option, - uri: Option, - version: Option, + method: Method, + uri: Uri, + version: Version, headers: Option, extensions: Option, body: Option, @@ -391,9 +391,9 @@ impl RequestParts { ) = req.into_parts(); RequestParts { - method: Some(method), - uri: Some(uri), - version: Some(version), + method, + uri, + version, headers: Some(headers), extensions: Some(extensions), body: Some(body), @@ -413,17 +413,9 @@ impl RequestParts { let mut req = Request::new(body.take().expect("body already extracted")); - if let Some(method) = method.take() { - *req.method_mut() = method; - } - - if let Some(uri) = uri.take() { - *req.uri_mut() = uri; - } - - if let Some(version) = version.take() { - *req.version_mut() = version; - } + *req.method_mut() = method.clone(); + *req.uri_mut() = uri.clone(); + *req.version_mut() = *version; if let Some(headers) = headers.take() { *req.headers_mut() = headers; @@ -436,61 +428,34 @@ impl RequestParts { req } - /// Gets a reference to the request method. - /// - /// Returns `None` if the method has been taken by another extractor. - pub fn method(&self) -> Option<&Method> { - self.method.as_ref() + /// Gets a reference the request method. + pub fn method(&self) -> &Method { + &self.method } /// Gets a mutable reference to the request method. - /// - /// Returns `None` if the method has been taken by another extractor. - pub fn method_mut(&mut self) -> Option<&mut Method> { - self.method.as_mut() + pub fn method_mut(&mut self) -> &mut Method { + &mut self.method } - /// Takes the method out of the request, leaving a `None` in its place. - pub fn take_method(&mut self) -> Option { - self.method.take() - } - - /// Gets a reference to the request URI. - /// - /// Returns `None` if the URI has been taken by another extractor. - pub fn uri(&self) -> Option<&Uri> { - self.uri.as_ref() + /// Gets a reference the request URI. + pub fn uri(&self) -> &Uri { + &self.uri } /// Gets a mutable reference to the request URI. - /// - /// Returns `None` if the URI has been taken by another extractor. - pub fn uri_mut(&mut self) -> Option<&mut Uri> { - self.uri.as_mut() + pub fn uri_mut(&mut self) -> &mut Uri { + &mut self.uri } - /// Takes the URI out of the request, leaving a `None` in its place. - pub fn take_uri(&mut self) -> Option { - self.uri.take() - } - - /// Gets a reference to the request HTTP version. - /// - /// Returns `None` if the HTTP version has been taken by another extractor. - pub fn version(&self) -> Option { + /// Get the request HTTP version. + pub fn version(&self) -> Version { self.version } /// Gets a mutable reference to the request HTTP version. - /// - /// Returns `None` if the HTTP version has been taken by another extractor. - pub fn version_mut(&mut self) -> Option<&mut Version> { - self.version.as_mut() - } - - /// Takes the HTTP version out of the request, leaving a `None` in its place. - pub fn take_version(&mut self) -> Option { - self.version.take() + pub fn version_mut(&mut self) -> &mut Version { + &mut self.version } /// Gets a reference to the request headers. diff --git a/src/extract/query.rs b/src/extract/query.rs index fd3786ba..a5986e98 100644 --- a/src/extract/query.rs +++ b/src/extract/query.rs @@ -47,11 +47,7 @@ where type Rejection = QueryRejection; async fn from_request(req: &mut RequestParts) -> Result { - let query = req - .uri() - .ok_or(UriAlreadyExtracted)? - .query() - .unwrap_or_default(); + let query = req.uri().query().unwrap_or_default(); let value = serde_urlencoded::from_str(query) .map_err(FailedToDeserializeQueryString::new::)?; Ok(Query(value)) diff --git a/src/extract/raw_query.rs b/src/extract/raw_query.rs index 9c8238f1..0c731c7d 100644 --- a/src/extract/raw_query.rs +++ b/src/extract/raw_query.rs @@ -30,10 +30,7 @@ where type Rejection = Infallible; async fn from_request(req: &mut RequestParts) -> Result { - let query = req - .uri() - .and_then(|uri| uri.query()) - .map(|query| query.to_string()); + let query = req.uri().query().map(|query| query.to_string()); Ok(Self(query)) } } diff --git a/src/extract/rejection.rs b/src/extract/rejection.rs index adfa3628..44d1e013 100644 --- a/src/extract/rejection.rs +++ b/src/extract/rejection.rs @@ -10,27 +10,6 @@ use http_body::Full; use std::convert::Infallible; use tower::BoxError; -define_rejection! { - #[status = INTERNAL_SERVER_ERROR] - #[body = "Version taken by other extractor"] - /// Rejection used if the HTTP version has been taken by another extractor. - pub struct VersionAlreadyExtracted; -} - -define_rejection! { - #[status = INTERNAL_SERVER_ERROR] - #[body = "URI taken by other extractor"] - /// Rejection used if the URI has been taken by another extractor. - pub struct UriAlreadyExtracted; -} - -define_rejection! { - #[status = INTERNAL_SERVER_ERROR] - #[body = "Method taken by other extractor"] - /// Rejection used if the method has been taken by another extractor. - pub struct MethodAlreadyExtracted; -} - define_rejection! { #[status = INTERNAL_SERVER_ERROR] #[body = "Extensions taken by other extractor"] @@ -222,7 +201,6 @@ composite_rejection! { /// Contains one variant for each way the [`Query`](super::Query) extractor /// can fail. pub enum QueryRejection { - UriAlreadyExtracted, FailedToDeserializeQueryString, } } @@ -237,9 +215,7 @@ composite_rejection! { FailedToDeserializeQueryString, FailedToBufferBody, BodyAlreadyExtracted, - UriAlreadyExtracted, HeadersAlreadyExtracted, - MethodAlreadyExtracted, } } diff --git a/src/extract/request_parts.rs b/src/extract/request_parts.rs index f4afcdd4..a6c3f46d 100644 --- a/src/extract/request_parts.rs +++ b/src/extract/request_parts.rs @@ -4,6 +4,7 @@ use bytes::Bytes; use futures_util::stream::Stream; use http::{HeaderMap, Method, Request, Uri, Version}; use std::{ + convert::Infallible, pin::Pin, task::{Context, Poll}, }; @@ -18,21 +19,15 @@ where async fn from_request(req: &mut RequestParts) -> Result { let RequestParts { - method, - uri, - version, + method: _, + uri: _, + version: _, headers, extensions, body, } = req; - let all_parts = method - .as_ref() - .zip(version.as_ref()) - .zip(uri.as_ref()) - .zip(extensions.as_ref()) - .zip(body.as_ref()) - .zip(headers.as_ref()); + let all_parts = extensions.as_ref().zip(body.as_ref()).zip(headers.as_ref()); if all_parts.is_some() { Ok(req.into_request()) @@ -60,10 +55,10 @@ impl FromRequest for Method where B: Send, { - type Rejection = MethodAlreadyExtracted; + type Rejection = Infallible; async fn from_request(req: &mut RequestParts) -> Result { - req.take_method().ok_or(MethodAlreadyExtracted) + Ok(req.method().clone()) } } @@ -72,10 +67,10 @@ impl FromRequest for Uri where B: Send, { - type Rejection = UriAlreadyExtracted; + type Rejection = Infallible; async fn from_request(req: &mut RequestParts) -> Result { - req.take_uri().ok_or(UriAlreadyExtracted) + Ok(req.uri().clone()) } } @@ -84,10 +79,10 @@ impl FromRequest for Version where B: Send, { - type Rejection = VersionAlreadyExtracted; + type Rejection = Infallible; async fn from_request(req: &mut RequestParts) -> Result { - req.take_version().ok_or(VersionAlreadyExtracted) + Ok(req.version()) } } diff --git a/src/extract/ws.rs b/src/extract/ws.rs index d2c07d7d..e2288f37 100644 --- a/src/extract/ws.rs +++ b/src/extract/ws.rs @@ -167,7 +167,7 @@ where type Rejection = WebSocketUpgradeRejection; async fn from_request(req: &mut RequestParts) -> Result { - if req.method().ok_or(MethodAlreadyExtracted)? != Method::GET { + if req.method() != Method::GET { return Err(MethodNotGet.into()); } @@ -572,7 +572,6 @@ pub mod rejection { InvalidUpgradeHeader, InvalidWebsocketVersionHeader, WebsocketKeyHeaderMissing, - MethodAlreadyExtracted, HeadersAlreadyExtracted, ExtensionsAlreadyExtracted, }