From 16ceb741b8e0f225489397b9d2528b462489af01 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 29 Nov 2017 13:26:55 -0800 Subject: [PATCH] refactor RouteHandler trait --- examples/basic.rs | 9 +- examples/websocket.rs | 3 +- src/application.rs | 35 +++----- src/dev.rs | 1 + src/lib.rs | 2 +- src/resource.rs | 34 +++----- src/route.rs | 197 +++++++++++++++++++----------------------- src/staticfiles.rs | 32 +++---- 8 files changed, 144 insertions(+), 169 deletions(-) diff --git a/examples/basic.rs b/examples/basic.rs index e01bb637f..1b850c7b8 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -82,8 +82,15 @@ fn main() { .header("LOCATION", "/index.html") .body(Body::Empty) })) + .handler("/test", |req| { + match *req.method() { + Method::GET => httpcodes::HTTPOk, + Method::POST => httpcodes::HTTPMethodNotAllowed, + _ => httpcodes::HTTPNotFound, + } + }) // static files - .route_handler("/static", StaticFiles::new("examples/static/", true))) + .route("/static", StaticFiles::new("examples/static/", true))) .serve::<_, ()>("127.0.0.1:8080").unwrap(); println!("Started http server: 127.0.0.1:8080"); diff --git a/examples/websocket.rs b/examples/websocket.rs index f8f4e5672..31d439895 100644 --- a/examples/websocket.rs +++ b/examples/websocket.rs @@ -66,7 +66,8 @@ fn main() { .middleware(middlewares::Logger::default()) // websocket route .resource("/ws/", |r| r.get(ws_index)) - .route_handler("/", StaticFiles::new("examples/static/", true))) + // static files + .route("/", StaticFiles::new("examples/static/", true))) // start http server on 127.0.0.1:8080 .serve::<_, ()>("127.0.0.1:8080").unwrap(); diff --git a/src/application.rs b/src/application.rs index 08160edb9..5f5ead242 100644 --- a/src/application.rs +++ b/src/application.rs @@ -2,7 +2,7 @@ use std::rc::Rc; use std::collections::HashMap; use task::Task; -use route::{RouteHandler, FnHandler, Reply}; +use route::{RouteHandler, WrapHandler, Reply, Handler}; use resource::Resource; use recognizer::{RouteRecognizer, check_pattern}; use httprequest::HttpRequest; @@ -126,10 +126,7 @@ impl ApplicationBuilder where S: 'static { /// store userid and friend in the exposed Params object: /// /// ```rust - /// extern crate actix; /// extern crate actix_web; - /// - /// use actix::*; /// use actix_web::*; /// /// fn main() { @@ -158,7 +155,7 @@ impl ApplicationBuilder where S: 'static { self } - /// Default resource is used if no matches route could be found. + /// Default resource is used if no match route could be found. pub fn default_resource(&mut self, f: F) -> &mut Self where F: FnOnce(&mut Resource) + 'static { @@ -178,7 +175,7 @@ impl ApplicationBuilder where S: 'static { /// /// fn main() { /// let app = Application::default("/") - /// .handler("/test", |req| { + /// .inline("/test", |req| { /// match *req.method() { /// Method::GET => httpcodes::HTTPOk, /// Method::POST => httpcodes::HTTPMethodNotAllowed, @@ -189,28 +186,22 @@ impl ApplicationBuilder where S: 'static { /// } /// ``` pub fn handler(&mut self, path: P, handler: F) -> &mut Self - where F: Fn(HttpRequest) -> R + 'static, - R: Into + 'static, - P: Into, + where P: Into, + F: Fn(HttpRequest) -> R + 'static, + R: Into + 'static { self.parts.as_mut().expect("Use after finish") - .handlers.insert(path.into(), Box::new(FnHandler::new(handler))); + .handlers.insert(path.into(), Box::new(WrapHandler::new(handler))); self } - /// Add path handler - pub fn route_handler(&mut self, path: P, h: H) -> &mut Self - where H: RouteHandler + 'static, P: Into + /// This method register handler for specified path prefix. + /// Any path that starts with this prefix matches handler. + pub fn route(&mut self, path: P, handler: H) -> &mut Self + where P: Into, H: Handler { - { - // add resource - let parts = self.parts.as_mut().expect("Use after finish"); - let path = path.into(); - if parts.handlers.contains_key(&path) { - panic!("Handler already registered: {:?}", path); - } - parts.handlers.insert(path, Box::new(h)); - } + self.parts.as_mut().expect("Use after finish") + .handlers.insert(path.into(), Box::new(WrapHandler::new(handler))); self } diff --git a/src/dev.rs b/src/dev.rs index 8ec4a50d1..cd8b7a8a7 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -10,6 +10,7 @@ // dev specific pub use task::Task; +pub use route::Handler; pub use pipeline::Pipeline; pub use recognizer::RouteRecognizer; pub use channel::HttpChannel; diff --git a/src/lib.rs b/src/lib.rs index 7efa9aa86..c94e67897 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,7 +82,7 @@ pub use application::Application; pub use httprequest::{HttpRequest, UrlEncoded}; pub use httpresponse::HttpResponse; pub use payload::{Payload, PayloadItem}; -pub use route::{Frame, RouteHandler, Reply}; +pub use route::{Frame, Reply}; pub use resource::Resource; pub use recognizer::Params; pub use server::HttpServer; diff --git a/src/resource.rs b/src/resource.rs index 93d31cb33..f485b5186 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -6,7 +6,7 @@ use futures::Stream; use task::Task; use error::Error; -use route::{Reply, RouteHandler, Frame, FnHandler, StreamHandler}; +use route::{Reply, RouteHandler, Frame, WrapHandler, Handler, StreamHandler}; use httprequest::HttpRequest; use httpcodes::{HTTPNotFound, HTTPMethodNotAllowed}; @@ -17,13 +17,12 @@ use httpcodes::{HTTPNotFound, HTTPMethodNotAllowed}; /// Resource in turn has at least one route. /// Route corresponds to handling HTTP method by calling route handler. /// -/// ```rust,ignore -/// -/// struct MyRoute; +/// ```rust +/// extern crate actix_web; /// /// fn main() { -/// let router = RoutingMap::default() -/// .resource("/", |r| r.post::()) +/// let app = actix_web::Application::default("/") +/// .resource("/", |r| r.get(|_| actix_web::HttpResponse::Ok())) /// .finish(); /// } pub struct Resource { @@ -63,7 +62,7 @@ impl Resource where S: 'static { where F: Fn(HttpRequest) -> R + 'static, R: Into + 'static, { - self.routes.insert(method, Box::new(FnHandler::new(handler))); + self.routes.insert(method, Box::new(WrapHandler::new(handler))); } /// Register async handler for specified method. @@ -74,51 +73,42 @@ impl Resource where S: 'static { self.routes.insert(method, Box::new(StreamHandler::new(handler))); } - /// Register handler for specified method. - pub fn route_handler(&mut self, method: Method, handler: H) - where H: RouteHandler - { - self.routes.insert(method, Box::new(handler)); - } - /// Default handler is used if no matched route found. /// By default `HTTPMethodNotAllowed` is used. - pub fn default_handler(&mut self, handler: H) - where H: RouteHandler + pub fn default_handler(&mut self, handler: H) where H: Handler { - self.default = Box::new(handler); + self.default = Box::new(WrapHandler::new(handler)); } /// Handler for `GET` method. pub fn get(&mut self, handler: F) where F: Fn(HttpRequest) -> R + 'static, R: Into + 'static, { - self.routes.insert(Method::GET, Box::new(FnHandler::new(handler))); + self.routes.insert(Method::GET, Box::new(WrapHandler::new(handler))); } /// Handler for `POST` method. pub fn post(&mut self, handler: F) where F: Fn(HttpRequest) -> R + 'static, R: Into + 'static, { - self.routes.insert(Method::POST, Box::new(FnHandler::new(handler))); + self.routes.insert(Method::POST, Box::new(WrapHandler::new(handler))); } /// Handler for `PUT` method. pub fn put(&mut self, handler: F) where F: Fn(HttpRequest) -> R + 'static, R: Into + 'static, { - self.routes.insert(Method::PUT, Box::new(FnHandler::new(handler))); + self.routes.insert(Method::PUT, Box::new(WrapHandler::new(handler))); } /// Handler for `DELETE` method. pub fn delete(&mut self, handler: F) where F: Fn(HttpRequest) -> R + 'static, R: Into + 'static, { - self.routes.insert(Method::DELETE, Box::new(FnHandler::new(handler))); + self.routes.insert(Method::DELETE, Box::new(WrapHandler::new(handler))); } } - impl RouteHandler for Resource { fn handle(&self, req: HttpRequest, task: &mut Task) { diff --git a/src/route.rs b/src/route.rs index 3842c769c..f97106384 100644 --- a/src/route.rs +++ b/src/route.rs @@ -4,15 +4,14 @@ use std::marker::PhantomData; use std::result::Result as StdResult; use actix::Actor; -// use http::{header, Version}; use futures::Stream; -use task::{Task, DrainFut, IoContext}; use body::Binary; -use error::{Error}; //, ExpectError, Result}; +use error::Error; use context::HttpContext; use httprequest::HttpRequest; use httpresponse::HttpResponse; +use task::{Task, DrainFut, IoContext}; #[doc(hidden)] #[derive(Debug)] @@ -28,118 +27,32 @@ impl Frame { } } -/// Trait defines object that could be regestered as resource route +/// Trait defines object that could be regestered as route handler #[allow(unused_variables)] -pub trait RouteHandler: 'static { +pub trait Handler: 'static { + type Result: Into; + /// Handle request - fn handle(&self, req: HttpRequest, task: &mut Task); + fn handle(&self, req: HttpRequest) -> Self::Result; /// Set route prefix fn set_prefix(&mut self, prefix: String) {} } -/* -/// Actors with ability to handle http requests. -#[allow(unused_variables)] -pub trait RouteState { - /// Shared state. State is shared with all routes within same application - /// and could be accessed with `HttpRequest::state()` method. - type State; - - /// Handle `EXPECT` header. By default respones with `HTTP/1.1 100 Continue` - fn expect(req: &mut HttpRequest, ctx: &mut Self::Context) -> Result<()> - where Self: Actor> - { - // handle expect header only for HTTP/1.1 - if req.version() == Version::HTTP_11 { - if let Some(expect) = req.headers().get(header::EXPECT) { - if let Ok(expect) = expect.to_str() { - if expect.to_lowercase() == "100-continue" { - ctx.write("HTTP/1.1 100 Continue\r\n\r\n"); - Ok(()) - } else { - Err(ExpectError::UnknownExpect.into()) - } - } else { - Err(ExpectError::Encoding.into()) - } - } else { - Ok(()) - } - } else { - Ok(()) - } - } - - /// Handle incoming request with http actor. - fn handle(req: HttpRequest) -> Result - where Self: Default, Self: Actor> - { - Ok(HttpContext::new(req, Self::default()).into()) - } -}*/ - -/// Fn() route handler -pub(crate) -struct FnHandler +/// Handler for Fn() +impl Handler for F where F: Fn(HttpRequest) -> R + 'static, - R: Into, - S: 'static, + R: Into + 'static { - f: Box, - s: PhantomData, -} + type Result = R; -impl FnHandler - where F: Fn(HttpRequest) -> R + 'static, - R: Into + 'static, - S: 'static, -{ - pub fn new(f: F) -> Self { - FnHandler{f: Box::new(f), s: PhantomData} + fn handle(&self, req: HttpRequest) -> R { + (self)(req) } } -impl RouteHandler for FnHandler - where F: Fn(HttpRequest) -> R + 'static, - R: Into + 'static, - S: 'static, -{ - fn handle(&self, req: HttpRequest, task: &mut Task) { - (self.f)(req).into().into(task) - } -} - -/// Async route handler -pub(crate) -struct StreamHandler - where F: Fn(HttpRequest) -> R + 'static, - R: Stream + 'static, - S: 'static, -{ - f: Box, - s: PhantomData, -} - -impl StreamHandler - where F: Fn(HttpRequest) -> R + 'static, - R: Stream + 'static, - S: 'static, -{ - pub fn new(f: F) -> Self { - StreamHandler{f: Box::new(f), s: PhantomData} - } -} - -impl RouteHandler for StreamHandler - where F: Fn(HttpRequest) -> R + 'static, - R: Stream + 'static, - S: 'static, -{ - fn handle(&self, req: HttpRequest, task: &mut Task) { - task.stream((self.f)(req)) - } -} +/// Represents response process. +pub struct Reply(ReplyItem); enum ReplyItem { Message(HttpResponse), @@ -147,11 +60,8 @@ enum ReplyItem { Stream(Box>), } -/// Represents response process. -pub struct Reply(ReplyItem); +impl Reply { -impl Reply -{ /// Create actor response pub fn actor(ctx: HttpContext) -> Reply where A: Actor>, S: 'static @@ -209,3 +119,78 @@ impl>, S: 'static> From> fo Reply(ReplyItem::Actor(Box::new(item))) } } + +/// Trait defines object that could be regestered as resource route +pub(crate) trait RouteHandler: 'static { + /// Handle request + fn handle(&self, req: HttpRequest, task: &mut Task); + + /// Set route prefix + fn set_prefix(&mut self, _prefix: String) {} +} + +/// Route handler wrapper for Handler +pub(crate) +struct WrapHandler + where H: Handler, + R: Into, + S: 'static, +{ + h: H, + s: PhantomData, +} + +impl WrapHandler + where H: Handler, + R: Into, + S: 'static, +{ + pub fn new(h: H) -> Self { + WrapHandler{h: h, s: PhantomData} + } +} + +impl RouteHandler for WrapHandler + where H: Handler, + R: Into + 'static, + S: 'static, +{ + fn handle(&self, req: HttpRequest, task: &mut Task) { + self.h.handle(req).into().into(task) + } + + fn set_prefix(&mut self, prefix: String) { + self.h.set_prefix(prefix) + } +} + +/// Async route handler +pub(crate) +struct StreamHandler + where F: Fn(HttpRequest) -> R + 'static, + R: Stream + 'static, + S: 'static, +{ + f: Box, + s: PhantomData, +} + +impl StreamHandler + where F: Fn(HttpRequest) -> R + 'static, + R: Stream + 'static, + S: 'static, +{ + pub fn new(f: F) -> Self { + StreamHandler{f: Box::new(f), s: PhantomData} + } +} + +impl RouteHandler for StreamHandler + where F: Fn(HttpRequest) -> R + 'static, + R: Stream + 'static, + S: 'static, +{ + fn handle(&self, req: HttpRequest, task: &mut Task) { + task.stream((self.f)(req)) + } +} diff --git a/src/staticfiles.rs b/src/staticfiles.rs index 0e3761f7a..b4ab7214d 100644 --- a/src/staticfiles.rs +++ b/src/staticfiles.rs @@ -7,9 +7,8 @@ use std::fmt::Write; use std::fs::{File, DirEntry}; use std::path::PathBuf; -use task::Task; -use route::RouteHandler; use mime_guess::get_mime_type; +use route::Handler; use httprequest::HttpRequest; use httpresponse::HttpResponse; use httpcodes::{HTTPOk, HTTPNotFound, HTTPForbidden}; @@ -24,7 +23,7 @@ use httpcodes::{HTTPOk, HTTPNotFound, HTTPForbidden}; /// /// fn main() { /// let app = Application::default("/") -/// .route_handler("/static", StaticFiles::new(".", true)) +/// .handler("/static", StaticFiles::new(".", true)) /// .finish(); /// } /// ``` @@ -128,7 +127,8 @@ impl StaticFiles { } } -impl RouteHandler for StaticFiles { +impl Handler for StaticFiles { + type Result = Result; fn set_prefix(&mut self, prefix: String) { if prefix != "/" { @@ -136,9 +136,9 @@ impl RouteHandler for StaticFiles { } } - fn handle(&self, req: HttpRequest, task: &mut Task) { + fn handle(&self, req: HttpRequest) -> Self::Result { if !self.accessible { - task.reply(HTTPNotFound) + Ok(HTTPNotFound.into()) } else { let mut hidden = false; let filepath = req.path()[self.prefix.len()..] @@ -152,7 +152,7 @@ impl RouteHandler for StaticFiles { // hidden file if hidden { - task.reply(HTTPNotFound) + return Ok(HTTPNotFound.into()) } // full filepath @@ -160,19 +160,19 @@ impl RouteHandler for StaticFiles { let filename = match self.directory.join(&filepath[idx..]).canonicalize() { Ok(fname) => fname, Err(err) => return match err.kind() { - io::ErrorKind::NotFound => task.reply(HTTPNotFound), - io::ErrorKind::PermissionDenied => task.reply(HTTPForbidden), - _ => task.error(err), + io::ErrorKind::NotFound => Ok(HTTPNotFound.into()), + io::ErrorKind::PermissionDenied => Ok(HTTPForbidden.into()), + _ => Err(err), } }; if filename.is_dir() { match self.index(&filepath[idx..], &filename) { - Ok(resp) => task.reply(resp), + Ok(resp) => Ok(resp), Err(err) => match err.kind() { - io::ErrorKind::NotFound => task.reply(HTTPNotFound), - io::ErrorKind::PermissionDenied => task.reply(HTTPForbidden), - _ => task.error(err), + io::ErrorKind::NotFound => Ok(HTTPNotFound.into()), + io::ErrorKind::PermissionDenied => Ok(HTTPForbidden.into()), + _ => Err(err), } } } else { @@ -185,9 +185,9 @@ impl RouteHandler for StaticFiles { Ok(mut file) => { let mut data = Vec::new(); let _ = file.read_to_end(&mut data); - task.reply(resp.body(data).unwrap()) + Ok(resp.body(data).unwrap()) }, - Err(err) => task.error(err), + Err(err) => Err(err), } } }