From 003fb0465e3850b66ab5c996f8eb80cbb21d13a8 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 11:56:17 +0200 Subject: [PATCH 01/10] Working implementation --- tower-http/Cargo.toml | 2 + tower-http/src/conditional_response.rs | 300 +++++++++++++++++++++++++ tower-http/src/lib.rs | 3 + 3 files changed, 305 insertions(+) create mode 100644 tower-http/src/conditional_response.rs diff --git a/tower-http/Cargo.toml b/tower-http/Cargo.toml index b88ddeb3..f460a81e 100644 --- a/tower-http/Cargo.toml +++ b/tower-http/Cargo.toml @@ -20,6 +20,8 @@ futures-util = { version = "0.3.14", default_features = false, features = [] } http = "0.2.7" http-body = "0.4.5" pin-project-lite = "0.2.7" +# Required for some semantics in some middlewares that pin-project-lite does not support +pin-project = "1.1.3" tower-layer = "0.3" tower-service = "0.3" diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs new file mode 100644 index 00000000..05b4b79d --- /dev/null +++ b/tower-http/src/conditional_response.rs @@ -0,0 +1,300 @@ +//! +//! Conditionally provide a response instead of calling the inner service. +//! +//! This middleware provides a way to conditionally skip calling the inner service +//! if a response is already available for the request. +//! +//! Probably the simplest visual for this is providing a cached response, though it +//! is unlikely that this middleware is suitable for a robust response cache interface +//! (or, more accurately, it's not the motivation for developing this so I haven't +//! looked into it adequately enough to provide a robust argument for it being so!). +//! +//! The premise is simple - write a (non-async) function that assesses the current request +//! for the possibility of providing a response before invoking the inner service. Return +//! the "early" response if that is possible, otherwise return the request. +//! +//! The differences between using this and returning an error from a pre-inner layer are. +//! +//! 1. The response will still pass through any _post_-inner layer processing +//! 2. You aren't "hacking" the idea of an error when all you are trying to do is avoid +//! calling the inner service when it isn't necessary. +//! +//! Possible uses: +//! +//! * A pre-inner layer produces a successful response before the inner service is called +//! * Caching (though see above - this could, however, be the layer that skips the inner +//! call while a more robust pre-inner layer implements the actual caching) +//! * Mocking +//! * Debugging +//! * ... +//! +//! # Example +//! +//! ```rust +//! use http::{Request, Response}; +//! use std::convert::Infallible; +//! use tower::{Service, ServiceExt, ServiceBuilder}; +//! use tower::conditional_response::CondResponseLayer; +//! +//! # #[tokio::main] +//! # async fn main() -> Result<(), Box> { +//! +//! // +//! // The responder function here decides whether to return an early response based +//! // upon the presence and value of a specific header.L +//! // +//! fn responder(request: Request) -> ConditionalResponse,Response> { +//! match request.headers().get("x-so-we-skip") { +//! Some(a) if a.to_str().unwrap() == "true" => ConditionalResponse::Response(Response::new("We skipped it".to_string())), +//! _ => ConditionalResponse::Request(request) +//! } +//! } +//! +//! async fn handle(_req: Request) -> Result, Infallible> { +//! // ... +//! Ok(Response::new("We ran it".to_string())) +//! } +//! +//! let mut svc = ServiceBuilder::new() +//! // +//! // Directly wrap the target service with the conditional responder layer +//! // +//! .layer(CondResponseLayer::new(responder)) +//! .service_fn(handle); +//! +//! let request = Request::builder().header("x-so-we-skip","true").body("".to_string()).expect("Expected an empty body"); + +//! // Call the service. +//! let ready = futures::executor::block_on(svc.ready()).expect("Expected the service to be ready"); +//! let response = futures::executor::block_on(ready.call(request)).expect("Expected the service to be successful"); +//! assert_eq!(response.body(), "We skipped it"); +//! # +//! # Ok(()) +//! # } +//! ``` + +use http::{Request, Response}; +use std::future::Future; +use std::{ + pin::Pin, + task::{Context, Poll}, +}; +use tower_layer::Layer; +use tower_service::Service; +use pin_project::pin_project; + +/// Layer that applies [`CondResponse`] which allows the caller to generate/return a response instead of calling the +/// inner service - useful for stacks where a default response (rather than an error) is determined by a pre-service +/// filter. +/// +/// See the [module docs](crate::conditional_response) for more details. +#[derive(Clone, Debug)] +pub struct CondResponseLayer

{ + responder: P +} + +impl

CondResponseLayer

+{ + /// Create a new [`CondResponseLayer`]. + pub fn new(responder:P) -> Self { + Self { responder } + } +} + +impl Layer for CondResponseLayer

+where + P: Clone +{ + type Service = CondResponse; + + fn layer(&self, inner: S) -> Self::Service { + CondResponse:: { + inner, + responder: self.responder.clone(), + } + } +} + +/// Middleware that conditionally provides a response to a request in lieu of calling the inner service. +/// +/// See the [module docs](crate::propagate_extension) for more details. +#[derive(Clone,Debug)] +pub struct CondResponse { + inner: S, + responder: P, +} + +impl CondResponse +{ + /// Create a new [`CondResponse`] with the inner service and the "responder" function. + pub fn new(inner: S, responder: P) -> Self { + Self { inner, responder } + } + + define_inner_service_accessors!(); + + /// Returns a new [`Layer`] that wraps services with a `CondResponse` middleware. + /// + /// [`Layer`]: tower_layer::Layer + pub fn layer(responder: P) -> CondResponseLayer

{ + CondResponseLayer::

::new(responder) + } +} + +impl Service> for CondResponse +where + S: Service, Response = Response> + Clone + Send + 'static, + P: ConditionalResponder,Response>, + ReqBody: Send + Sync + Clone +{ + type Response = S::Response; + type Error = S::Error; + type Future = ResponseFuture; + + #[inline] + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, req: Request) -> Self::Future { + match self.responder.has_response(req) { + ConditionalResponse::Request(t) => ResponseFuture::::Future(self.inner.call(t)), + ConditionalResponse::Response(r) => ResponseFuture::::Response(Some(r)) + } + } +} + + +/// Response future for [`CondResponse`]. +/// +/// We use an enum because the inner content may be a future or +/// or may be a direct response. +/// +/// We use an option for the direct response so that ownership can be taken. +/// +#[derive(Debug)] +#[pin_project(project = ResponseFutureProj)] +pub enum ResponseFuture { + Response(Option), + Future(#[pin] F), +} + +impl Future for ResponseFuture> +where + F: Future, E>>, +{ + type Output = F::Output; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + match self.project() { + ResponseFutureProj::Response(r) => Poll::Ready(Ok(r.take().unwrap())), + ResponseFutureProj::Future(ref mut future) => future.as_mut().poll(cx) + } + } +} + +///////////////////////////////////////////////////////////////////////// + +/// +/// The response required from the responder function. +/// +pub enum ConditionalResponse { + /// + /// No response is available, so return the request + /// + Request(T), + /// + /// A response is available, so return the response + /// + Response(R) +} + +/// +/// Fn trait for functions that consume a request and return a +/// ConditionalResponse variant. +/// + +pub trait ConditionalResponder { + /// The type of requests returned by [`has_response`]. + /// + /// This request is forwarded to the inner service if the responder + /// succeeds. + /// + /// [`has_response`]: crate::filter::responder::has_response + /// has_response whether the given request should be forwarded. + /// + /// If the future resolves with [`Ok`], the request is forwarded to the inner service. + fn has_response(&mut self, request: T) -> ConditionalResponse; +} + +impl ConditionalResponder for F +where + F: FnMut(T) -> ConditionalResponse, +{ + fn has_response(&mut self, request: T) -> ConditionalResponse { + self(request) + } +} + +#[cfg(test)] + mod tests { + use super::*; + + use http::{Request, Response}; + use std::convert::Infallible; + use tower::{Service, ServiceExt, ServiceBuilder}; + use crate::conditional_response::CondResponseLayer; + + fn responder(request: Request) -> ConditionalResponse,Response> { + match request.headers().get("x-so-we-skip") { + Some(a) if a.to_str().unwrap() == "true" => ConditionalResponse::Response(Response::new("We skipped it".to_string())), + _ => ConditionalResponse::Request(request) + } + } + + async fn handle(_req: Request) -> Result, Infallible> { + Ok(Response::new("We ran it".to_string())) + } + + #[test] + fn skip_test() { + let mut svc = ServiceBuilder::new() + .layer(CondResponseLayer::new(responder)) + .service_fn(handle); + + let request = Request::builder().header("x-so-we-skip","true").body("".to_string()).expect("Expected an empty body"); + + // Call the service. + let ready = futures::executor::block_on(svc.ready()).expect("Expected the service to be ready"); + let response = futures::executor::block_on(ready.call(request)).expect("Expected the service to be successful"); + assert_eq!(response.body(), "We skipped it"); + } + + #[test] + fn no_skip_test_header() { + let mut svc = ServiceBuilder::new() + .layer(CondResponseLayer::new(responder)) + .service_fn(handle); + + let request = Request::builder().header("x-so-we-skip","not true").body("".to_string()).expect("Expected an empty body"); + + // Call the service. + let ready = futures::executor::block_on(svc.ready()).expect("Expected the service to be ready"); + let response = futures::executor::block_on(ready.call(request)).expect("Expected the service to be successful"); + assert_eq!(response.body(), "We ran it"); + } + + #[test] + fn no_skip_test_no_header() { + let mut svc = ServiceBuilder::new() + .layer(CondResponseLayer::new(responder)) + .service_fn(handle); + + let request = Request::builder().body("".to_string()).expect("Expected an empty body"); + + // Call the service. + let ready = futures::executor::block_on(svc.ready()).expect("Expected the service to be ready"); + let response = futures::executor::block_on(ready.call(request)).expect("Expected the service to be successful"); + assert_eq!(response.body(), "We ran it"); + } +} diff --git a/tower-http/src/lib.rs b/tower-http/src/lib.rs index 6719ddbd..e0568e4f 100644 --- a/tower-http/src/lib.rs +++ b/tower-http/src/lib.rs @@ -320,6 +320,9 @@ pub mod request_id; #[cfg(feature = "catch-panic")] pub mod catch_panic; +#[cfg(any(test,feature = "cond-skip-inner"))] +pub mod conditional_response; + #[cfg(feature = "set-status")] pub mod set_status; From db7c33c50b5c1cad753ff75336c39e496def7616 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 12:39:58 +0200 Subject: [PATCH 02/10] Complete the implementation --- tower-http/Cargo.toml | 2 ++ tower-http/src/builder.rs | 19 +++++++++++++++++++ tower-http/src/conditional_response.rs | 2 +- tower-http/src/lib.rs | 2 +- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/tower-http/Cargo.toml b/tower-http/Cargo.toml index f460a81e..71208b62 100644 --- a/tower-http/Cargo.toml +++ b/tower-http/Cargo.toml @@ -61,6 +61,7 @@ full = [ "auth", "catch-panic", "compression-full", + "conditional_response", "cors", "decompression-full", "follow-redirect", @@ -85,6 +86,7 @@ full = [ add-extension = [] auth = ["base64", "validate-request"] catch-panic = ["tracing", "futures-util/std"] +conditional_response = [] cors = [] follow-redirect = ["iri-string", "tower/util"] fs = ["tokio/fs", "tokio-util/io", "tokio/io-util", "dep:http-range-header", "mime_guess", "mime", "percent-encoding", "httpdate", "set-status", "futures-util/alloc", "tracing"] diff --git a/tower-http/src/builder.rs b/tower-http/src/builder.rs index 2cb4f94a..6d525bf8 100644 --- a/tower-http/src/builder.rs +++ b/tower-http/src/builder.rs @@ -66,6 +66,17 @@ pub trait ServiceBuilderExt: crate::sealed::Sealed + Sized { value: T, ) -> ServiceBuilder, L>>; + /// Conditionally bypass the inner service by providing an "early" response. + /// + /// See [`tower_http::conditional_response`] for more details. + /// + /// [`tower_http::conditional_response`]: crate::conditional_response + #[cfg(feature = "conditional-response")] + fn conditional_response( + self, + responder: ConditionalResponder, + ) -> ServiceBuilder>; + /// Apply a transformation to the request body. /// /// See [`tower_http::map_request_body`] for more details. @@ -388,6 +399,14 @@ impl ServiceBuilderExt for ServiceBuilder { self.layer(crate::add_extension::AddExtensionLayer::new(value)) } + #[cfg(feature = "conditional-response")] + fn conditional_response( + self, + responder: ConditionalResponder, + ) -> ServiceBuilder> { + self.layer(crate::conditional_response::CondResponseLayer::new(f)) + } + #[cfg(feature = "map-request-body")] fn map_request_body( self, diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index 05b4b79d..e545e0b4 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -117,7 +117,7 @@ where /// Middleware that conditionally provides a response to a request in lieu of calling the inner service. /// -/// See the [module docs](crate::propagate_extension) for more details. +/// See the [module docs](crate::conditional_response) for more details. #[derive(Clone,Debug)] pub struct CondResponse { inner: S, diff --git a/tower-http/src/lib.rs b/tower-http/src/lib.rs index e0568e4f..da644534 100644 --- a/tower-http/src/lib.rs +++ b/tower-http/src/lib.rs @@ -320,7 +320,7 @@ pub mod request_id; #[cfg(feature = "catch-panic")] pub mod catch_panic; -#[cfg(any(test,feature = "cond-skip-inner"))] +#[cfg(any(test,feature = "conditional-response"))] pub mod conditional_response; #[cfg(feature = "set-status")] From 4c21041d5f37f6fe9a7d79cc373ecaee38ada689 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 14:06:49 +0200 Subject: [PATCH 03/10] Fix feature name --- tower-http/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tower-http/Cargo.toml b/tower-http/Cargo.toml index 71208b62..598c376f 100644 --- a/tower-http/Cargo.toml +++ b/tower-http/Cargo.toml @@ -61,7 +61,7 @@ full = [ "auth", "catch-panic", "compression-full", - "conditional_response", + "conditional-response", "cors", "decompression-full", "follow-redirect", @@ -86,7 +86,7 @@ full = [ add-extension = [] auth = ["base64", "validate-request"] catch-panic = ["tracing", "futures-util/std"] -conditional_response = [] +conditional-response = [] cors = [] follow-redirect = ["iri-string", "tower/util"] fs = ["tokio/fs", "tokio-util/io", "tokio/io-util", "dep:http-range-header", "mime_guess", "mime", "percent-encoding", "httpdate", "set-status", "futures-util/alloc", "tracing"] From fb8bbb5c600757a25c3227c2836d98d13f7dee93 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 14:43:55 +0200 Subject: [PATCH 04/10] Fix ServiceBuilderExt so it appears in tests --- tower-http/src/builder.rs | 20 ++++++++++---------- tower-http/src/lib.rs | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tower-http/src/builder.rs b/tower-http/src/builder.rs index 6d525bf8..b5a841de 100644 --- a/tower-http/src/builder.rs +++ b/tower-http/src/builder.rs @@ -40,7 +40,7 @@ use tower_layer::Stack; /// # service.ready().await.unwrap().call(Request::new(Body::empty())).await.unwrap(); /// # } /// ``` -#[cfg(feature = "util")] +#[cfg(any(test,feature = "util"))] // ^ work around rustdoc not inferring doc(cfg)s for cfg's from surrounding scopes pub trait ServiceBuilderExt: crate::sealed::Sealed + Sized { /// Propagate a header from the request to the response. @@ -71,11 +71,11 @@ pub trait ServiceBuilderExt: crate::sealed::Sealed + Sized { /// See [`tower_http::conditional_response`] for more details. /// /// [`tower_http::conditional_response`]: crate::conditional_response - #[cfg(feature = "conditional-response")] - fn conditional_response( + #[cfg(any(test,feature = "conditional-response"))] + fn conditional_response( self, - responder: ConditionalResponder, - ) -> ServiceBuilder>; + responder: R, + ) -> ServiceBuilder, L>>; /// Apply a transformation to the request body. /// @@ -399,12 +399,12 @@ impl ServiceBuilderExt for ServiceBuilder { self.layer(crate::add_extension::AddExtensionLayer::new(value)) } - #[cfg(feature = "conditional-response")] - fn conditional_response( + #[cfg(any(test,feature = "conditional-response"))] + fn conditional_response( self, - responder: ConditionalResponder, - ) -> ServiceBuilder> { - self.layer(crate::conditional_response::CondResponseLayer::new(f)) + responder: R, + ) -> ServiceBuilder, L>> { + self.layer(crate::conditional_response::CondResponseLayer::new(responder)) } #[cfg(feature = "map-request-body")] diff --git a/tower-http/src/lib.rs b/tower-http/src/lib.rs index da644534..e3a070cf 100644 --- a/tower-http/src/lib.rs +++ b/tower-http/src/lib.rs @@ -335,10 +335,10 @@ pub mod normalize_path; pub mod classify; pub mod services; -#[cfg(feature = "util")] +#[cfg(any(test,feature = "util"))] mod builder; -#[cfg(feature = "util")] +#[cfg(any(test,feature = "util"))] #[doc(inline)] pub use self::builder::ServiceBuilderExt; From dd623bdfcce48c446626df4524b228d2c85d9c8e Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 14:44:08 +0200 Subject: [PATCH 05/10] Test service builder function too --- tower-http/src/conditional_response.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index e545e0b4..70015f9e 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -243,6 +243,7 @@ where use http::{Request, Response}; use std::convert::Infallible; use tower::{Service, ServiceExt, ServiceBuilder}; + use crate::builder::ServiceBuilderExt; use crate::conditional_response::CondResponseLayer; fn responder(request: Request) -> ConditionalResponse,Response> { @@ -287,7 +288,7 @@ where #[test] fn no_skip_test_no_header() { let mut svc = ServiceBuilder::new() - .layer(CondResponseLayer::new(responder)) + .conditional_response(responder) .service_fn(handle); let request = Request::builder().body("".to_string()).expect("Expected an empty body"); From 9b6d7a036f2685df14f5bcb5a85069518d9b7d60 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 15:03:42 +0200 Subject: [PATCH 06/10] Better naming --- tower-http/src/builder.rs | 6 ++-- tower-http/src/conditional_response.rs | 40 +++++++++++++------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/tower-http/src/builder.rs b/tower-http/src/builder.rs index b5a841de..6db0528f 100644 --- a/tower-http/src/builder.rs +++ b/tower-http/src/builder.rs @@ -75,7 +75,7 @@ pub trait ServiceBuilderExt: crate::sealed::Sealed + Sized { fn conditional_response( self, responder: R, - ) -> ServiceBuilder, L>>; + ) -> ServiceBuilder, L>>; /// Apply a transformation to the request body. /// @@ -403,8 +403,8 @@ impl ServiceBuilderExt for ServiceBuilder { fn conditional_response( self, responder: R, - ) -> ServiceBuilder, L>> { - self.layer(crate::conditional_response::CondResponseLayer::new(responder)) + ) -> ServiceBuilder, L>> { + self.layer(crate::conditional_response::ConditionalResponseLayer::new(responder)) } #[cfg(feature = "map-request-body")] diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index 70015f9e..54e70df5 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -34,7 +34,7 @@ //! use http::{Request, Response}; //! use std::convert::Infallible; //! use tower::{Service, ServiceExt, ServiceBuilder}; -//! use tower::conditional_response::CondResponseLayer; +//! use tower::conditional_response::ConditionalResponseLayer; //! //! # #[tokio::main] //! # async fn main() -> Result<(), Box> { @@ -59,7 +59,7 @@ //! // //! // Directly wrap the target service with the conditional responder layer //! // -//! .layer(CondResponseLayer::new(responder)) +//! .layer(ConditionalResponseLayer::new(responder)) //! .service_fn(handle); //! //! let request = Request::builder().header("x-so-we-skip","true").body("".to_string()).expect("Expected an empty body"); @@ -83,32 +83,32 @@ use tower_layer::Layer; use tower_service::Service; use pin_project::pin_project; -/// Layer that applies [`CondResponse`] which allows the caller to generate/return a response instead of calling the +/// Layer that applies [`ConditionalResponseService`] which allows the caller to generate/return a response instead of calling the /// inner service - useful for stacks where a default response (rather than an error) is determined by a pre-service /// filter. /// /// See the [module docs](crate::conditional_response) for more details. #[derive(Clone, Debug)] -pub struct CondResponseLayer

{ +pub struct ConditionalResponseLayer

{ responder: P } -impl

CondResponseLayer

+impl

ConditionalResponseLayer

{ - /// Create a new [`CondResponseLayer`]. + /// Create a new [`ConditionalResponseLayer`]. pub fn new(responder:P) -> Self { Self { responder } } } -impl Layer for CondResponseLayer

+impl Layer for ConditionalResponseLayer

where P: Clone { - type Service = CondResponse; + type Service = ConditionalResponseService; fn layer(&self, inner: S) -> Self::Service { - CondResponse:: { + ConditionalResponseService:: { inner, responder: self.responder.clone(), } @@ -119,29 +119,29 @@ where /// /// See the [module docs](crate::conditional_response) for more details. #[derive(Clone,Debug)] -pub struct CondResponse { +pub struct ConditionalResponseService { inner: S, responder: P, } -impl CondResponse +impl ConditionalResponseService { - /// Create a new [`CondResponse`] with the inner service and the "responder" function. + /// Create a new [`ConditionalResponseService`] with the inner service and the "responder" function. pub fn new(inner: S, responder: P) -> Self { Self { inner, responder } } define_inner_service_accessors!(); - /// Returns a new [`Layer`] that wraps services with a `CondResponse` middleware. + /// Returns a new [`Layer`] that wraps services with a `ConditionalResponseService` middleware. /// /// [`Layer`]: tower_layer::Layer - pub fn layer(responder: P) -> CondResponseLayer

{ - CondResponseLayer::

::new(responder) + pub fn layer(responder: P) -> ConditionalResponseLayer

{ + ConditionalResponseLayer::

::new(responder) } } -impl Service> for CondResponse +impl Service> for ConditionalResponseService where S: Service, Response = Response> + Clone + Send + 'static, P: ConditionalResponder,Response>, @@ -165,7 +165,7 @@ where } -/// Response future for [`CondResponse`]. +/// Response future for [`ConditionalResponseService`]. /// /// We use an enum because the inner content may be a future or /// or may be a direct response. @@ -244,7 +244,7 @@ where use std::convert::Infallible; use tower::{Service, ServiceExt, ServiceBuilder}; use crate::builder::ServiceBuilderExt; - use crate::conditional_response::CondResponseLayer; + use crate::conditional_response::ConditionalResponseLayer; fn responder(request: Request) -> ConditionalResponse,Response> { match request.headers().get("x-so-we-skip") { @@ -260,7 +260,7 @@ where #[test] fn skip_test() { let mut svc = ServiceBuilder::new() - .layer(CondResponseLayer::new(responder)) + .layer(ConditionalResponseLayer::new(responder)) .service_fn(handle); let request = Request::builder().header("x-so-we-skip","true").body("".to_string()).expect("Expected an empty body"); @@ -274,7 +274,7 @@ where #[test] fn no_skip_test_header() { let mut svc = ServiceBuilder::new() - .layer(CondResponseLayer::new(responder)) + .layer(ConditionalResponseLayer::new(responder)) .service_fn(handle); let request = Request::builder().header("x-so-we-skip","not true").body("".to_string()).expect("Expected an empty body"); From cb8cfb71f1b4587b6d01c70039387b34f13ba9a1 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 15:41:04 +0200 Subject: [PATCH 07/10] Discovering the --all-features flag on cargo test! --- tower-http/src/builder.rs | 6 +++--- tower-http/src/conditional_response.rs | 9 ++++++++- tower-http/src/lib.rs | 6 +++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tower-http/src/builder.rs b/tower-http/src/builder.rs index 6db0528f..c8985a3d 100644 --- a/tower-http/src/builder.rs +++ b/tower-http/src/builder.rs @@ -40,7 +40,7 @@ use tower_layer::Stack; /// # service.ready().await.unwrap().call(Request::new(Body::empty())).await.unwrap(); /// # } /// ``` -#[cfg(any(test,feature = "util"))] +#[cfg(feature = "util")] // ^ work around rustdoc not inferring doc(cfg)s for cfg's from surrounding scopes pub trait ServiceBuilderExt: crate::sealed::Sealed + Sized { /// Propagate a header from the request to the response. @@ -71,7 +71,7 @@ pub trait ServiceBuilderExt: crate::sealed::Sealed + Sized { /// See [`tower_http::conditional_response`] for more details. /// /// [`tower_http::conditional_response`]: crate::conditional_response - #[cfg(any(test,feature = "conditional-response"))] + #[cfg(feature = "conditional-response")] fn conditional_response( self, responder: R, @@ -399,7 +399,7 @@ impl ServiceBuilderExt for ServiceBuilder { self.layer(crate::add_extension::AddExtensionLayer::new(value)) } - #[cfg(any(test,feature = "conditional-response"))] + #[cfg(feature = "conditional-response")] fn conditional_response( self, responder: R, diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index 54e70df5..eed1523b 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -34,7 +34,8 @@ //! use http::{Request, Response}; //! use std::convert::Infallible; //! use tower::{Service, ServiceExt, ServiceBuilder}; -//! use tower::conditional_response::ConditionalResponseLayer; +//! use tower_http::conditional_response::ConditionalResponse; +//! use tower_http::conditional_response::ConditionalResponseLayer; //! //! # #[tokio::main] //! # async fn main() -> Result<(), Box> { @@ -175,7 +176,13 @@ where #[derive(Debug)] #[pin_project(project = ResponseFutureProj)] pub enum ResponseFuture { + /// + /// The future contains a direct response to return on first poll + /// Response(Option), + /// + /// The future contains a "child" future that should be polled + /// Future(#[pin] F), } diff --git a/tower-http/src/lib.rs b/tower-http/src/lib.rs index e3a070cf..734c4484 100644 --- a/tower-http/src/lib.rs +++ b/tower-http/src/lib.rs @@ -320,7 +320,7 @@ pub mod request_id; #[cfg(feature = "catch-panic")] pub mod catch_panic; -#[cfg(any(test,feature = "conditional-response"))] +#[cfg(feature = "conditional-response")] pub mod conditional_response; #[cfg(feature = "set-status")] @@ -335,10 +335,10 @@ pub mod normalize_path; pub mod classify; pub mod services; -#[cfg(any(test,feature = "util"))] +#[cfg(feature = "util")] mod builder; -#[cfg(any(test,feature = "util"))] +#[cfg(feature = "util")] #[doc(inline)] pub use self::builder::ServiceBuilderExt; From da15ca81fd074f77f5d661a323c22007b3b048e7 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 15:52:35 +0200 Subject: [PATCH 08/10] Better use of ServiceBuilderExt in example --- tower-http/src/conditional_response.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index eed1523b..39b91521 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -35,7 +35,7 @@ //! use std::convert::Infallible; //! use tower::{Service, ServiceExt, ServiceBuilder}; //! use tower_http::conditional_response::ConditionalResponse; -//! use tower_http::conditional_response::ConditionalResponseLayer; +//! use tower_http::ServiceBuilderExt; //! //! # #[tokio::main] //! # async fn main() -> Result<(), Box> { @@ -60,7 +60,7 @@ //! // //! // Directly wrap the target service with the conditional responder layer //! // -//! .layer(ConditionalResponseLayer::new(responder)) +//! .conditional_response(responder) //! .service_fn(handle); //! //! let request = Request::builder().header("x-so-we-skip","true").body("".to_string()).expect("Expected an empty body"); From f7b24798e09d38ca63f8b168e6083fa29e86201a Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 16:25:44 +0200 Subject: [PATCH 09/10] Add reasoning for not using Result --- tower-http/src/conditional_response.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index 39b91521..155adeda 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -27,7 +27,16 @@ //! * Mocking //! * Debugging //! * ... + +//! The function signature has to be: +//! +//! ``` +//! fn responder(request: request type) -> conditional_response::ConditionalResponse { +//! ``` //! +//! Note in particular that there is no [`Result`] - if you have an error you should just generate an error response +//! (or panic and rely on a panic_trapping layer to sort things out) +//! //! # Example //! //! ```rust From a37fd8c3cc38883865f20ad6e1db80297bce02c3 Mon Sep 17 00:00:00 2001 From: Martin Bartlett Date: Mon, 23 Oct 2023 18:18:53 +0200 Subject: [PATCH 10/10] Ignore doc that's not supposed to be a test --- tower-http/src/conditional_response.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tower-http/src/conditional_response.rs b/tower-http/src/conditional_response.rs index 155adeda..3c6095a3 100644 --- a/tower-http/src/conditional_response.rs +++ b/tower-http/src/conditional_response.rs @@ -30,8 +30,8 @@ //! The function signature has to be: //! -//! ``` -//! fn responder(request: request type) -> conditional_response::ConditionalResponse { +//! ```ignore +//! fn responder(request: request type) -> conditional_response::ConditionalResponse //! ``` //! //! Note in particular that there is no [`Result`] - if you have an error you should just generate an error response