From 2f8fbf8d3bf46f3b73cb2bc6c8f0b4fa18893b0e Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 17 Nov 2021 21:09:33 +0300 Subject: [PATCH] FromRequestX --- src/data.rs | 30 +++++++++- src/extract.rs | 89 ++++++++++++++++++++++++----- src/handler.rs | 140 +++++++++++++++++++++++++++++++++++----------- src/lib.rs | 2 +- src/resource.rs | 12 ++-- src/route.rs | 13 ++--- src/types/path.rs | 12 ++-- src/web.rs | 12 ++-- 8 files changed, 233 insertions(+), 77 deletions(-) diff --git a/src/data.rs b/src/data.rs index d27ad196b..203b5d4dd 100644 --- a/src/data.rs +++ b/src/data.rs @@ -6,8 +6,8 @@ use futures_core::future::LocalBoxFuture; use serde::Serialize; use crate::{ - dev::Payload, error::ErrorInternalServerError, extract::FromRequest, request::HttpRequest, - Error, + dev::Payload, error::ErrorInternalServerError, request::HttpRequest, Error, FromRequest, + FromRequestX, }; /// Data factory. @@ -143,6 +143,29 @@ impl FromRequest for Data { } } +impl<'a, T: ?Sized + 'static> FromRequestX<'a> for &Data { + type Output = &'a Data; + type Error = Error; + type Future = Ready>; + + #[inline] + fn from_request(req: &'a HttpRequest, _: &mut Payload) -> Self::Future { + if let Some(st) = req.app_data::>() { + ok(st) + } else { + log::debug!( + "Failed to construct App-level Data extractor. \ + Request path: {:?} (type: {})", + req.path(), + type_name::(), + ); + err(ErrorInternalServerError( + "App data is not configured, to configure construct it with web::Data::new() and pass it to App::app_data()", + )) + } + } +} + impl DataFactory for Data { fn create(&self, extensions: &mut Extensions) -> bool { extensions.insert(Data(self.0.clone())); @@ -160,6 +183,9 @@ mod tests { web, App, HttpResponse, }; + // shadow + trait FromRequest {} + // allow deprecated App::data #[allow(deprecated)] #[actix_rt::test] diff --git a/src/extract.rs b/src/extract.rs index 29fd0d05e..7e561fd20 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -76,6 +76,38 @@ pub trait FromRequest: Sized { } } +pub trait FromRequestX<'a>: Sized { + /// Must be Self unless the extractor borrows request. + type Output; + + /// The associated error which can be returned. + // TODO Consider adding 'static bound + type Error: Into; + + /// Future that resolves to a Self. + type Future: Future>; + + /// Create a Self from request parts asynchronously. + fn from_request(req: &'a HttpRequest, payload: &mut Payload) -> Self::Future; + + /// Create a Self from request head asynchronously. + /// + /// This method is short for `T::from_request(req, &mut Payload::None)`. + fn extract(req: &'a HttpRequest) -> Self::Future { + Self::from_request(req, &mut Payload::None) + } +} + +impl<'a, T: FromRequest> FromRequestX<'a> for T { + type Output = Self; + type Error = ::Error; + type Future = ::Future; + + fn from_request(req: &'a HttpRequest, payload: &mut Payload) -> Self::Future { + Self::from_request(req, payload) + } +} + /// Optionally extract a field from the request /// /// If the FromRequest for T fails, return None rather than returning an error response @@ -123,16 +155,16 @@ pub trait FromRequest: Sized { /// ); /// } /// ``` -impl FromRequest for Option +impl<'a, T> FromRequestX<'a> for Option where - T: FromRequest, - T::Future: 'static, + T: FromRequestX<'a>, { + type Output = Option; type Error = Error; type Future = FromRequestOptFuture; #[inline] - fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { + fn from_request(req: &'a HttpRequest, payload: &mut Payload) -> Self::Future { FromRequestOptFuture { fut: T::from_request(req, payload), } @@ -209,6 +241,7 @@ where /// ); /// } /// ``` +// TODO impl FromRequest for Result where T: FromRequest + 'static, @@ -266,6 +299,26 @@ impl FromRequest for Uri { } } +impl<'a> FromRequestX<'a> for &Uri { + type Output = &'a Uri; + type Error = Infallible; + type Future = Ready>; + + fn from_request(req: &'a HttpRequest, _: &mut Payload) -> Self::Future { + ok(req.uri()) + } +} + +impl<'a> FromRequestX<'a> for &HttpRequest { + type Output = &'a HttpRequest; + type Error = Infallible; + type Future = Ready>; + + fn from_request(req: &'a HttpRequest, _: &mut Payload) -> Self::Future { + ok(req) + } +} + /// Extract the request's method. /// /// # Examples @@ -318,38 +371,41 @@ macro_rules! tuple_from_req ({$fut_type:ident, $(($n:tt, $T:ident)),+} => { // redundant imports use super::*; + use std::marker::PhantomData; + /// A helper struct to allow us to pin-project through /// to individual fields #[pin_project::pin_project] - struct FutWrapper<$($T: FromRequest),+>($(#[pin] $T::Future),+); + struct FutWrapper<'a, $($T: FromRequestX<'a>),+>($(#[pin] $T::Future,)+ PhantomData<&'a ()>); /// FromRequest implementation for tuple #[doc(hidden)] #[allow(unused_parens)] - impl<$($T: FromRequest + 'static),+> FromRequest for ($($T,)+) + impl<'a, $($T: FromRequestX<'a>),+> FromRequestX<'a> for ($($T,)+) { + type Output = ($($T::Output,)+); type Error = Error; - type Future = $fut_type<$($T),+>; + type Future = $fut_type<'a, $($T),+>; - fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { + fn from_request(req: &'a HttpRequest, payload: &mut Payload) -> Self::Future { $fut_type { - items: <($(Option<$T>,)+)>::default(), - futs: FutWrapper($($T::from_request(req, payload),)+), + items: <($(Option<$T::Output>,)+)>::default(), + futs: FutWrapper($($T::from_request(req, payload),)+ PhantomData), } } } #[doc(hidden)] #[pin_project::pin_project] - pub struct $fut_type<$($T: FromRequest),+> { - items: ($(Option<$T>,)+), + pub struct $fut_type<'a, $($T: FromRequestX<'a>),+> { + items: ($(Option<$T::Output>,)+), #[pin] - futs: FutWrapper<$($T,)+>, + futs: FutWrapper<'a, $($T,)+>, } - impl<$($T: FromRequest),+> Future for $fut_type<$($T),+> + impl<'a, $($T: FromRequestX<'a>),+> Future for $fut_type<'a, $($T),+> { - type Output = Result<($($T,)+), Error>; + type Output = Result<($($T::Output,)+), Error>; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let mut this = self.project(); @@ -405,6 +461,9 @@ mod tests { use crate::test::TestRequest; use crate::types::{Form, FormConfig}; + // shadow + trait FromRequest {} + #[derive(Deserialize, Debug, PartialEq)] struct Info { hello: String, diff --git a/src/handler.rs b/src/handler.rs index ddefe8d53..1f8cdd965 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -7,31 +7,47 @@ use actix_service::{ use crate::{ service::{ServiceRequest, ServiceResponse}, - Error, FromRequest, HttpResponse, Responder, + Error, FromRequestX, HttpResponse, Responder, }; +// TODO inaccessible docs /// A request handler is an async function that accepts zero or more parameters that can be /// extracted from a request (i.e., [`impl FromRequest`](crate::FromRequest)) and returns a type /// that can be converted into an [`HttpResponse`] (that is, it impls the [`Responder`] trait). /// /// If you got the error `the trait Handler<_, _, _> is not implemented`, then your function is not /// a valid handler. See [Request Handlers](https://actix.rs/docs/handlers/) for more information. -pub trait Handler: Clone + 'static -where - R: Future, - R::Output: Responder, -{ - fn call(&self, param: T) -> R; +pub trait Handler<'a, T: FromRequestX<'a>>: Clone + 'static { + // TODO why 'static ?? + type Response: Responder + 'static; + type Future: Future; + + fn handle(&'a self, _: T::Output) -> Self::Future; } -pub fn handler_service( - handler: F, +impl<'a, F, T, Fut, Resp> Handler<'a, T> for F +where + F: FnX, + F: FnX, + F: Clone + 'static, + T: FromRequestX<'a>, + Fut: Future, + Resp: Responder + 'static, +{ + type Response = Resp; + type Future = Fut; + + fn handle(&'a self, data: T::Output) -> Self::Future { + self.call(data) + } +} + +pub fn handler_service( + handler: H, ) -> BoxServiceFactory<(), ServiceRequest, ServiceResponse, Error, ()> where - F: Handler, - T: FromRequest, - R: Future, - R::Output: Responder, + H: for<'a> Handler<'a, T>, + T: for<'a> FromRequestX<'a>, { boxed::factory(fn_service(move |req: ServiceRequest| { let handler = handler.clone(); @@ -39,37 +55,95 @@ where let (req, mut payload) = req.into_parts(); let res = match T::from_request(&req, &mut payload).await { Err(err) => HttpResponse::from_error(err), - Ok(data) => handler.call(data).await.respond_to(&req), + Ok(data) => handler.handle(data).await.respond_to(&req), }; Ok(ServiceResponse::new(req, res)) } })) } +/// Same as [`std::ops::Fn`] +pub trait FnX { + type Output; + fn call(&self, args: Args) -> Self::Output; +} + /// FromRequest trait impl for tuples -macro_rules! factory_tuple ({ $($param:ident)* } => { - impl Handler<($($param,)*), Res> for Func - where Func: Fn($($param),*) -> Res + Clone + 'static, - Res: Future, - Res::Output: Responder, +macro_rules! fn_tuple ({ $($param:ident)* } => { + impl FnX<($($param,)*)> for Func + where Func: Fn($($param),*) -> O, { + type Output = O; + #[allow(non_snake_case)] - fn call(&self, ($($param,)*): ($($param,)*)) -> Res { + fn call(&self, ($($param,)*): ($($param,)*)) -> O { (self)($($param,)*) } } }); -factory_tuple! {} -factory_tuple! { A } -factory_tuple! { A B } -factory_tuple! { A B C } -factory_tuple! { A B C D } -factory_tuple! { A B C D E } -factory_tuple! { A B C D E F } -factory_tuple! { A B C D E F G } -factory_tuple! { A B C D E F G H } -factory_tuple! { A B C D E F G H I } -factory_tuple! { A B C D E F G H I J } -factory_tuple! { A B C D E F G H I J K } -factory_tuple! { A B C D E F G H I J K L } +fn_tuple! {} +fn_tuple! { A } +fn_tuple! { A B } +fn_tuple! { A B C } +fn_tuple! { A B C D } +fn_tuple! { A B C D E } +fn_tuple! { A B C D E F } +fn_tuple! { A B C D E F G } +fn_tuple! { A B C D E F G H } +fn_tuple! { A B C D E F G H I } +fn_tuple! { A B C D E F G H I J } +fn_tuple! { A B C D E F G H I J K } +fn_tuple! { A B C D E F G H I J K L } + +#[cfg(test)] +mod test { + use serde::Deserialize; + + use super::*; + use crate::{ + dev::Service, + http::StatusCode, + test::{init_service, TestRequest}, + web, App, HttpRequest, HttpResponse, + }; + + #[derive(Deserialize)] + struct Params<'a> { + name: &'a str, + } + + #[derive(Deserialize)] + struct ParamsOwned { + name: String, + } + + async fn handler( + req: &HttpRequest, + (data, params1): (Option<&web::Data>, web::Path), + ) -> HttpResponse { + let params2 = web::Path::>::extract(req).await.unwrap(); + assert_eq!(params1.name, "named"); + assert_eq!(params2.name, "named"); + + assert_eq!(data.unwrap().as_ref(), &42); + + HttpResponse::Ok().finish() + } + + #[actix_rt::test] + async fn test_borrowed_extractor() { + let srv = init_service( + App::new().service( + web::resource("/{name}") + .app_data(web::Data::new(42usize)) + .route(web::get().to(handler)), + ), + ) + .await; + + let req = TestRequest::with_uri("/named").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } +} diff --git a/src/lib.rs b/src/lib.rs index 3ad77ff5f..c67eb586a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,7 +105,7 @@ pub use cookie; pub use crate::app::App; pub use crate::error::{Error, ResponseError, Result}; -pub use crate::extract::FromRequest; +pub use crate::extract::{FromRequest, FromRequestX}; pub use crate::request::HttpRequest; pub use crate::resource::Resource; pub use crate::responder::Responder; diff --git a/src/resource.rs b/src/resource.rs index 851ce0fc9..82c50bee6 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -16,12 +16,12 @@ use futures_util::future::join_all; use crate::{ data::Data, dev::{ensure_leading_slash, AppService, HttpServiceFactory, ResourceDef}, + extract::FromRequestX, guard::Guard, handler::Handler, - responder::Responder, route::{Route, RouteService}, service::{ServiceRequest, ServiceResponse}, - Error, FromRequest, HttpResponse, + Error, HttpResponse, }; type HttpService = BoxService; @@ -236,12 +236,10 @@ where /// # fn index(req: HttpRequest) -> HttpResponse { unimplemented!() } /// App::new().service(web::resource("/").route(web::route().to(index))); /// ``` - pub fn to(mut self, handler: F) -> Self + pub fn to(mut self, handler: F) -> Self where - F: Handler, - I: FromRequest + 'static, - R: Future + 'static, - R::Output: Responder + 'static, + F: for<'a> Handler<'a, I>, + I: for<'a> FromRequestX<'a>, { self.routes.push(Route::new().to(handler)); self diff --git a/src/route.rs b/src/route.rs index 0c0699430..58824e150 100644 --- a/src/route.rs +++ b/src/route.rs @@ -1,6 +1,6 @@ #![allow(clippy::rc_buffer)] // inner value is mutated before being shared (`Rc::get_mut`) -use std::{future::Future, rc::Rc}; +use std::rc::Rc; use actix_http::http::Method; use actix_service::{ @@ -10,10 +10,11 @@ use actix_service::{ use futures_core::future::LocalBoxFuture; use crate::{ + extract::FromRequestX, guard::{self, Guard}, handler::{handler_service, Handler}, service::{ServiceRequest, ServiceResponse}, - Error, FromRequest, HttpResponse, Responder, + Error, HttpResponse, }; /// Resource route definition @@ -175,12 +176,10 @@ impl Route { /// ); /// } /// ``` - pub fn to(mut self, handler: F) -> Self + pub fn to(mut self, handler: F) -> Self where - F: Handler, - T: FromRequest + 'static, - R: Future + 'static, - R::Output: Responder + 'static, + F: for<'a> Handler<'a, T>, + T: for<'a> FromRequestX<'a>, { self.service = handler_service(handler); self diff --git a/src/types/path.rs b/src/types/path.rs index cd24deb81..af4bd8b46 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -9,7 +9,7 @@ use serde::de; use crate::{ dev::Payload, error::{Error, ErrorNotFound, PathError}, - FromRequest, HttpRequest, + FromRequestX, HttpRequest, }; /// Extract typed data from request path segments. @@ -91,15 +91,16 @@ impl fmt::Display for Path { } /// See [here](#usage) for example of usage as an extractor. -impl FromRequest for Path +impl<'a, T> FromRequestX<'a> for Path where - T: de::DeserializeOwned, + T: de::Deserialize<'a>, { + type Output = Self; type Error = Error; - type Future = Ready>; + type Future = Ready>; #[inline] - fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + fn from_request(req: &'a HttpRequest, _: &mut Payload) -> Self::Future { let error_handler = req .app_data::() .and_then(|c| c.err_handler.clone()); @@ -266,6 +267,7 @@ mod tests { resource.capture_match_info(req.match_info_mut()); let (req, mut pl) = req.into_parts(); + let s = Path::::from_request(&req, &mut pl).await.unwrap(); assert_eq!(s.as_ref().key, "name"); assert_eq!(s.value, 32); diff --git a/src/web.rs b/src/web.rs index e9f5c8518..bd080ba9f 100644 --- a/src/web.rs +++ b/src/web.rs @@ -7,8 +7,8 @@ use actix_router::IntoPatterns; pub use bytes::{Buf, BufMut, Bytes, BytesMut}; use crate::{ - error::BlockingError, extract::FromRequest, handler::Handler, resource::Resource, - responder::Responder, route::Route, scope::Scope, service::WebService, + error::BlockingError, extract::FromRequestX, handler::Handler, resource::Resource, + route::Route, scope::Scope, service::WebService, }; pub use crate::config::ServiceConfig; @@ -139,12 +139,10 @@ pub fn method(method: Method) -> Route { /// web::to(index)) /// ); /// ``` -pub fn to(handler: F) -> Route +pub fn to(handler: F) -> Route where - F: Handler, - I: FromRequest + 'static, - R: Future + 'static, - R::Output: Responder + 'static, + F: for<'a> Handler<'a, I>, + I: for<'a> FromRequestX<'a>, { Route::new().to(handler) }