From 6a33b65f02cd9db179b866af157868409998baeb Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sat, 21 Oct 2017 18:54:24 -0700 Subject: [PATCH] refactor server router --- examples/multipart/src/main.rs | 15 +- examples/websocket-chat/src/main.rs | 29 ++-- examples/websocket/src/main.rs | 8 +- src/application.rs | 219 +++++++++++----------------- src/dev.rs | 1 - src/lib.rs | 2 - src/router.rs | 175 ---------------------- src/server.rs | 81 +++++++--- src/staticfiles.rs | 2 +- tests/test_server.rs | 14 +- 10 files changed, 173 insertions(+), 373 deletions(-) delete mode 100644 src/router.rs diff --git a/examples/multipart/src/main.rs b/examples/multipart/src/main.rs index dc0d6fef8..5b210b2b2 100644 --- a/examples/multipart/src/main.rs +++ b/examples/multipart/src/main.rs @@ -20,7 +20,7 @@ impl Route for MyRoute { let multipart = match req.multipart(payload) { Ok(mp) => mp, - Err(e) => return Reply::reply(e), + Err(e) => return e.into(), }; // get Multipart stream @@ -68,13 +68,12 @@ fn main() { let sys = actix::System::new("multipart-example"); HttpServer::new( - RoutingMap::default() - .app("/", Application::default() - .resource("/multipart", |r| { - r.post::(); - }) - .finish()) - .finish()) + vec![ + Application::default("/") + .resource("/multipart", |r| { + r.post::(); + }).finish() + ]) .serve::<_, ()>("127.0.0.1:8080").unwrap(); let _ = sys.run(); diff --git a/examples/websocket-chat/src/main.rs b/examples/websocket-chat/src/main.rs index 6f706ac64..252ef6d3c 100644 --- a/examples/websocket-chat/src/main.rs +++ b/examples/websocket-chat/src/main.rs @@ -212,22 +212,19 @@ fn main() { // Create Http server with websocket support HttpServer::new( - RoutingMap::default() - .app("/", Application::builder(state) - // redirect to websocket.html - .resource("/", |r| - r.handler(Method::GET, |req, payload, state| { - httpcodes::HTTPFound - .builder() - .header("LOCATION", "/static/websocket.html") - .body(Body::Empty) - })) - // websocket - .resource("/ws/", |r| r.get::()) - // static resources - .route_handler("/static", StaticFiles::new("static/", true)) - .finish()) - .finish()) + Application::builder("/", state) + // redirect to websocket.html + .resource("/", |r| + r.handler(Method::GET, |req, payload, state| { + httpcodes::HTTPFound + .builder() + .header("LOCATION", "/static/websocket.html") + .body(Body::Empty) + })) + // websocket + .resource("/ws/", |r| r.get::()) + // static resources + .route_handler("/static", StaticFiles::new("static/", true))) .serve::<_, ()>("127.0.0.1:8080").unwrap(); let _ = sys.run(); diff --git a/examples/websocket/src/main.rs b/examples/websocket/src/main.rs index 48d335086..2ab22989d 100644 --- a/examples/websocket/src/main.rs +++ b/examples/websocket/src/main.rs @@ -58,17 +58,13 @@ impl Handler for MyWebSocket { } } - fn main() { let sys = actix::System::new("ws-example"); HttpServer::new( - RoutingMap::default() + Application::default("/") .resource("/ws/", |r| r.get::()) - .app("/", Application::default() - .route_handler("/", StaticFiles::new("static/", true)) - .finish()) - .finish()) + .route_handler("/", StaticFiles::new("static/", true))) .serve::<_, ()>("127.0.0.1:8080").unwrap(); println!("Started http server: 127.0.0.1:8080"); diff --git a/src/application.rs b/src/application.rs index 5a152669c..baa8659fc 100644 --- a/src/application.rs +++ b/src/application.rs @@ -5,55 +5,55 @@ use std::collections::HashMap; use task::Task; use payload::Payload; use route::{RouteHandler, FnHandler}; -use router::Handler; use resource::Resource; use recognizer::{RouteRecognizer, check_pattern}; use httprequest::HttpRequest; use httpresponse::HttpResponse; +use server::HttpHandler; /// Application -pub struct Application { - state: S, +pub struct Application { + state: Rc, + prefix: String, default: Resource, handlers: HashMap>>, - resources: HashMap>, + router: RouteRecognizer>, } -impl Application where S: 'static -{ - pub(crate) fn prepare(self, prefix: String) -> Box { - let mut handlers = HashMap::new(); - let prefix = if prefix.ends_with('/') { prefix } else { prefix + "/" }; +impl HttpHandler for Application { - let mut routes = Vec::new(); - for (path, handler) in self.resources { - routes.push((path, handler)) + fn prefix(&self) -> &str { + &self.prefix + } + + fn handle(&self, req: HttpRequest, payload: Payload) -> Task { + if let Some((params, h)) = self.router.recognize(req.path()) { + if let Some(params) = params { + h.handle( + req.with_match_info(params), payload, Rc::clone(&self.state)) + } else { + h.handle(req, payload, Rc::clone(&self.state)) + } + } else { + for (prefix, handler) in &self.handlers { + if req.path().starts_with(prefix) { + return handler.handle(req, payload, Rc::clone(&self.state)) + } + } + self.default.handle(req, payload, Rc::clone(&self.state)) } - - for (path, mut handler) in self.handlers { - let path = prefix.clone() + path.trim_left_matches('/'); - handler.set_prefix(path.clone()); - handlers.insert(path, handler); - } - Box::new( - InnerApplication { - state: Rc::new(self.state), - default: self.default, - handlers: handlers, - router: RouteRecognizer::new(prefix, routes) } - ) } } - impl Application<()> { /// Create default `ApplicationBuilder` with no state - pub fn default() -> ApplicationBuilder<()> { + pub fn default(prefix: T) -> ApplicationBuilder<()> { ApplicationBuilder { parts: Some(ApplicationBuilderParts { state: (), + prefix: prefix.to_string(), default: Resource::default(), handlers: HashMap::new(), resources: HashMap::new()}) @@ -63,104 +63,29 @@ impl Application<()> { impl Application where S: 'static { - /// Create application builder - pub fn builder(state: S) -> ApplicationBuilder { + /// Create application builder with specific state. State is shared with all + /// routes within same application and could be + /// accessed with `HttpContext::state()` method. + pub fn builder(prefix: T, state: S) -> ApplicationBuilder { ApplicationBuilder { parts: Some(ApplicationBuilderParts { state: state, + prefix: prefix.to_string(), default: Resource::default(), handlers: HashMap::new(), resources: HashMap::new()}) } } - - /// Create http application with specific state. State is shared with all - /// routes within same application and could be - /// accessed with `HttpContext::state()` method. - pub fn new(state: S) -> Application { - Application { - state: state, - default: Resource::default(), - handlers: HashMap::new(), - resources: HashMap::new(), - } - } - - /// Add resource by path. - pub fn resource(&mut self, path: P) -> &mut Resource - { - let path = path.to_string(); - - // add resource - if !self.resources.contains_key(&path) { - check_pattern(&path); - self.resources.insert(path.clone(), Resource::default()); - } - - self.resources.get_mut(&path).unwrap() - } - - /// This method register handler for specified path. - /// - /// ```rust - /// extern crate actix_web; - /// use actix_web::*; - /// - /// fn main() { - /// let mut app = Application::new(()); - /// - /// app.handler("/test", |req, payload, state| { - /// httpcodes::HTTPOk - /// }); - /// } - /// ``` - pub fn handler(&mut self, path: P, handler: F) -> &mut Self - where F: Fn(HttpRequest, Payload, &S) -> R + 'static, - R: Into + 'static, - P: ToString, - { - self.handlers.insert(path.to_string(), Box::new(FnHandler::new(handler))); - self - } - - /// Add path handler - pub fn route_handler(&mut self, path: P, h: H) - where H: RouteHandler + 'static, P: ToString - { - let path = path.to_string(); - - // add resource - if self.handlers.contains_key(&path) { - panic!("Handler already registered: {:?}", path); - } - - self.handlers.insert(path, Box::new(h)); - } - - /// Default resource is used if no matches route could be found. - pub fn default_resource(&mut self) -> &mut Resource { - &mut self.default - } } struct ApplicationBuilderParts { state: S, + prefix: String, default: Resource, handlers: HashMap>>, resources: HashMap>, } -impl From> for Application { - fn from(b: ApplicationBuilderParts) -> Self { - Application { - state: b.state, - default: b.default, - handlers: b.handlers, - resources: b.resources, - } - } -} - /// Application builder pub struct ApplicationBuilder { parts: Option>, @@ -170,6 +95,22 @@ impl ApplicationBuilder where S: 'static { /// Configure resource for specific path. /// + /// Resource may have variable path also. For instance, a resource with + /// the path */a/{name}/c* would match all incoming requests with paths + /// such as */a/b/c*, */a/1/c*, and */a/etc/c*. + /// + /// A variable part is specified in the form `{identifier}`, where + /// the identifier can be used later in a request handler to access the matched + /// value for that part. This is done by looking up the identifier + /// in the `Params` object returned by `Request.match_info()` method. + /// + /// By default, each part matches the regular expression `[^{}/]+`. + /// + /// You can also specify a custom regex in the form `{identifier:regex}`: + /// + /// For instance, to route Get requests on any route matching `/users/{userid}/{friend}` and + /// store userid and friend in the exposed Params object: + /// /// ```rust /// extern crate actix; /// extern crate actix_web; @@ -193,7 +134,7 @@ impl ApplicationBuilder where S: 'static { /// } /// } /// fn main() { - /// let app = Application::default() + /// let app = Application::default("/") /// .resource("/test", |r| { /// r.get::(); /// r.handler(Method::HEAD, |req, payload, state| { @@ -238,7 +179,7 @@ impl ApplicationBuilder where S: 'static { /// use actix_web::*; /// /// fn main() { - /// let app = Application::default() + /// let app = Application::default("/") /// .handler("/test", |req, payload, state| { /// match *req.method() { /// Method::GET => httpcodes::HTTPOk, @@ -277,36 +218,48 @@ impl ApplicationBuilder where S: 'static { /// Construct application pub fn finish(&mut self) -> Application { - self.parts.take().expect("Use after finish").into() + let parts = self.parts.take().expect("Use after finish"); + + let mut handlers = HashMap::new(); + let prefix = if parts.prefix.ends_with('/') { + parts.prefix + } else { + parts.prefix + "/" + }; + + let mut routes = Vec::new(); + for (path, handler) in parts.resources { + routes.push((path, handler)) + } + + for (path, mut handler) in parts.handlers { + let path = prefix.clone() + path.trim_left_matches('/'); + handler.set_prefix(path.clone()); + handlers.insert(path, handler); + } + Application { + state: Rc::new(parts.state), + prefix: prefix.clone(), + default: parts.default, + handlers: handlers, + router: RouteRecognizer::new(prefix, routes) } } } -pub(crate) -struct InnerApplication { - state: Rc, - default: Resource, - handlers: HashMap>>, - router: RouteRecognizer>, +impl From> for Application { + fn from(mut builder: ApplicationBuilder) -> Application { + builder.finish() + } } +impl Iterator for ApplicationBuilder { + type Item = Application; -impl Handler for InnerApplication { - - fn handle(&self, req: HttpRequest, payload: Payload) -> Task { - if let Some((params, h)) = self.router.recognize(req.path()) { - if let Some(params) = params { - h.handle( - req.with_match_info(params), payload, Rc::clone(&self.state)) - } else { - h.handle(req, payload, Rc::clone(&self.state)) - } + fn next(&mut self) -> Option { + if self.parts.is_some() { + Some(self.finish()) } else { - for (prefix, handler) in &self.handlers { - if req.path().starts_with(prefix) { - return handler.handle(req, payload, Rc::clone(&self.state)) - } - } - self.default.handle(req, payload, Rc::clone(&self.state)) + None } } } diff --git a/src/dev.rs b/src/dev.rs index 71c6725c4..aa209fb09 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -14,7 +14,6 @@ pub use application::{Application, ApplicationBuilder}; pub use httprequest::HttpRequest; pub use httpresponse::{Body, HttpResponse, HttpResponseBuilder}; pub use payload::{Payload, PayloadItem, PayloadError}; -pub use router::RoutingMap; pub use resource::{Reply, Resource}; pub use route::{Route, RouteFactory, RouteHandler}; pub use recognizer::Params; diff --git a/src/lib.rs b/src/lib.rs index 2b766c410..34e45b21e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,7 +36,6 @@ mod payload; mod resource; mod recognizer; mod route; -mod router; mod reader; mod task; mod staticfiles; @@ -53,7 +52,6 @@ pub use application::{Application, ApplicationBuilder}; pub use httprequest::{HttpRequest, UrlEncoded}; pub use httpresponse::{Body, HttpResponse, HttpResponseBuilder}; pub use payload::{Payload, PayloadItem, PayloadError}; -pub use router::{Router, RoutingMap}; pub use route::{Route, RouteFactory, RouteHandler}; pub use resource::{Reply, Resource}; pub use recognizer::{Params, RouteRecognizer}; diff --git a/src/router.rs b/src/router.rs deleted file mode 100644 index eb1cd38d1..000000000 --- a/src/router.rs +++ /dev/null @@ -1,175 +0,0 @@ -use std::rc::Rc; -use std::string::ToString; -use std::collections::HashMap; - -use task::Task; -use payload::Payload; -use route::RouteHandler; -use resource::Resource; -use recognizer::{RouteRecognizer, check_pattern}; -use application::Application; -use httpcodes::HTTPNotFound; -use httprequest::HttpRequest; - -pub(crate) trait Handler: 'static { - fn handle(&self, req: HttpRequest, payload: Payload) -> Task; -} - -/// Server routing map -pub struct Router { - apps: HashMap>, - resources: RouteRecognizer, -} - -impl Router { - - pub(crate) fn call(&self, req: HttpRequest, payload: Payload) -> Task - { - if let Some((params, h)) = self.resources.recognize(req.path()) { - if let Some(params) = params { - h.handle( - req.with_match_info(params), payload, Rc::new(())) - } else { - h.handle(req, payload, Rc::new(())) - } - } else { - for (prefix, app) in &self.apps { - if req.path().starts_with(prefix) { - return app.handle(req, payload) - } - } - Task::reply(HTTPNotFound.response()) - } - } -} - -/// Request routing map builder -/// -/// Resource may have variable path also. For instance, a resource with -/// the path */a/{name}/c* would match all incoming requests with paths -/// such as */a/b/c*, */a/1/c*, and */a/etc/c*. -/// -/// A variable part is specified in the form `{identifier}`, where -/// the identifier can be used later in a request handler to access the matched -/// value for that part. This is done by looking up the identifier -/// in the `Params` object returned by `Request.match_info()` method. -/// -/// By default, each part matches the regular expression `[^{}/]+`. -/// -/// You can also specify a custom regex in the form `{identifier:regex}`: -/// -/// For instance, to route Get requests on any route matching `/users/{userid}/{friend}` and -/// store userid and friend in the exposed Params object: -/// -/// ```rust,ignore -/// let mut map = RoutingMap::default(); -/// -/// map.resource("/users/{userid}/{friend}", |r| r.get::()); -/// ``` -pub struct RoutingMap { - parts: Option, -} - -struct RoutingMapParts { - apps: HashMap>, - resources: HashMap, -} - -impl Default for RoutingMap { - fn default() -> Self { - RoutingMap { - parts: Some(RoutingMapParts { - apps: HashMap::new(), - resources: HashMap::new()}), - } - } -} - -impl RoutingMap { - - /// Add `Application` object with specific prefix. - /// Application prefixes all registered resources with specified prefix. - /// - /// ```rust,ignore - /// - /// struct MyRoute; - /// - /// fn main() { - /// let mut router = - /// RoutingMap::default() - /// .app("/pre", Application::default() - /// .resource("/users/{userid}", |r| { - /// r.get::(); - /// r.post::(); - /// }) - /// .finish() - /// ).finish(); - /// } - /// ``` - /// In this example, `MyRoute` route is available as `http://.../pre/test` url. - pub fn app(&mut self, prefix: P, app: Application) -> &mut Self - where P: ToString - { - { - let parts = self.parts.as_mut().expect("Use after finish"); - - // we can not override registered resource - let prefix = prefix.to_string(); - if parts.apps.contains_key(&prefix) { - panic!("Resource is registered: {}", prefix); - } - - // add application - parts.apps.insert(prefix.clone(), app.prepare(prefix)); - } - self - } - - /// Configure resource for specific path. - /// - /// ```rust,ignore - /// - /// struct MyRoute; - /// - /// fn main() { - /// RoutingMap::default().resource("/test", |r| { - /// r.post::(); - /// }).finish(); - /// } - /// ``` - /// In this example, `MyRoute` route is available as `http://.../test` url. - pub fn resource(&mut self, path: P, f: F) -> &mut Self - where F: FnOnce(&mut Resource<()>) + 'static, - P: ToString, - { - { - let parts = self.parts.as_mut().expect("Use after finish"); - - // add resource - let path = path.to_string(); - if !parts.resources.contains_key(&path) { - check_pattern(&path); - parts.resources.insert(path.clone(), Resource::default()); - } - // configure resource - f(parts.resources.get_mut(&path).unwrap()); - } - self - } - - /// Finish configuration and create `Router` instance - pub fn finish(&mut self) -> Router - { - let parts = self.parts.take().expect("Use after finish"); - - let mut routes = Vec::new(); - for (path, resource) in parts.resources { - routes.push((path, resource)) - } - - Router { - apps: parts.apps, - resources: RouteRecognizer::new("/".to_owned(), routes), - } - } -} diff --git a/src/server.rs b/src/server.rs index 43bd06acf..45f77525c 100644 --- a/src/server.rs +++ b/src/server.rs @@ -11,34 +11,52 @@ use tokio_core::net::{TcpListener, TcpStream}; use tokio_io::{AsyncRead, AsyncWrite}; use task::{Task, RequestInfo}; -use router::Router; use reader::{Reader, ReaderError}; +use payload::Payload; +use httpcodes::HTTPNotFound; +use httprequest::HttpRequest; + +/// Low level http request handler +pub trait HttpHandler: 'static { + /// Http handler prefix + fn prefix(&self) -> &str; + /// Handle request + fn handle(&self, req: HttpRequest, payload: Payload) -> Task; +} /// An HTTP Server /// /// `T` - async stream, anything that implements `AsyncRead` + `AsyncWrite`. /// /// `A` - peer address -pub struct HttpServer { - router: Rc, +/// +/// `H` - request handler +pub struct HttpServer { + h: Rc>, io: PhantomData, addr: PhantomData, } -impl Actor for HttpServer { +impl Actor for HttpServer { type Context = Context; } -impl HttpServer { - /// Create new http server with specified `RoutingMap` - pub fn new(router: Router) -> Self { - HttpServer {router: Rc::new(router), io: PhantomData, addr: PhantomData} +impl HttpServer where H: HttpHandler +{ + /// Create new http server with vec of http handlers + pub fn new>(handler: U) -> Self { + let apps: Vec<_> = handler.into_iter().map(|h| h.into()).collect(); + + HttpServer {h: Rc::new(apps), + io: PhantomData, + addr: PhantomData} } } -impl HttpServer +impl HttpServer where T: AsyncRead + AsyncWrite + 'static, - A: 'static + A: 'static, + H: HttpHandler, { /// Start listening for incomming connections from stream. pub fn serve_incoming(self, stream: S) -> io::Result @@ -52,7 +70,7 @@ impl HttpServer } } -impl HttpServer { +impl HttpServer { /// Start listening for incomming connections. /// @@ -99,18 +117,21 @@ impl ResponseType for IoStream type Error = (); } -impl StreamHandler, io::Error> for HttpServer - where T: AsyncRead + AsyncWrite + 'static, A: 'static {} - -impl Handler, io::Error> for HttpServer +impl StreamHandler, io::Error> for HttpServer where T: AsyncRead + AsyncWrite + 'static, - A: 'static + A: 'static, + H: HttpHandler + 'static {} + +impl Handler, io::Error> for HttpServer + where T: AsyncRead + AsyncWrite + 'static, + A: 'static, + H: HttpHandler + 'static, { fn handle(&mut self, msg: IoStream, _: &mut Context) -> Response> { Arbiter::handle().spawn( - HttpChannel{router: Rc::clone(&self.router), + HttpChannel{router: Rc::clone(&self.h), addr: msg.1, stream: msg.0, reader: Reader::new(), @@ -136,8 +157,8 @@ struct Entry { const KEEPALIVE_PERIOD: u64 = 15; // seconds const MAX_PIPELINED_MESSAGES: usize = 16; -pub struct HttpChannel { - router: Rc, +pub struct HttpChannel { + router: Rc>, #[allow(dead_code)] addr: A, stream: T, @@ -155,14 +176,18 @@ pub struct HttpChannel { } }*/ -impl Actor for HttpChannel - where T: AsyncRead + AsyncWrite + 'static, A: 'static +impl Actor for HttpChannel + where T: AsyncRead + AsyncWrite + 'static, + A: 'static, + H: HttpHandler + 'static { type Context = Context; } -impl Future for HttpChannel - where T: AsyncRead + AsyncWrite + 'static, A: 'static +impl Future for HttpChannel + where T: AsyncRead + AsyncWrite + 'static, + A: 'static, + H: HttpHandler + 'static { type Item = (); type Error = (); @@ -261,8 +286,16 @@ impl Future for HttpChannel // start request processing let info = RequestInfo::new(&req); + let mut task = None; + for h in self.router.iter() { + if req.path().starts_with(h.prefix()) { + task = Some(h.handle(req, payload)); + break + } + } + self.items.push_back( - Entry {task: self.router.call(req, payload), + Entry {task: task.unwrap_or_else(|| Task::reply(HTTPNotFound)), req: info, eof: false, error: false, diff --git a/src/staticfiles.rs b/src/staticfiles.rs index b22c150ee..c37c7d978 100644 --- a/src/staticfiles.rs +++ b/src/staticfiles.rs @@ -26,7 +26,7 @@ use httpcodes::{HTTPOk, HTTPNotFound, HTTPForbidden, HTTPInternalServerError}; /// use actix_web::*; /// /// fn main() { -/// let app = Application::default() +/// let app = Application::default("/") /// .route_handler("/static", StaticFiles::new(".", true)) /// .finish(); /// } diff --git a/tests/test_server.rs b/tests/test_server.rs index 5111f3e6d..29a3f992d 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -12,14 +12,14 @@ use futures::Future; use tokio_core::net::{TcpStream, TcpListener}; -fn create_server() -> HttpServer { +fn create_server() -> HttpServer> { HttpServer::new( - RoutingMap::default() - .resource("/", |r| - r.handler(Method::GET, |_, _, _| { - httpcodes::HTTPOk - })) - .finish()) + vec![Application::default("/") + .resource("/", |r| + r.handler(Method::GET, |_, _, _| { + httpcodes::HTTPOk + })) + .finish()]) } #[test]