refactor Handler trait, use mut self

guide/src/qs_4.md

@@ -39,6 +39,81 @@ fn index(req: HttpRequest) -> Box<Future<Item=HttpResponse, Error=Error>> {
 }
 ```
 
+Some notes on shared application state and handler state. If you noticed
+*Handler* trait is generic over *S*, which defines application state type. So
+application state is accessible from handler with `HttpRequest::state()` method. 
+But state is accessible as a read-only reference, if you need mutable access to state
+you have to implement it yourself. On other hand handler can mutable access it's own state
+as `handle` method takes mutable reference to *self*. Beware, actix creates multiple copies
+of application state and handlers, unique for each thread, so if you run your
+application in several threads actix will create same amount as number of threads 
+of application state objects and handler objects.
+
+Here is example of handler that stores number of processed requests:
+
+```rust
+# extern crate actix;
+# extern crate actix_web;
+use actix_web::*;
+use actix_web::dev::Handler;
+
+struct MyHandler(usize);
+
+impl<S> Handler<S> for MyHandler {
+    type Result = HttpResponse;
+
+    /// Handle request
+    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
+        self.0 += 1;
+        httpcodes::HTTPOk.response()
+    }
+}
+# fn main() {}
+```
+
+This handler will work, but `self.0` value will be different depends on number of threads and
+number of requests processed per thread. Proper implementation would use `Arc` and `AtomicUsize`
+
+```rust
+# extern crate actix;
+# extern crate actix_web;
+use actix_web::*;
+use std::sync::Arc;
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+struct MyHandler(Arc<AtomicUsize>);
+
+impl<S> Handler<S> for MyHandler {
+    type Result = HttpResponse;
+
+    /// Handle request
+    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
+        let num = self.0.load(Ordering::Relaxed) + 1;
+        self.0.store(num, Ordering::Relaxed);
+        httpcodes::HTTPOk.response()
+    }
+}
+
+fn main() {
+    let sys = actix::System::new("example");
+
+    let inc = Arc::new(AtomicUsize::new(0));
+
+    HttpServer::new(
+        move || { 
+            let cloned = inc.clone();
+            Application::new()
+            .resource("/", move |r| r.h(MyHandler(cloned)))
+        })
+        .bind("127.0.0.1:8088").unwrap()
+        .start();
+
+    println!("Started http server: 127.0.0.1:8088");
+#    actix::Arbiter::system().send(actix::msgs::SystemExit(0));
+    let _ = sys.run();
+}
+```
+
 ## Response with custom type
 
 To return custom type directly from handler function type needs to implement `Responder` trait.

src/application.rs

@@ -15,7 +15,8 @@ pub struct HttpApplication<S=()> {
     state: Rc<S>,
     prefix: String,
     default: Resource<S>,
-    router: Router<S>,
+    router: Router,
+    resources: Vec<Resource<S>>,
     middlewares: Rc<Vec<Box<Middleware<S>>>>,
 }
 
@@ -25,9 +26,9 @@ impl<S: 'static> HttpApplication<S> {
         req.with_state(Rc::clone(&self.state), self.router.clone())
     }
 
-    pub(crate) fn run(&self, mut req: HttpRequest<S>) -> Reply {
+    pub(crate) fn run(&mut self, mut req: HttpRequest<S>) -> Reply {
-        if let Some(h) = self.router.recognize(&mut req) {
+        if let Some(idx) = self.router.recognize(&mut req) {
-            h.handle(req.clone(), Some(&self.default))
+            self.resources[idx].handle(req.clone(), Some(&mut self.default))
         } else {
             self.default.handle(req, None)
         }
@@ -36,11 +37,12 @@ impl<S: 'static> HttpApplication<S> {
 
 impl<S: 'static> HttpHandler for HttpApplication<S> {
 
-    fn handle(&self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest> {
+    fn handle(&mut self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest> {
         if req.path().starts_with(&self.prefix) {
             let req = self.prepare_request(req);
+            // TODO: redesign run callback
             Ok(Box::new(Pipeline::new(req, Rc::clone(&self.middlewares),
-                                      &|req: HttpRequest<S>| self.run(req))))
+                                      &mut |req: HttpRequest<S>| self.run(req))))
         } else {
             Err(req)
         }
@@ -264,11 +266,13 @@ impl<S> Application<S> where S: 'static {
             resources.insert(pattern, None);
         }
 
+        let (router, resources) = Router::new(prefix, resources);
         HttpApplication {
             state: Rc::new(parts.state),
             prefix: prefix.to_owned(),
             default: parts.default,
-            router: Router::new(prefix, resources),
+            router: router,
+            resources: resources,
             middlewares: Rc::new(parts.middlewares),
         }
     }
@@ -314,7 +318,7 @@ mod tests {
 
     #[test]
     fn test_default_resource() {
-        let app = Application::new()
+        let mut app = Application::new()
             .resource("/test", |r| r.h(httpcodes::HTTPOk))
             .finish();
 
@@ -330,7 +334,7 @@ mod tests {
         let resp = app.run(req);
         assert_eq!(resp.as_response().unwrap().status(), StatusCode::NOT_FOUND);
 
-        let app = Application::new()
+        let mut app = Application::new()
             .default_resource(|r| r.h(httpcodes::HTTPMethodNotAllowed))
             .finish();
         let req = HttpRequest::new(
@@ -342,7 +346,7 @@ mod tests {
 
     #[test]
     fn test_unhandled_prefix() {
-        let app = Application::new()
+        let mut app = Application::new()
             .prefix("/test")
             .resource("/test", |r| r.h(httpcodes::HTTPOk))
             .finish();
@@ -351,7 +355,7 @@ mod tests {
 
     #[test]
     fn test_state() {
-        let app = Application::with_state(10)
+        let mut app = Application::with_state(10)
             .resource("/", |r| r.h(httpcodes::HTTPOk))
             .finish();
         let req = HttpRequest::default().with_state(Rc::clone(&app.state), app.router.clone());

src/channel.rs

@@ -18,7 +18,7 @@ use server::{ServerSettings, WorkerSettings};
 pub trait HttpHandler: 'static {
 
     /// Handle request
-    fn handle(&self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest>;
+    fn handle(&mut self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest>;
 
     /// Set server settings
     fn server_settings(&mut self, settings: ServerSettings) {}

src/fs.rs

@@ -252,7 +252,7 @@ impl StaticFiles {
 impl<S> Handler<S> for StaticFiles {
     type Result = Result<FilesystemElement, io::Error>;
 
-    fn handle(&self, req: HttpRequest<S>) -> Self::Result {
+    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
         if !self.accessible {
             Err(io::Error::new(io::ErrorKind::NotFound, "not found"))
         } else {

src/h1.rs

@@ -231,7 +231,7 @@ impl<T, H> Http1<T, H>
 
                         // start request processing
                         let mut pipe = None;
-                        for h in self.settings.handlers().iter() {
+                        for h in self.settings.handlers().iter_mut() {
                             req = match h.handle(req) {
                                 Ok(t) => {
                                     pipe = Some(t);

src/h2.rs

@@ -261,7 +261,7 @@ impl Entry {
 
         // start request processing
         let mut task = None;
-        for h in settings.handlers().iter() {
+        for h in settings.handlers().iter_mut() {
             req = match h.handle(req) {
                 Ok(t) => {
                     task = Some(t);

src/handler.rs

@@ -19,7 +19,7 @@ pub trait Handler<S>: 'static {
     type Result: Responder;
 
     /// Handle request
-    fn handle(&self, req: HttpRequest<S>) -> Self::Result;
+    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result;
 }
 
 /// Trait implemented by types that generate responses for clients.
@@ -59,7 +59,7 @@ impl<F, R, S> Handler<S> for F
 {
     type Result = R;
 
-    fn handle(&self, req: HttpRequest<S>) -> R {
+    fn handle(&mut self, req: HttpRequest<S>) -> R {
         (self)(req)
     }
 }
@@ -207,7 +207,7 @@ impl<I, E> Responder for Box<Future<Item=I, Error=E>>
 
 /// Trait defines object that could be regestered as resource route
 pub(crate) trait RouteHandler<S>: 'static {
-    fn handle(&self, req: HttpRequest<S>) -> Reply;
+    fn handle(&mut self, req: HttpRequest<S>) -> Reply;
 }
 
 /// Route handler wrapper for Handler
@@ -236,7 +236,7 @@ impl<S, H, R> RouteHandler<S> for WrapHandler<S, H, R>
           R: Responder + 'static,
           S: 'static,
 {
-    fn handle(&self, req: HttpRequest<S>) -> Reply {
+    fn handle(&mut self, req: HttpRequest<S>) -> Reply {
         let req2 = req.clone_without_state();
         match self.h.handle(req).respond_to(req2) {
             Ok(reply) => reply.into(),
@@ -277,7 +277,7 @@ impl<S, H, F, R, E> RouteHandler<S> for AsyncHandler<S, H, F, R, E>
           E: Into<Error> + 'static,
           S: 'static,
 {
-    fn handle(&self, req: HttpRequest<S>) -> Reply {
+    fn handle(&mut self, req: HttpRequest<S>) -> Reply {
         let req2 = req.clone_without_state();
         let fut = (self.h)(req)
             .map_err(|e| e.into())
@@ -368,7 +368,7 @@ impl NormalizePath {
 impl<S> Handler<S> for NormalizePath {
     type Result = Result<HttpResponse, HttpError>;
 
-    fn handle(&self, req: HttpRequest<S>) -> Self::Result {
+    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
         if let Some(router) = req.router() {
             let query = req.query_string();
             if self.merge {
@@ -420,7 +420,7 @@ mod tests {
 
     #[test]
     fn test_normalize_path_trailing_slashes() {
-        let app = Application::new()
+        let mut app = Application::new()
             .resource("/resource1", |r| r.method(Method::GET).f(index))
             .resource("/resource2/", |r| r.method(Method::GET).f(index))
             .default_resource(|r| r.h(NormalizePath::default()))
@@ -452,7 +452,7 @@ mod tests {
 
     #[test]
     fn test_normalize_path_trailing_slashes_disabled() {
-        let app = Application::new()
+        let mut app = Application::new()
             .resource("/resource1", |r| r.method(Method::GET).f(index))
             .resource("/resource2/", |r| r.method(Method::GET).f(index))
             .default_resource(|r| r.h(
@@ -479,7 +479,7 @@ mod tests {
 
     #[test]
     fn test_normalize_path_merge_slashes() {
-        let app = Application::new()
+        let mut app = Application::new()
             .resource("/resource1", |r| r.method(Method::GET).f(index))
             .resource("/resource1/a/b", |r| r.method(Method::GET).f(index))
             .default_resource(|r| r.h(NormalizePath::default()))
@@ -515,7 +515,7 @@ mod tests {
 
     #[test]
     fn test_normalize_path_merge_and_append_slashes() {
-        let app = Application::new()
+        let mut app = Application::new()
             .resource("/resource1", |r| r.method(Method::GET).f(index))
             .resource("/resource2/", |r| r.method(Method::GET).f(index))
             .resource("/resource1/a/b", |r| r.method(Method::GET).f(index))

src/httpcodes.rs

@@ -70,13 +70,13 @@ impl StaticResponse {
 impl<S> Handler<S> for StaticResponse {
     type Result = HttpResponse;
 
-    fn handle(&self, _: HttpRequest<S>) -> HttpResponse {
+    fn handle(&mut self, _: HttpRequest<S>) -> HttpResponse {
         HttpResponse::new(self.0, Body::Empty)
     }
 }
 
 impl<S> RouteHandler<S> for StaticResponse {
-    fn handle(&self, _: HttpRequest<S>) -> Reply {
+    fn handle(&mut self, _: HttpRequest<S>) -> Reply {
         Reply::response(HttpResponse::new(self.0, Body::Empty))
     }
 }

src/httprequest.rs

@@ -87,7 +87,7 @@ impl HttpMessage {
 }
 
 /// An HTTP Request
-pub struct HttpRequest<S=()>(SharedHttpMessage, Option<Rc<S>>, Option<Router<S>>);
+pub struct HttpRequest<S=()>(SharedHttpMessage, Option<Rc<S>>, Option<Router>);
 
 impl HttpRequest<()> {
     /// Construct a new Request.
@@ -146,7 +146,7 @@ impl HttpRequest<()> {
 
     #[inline]
     /// Construct new http request with state.
-    pub fn with_state<S>(self, state: Rc<S>, router: Router<S>) -> HttpRequest<S> {
+    pub fn with_state<S>(self, state: Rc<S>, router: Router) -> HttpRequest<S> {
         HttpRequest(self.0, Some(state), Some(router))
     }
 }
@@ -277,7 +277,7 @@ impl<S> HttpRequest<S> {
 
     /// This method returns reference to current `Router` object.
     #[inline]
-    pub fn router(&self) -> Option<&Router<S>> {
+    pub fn router(&self) -> Option<&Router> {
         self.2.as_ref()
     }
 
@@ -736,11 +736,11 @@ mod tests {
         let mut req = HttpRequest::new(Method::GET, Uri::from_str("/value/?id=test").unwrap(),
                                        Version::HTTP_11, HeaderMap::new(), None);
 
-        let mut resource = Resource::default();
+        let mut resource = Resource::<()>::default();
         resource.name("index");
         let mut map = HashMap::new();
         map.insert(Pattern::new("index", "/{key}/"), Some(resource));
-        let router = Router::new("", map);
+        let (router, _) = Router::new("", map);
         assert!(router.recognize(&mut req).is_some());
 
         assert_eq!(req.match_info().get("key"), Some("value"));
@@ -843,11 +843,11 @@ mod tests {
         let req = HttpRequest::new(
             Method::GET, Uri::from_str("/").unwrap(), Version::HTTP_11, headers, None);
 
-        let mut resource = Resource::default();
+        let mut resource = Resource::<()>::default();
         resource.name("index");
         let mut map = HashMap::new();
         map.insert(Pattern::new("index", "/user/{name}.{ext}"), Some(resource));
-        let router = Router::new("", map);
+        let (router, _) = Router::new("", map);
         assert!(router.has_route("/user/test.html"));
         assert!(!router.has_route("/test/unknown"));
 
@@ -874,7 +874,7 @@ mod tests {
         resource.name("index");
         let mut map = HashMap::new();
         map.insert(Pattern::new("youtube", "https://youtube.com/watch/{video_id}"), None);
-        let router = Router::new("", map);
+        let (router, _) = Router::new::<()>("", map);
         assert!(!router.has_route("https://youtube.com/watch/unknown"));
 
         let req = req.with_state(Rc::new(()), router);

src/pipeline.rs

@@ -15,8 +15,8 @@ use httprequest::HttpRequest;
 use httpresponse::HttpResponse;
 use middlewares::{Middleware, Finished, Started, Response};
 
-type Handler<S> = Fn(HttpRequest<S>) -> Reply;
+type Handler<S> = FnMut(HttpRequest<S>) -> Reply;
-pub(crate) type PipelineHandler<'a, S> = &'a Fn(HttpRequest<S>) -> Reply;
+pub(crate) type PipelineHandler<'a, S> = &'a mut FnMut(HttpRequest<S>) -> Reply;
 
 pub struct Pipeline<S>(PipelineInfo<S>, PipelineState<S>);
 
@@ -287,7 +287,7 @@ impl<S> StartMiddlewares<S> {
         let len = info.mws.len();
         loop {
             if info.count == len {
-                let reply = (&*handler)(info.req.clone());
+                let reply = (&mut *handler)(info.req.clone());
                 return WaitingResponse::init(info, reply)
             } else {
                 match info.mws[info.count].start(&mut info.req) {
@@ -329,7 +329,7 @@ impl<S> StartMiddlewares<S> {
                         return Ok(RunMiddlewares::init(info, resp));
                     }
                     if info.count == len {
-                        let reply = (unsafe{&*self.hnd})(info.req.clone());
+                        let reply = (unsafe{&mut *self.hnd})(info.req.clone());
                         return Ok(WaitingResponse::init(info, reply));
                     } else {
                         loop {

src/resource.rs

@@ -126,10 +126,10 @@ impl<S: 'static> Resource<S> {
         self.routes.last_mut().unwrap().f(handler)
     }
 
-    pub(crate) fn handle(&self, mut req: HttpRequest<S>, default: Option<&Resource<S>>)
+    pub(crate) fn handle(&mut self, mut req: HttpRequest<S>, default: Option<&mut Resource<S>>)
                          -> Reply
     {
-        for route in &self.routes {
+        for route in &mut self.routes {
             if route.check(&mut req) {
                 return route.handle(req)
             }

src/route.rs

@@ -36,7 +36,7 @@ impl<S: 'static> Route<S> {
         true
     }
 
-    pub(crate) fn handle(&self, req: HttpRequest<S>) -> Reply {
+    pub(crate) fn handle(&mut self, req: HttpRequest<S>) -> Reply {
         self.handler.handle(req)
     }
 

src/router.rs

@@ -12,20 +12,20 @@ use server::ServerSettings;
 
 
 /// Interface for application router.
-pub struct Router<S>(Rc<Inner<S>>);
+pub struct Router(Rc<Inner>);
 
-struct Inner<S> {
+struct Inner {
     prefix: String,
     regset: RegexSet,
     named: HashMap<String, (Pattern, bool)>,
     patterns: Vec<Pattern>,
-    resources: Vec<Resource<S>>,
     srv: ServerSettings,
 }
 
-impl<S> Router<S> {
+impl Router {
     /// Create new router
-    pub fn new(prefix: &str, map: HashMap<Pattern, Option<Resource<S>>>) -> Router<S>
+    pub fn new<S>(prefix: &str, map: HashMap<Pattern, Option<Resource<S>>>)
+                  -> (Router, Vec<Resource<S>>)
     {
         let prefix = prefix.trim().trim_right_matches('/').to_owned();
         let mut named = HashMap::new();
@@ -46,13 +46,12 @@ impl<S> Router<S> {
             }
         }
 
-        Router(Rc::new(
+        (Router(Rc::new(
             Inner{ prefix: prefix,
                    regset: RegexSet::new(&paths).unwrap(),
                    named: named,
                    patterns: patterns,
-                   resources: resources,
+                   srv: ServerSettings::default() })), resources)
-                   srv: ServerSettings::default() }))
     }
 
     pub(crate) fn set_server_settings(&mut self, settings: ServerSettings) {
@@ -72,7 +71,7 @@ impl<S> Router<S> {
     }
 
     /// Query for matched resource
-    pub fn recognize(&self, req: &mut HttpRequest<S>) -> Option<&Resource<S>> {
+    pub fn recognize<S>(&self, req: &mut HttpRequest<S>) -> Option<usize> {
         let mut idx = None;
         {
             let path = &req.path()[self.0.prefix.len()..];
@@ -88,7 +87,7 @@ impl<S> Router<S> {
         if let Some(idx) = idx {
             let path: &str = unsafe{ mem::transmute(&req.path()[self.0.prefix.len()..]) };
             self.0.patterns[idx].update_match_info(path, req);
-            return Some(&self.0.resources[idx])
+            return Some(idx)
         } else {
             None
         }
@@ -128,8 +127,8 @@ impl<S> Router<S> {
     }
 }
 
-impl<S: 'static> Clone for Router<S> {
+impl Clone for Router {
-    fn clone(&self) -> Router<S> {
+    fn clone(&self) -> Router {
         Router(Rc::clone(&self.0))
     }
 }
@@ -315,7 +314,7 @@ mod tests {
         routes.insert(Pattern::new("", "/v{val}/{val2}/index.html"), Some(Resource::default()));
         routes.insert(Pattern::new("", "/v/{tail:.*}"), Some(Resource::default()));
         routes.insert(Pattern::new("", "{test}/index.html"), Some(Resource::default()));
-        let rec = Router::new("", routes);
+        let (rec, _) = Router::new::<()>("", routes);
 
         let mut req = HttpRequest::new(
             Method::GET, Uri::from_str("/name").unwrap(),

src/server.rs

@@ -1,5 +1,6 @@
 use std::{io, net, thread};
 use std::rc::Rc;
+use std::cell::{RefCell, RefMut};
 use std::sync::Arc;
 use std::time::Duration;
 use std::marker::PhantomData;
@@ -447,7 +448,7 @@ struct Worker<H> {
 }
 
 pub(crate) struct WorkerSettings<H> {
-    h: Vec<H>,
+    h: RefCell<Vec<H>>,
     enabled: bool,
     keep_alive: u64,
     bytes: Rc<helpers::SharedBytesPool>,
@@ -457,7 +458,7 @@ pub(crate) struct WorkerSettings<H> {
 impl<H> WorkerSettings<H> {
     pub(crate) fn new(h: Vec<H>, keep_alive: Option<u64>) -> WorkerSettings<H> {
         WorkerSettings {
-            h: h,
+            h: RefCell::new(h),
             enabled: if let Some(ka) = keep_alive { ka > 0 } else { false },
             keep_alive: keep_alive.unwrap_or(0),
             bytes: Rc::new(helpers::SharedBytesPool::new()),
@@ -465,8 +466,8 @@ impl<H> WorkerSettings<H> {
         }
     }
 
-    pub fn handlers(&self) -> &Vec<H> {
+    pub fn handlers(&self) -> RefMut<Vec<H>> {
-        &self.h
+        self.h.borrow_mut()
     }
     pub fn keep_alive(&self) -> u64 {
         self.keep_alive