From 3de928459273a3c56f2a2ff823a17d549096d8be Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 21 Jun 2018 17:07:54 +0600 Subject: [PATCH] Handler::handle uses &self instead of mutabble reference --- CHANGES.md | 2 +- MIGRATION.md | 2 ++ src/application.rs | 23 ++++++++++++----------- src/fs.rs | 2 +- src/handler.rs | 13 ++++++------- src/helpers.rs | 2 +- src/resource.rs | 4 ++-- src/route.rs | 4 ++-- src/scope.rs | 43 ++++++++++++++++++++++++------------------- src/server/mod.rs | 6 +++--- src/test.rs | 2 +- src/with.rs | 4 ++-- 12 files changed, 57 insertions(+), 50 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4bbfe5bd2..71a5ae59b 100644 --- a/CHANGES.md +++ b/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/` to default headers when building a request diff --git a/MIGRATION.md b/MIGRATION.md index 73e2d5653..3b61e98cd 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -17,6 +17,8 @@ fn index((query, json): (Query<..>, Json 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. diff --git a/src/application.rs b/src/application.rs index 9ce99e5b1..fdeb164c5 100644 --- a/src/application.rs +++ b/src/application.rs @@ -29,7 +29,7 @@ pub struct HttpApplication { #[doc(hidden)] pub struct Inner { prefix: usize, - default: Rc>>, + default: Rc>, encoding: ContentEncoding, resources: Vec>, handlers: Vec>, @@ -51,7 +51,7 @@ impl PipelineHandler for Inner { 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 PipelineHandler for Inner { 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 HttpApplication { impl HttpHandler for HttpApplication { type Task = Pipeline>; - fn handle( - &mut self, req: HttpRequest, - ) -> Result>, HttpRequest> { + fn handle(&self, req: HttpRequest) -> Result>, HttpRequest> { let m = { let path = req.path(); path.starts_with(&self.prefix) @@ -172,7 +170,7 @@ struct ApplicationParts { state: S, prefix: String, settings: ServerSettings, - default: Rc>>, + default: Rc>, resources: Vec<(Resource, Option>)>, handlers: Vec>, external: HashMap, @@ -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 + 'static, { { - let parts: &mut ApplicationParts = self.parts.as_mut().expect("Use after finish"); + let parts: &mut ApplicationParts = + 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 { impl HttpHandler for BoxedApplication { type Task = Box; - fn handle(&mut self, req: HttpRequest) -> Result { + fn handle(&self, req: HttpRequest) -> Result { self.app.handle(req).map(|t| { let task: Self::Task = Box::new(t); task diff --git a/src/fs.rs b/src/fs.rs index 639626c57..61fa207e2 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -654,7 +654,7 @@ impl StaticFiles { impl Handler for StaticFiles { type Result = Result, Error>; - fn handle(&mut self, req: HttpRequest) -> Self::Result { + fn handle(&self, req: HttpRequest) -> Self::Result { if !self.accessible { Ok(self.default.handle(req)) } else { diff --git a/src/handler.rs b/src/handler.rs index 4428ce83f..61dd5694b 100644 --- a/src/handler.rs +++ b/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: 'static { type Result: Responder; /// Handle request - fn handle(&mut self, req: HttpRequest) -> Self::Result; + fn handle(&self, req: HttpRequest) -> 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) -> R { + fn handle(&self, req: HttpRequest) -> R { (self)(req) } } @@ -405,13 +404,13 @@ where // /// Trait defines object that could be registered as resource route pub(crate) trait RouteHandler: 'static { - fn handle(&mut self, req: HttpRequest) -> AsyncResult; + fn handle(&self, req: HttpRequest) -> AsyncResult; fn has_default_resource(&self) -> bool { false } - fn default_resource(&mut self, default: Rc>>) { + fn default_resource(&mut self, _: Rc>) { unimplemented!() } } @@ -444,7 +443,7 @@ where R: Responder + 'static, S: 'static, { - fn handle(&mut self, req: HttpRequest) -> AsyncResult { + fn handle(&self, req: HttpRequest) -> AsyncResult { 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 + 'static, S: 'static, { - fn handle(&mut self, req: HttpRequest) -> AsyncResult { + fn handle(&self, req: HttpRequest) -> AsyncResult { 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() { diff --git a/src/helpers.rs b/src/helpers.rs index c94c24d90..8dbf1b909 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -86,7 +86,7 @@ impl NormalizePath { impl Handler for NormalizePath { type Result = HttpResponse; - fn handle(&mut self, req: HttpRequest) -> Self::Result { + fn handle(&self, req: HttpRequest) -> Self::Result { if let Some(router) = req.router() { let query = req.query_string(); if self.merge { diff --git a/src/resource.rs b/src/resource.rs index 49e9ab0cc..570b79095 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -282,9 +282,9 @@ impl ResourceHandler { } pub(crate) fn handle( - &mut self, mut req: HttpRequest, + &self, mut req: HttpRequest, ) -> Result, HttpRequest> { - 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)) diff --git a/src/route.rs b/src/route.rs index 4c82926e8..42d68f199 100644 --- a/src/route.rs +++ b/src/route.rs @@ -49,13 +49,13 @@ impl Route { } #[inline] - pub(crate) fn handle(&mut self, req: HttpRequest) -> AsyncResult { + pub(crate) fn handle(&self, req: HttpRequest) -> AsyncResult { self.handler.handle(req) } #[inline] pub(crate) fn compose( - &mut self, req: HttpRequest, mws: Rc>>>>, + &self, req: HttpRequest, mws: Rc>>>>, ) -> AsyncResult { AsyncResult::async(Box::new(Compose::new(req, mws, self.handler.clone()))) } diff --git a/src/scope.rs b/src/scope.rs index 70fb17287..23e4a7238 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -18,7 +18,7 @@ use pred::Predicate; use resource::ResourceHandler; use router::Resource; -type ScopeResource = Rc>>; +type ScopeResource = Rc>; type Route = UnsafeCell>>; type ScopeResources = Rc)>>; type NestedInfo = (Resource, Route, Vec>>); @@ -236,9 +236,13 @@ impl Scope { } 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 Scope { ); 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 Scope { ); 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 Scope { F: FnOnce(&mut ResourceHandler) -> 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 Scope { } impl RouteHandler for Scope { - fn handle(&mut self, mut req: HttpRequest) -> AsyncResult { + fn handle(&self, mut req: HttpRequest) -> AsyncResult { 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 RouteHandler for Scope { // 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 { } impl RouteHandler for Wrapper { - fn handle(&mut self, req: HttpRequest) -> AsyncResult { + fn handle(&self, req: HttpRequest) -> AsyncResult { self.scope.handle(req.change_state(Rc::clone(&self.state))) } } @@ -453,7 +460,7 @@ struct ComposeInfo { count: usize, req: HttpRequest, mws: Rc>>>>, - resource: Rc>>, + resource: Rc>, } enum ComposeState { @@ -479,7 +486,7 @@ impl ComposeState { impl Compose { fn new( req: HttpRequest, mws: Rc>>>>, - resource: Rc>>, + resource: Rc>, ) -> Self { let mut info = ComposeInfo { count: 0, @@ -527,8 +534,7 @@ impl StartMiddlewares { 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 StartMiddlewares { 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 { diff --git a/src/server/mod.rs b/src/server/mod.rs index c0dabb263..b65fc3a86 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -126,14 +126,14 @@ pub trait HttpHandler: 'static { type Task: HttpHandlerTask; /// Handle request - fn handle(&mut self, req: HttpRequest) -> Result; + fn handle(&self, req: HttpRequest) -> Result; } impl HttpHandler for Box>> { type Task = Box; - fn handle(&mut self, req: HttpRequest) -> Result, HttpRequest> { - self.as_mut().handle(req) + fn handle(&self, req: HttpRequest) -> Result, HttpRequest> { + self.as_ref().handle(req) } } diff --git a/src/test.rs b/src/test.rs index 19e682d8d..7bf0f149e 100644 --- a/src/test.rs +++ b/src/test.rs @@ -564,7 +564,7 @@ impl TestRequest { /// with generated request. /// /// This method panics is handler returns actor or async result. - pub fn run>(self, mut h: H) -> Result { + pub fn run>(self, h: H) -> Result { let req = self.finish(); let resp = h.handle(req.clone()); diff --git a/src/with.rs b/src/with.rs index 126958b50..ac220958b 100644 --- a/src/with.rs +++ b/src/with.rs @@ -56,7 +56,7 @@ where { type Result = AsyncResult; - fn handle(&mut self, req: HttpRequest) -> Self::Result { + fn handle(&self, req: HttpRequest) -> Self::Result { let mut fut = WithHandlerFut { req, started: false, @@ -192,7 +192,7 @@ where { type Result = AsyncResult; - fn handle(&mut self, req: HttpRequest) -> Self::Result { + fn handle(&self, req: HttpRequest) -> Self::Result { let mut fut = WithAsyncHandlerFut { req, started: false,