feat(unboxed_closures): allow unboxed closures for handlers and error handlers

examples/example_route_data.rs

@@ -1,25 +1,13 @@
-extern crate nickel;
+#[macro_use] extern crate nickel;
 
-use nickel::{Nickel, Request, Response, HttpRouter, MiddlewareResult};
+use nickel::{Nickel, HttpRouter};
 use std::sync::atomic::AtomicUsize;
 use std::sync::atomic::Ordering::Relaxed;
 
-struct Logger {
-    visits: AtomicUsize
-}
-
 fn main() {
     let mut server = Nickel::new();
+    let visits = AtomicUsize::new(0);
 
-    fn root_handler<'a>(_: &mut Request, response: Response<'a>, logger: &Logger)
-            -> MiddlewareResult<'a> {
-        let text = format!("{}", logger.visits.fetch_add(1, Relaxed));
-        response.send(text)
-    }
-
-    // issue #20178
-    let rhandler: for <'a> fn(&mut Request, Response<'a>, &Logger) -> MiddlewareResult<'a> = root_handler;
-
-    server.get("/", (rhandler, Logger{visits: AtomicUsize::new(0)}));
+    server.get("/", middleware! { format!("{}", visits.fetch_add(1, Relaxed)) });
     server.listen("127.0.0.1:6767");
 }

src/macros.rs

@@ -12,6 +12,31 @@ macro_rules! router {
     )
 }
 
+/// Macro to reduce the boilerplate required for using unboxed
+/// closures as `Middleware` due to current type inference behaviour.
+///
+/// In future, the macro should hopefully be able to be removed while
+/// having minimal changes to the closure's code.
+///
+/// # Examples
+/// ```rust,no_run
+/// # #[macro_use] extern crate nickel;
+/// # fn main() {
+/// use nickel::{Nickel, HttpRouter};
+/// use std::sync::atomic::{AtomicUsize, Ordering};
+///
+/// let mut server = Nickel::new();
+///
+/// // Some shared resource between requests, must be `Sync + Send`
+/// let visits = AtomicUsize::new(0);
+///
+/// server.get("/", middleware! { |_req, _res|
+///     format!("{}", visits.fetch_add(1, Ordering::Relaxed))
+/// });
+///
+/// server.listen("127.0.0.1:6767");
+/// # }
+/// ```
 #[macro_export]
 macro_rules! middleware {
     (@$f:ident) => {{
@@ -28,16 +53,26 @@ macro_rules! middleware {
             r.respond(res)
         }
 
-        #[allow(unused_variables)]
-        fn f<'a>($req: &mut Request, $res: Response<'a>)
-                -> MiddlewareResult<'a> {
-            restrict(as_block!({$($b)+}), $res)
-        }
+        #[inline(always)]
+        fn ignore_unused(_: &Request, _: &Response) {}
 
-        middleware!(@f)
+        // Inference fails due to thinking it's a (&Request, Response) with
+        // different mutability requirements
+        #[inline(always)]
+        fn restrict_closure<F>(f: F) -> F
+            where F: for<'r, 'b, 'a>
+                        Fn(&'r mut Request<'b, 'a, 'b>, Response<'a>)
+                            -> MiddlewareResult<'a> + Send + Sync { f }
+
+        restrict_closure(move |$req, $res| {
+            ignore_unused($req, &$res);
+            restrict(as_block!({$($b)+}), $res)
+        })
     }};
     (|$req:ident| $($b:tt)+) => { middleware!(|$req, res| $($b)+) };
     ($($b:tt)+) => { middleware!(|req, res| $($b)+) };
 }
 
-#[macro_export] macro_rules! as_block { ($b:block) => ( $b ) }
+#[doc(hidden)]
+#[macro_export]
+macro_rules! as_block { ($b:block) => ( $b ) }

src/middleware_handler.rs

@@ -20,21 +20,14 @@ use middleware::{Middleware, MiddlewareResult, Halt, Continue};
 use serialize::json;
 use mimes::{MediaType, get_media_type};
 use std::io::Write;
+use std::ops::Fn;
 
-impl Middleware for for<'a> fn(&mut Request, Response<'a>) -> MiddlewareResult<'a> {
+impl<T> Middleware for T where T: for<'r, 'b, 'a> Fn(&'r mut Request<'b, 'a, 'b>, Response<'a>) -> MiddlewareResult<'a> + Send + Sync + 'static {
     fn invoke<'a, 'b>(&'a self, req: &mut Request<'b, 'a, 'b>, res: Response<'a>) -> MiddlewareResult<'a> {
         (*self)(req, res)
     }
 }
 
-impl<T> Middleware for (for <'a> fn(&mut Request, Response<'a>, &T) -> MiddlewareResult<'a>, T)
-        where T: Send + Sync + 'static {
-    fn invoke<'a, 'b>(&'a self, req: &mut Request<'b, 'a, 'b>, res: Response<'a>) -> MiddlewareResult<'a> {
-        let (f, ref data) = *self;
-        f(req, res, data)
-    }
-}
-
 /// This trait provides convenience for translating a number
 /// of common return types into a `MiddlewareResult` while
 /// also modifying the `Response` as required.

src/router/http_router.rs

@@ -18,21 +18,22 @@ pub trait HttpRouter {
     /// use regex::Regex;
     ///
     /// fn main() {
-    ///     let read_handler = middleware! { "Get request! "};
-    ///     let modify_handler = middleware! { |request|
-    ///         format!("Method is: {}", request.origin.method)
-    ///     };
-    ///
     ///     let mut server = Nickel::new();
     ///
-    ///     server.add_route(Get, "/foo", read_handler);
-    ///     server.add_route(Post, "/foo", modify_handler);
-    ///     server.add_route(Put, "/foo", modify_handler);
-    ///     server.add_route(Delete, "/foo", modify_handler);
+    ///     server.add_route(Get, "/foo", middleware! { "Get request! "});
+    ///     server.add_route(Post, "/foo", middleware! { |request|
+    ///         format!("Method is: {}", request.origin.method)
+    ///     });
+    ///     server.add_route(Put, "/foo", middleware! { |request|
+    ///         format!("Method is: {}", request.origin.method)
+    ///     });
+    ///     server.add_route(Delete, "/foo", middleware! { |request|
+    ///         format!("Method is: {}", request.origin.method)
+    ///     });
     ///
     ///     // Regex path
     ///     let regex = Regex::new("/(foo|bar)").unwrap();
-    ///     server.add_route(Get, regex, read_handler);
+    ///     server.add_route(Get, regex, middleware! { "Regex Get request! "});
     /// }
     /// ```
     fn add_route<M: Into<Matcher>, H: Middleware>(&mut self, Method, M, H);

src/router/router.rs

@@ -181,11 +181,10 @@ fn creates_valid_regex_for_routes () {
 #[test]
 fn can_match_var_routes () {
     let route_store = &mut Router::new();
-    let handler = middleware! { "hello from foo" };
 
-    route_store.add_route(Method::Get, "/foo/:userid", handler);
-    route_store.add_route(Method::Get, "/bar", handler);
-    route_store.add_route(Method::Get, "/file/:format/:file", handler);
+    route_store.add_route(Method::Get, "/foo/:userid", middleware! { "hello from foo" });
+    route_store.add_route(Method::Get, "/bar", middleware! { "hello from foo" });
+    route_store.add_route(Method::Get, "/file/:format/:file", middleware! { "hello from foo" });
 
     let route_result = route_store.match_route(&Method::Get, "/foo/4711").unwrap();
     assert_eq!(route_result.param("userid"), "4711");
@@ -249,10 +248,9 @@ fn regex_path() {
     use regex::Regex;
 
     let route_store = &mut Router::new();
-    let handler = middleware! { "hello from foo" };
 
     let regex = Regex::new("/(foo|bar)").unwrap();
-    route_store.add_route(Method::Get, regex, handler);
+    route_store.add_route(Method::Get, regex, middleware! { "hello from foo" });
 
     let route_result = route_store.match_route(&Method::Get, "/foo");
     assert!(route_result.is_some());
@@ -272,10 +270,9 @@ fn regex_path_named() {
     use regex::Regex;
 
     let route_store = &mut Router::new();
-    let handler = middleware! { "hello from foo" };
 
     let regex = Regex::new("/(?P<a>foo|bar)/b").unwrap();
-    route_store.add_route(Method::Get, regex, handler);
+    route_store.add_route(Method::Get, regex, middleware! { "hello from foo" });
 
     let route_result = route_store.match_route(&Method::Get, "/foo/b");
     assert!(route_result.is_some());
@@ -298,11 +295,10 @@ fn ignores_querystring() {
     use regex::Regex;
 
     let route_store = &mut Router::new();
-    let handler = middleware! { "hello from foo" };
 
     let regex = Regex::new("/(?P<a>foo|bar)/b").unwrap();
-    route_store.add_route(Method::Get, regex, handler);
-    route_store.add_route(Method::Get, "/:foo", handler);
+    route_store.add_route(Method::Get, regex, middleware! { "hello from foo" });
+    route_store.add_route(Method::Get, "/:foo", middleware! { "hello from foo" });
 
     // Should ignore the querystring
     let route_result = route_store.match_route(&Method::Get, "/moo?foo");