Repository: seanmonstar/warp (language: rust)

Issue:
Custom `with` filters
Under the current api, `Filter::with` requires that the type be `W: Wrap` however the `Wrap` trait is sealed making it difficult to write custom wrapping filters. For instance, I'm trying to write a filter that will forward a header from a request to the response. So far I've come up with the filter to generate the either grab the request id or generate a new one:

```rust
let req_id = warp::header::<String>("x-request-id").or_else(|_| Ok(("generated".to_owned(),)))
```
But I can't figure out using the combinators provided how to store the request-id in the filter and use it on the response.

It also seems that you cannot call `map` after returning a `warp::reply` in the combinator chain so I can't even make the above work with boiler plate something like

```rust
let my_route = req_id.map(warp::reply).map(|id: String, reply: warp::Reply| {
    reply.set_header("x-request-id", id)
})
```


Discussion / hints:
Right, the `with` and `Wrap` thing is sealed since it is pretty new, and I wasn't sure yet if it could fit all needs.

In your example, I wouldn't expect `req_id.map(warp::reply)` to work, since the `req_id` filter has 1 value extracted, and `warp::reply()` takes 0 arguments...

Would this work for you?

```rust
let my_route = req_id.map(|id| {
    Response::builder()
        .header("x-request-id", id)
        .body("")
})
```
> Right, the with and Wrap thing is sealed since it is pretty new, and I wasn't sure yet if it could fit all needs.

Totally understood. The library is excellent so far and after handlers have a lot more complexity.

> Would this work for you?

Ya the composition is a little awkward since this is a filter that I'd like to be universally be a applied. But I landed on something like this:

```rust
    let req_id = warp::header::<String>("x-request-id").or_else(|_| {
        Ok(("deadbeef".to_owned(),))
    }).map(|id: String| {
        let mut res = Response::builder();
        res.header("x-request-id", id.as_str());

        res
    }).map(|mut res: response::Builder| {
        res.body("bar").unwrap()
    });
```

Which allows me to compose different body handlers after, and filters that don't pass along arguments before.
I'm learning that writing these is kinda non-trivial: you need to name the `Wrapped` filter type, which means you effectively have to write the `Filter` impl by hand, and avoid any closures. This in turn leads you to need to write a `Future` impl by hand. I hope some of this gets easier with existential types, but in the meantime it's kinda rough.

This abstraction, or one like it, is pretty necessary though: it's the only way to put something on "both sides" of a filter, which is a common middleware requirement.
@kamalmarhubi yep exactly! I've been working on a CORS filter, and it needs to be a `Wrap`, and I want to make use of the other filters, so I've resigned it to just returning `Filter::boxed()` for now.
Has there been any progress on this issue recently? Warp is awesome and adding the ability to inject middleware using custom `with` filters is the last major piece I'm missing for productivity.
I think this can now be achieved via:
```Rust
use std::convert::Infallible;
use warp::Filter;
use hyper::{Body, Request};
use tower_service::Service;

#[tokio::main]
async fn main() -> Result<(), hyper::error::Error> {
    let route = warp::any().map(|| "Hello From Warp!");
    let mut warp_svc = warp::service(route);
    let make_svc = hyper::service::make_service_fn(move |_| async move {
        let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
            println!("before request");
            let resp = warp_svc.call(req).await;
            println!("after request");
            resp
        });
        Ok::<_, Infallible>(svc)
    });

    hyper::Server::bind(&([127, 0, 0, 1], 3030).into())
        .serve(make_svc)
        .await?;

    Ok(())
}
```
@jxs it worked for me, but my use case is a little more complex:

If I change the route to be a BoxedFilter (to turn easier return the route from a function):
```rust
let route = warp::any().map(|| "Hello From Warp!").boxed();
```
It fails to compile:
```shell
$ cargo run

   Compiling warp-report-1 v0.1.0 (/home/fernando/repos/sandbox/warp-report-1)
error[E0507]: cannot move out of `warp_svc`, a captured variable in an `FnMut` closure
  --> src/main.rs:11:83
   |
9  |       let mut warp_svc = warp::service(route);
   |           ------------ captured outer variable
10 |       let make_svc = hyper::service::make_service_fn(move |_| async move {
11 |           let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
   |  ___________________________________________________________________________________^
12 | |             println!("before request");
13 | |             let resp = warp_svc.call(req).await;
   | |                        --------
   | |                        |
   | |                        move occurs because `warp_svc` has type `warp::filter::service::FilteredService<warp::filter::boxed::BoxedFilter<(&str,)>>`, which does not implement the `Copy` trait
   | |                        move occurs due to use in generator
14 | |             println!("after request");
15 | |             resp
16 | |         });
   | |_________^ move out of `warp_svc` occurs here

error[E0507]: cannot move out of `warp_svc`, a captured variable in an `FnMut` closure
  --> src/main.rs:10:72
   |
9  |       let mut warp_svc = warp::service(route);
   |           ------------ captured outer variable
10 |       let make_svc = hyper::service::make_service_fn(move |_| async move {
   |  ________________________________________________________________________^
11 | |         let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
12 | |             println!("before request");
13 | |             let resp = warp_svc.call(req).await;
   | |                        --------
   | |                        |
   | |                        move occurs because `warp_svc` has type `warp::filter::service::FilteredService<warp::filter::boxed::BoxedFilter<(&str,)>>`, which does not implement the `Copy` trait
   | |                        move occurs due to use in generator
...  |
17 | |         Ok::<_, Infallible>(svc)
18 | |     });
   | |_____^ move out of `warp_svc` occurs here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0507`.
error: could not compile `warp-report-1`.

To learn more, run the command again with --verbose.
```

-----

And if I don't use the boxed filter, but add "state" filter:
```rust
    let http_client = hyper::Client::new();

    let route = warp::any()
            .and(warp::any().map(move || http_client.clone()))
            .map(|_http_client| "Hello From Warp!");
```

it will fail with a similar error:
```shell
$ cargo run
   Compiling warp-report-1 v0.1.0 (/home/fernando/repos/sandbox/warp-report-1)
error[E0507]: cannot move out of `warp_svc`, a captured variable in an `FnMut` closure
  --> src/main.rs:20:83
   |
18 |       let mut warp_svc = warp::service(route);
   |           ------------ captured outer variable
19 |       let make_svc = hyper::service::make_service_fn(move |_| async move {
20 |           let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
   |  ___________________________________________________________________________________^
21 | |             println!("before request");
22 | |             let resp = warp_svc.call(req).await;
   | |                        --------
   | |                        |
   | |                        move occurs because `warp_svc` has type `warp::filter::service::FilteredService<warp::filter::map::Map<warp::filter::and::And<impl warp::filter::Filter+std::marker::Copy, warp::filter::map::Map<impl warp::filter::Filter+std::marker::Copy, [closure@src/main.rs:15:34: 15:61 http_client:hyper::client::Client<hyper::client::connect::http::HttpConnector>]>>, [closure@src/main.rs:16:18: 16:51]>>`, which does not implement the `Copy` trait
   | |                        move occurs due to use in generator
23 | |             println!("after request");
24 | |             resp
25 | |         });
   | |_________^ move out of `warp_svc` occurs here

error[E0507]: cannot move out of `warp_svc`, a captured variable in an `FnMut` closure
  --> src/main.rs:19:72
   |
18 |       let mut warp_svc = warp::service(route);
   |           ------------ captured outer variable
19 |       let make_svc = hyper::service::make_service_fn(move |_| async move {
   |  ________________________________________________________________________^
20 | |         let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
21 | |             println!("before request");
22 | |             let resp = warp_svc.call(req).await;
   | |                        --------
   | |                        |
   | |                        move occurs because `warp_svc` has type `warp::filter::service::FilteredService<warp::filter::map::Map<warp::filter::and::And<impl warp::filter::Filter+std::marker::Copy, warp::filter::map::Map<impl warp::filter::Filter+std::marker::Copy, [closure@src/main.rs:15:34: 15:61 http_client:hyper::client::Client<hyper::client::connect::http::HttpConnector>]>>, [closure@src/main.rs:16:18: 16:51]>>`, which does not implement the `Copy` trait
   | |                        move occurs due to use in generator
...  |
26 | |         Ok::<_, Infallible>(svc)
27 | |     });
   | |_____^ move out of `warp_svc` occurs here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0507`.
error: could not compile `warp-report-1`.

To learn more, run the command again with --verbose.
```

-----

The same seems to occur when I added the cors wrapper:
```rust
    let route = warp::any()
        .map(|| "Hello From Warp!")
        .with(warp::cors().allow_any_origin());
```
it failed with a similar error too:
```shell
$ cargo run
   Compiling warp-report-1 v0.1.0 (/home/fernando/repos/sandbox/warp-report-1)
error[E0507]: cannot move out of `warp_svc`, a captured variable in an `FnMut` closure
  --> src/main.rs:18:83
   |
16 |       let mut warp_svc = warp::service(route);
   |           ------------ captured outer variable
17 |       let make_svc = hyper::service::make_service_fn(move |_| async move {
18 |           let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
   |  ___________________________________________________________________________________^
19 | |             println!("before request");
20 | |             let resp = warp_svc.call(req).await;
   | |                        --------
   | |                        |
   | |                        move occurs because `warp_svc` has type `warp::filter::service::FilteredService<warp::filters::cors::internal::CorsFilter<warp::filter::map::Map<impl warp::filter::Filter+std::marker::Copy, [closure@src/main.rs:13:14: 13:35]>>>`, which does not implement the `Copy` trait
   | |                        move occurs due to use in generator
21 | |             println!("after request");
22 | |             resp
23 | |         });
   | |_________^ move out of `warp_svc` occurs here

error[E0507]: cannot move out of `warp_svc`, a captured variable in an `FnMut` closure
  --> src/main.rs:17:72
   |
16 |       let mut warp_svc = warp::service(route);
   |           ------------ captured outer variable
17 |       let make_svc = hyper::service::make_service_fn(move |_| async move {
   |  ________________________________________________________________________^
18 | |         let svc = hyper::service::service_fn(move |req: Request<Body>| async move {
19 | |             println!("before request");
20 | |             let resp = warp_svc.call(req).await;
   | |                        --------
   | |                        |
   | |                        move occurs because `warp_svc` has type `warp::filter::service::FilteredService<warp::filters::cors::internal::CorsFilter<warp::filter::map::Map<impl warp::filter::Filter+std::marker::Copy, [closure@src/main.rs:13:14: 13:35]>>>`, which does not implement the `Copy` trait
   | |                        move occurs due to use in generator
...  |
24 | |         Ok::<_, Infallible>(svc)
25 | |     });
   | |_____^ move out of `warp_svc` occurs here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0507`.
error: could not compile `warp-report-1`.

To learn more, run the command again with --verbose.
```

-----

I'm new to the language, so I don't know if I'm doing something wrong on the code, but it seems that some filters are missing the implementation of the `Copy` trait. 

The custom `with` filter is super important to my application, and it is blocking me to release it in production.

Please, let me know if I should do something in a different way, or if I can help in someway!

Thank you!
In order to avoid the problem of "move out of" errors, you can try putting `.clone()` on filters. Not all of them implement `Copy`, but they should all implement `Clone`.

It may require cloning at each step of the `move` closures to actually work, but it shouldn't be impossible
@asonix thank you! It worked! I've tried clone the route before, but not as much as I needed.

I'll post an example of that here, for future reference if anyone face the same error:

```rust
use std::convert::Infallible;
use warp::Filter;
use hyper::{Body, Request};
use tower_service::Service;

#[tokio::main]
async fn main() -> Result<(), hyper::error::Error> {
    let http_client = hyper::Client::new();

    let route = warp::any()
        .and(warp::any().map(move || http_client.clone()))
        .map(|_http_client| "Hello From Warp!")
        .with(warp::cors().allow_any_origin())
        .boxed();

    let warp_svc = warp::service(route);
    let make_svc = hyper::service::make_service_fn(move |_| {
        let warp_svc = warp_svc.clone();
        async move {
            let svc = hyper::service::service_fn(move |req: Request<Body>| {
                let mut warp_svc = warp_svc.clone();    
                async move {
                    println!("before request");
                    let resp = warp_svc.call(req).await;
                    println!("after request");
                    resp
                }
            });
            Ok::<_, Infallible>(svc)
        }
    });

    hyper::Server::bind(&([127, 0, 0, 1], 3030).into())
        .serve(make_svc)
        .await?;

    Ok(())
}
```
In my opinion the current advised solution of using hyper service conversion to implement "wrapping" functionality isn't good enough. As @hawkw put it in #408, this is just not a very "warpy" API. If I'm not missing something, you can't convert a hyper service back to a filter, which breaks the complete composability chain.

I understood the reason to seal wrap was that @seanmonstar wasn't sure yet if it could fit all needs. This was almost 2 years ago though. The API didn't really change since and PRs like #408 and #513 show that it is useful in its current form. So to move forward with this RFC I think it would make sense to either collect problems we are seeing with the current API, or allow custom `with` / `Wrap` implementations in user land.
I must also admit to becoming a bit frustrated with the lack of easy customization with warp filters.  One of the strengths of FP is the ability to mix and match and customize due to referential transparency, but warp hides all of its customizable parts behind `private(crate)` flags, and doesn't provide any hooks to make our own custom filters.

I'm looking to make a custom authentication filter which combines a few steps (getting the auth header, checking the token with a configurable service, and returning the identity data).  I can do it with a ton of snaking .ands .and_thens .or_else and .recovers, but it'd be easier if I could just have a `.and(authn(service_config))` and I can get all of the twisty passages, all alike into a simple filter for others to use.

Why is this being locked away behind private flags?

Fix patch:
diff --git a/examples/wrapping.rs b/examples/wrapping.rs
new file mode 100644
index 000000000..bfc84f980
--- /dev/null
+++ b/examples/wrapping.rs
@@ -0,0 +1,31 @@
+#![deny(warnings)]
+use warp::Filter;
+
+fn hello_wrapper<F, T>(
+    filter: F,
+) -> impl Filter<Extract = (&'static str,)> + Clone + Send + Sync + 'static
+where
+    F: Filter<Extract = (T,), Error = std::convert::Infallible> + Clone + Send + Sync + 'static,
+    F::Extract: warp::Reply,
+{
+    warp::any()
+        .map(|| {
+            println!("before filter");
+        })
+        .untuple_one()
+        .and(filter)
+        .map(|_arg| "wrapped hello world")
+}
+
+#[tokio::main]
+async fn main() {
+    // Match any request and return hello world!
+    let routes = warp::any()
+        .map(|| "hello world")
+        .boxed()
+        .recover(|_err| async { Ok("recovered") })
+        // wrap the filter with hello_wrapper
+        .with(warp::wrap_fn(hello_wrapper));
+
+    warp::serve(routes).run(([127, 0, 0, 1], 3030)).await;
+}
diff --git a/src/filter/mod.rs b/src/filter/mod.rs
index 55a458d95..851806bb7 100644
--- a/src/filter/mod.rs
+++ b/src/filter/mod.rs
@@ -30,6 +30,7 @@ use self::or_else::OrElse;
 use self::recover::Recover;
 use self::unify::Unify;
 use self::untuple_one::UntupleOne;
+pub use self::wrap::wrap_fn;
 pub(crate) use self::wrap::{Wrap, WrapSealed};
 
 // A crate-private base trait, allowing the actual `filter` method to change
diff --git a/src/filter/wrap.rs b/src/filter/wrap.rs
index 2226286e2..377b0ef0e 100644
--- a/src/filter/wrap.rs
+++ b/src/filter/wrap.rs
@@ -25,3 +25,31 @@ where
     F: Filter,
 {
 }
+
+/// Function that receives a filter to be combined with pre and after filters
+pub fn wrap_fn<F, T, U>(func: F) -> WrapFn<F>
+where
+    F: Fn(T) -> U,
+    T: Filter,
+    U: Filter,
+{
+    WrapFn { func }
+}
+
+#[derive(Debug)]
+pub struct WrapFn<F> {
+    func: F,
+}
+
+impl<F, T, U> WrapSealed<T> for WrapFn<F>
+where
+    F: Fn(T) -> U,
+    T: Filter,
+    U: Filter,
+{
+    type Wrapped = U;
+
+    fn wrap(&self, filter: T) -> Self::Wrapped {
+        (self.func)(filter)
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 65469dc5d..c4c6a016f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -149,6 +149,7 @@ pub use self::filters::{
     trace::trace,
 };
 // ws() function
+pub use self::filter::wrap_fn;
 #[cfg(feature = "websocket")]
 #[doc(hidden)]
 pub use self::filters::ws::ws;


Test patch: