Handler::handle uses &self instead of mutabble reference

CHANGES.md

@@ -37,7 +37,7 @@
 
 * Use tokio instead of tokio-core
 
-* Use `&mut self` instead of `&self` for Middleware trait
+* For safety and performance reasons `Handler::handle()` uses `&self` instead of `&mut self`
 
 * Added header `User-Agent: Actix-web/<current_version>` to default headers when building a request
 

MIGRATION.md

@@ -17,6 +17,8 @@
     fn index((query, json): (Query<..>, Json<MyStruct)) -> impl Responder {}
     ```
 
+* `Handler::handle()` uses `&self` instead of `&mut self`
+
 * Removed deprecated `HttpServer::threads()`, use 
   [HttpServer::workers()](https://actix.rs/actix-web/actix_web/server/struct.HttpServer.html#method.workers) instead.
 

src/application.rs

@@ -29,7 +29,7 @@ pub struct HttpApplication<S = ()> {
 #[doc(hidden)]
 pub struct Inner<S> {
     prefix: usize,
-    default: Rc<RefCell<ResourceHandler<S>>>,
+    default: Rc<ResourceHandler<S>>,
     encoding: ContentEncoding,
     resources: Vec<ResourceHandler<S>>,
     handlers: Vec<PrefixHandlerType<S>>,
@@ -51,7 +51,7 @@ impl<S: 'static> PipelineHandler<S> for Inner<S> {
         match htype {
             HandlerType::Normal(idx) => match self.resources[idx].handle(req) {
                 Ok(result) => result,
-                Err(req) => match self.default.borrow_mut().handle(req) {
+                Err(req) => match self.default.handle(req) {
                     Ok(result) => result,
                     Err(_) => AsyncResult::ok(HttpResponse::new(StatusCode::NOT_FOUND)),
                 },
@@ -60,7 +60,7 @@ impl<S: 'static> PipelineHandler<S> for Inner<S> {
                 PrefixHandlerType::Handler(_, ref mut hnd) => hnd.handle(req),
                 PrefixHandlerType::Scope(_, ref mut hnd, _) => hnd.handle(req),
             },
-            HandlerType::Default => match self.default.borrow_mut().handle(req) {
+            HandlerType::Default => match self.default.handle(req) {
                 Ok(result) => result,
                 Err(_) => AsyncResult::ok(HttpResponse::new(StatusCode::NOT_FOUND)),
             },
@@ -138,9 +138,7 @@ impl<S: 'static> HttpApplication<S> {
 impl<S: 'static> HttpHandler for HttpApplication<S> {
     type Task = Pipeline<S, Inner<S>>;
 
-    fn handle(
-        &mut self, req: HttpRequest,
-    ) -> Result<Pipeline<S, Inner<S>>, HttpRequest> {
+    fn handle(&self, req: HttpRequest) -> Result<Pipeline<S, Inner<S>>, HttpRequest> {
         let m = {
             let path = req.path();
             path.starts_with(&self.prefix)
@@ -172,7 +170,7 @@ struct ApplicationParts<S> {
     state: S,
     prefix: String,
     settings: ServerSettings,
-    default: Rc<RefCell<ResourceHandler<S>>>,
+    default: Rc<ResourceHandler<S>>,
     resources: Vec<(Resource, Option<ResourceHandler<S>>)>,
     handlers: Vec<PrefixHandlerType<S>>,
     external: HashMap<String, Resource>,
@@ -223,7 +221,7 @@ where
                 state,
                 prefix: "/".to_owned(),
                 settings: ServerSettings::default(),
-                default: Rc::new(RefCell::new(ResourceHandler::default_not_found())),
+                default: Rc::new(ResourceHandler::default_not_found()),
                 resources: Vec::new(),
                 handlers: Vec::new(),
                 external: HashMap::new(),
@@ -335,7 +333,8 @@ where
         T: FromRequest<S> + 'static,
     {
         {
-            let parts: &mut ApplicationParts<S> = self.parts.as_mut().expect("Use after finish");
+            let parts: &mut ApplicationParts<S> =
+                self.parts.as_mut().expect("Use after finish");
 
             let out = {
                 // get resource handler
@@ -474,7 +473,9 @@ where
     {
         {
             let parts = self.parts.as_mut().expect("Use after finish");
-            f(&mut parts.default.borrow_mut());
+            let default = Rc::get_mut(&mut parts.default)
+                .expect("Multiple App instance references are not allowed");
+            f(default);
         }
         self
     }
@@ -707,7 +708,7 @@ struct BoxedApplication<S> {
 impl<S: 'static> HttpHandler for BoxedApplication<S> {
     type Task = Box<HttpHandlerTask>;
 
-    fn handle(&mut self, req: HttpRequest) -> Result<Self::Task, HttpRequest> {
+    fn handle(&self, req: HttpRequest) -> Result<Self::Task, HttpRequest> {
         self.app.handle(req).map(|t| {
             let task: Self::Task = Box::new(t);
             task

src/fs.rs

@@ -654,7 +654,7 @@ impl<S: 'static> StaticFiles<S> {
 impl<S: 'static> Handler<S> for StaticFiles<S> {
     type Result = Result<AsyncResult<HttpResponse>, Error>;
 
-    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
+    fn handle(&self, req: HttpRequest<S>) -> Self::Result {
         if !self.accessible {
             Ok(self.default.handle(req))
         } else {

src/handler.rs

@@ -1,4 +1,3 @@
-use std::cell::RefCell;
 use std::marker::PhantomData;
 use std::ops::Deref;
 use std::rc::Rc;
@@ -19,7 +18,7 @@ pub trait Handler<S>: 'static {
     type Result: Responder;
 
     /// Handle request
-    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result;
+    fn handle(&self, req: HttpRequest<S>) -> Self::Result;
 }
 
 /// Trait implemented by types that generate responses for clients.
@@ -209,7 +208,7 @@ where
 {
     type Result = R;
 
-    fn handle(&mut self, req: HttpRequest<S>) -> R {
+    fn handle(&self, req: HttpRequest<S>) -> R {
         (self)(req)
     }
 }
@@ -405,13 +404,13 @@ where
 
 // /// Trait defines object that could be registered as resource route
 pub(crate) trait RouteHandler<S>: 'static {
-    fn handle(&mut self, req: HttpRequest<S>) -> AsyncResult<HttpResponse>;
+    fn handle(&self, req: HttpRequest<S>) -> AsyncResult<HttpResponse>;
 
     fn has_default_resource(&self) -> bool {
         false
     }
 
-    fn default_resource(&mut self, default: Rc<RefCell<ResourceHandler<S>>>) {
+    fn default_resource(&mut self, _: Rc<ResourceHandler<S>>) {
         unimplemented!()
     }
 }
@@ -444,7 +443,7 @@ where
     R: Responder + 'static,
     S: 'static,
 {
-    fn handle(&mut self, req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
+    fn handle(&self, req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
         match self.h.handle(req.clone()).respond_to(&req) {
             Ok(reply) => reply.into(),
             Err(err) => AsyncResult::err(err.into()),
@@ -489,7 +488,7 @@ where
     E: Into<Error> + 'static,
     S: 'static,
 {
-    fn handle(&mut self, req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
+    fn handle(&self, req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
         let fut = (self.h)(req.clone()).map_err(|e| e.into()).then(move |r| {
             match r.respond_to(&req) {
                 Ok(reply) => match reply.into().into() {

src/helpers.rs

@@ -86,7 +86,7 @@ impl NormalizePath {
 impl<S> Handler<S> for NormalizePath {
     type Result = HttpResponse;
 
-    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
+    fn handle(&self, req: HttpRequest<S>) -> Self::Result {
         if let Some(router) = req.router() {
             let query = req.query_string();
             if self.merge {

src/resource.rs

@@ -282,9 +282,9 @@ impl<S: 'static> ResourceHandler<S> {
     }
 
     pub(crate) fn handle(
-        &mut self, mut req: HttpRequest<S>,
+        &self, mut req: HttpRequest<S>,
     ) -> Result<AsyncResult<HttpResponse>, HttpRequest<S>> {
-        for route in &mut self.routes {
+        for route in &self.routes {
             if route.check(&mut req) {
                 return if self.middlewares.borrow().is_empty() {
                     Ok(route.handle(req))

src/route.rs

@@ -49,13 +49,13 @@ impl<S: 'static> Route<S> {
     }
 
     #[inline]
-    pub(crate) fn handle(&mut self, req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
+    pub(crate) fn handle(&self, req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
         self.handler.handle(req)
     }
 
     #[inline]
     pub(crate) fn compose(
-        &mut self, req: HttpRequest<S>, mws: Rc<RefCell<Vec<Box<Middleware<S>>>>>,
+        &self, req: HttpRequest<S>, mws: Rc<RefCell<Vec<Box<Middleware<S>>>>>,
     ) -> AsyncResult<HttpResponse> {
         AsyncResult::async(Box::new(Compose::new(req, mws, self.handler.clone())))
     }

src/scope.rs

@@ -18,7 +18,7 @@ use pred::Predicate;
 use resource::ResourceHandler;
 use router::Resource;
 
-type ScopeResource<S> = Rc<RefCell<ResourceHandler<S>>>;
+type ScopeResource<S> = Rc<ResourceHandler<S>>;
 type Route<S> = UnsafeCell<Box<RouteHandler<S>>>;
 type ScopeResources<S> = Rc<Vec<(Resource, ScopeResource<S>)>>;
 type NestedInfo<S> = (Resource, Route<S>, Vec<Box<Predicate<S>>>);
@@ -236,9 +236,13 @@ impl<S: 'static> Scope<S> {
         }
 
         if found {
-            for &(ref pattern, ref resource) in self.resources.iter() {
+            let resources = Rc::get_mut(&mut self.resources)
+                .expect("Multiple scope references are not allowed");
+            for &mut (ref pattern, ref mut resource) in resources.iter_mut() {
                 if pattern.pattern() == path {
-                    resource.borrow_mut().method(method).with(f);
+                    let res = Rc::get_mut(resource)
+                        .expect("Multiple scope references are not allowed");
+                    res.method(method).with(f);
                     break;
                 }
             }
@@ -253,7 +257,7 @@ impl<S: 'static> Scope<S> {
             );
             Rc::get_mut(&mut self.resources)
                 .expect("Can not use after configuration")
-                .push((pattern, Rc::new(RefCell::new(handler))));
+                .push((pattern, Rc::new(handler)));
         }
         self
     }
@@ -297,7 +301,7 @@ impl<S: 'static> Scope<S> {
         );
         Rc::get_mut(&mut self.resources)
             .expect("Can not use after configuration")
-            .push((pattern, Rc::new(RefCell::new(handler))));
+            .push((pattern, Rc::new(handler)));
 
         self
     }
@@ -308,10 +312,13 @@ impl<S: 'static> Scope<S> {
         F: FnOnce(&mut ResourceHandler<S>) -> R + 'static,
     {
         if self.default.is_none() {
-            self.default =
-                Some(Rc::new(RefCell::new(ResourceHandler::default_not_found())));
+            self.default = Some(Rc::new(ResourceHandler::default_not_found()));
+        }
+        {
+            let default = Rc::get_mut(self.default.as_mut().unwrap())
+                .expect("Multiple copies of default handler");
+            f(default);
         }
-        f(&mut *self.default.as_ref().unwrap().borrow_mut());
         self
     }
 
@@ -332,18 +339,18 @@ impl<S: 'static> Scope<S> {
 }
 
 impl<S: 'static> RouteHandler<S> for Scope<S> {
-    fn handle(&mut self, mut req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
+    fn handle(&self, mut req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
         let tail = req.match_info().tail as usize;
 
         // recognize resources
         for &(ref pattern, ref resource) in self.resources.iter() {
             if pattern.match_with_params(&mut req, tail, false) {
                 if self.middlewares.borrow().is_empty() {
-                    return match resource.borrow_mut().handle(req) {
+                    return match resource.handle(req) {
                         Ok(result) => result,
                         Err(req) => {
                             if let Some(ref default) = self.default {
-                                match default.borrow_mut().handle(req) {
+                                match default.handle(req) {
                                     Ok(result) => result,
                                     Err(_) => AsyncResult::ok(HttpResponse::new(
                                         StatusCode::NOT_FOUND,
@@ -388,7 +395,7 @@ impl<S: 'static> RouteHandler<S> for Scope<S> {
         // default handler
         if self.middlewares.borrow().is_empty() {
             if let Some(ref default) = self.default {
-                match default.borrow_mut().handle(req) {
+                match default.handle(req) {
                     Ok(result) => result,
                     Err(_) => AsyncResult::ok(HttpResponse::new(StatusCode::NOT_FOUND)),
                 }
@@ -421,7 +428,7 @@ struct Wrapper<S: 'static> {
 }
 
 impl<S: 'static, S2: 'static> RouteHandler<S2> for Wrapper<S> {
-    fn handle(&mut self, req: HttpRequest<S2>) -> AsyncResult<HttpResponse> {
+    fn handle(&self, req: HttpRequest<S2>) -> AsyncResult<HttpResponse> {
         self.scope.handle(req.change_state(Rc::clone(&self.state)))
     }
 }
@@ -453,7 +460,7 @@ struct ComposeInfo<S: 'static> {
     count: usize,
     req: HttpRequest<S>,
     mws: Rc<RefCell<Vec<Box<Middleware<S>>>>>,
-    resource: Rc<RefCell<ResourceHandler<S>>>,
+    resource: Rc<ResourceHandler<S>>,
 }
 
 enum ComposeState<S: 'static> {
@@ -479,7 +486,7 @@ impl<S: 'static> ComposeState<S> {
 impl<S: 'static> Compose<S> {
     fn new(
         req: HttpRequest<S>, mws: Rc<RefCell<Vec<Box<Middleware<S>>>>>,
-        resource: Rc<RefCell<ResourceHandler<S>>>,
+        resource: Rc<ResourceHandler<S>>,
     ) -> Self {
         let mut info = ComposeInfo {
             count: 0,
@@ -527,8 +534,7 @@ impl<S: 'static> StartMiddlewares<S> {
             if info.count == len {
                 let reply = {
                     let req = info.req.clone();
-                    let mut resource = info.resource.borrow_mut();
-                    resource.handle(req).unwrap()
+                    info.resource.handle(req).unwrap()
                 };
                 return WaitingResponse::init(info, reply);
             } else {
@@ -564,8 +570,7 @@ impl<S: 'static> StartMiddlewares<S> {
                         if info.count == len {
                             let reply = {
                                 let req = info.req.clone();
-                                let mut resource = info.resource.borrow_mut();
-                                resource.handle(req).unwrap()
+                                info.resource.handle(req).unwrap()
                             };
                             return Some(WaitingResponse::init(info, reply));
                         } else {

src/server/mod.rs

@@ -126,14 +126,14 @@ pub trait HttpHandler: 'static {
     type Task: HttpHandlerTask;
 
     /// Handle request
-    fn handle(&mut self, req: HttpRequest) -> Result<Self::Task, HttpRequest>;
+    fn handle(&self, req: HttpRequest) -> Result<Self::Task, HttpRequest>;
 }
 
 impl HttpHandler for Box<HttpHandler<Task = Box<HttpHandlerTask>>> {
     type Task = Box<HttpHandlerTask>;
 
-    fn handle(&mut self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest> {
-        self.as_mut().handle(req)
+    fn handle(&self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest> {
+        self.as_ref().handle(req)
     }
 }
 

src/test.rs

@@ -564,7 +564,7 @@ impl<S: 'static> TestRequest<S> {
     /// with generated request.
     ///
     /// This method panics is handler returns actor or async result.
-    pub fn run<H: Handler<S>>(self, mut h: H) -> Result<HttpResponse, Error> {
+    pub fn run<H: Handler<S>>(self, h: H) -> Result<HttpResponse, Error> {
         let req = self.finish();
         let resp = h.handle(req.clone());
 

src/with.rs

@@ -56,7 +56,7 @@ where
 {
     type Result = AsyncResult<HttpResponse>;
 
-    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
+    fn handle(&self, req: HttpRequest<S>) -> Self::Result {
         let mut fut = WithHandlerFut {
             req,
             started: false,
@@ -192,7 +192,7 @@ where
 {
     type Result = AsyncResult<HttpResponse>;
 
-    fn handle(&mut self, req: HttpRequest<S>) -> Self::Result {
+    fn handle(&self, req: HttpRequest<S>) -> Self::Result {
         let mut fut = WithAsyncHandlerFut {
             req,
             started: false,