From 65ca563579afc3b55279847cdd4ff6df41ee0e08 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 21 Jun 2018 23:06:23 +0600 Subject: [PATCH] use read only self for Middleware --- src/application.rs | 59 ++++++++++++------------ src/fs.rs | 4 +- src/helpers.rs | 8 ++-- src/json.rs | 2 +- src/middleware/cors.rs | 18 ++++---- src/middleware/csrf.rs | 12 ++--- src/middleware/defaultheaders.rs | 4 +- src/middleware/errhandlers.rs | 6 +-- src/middleware/identity.rs | 4 +- src/middleware/logger.rs | 6 +-- src/middleware/mod.rs | 6 +-- src/middleware/session.rs | 4 +- src/pipeline.rs | 48 +++++++++----------- src/resource.rs | 10 ++--- src/route.rs | 29 ++++++------ src/scope.rs | 77 +++++++++++++++----------------- src/test.rs | 2 +- tests/test_middleware.rs | 24 ++++------ 18 files changed, 150 insertions(+), 173 deletions(-) diff --git a/src/application.rs b/src/application.rs index fdeb164c5..bdc55fe79 100644 --- a/src/application.rs +++ b/src/application.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; @@ -21,9 +20,9 @@ pub struct HttpApplication { prefix: String, prefix_len: usize, router: Router, - inner: Rc>>, + inner: Rc>, filters: Option>>>, - middlewares: Rc>>>>, + middlewares: Rc>>>, } #[doc(hidden)] @@ -41,12 +40,13 @@ enum PrefixHandlerType { } impl PipelineHandler for Inner { + #[inline] fn encoding(&self) -> ContentEncoding { self.encoding } fn handle( - &mut self, req: HttpRequest, htype: HandlerType, + &self, req: HttpRequest, htype: HandlerType, ) -> AsyncResult { match htype { HandlerType::Normal(idx) => match self.resources[idx].handle(req) { @@ -57,8 +57,8 @@ impl PipelineHandler for Inner { }, }, HandlerType::Handler(idx) => match self.handlers[idx] { - PrefixHandlerType::Handler(_, ref mut hnd) => hnd.handle(req), - PrefixHandlerType::Scope(_, ref mut hnd, _) => hnd.handle(req), + PrefixHandlerType::Handler(_, ref hnd) => hnd.handle(req), + PrefixHandlerType::Scope(_, ref hnd, _) => hnd.handle(req), }, HandlerType::Default => match self.default.handle(req) { Ok(result) => result, @@ -74,14 +74,13 @@ impl HttpApplication { if let Some(idx) = self.router.recognize(req) { HandlerType::Normal(idx) } else { - let inner = self.inner.borrow(); req.match_info_mut().set_tail(0); - 'outer: for idx in 0..inner.handlers.len() { - match inner.handlers[idx] { + 'outer: for idx in 0..self.inner.handlers.len() { + match self.inner.handlers[idx] { PrefixHandlerType::Handler(ref prefix, _) => { let m = { - let path = &req.path()[inner.prefix..]; + let path = &req.path()[self.inner.prefix..]; let path_len = path.len(); path.starts_with(prefix) @@ -90,7 +89,7 @@ impl HttpApplication { }; if m { - let prefix_len = (inner.prefix + prefix.len()) as u16; + let prefix_len = (self.inner.prefix + prefix.len()) as u16; let url = req.url().clone(); req.set_prefix_len(prefix_len); req.match_info_mut().set_url(url); @@ -100,7 +99,7 @@ impl HttpApplication { } PrefixHandlerType::Scope(ref pattern, _, ref filters) => { if let Some(prefix_len) = - pattern.match_prefix_with_params(req, inner.prefix) + pattern.match_prefix_with_params(req, self.inner.prefix) { for filter in filters { if !filter.check(req) { @@ -108,7 +107,7 @@ impl HttpApplication { } } - let prefix_len = (inner.prefix + prefix_len) as u16; + let prefix_len = (self.inner.prefix + prefix_len) as u16; let url = req.url().clone(); req.set_prefix_len(prefix_len); let params = req.match_info_mut(); @@ -124,9 +123,9 @@ impl HttpApplication { } #[cfg(test)] - pub(crate) fn run(&mut self, mut req: HttpRequest) -> AsyncResult { + pub(crate) fn run(&self, mut req: HttpRequest) -> AsyncResult { let tp = self.get_handler(&mut req); - self.inner.borrow_mut().handle(req, tp) + self.inner.handle(req, tp) } #[cfg(test)] @@ -629,7 +628,7 @@ where resources.push((pattern, None)); } - for ref mut handler in parts.handlers.iter_mut() { + for handler in &mut parts.handlers { if let PrefixHandlerType::Scope(_, ref mut route_handler, _) = handler { if !route_handler.has_default_resource() { route_handler.default_resource(Rc::clone(&parts.default)); @@ -639,13 +638,13 @@ where let (router, resources) = Router::new(&prefix, parts.settings, resources); - let inner = Rc::new(RefCell::new(Inner { + let inner = Rc::new(Inner { prefix: prefix_len, default: Rc::clone(&parts.default), encoding: parts.encoding, handlers: parts.handlers, resources, - })); + }); let filters = if parts.filters.is_empty() { None } else { @@ -655,7 +654,7 @@ where HttpApplication { state: Rc::new(parts.state), router: router.clone(), - middlewares: Rc::new(RefCell::new(parts.middlewares)), + middlewares: Rc::new(parts.middlewares), prefix, prefix_len, inner, @@ -765,7 +764,7 @@ mod tests { #[test] fn test_default_resource() { - let mut app = App::new() + let app = App::new() .resource("/test", |r| r.f(|_| HttpResponse::Ok())) .finish(); @@ -777,7 +776,7 @@ mod tests { let resp = app.run(req); assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); - let mut app = App::new() + let app = App::new() .default_resource(|r| r.f(|_| HttpResponse::MethodNotAllowed())) .finish(); let req = TestRequest::with_uri("/blah").finish(); @@ -787,7 +786,7 @@ mod tests { #[test] fn test_unhandled_prefix() { - let mut app = App::new() + let app = App::new() .prefix("/test") .resource("/test", |r| r.f(|_| HttpResponse::Ok())) .finish(); @@ -796,7 +795,7 @@ mod tests { #[test] fn test_state() { - let mut app = App::with_state(10) + let app = App::with_state(10) .resource("/", |r| r.f(|_| HttpResponse::Ok())) .finish(); let req = @@ -807,7 +806,7 @@ mod tests { #[test] fn test_prefix() { - let mut app = App::new() + let app = App::new() .prefix("/test") .resource("/blah", |r| r.f(|_| HttpResponse::Ok())) .finish(); @@ -830,7 +829,7 @@ mod tests { #[test] fn test_handler() { - let mut app = App::new().handler("/test", |_| HttpResponse::Ok()).finish(); + let app = App::new().handler("/test", |_| HttpResponse::Ok()).finish(); let req = TestRequest::with_uri("/test").finish(); let resp = app.run(req); @@ -855,7 +854,7 @@ mod tests { #[test] fn test_handler2() { - let mut app = App::new().handler("test", |_| HttpResponse::Ok()).finish(); + let app = App::new().handler("test", |_| HttpResponse::Ok()).finish(); let req = TestRequest::with_uri("/test").finish(); let resp = app.run(req); @@ -880,7 +879,7 @@ mod tests { #[test] fn test_handler_with_prefix() { - let mut app = App::new() + let app = App::new() .prefix("prefix") .handler("/test", |_| HttpResponse::Ok()) .finish(); @@ -908,7 +907,7 @@ mod tests { #[test] fn test_route() { - let mut app = App::new() + let app = App::new() .route("/test", Method::GET, |_: HttpRequest| HttpResponse::Ok()) .route("/test", Method::POST, |_: HttpRequest| { HttpResponse::Created() @@ -930,7 +929,7 @@ mod tests { #[test] fn test_handler_prefix() { - let mut app = App::new() + let app = App::new() .prefix("/app") .handler("/test", |_| HttpResponse::Ok()) .finish(); @@ -980,7 +979,7 @@ mod tests { #[test] fn test_option_responder() { - let mut app = App::new() + let app = App::new() .resource("/none", |r| r.f(|_| -> Option<&'static str> { None })) .resource("/some", |r| r.f(|_| Some("some"))) .finish(); diff --git a/src/fs.rs b/src/fs.rs index 61fa207e2..c5a7de615 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -1204,7 +1204,7 @@ mod tests { #[test] fn test_redirect_to_index() { - let mut st = StaticFiles::new(".").index_file("index.html"); + let st = StaticFiles::new(".").index_file("index.html"); let mut req = HttpRequest::default(); req.match_info_mut().add_static("tail", "tests"); @@ -1230,7 +1230,7 @@ mod tests { #[test] fn test_redirect_to_index_nested() { - let mut st = StaticFiles::new(".").index_file("mod.rs"); + let st = StaticFiles::new(".").index_file("mod.rs"); let mut req = HttpRequest::default(); req.match_info_mut().add_static("tail", "src/client"); diff --git a/src/helpers.rs b/src/helpers.rs index 8dbf1b909..0b35f047c 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -181,7 +181,7 @@ mod tests { #[test] fn test_normalize_path_trailing_slashes() { - let mut app = App::new() + let app = App::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())) @@ -222,7 +222,7 @@ mod tests { #[test] fn test_normalize_path_trailing_slashes_disabled() { - let mut app = App::new() + let app = App::new() .resource("/resource1", |r| r.method(Method::GET).f(index)) .resource("/resource2/", |r| r.method(Method::GET).f(index)) .default_resource(|r| { @@ -255,7 +255,7 @@ mod tests { #[test] fn test_normalize_path_merge_slashes() { - let mut app = App::new() + let app = App::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())) @@ -344,7 +344,7 @@ mod tests { #[test] fn test_normalize_path_merge_and_append_slashes() { - let mut app = App::new() + let app = App::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)) diff --git a/src/json.rs b/src/json.rs index 0b5cb96e4..3f9188c14 100644 --- a/src/json.rs +++ b/src/json.rs @@ -412,7 +412,7 @@ mod tests { fn test_with_json() { let mut cfg = JsonConfig::default(); cfg.limit(4096); - let mut handler = With::new(|data: Json| data, cfg); + let handler = With::new(|data: Json| data, cfg); let req = HttpRequest::default(); assert!(handler.handle(req).as_err().is_some()); diff --git a/src/middleware/cors.rs b/src/middleware/cors.rs index 454596567..734f7be4b 100644 --- a/src/middleware/cors.rs +++ b/src/middleware/cors.rs @@ -356,7 +356,7 @@ impl Cors { } impl Middleware for Cors { - fn start(&mut self, req: &mut HttpRequest) -> Result { + fn start(&self, req: &mut HttpRequest) -> Result { if self.inner.preflight && Method::OPTIONS == *req.method() { self.validate_origin(req)?; self.validate_allowed_method(req)?; @@ -434,7 +434,7 @@ impl Middleware for Cors { } fn response( - &mut self, req: &mut HttpRequest, mut resp: HttpResponse, + &self, req: &mut HttpRequest, mut resp: HttpResponse, ) -> Result { match self.inner.origins { AllOrSome::All => { @@ -944,7 +944,7 @@ mod tests { #[test] fn validate_origin_allows_all_origins() { - let mut cors = Cors::default(); + let cors = Cors::default(); let mut req = TestRequest::with_header("Origin", "https://www.example.com").finish(); @@ -1013,7 +1013,7 @@ mod tests { // #[test] // #[should_panic(expected = "MissingOrigin")] // fn test_validate_missing_origin() { - // let mut cors = Cors::build() + // let cors = Cors::build() // .allowed_origin("https://www.example.com") // .finish(); // let mut req = HttpRequest::default(); @@ -1023,7 +1023,7 @@ mod tests { #[test] #[should_panic(expected = "OriginNotAllowed")] fn test_validate_not_allowed_origin() { - let mut cors = Cors::build() + let cors = Cors::build() .allowed_origin("https://www.example.com") .finish(); @@ -1035,7 +1035,7 @@ mod tests { #[test] fn test_validate_origin() { - let mut cors = Cors::build() + let cors = Cors::build() .allowed_origin("https://www.example.com") .finish(); @@ -1048,7 +1048,7 @@ mod tests { #[test] fn test_no_origin_response() { - let mut cors = Cors::build().finish(); + let cors = Cors::build().finish(); let mut req = TestRequest::default().method(Method::GET).finish(); let resp: HttpResponse = HttpResponse::Ok().into(); @@ -1074,7 +1074,7 @@ mod tests { #[test] fn test_response() { - let mut cors = Cors::build() + let cors = Cors::build() .send_wildcard() .disable_preflight() .max_age(3600) @@ -1109,7 +1109,7 @@ mod tests { resp.headers().get(header::VARY).unwrap().as_bytes() ); - let mut cors = Cors::build() + let cors = Cors::build() .disable_vary_header() .allowed_origin("https://www.example.com") .finish(); diff --git a/src/middleware/csrf.rs b/src/middleware/csrf.rs index 8c2b06e72..670ec1c1f 100644 --- a/src/middleware/csrf.rs +++ b/src/middleware/csrf.rs @@ -209,7 +209,7 @@ impl CsrfFilter { } impl Middleware for CsrfFilter { - fn start(&mut self, req: &mut HttpRequest) -> Result { + fn start(&self, req: &mut HttpRequest) -> Result { self.validate(req)?; Ok(Started::Done) } @@ -223,7 +223,7 @@ mod tests { #[test] fn test_safe() { - let mut csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); + let csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); let mut req = TestRequest::with_header("Origin", "https://www.w3.org") .method(Method::HEAD) @@ -234,7 +234,7 @@ mod tests { #[test] fn test_csrf() { - let mut csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); + let csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); let mut req = TestRequest::with_header("Origin", "https://www.w3.org") .method(Method::POST) @@ -245,7 +245,7 @@ mod tests { #[test] fn test_referer() { - let mut csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); + let csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); let mut req = TestRequest::with_header( "Referer", @@ -258,9 +258,9 @@ mod tests { #[test] fn test_upgrade() { - let mut strict_csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); + let strict_csrf = CsrfFilter::new().allowed_origin("https://www.example.com"); - let mut lax_csrf = CsrfFilter::new() + let lax_csrf = CsrfFilter::new() .allowed_origin("https://www.example.com") .allow_upgrade(); diff --git a/src/middleware/defaultheaders.rs b/src/middleware/defaultheaders.rs index acccc552f..dca8dfbe1 100644 --- a/src/middleware/defaultheaders.rs +++ b/src/middleware/defaultheaders.rs @@ -75,7 +75,7 @@ impl DefaultHeaders { impl Middleware for DefaultHeaders { fn response( - &mut self, _: &mut HttpRequest, mut resp: HttpResponse, + &self, _: &mut HttpRequest, mut resp: HttpResponse, ) -> Result { for (key, value) in self.headers.iter() { if !resp.headers().contains_key(key) { @@ -100,7 +100,7 @@ mod tests { #[test] fn test_default_headers() { - let mut mw = DefaultHeaders::new().header(CONTENT_TYPE, "0001"); + let mw = DefaultHeaders::new().header(CONTENT_TYPE, "0001"); let mut req = HttpRequest::default(); diff --git a/src/middleware/errhandlers.rs b/src/middleware/errhandlers.rs index e1b484182..fe148fdd0 100644 --- a/src/middleware/errhandlers.rs +++ b/src/middleware/errhandlers.rs @@ -71,7 +71,7 @@ impl ErrorHandlers { impl Middleware for ErrorHandlers { fn response( - &mut self, req: &mut HttpRequest, resp: HttpResponse, + &self, req: &mut HttpRequest, resp: HttpResponse, ) -> Result { if let Some(handler) = self.handlers.get(&resp.status()) { handler(req, resp) @@ -99,7 +99,7 @@ mod tests { #[test] fn test_handler() { - let mut mw = + let mw = ErrorHandlers::new().handler(StatusCode::INTERNAL_SERVER_ERROR, render_500); let mut req = HttpRequest::default(); @@ -121,7 +121,7 @@ mod tests { struct MiddlewareOne; impl Middleware for MiddlewareOne { - fn start(&mut self, _req: &mut HttpRequest) -> Result { + fn start(&self, _req: &mut HttpRequest) -> Result { Err(ErrorInternalServerError("middleware error")) } } diff --git a/src/middleware/identity.rs b/src/middleware/identity.rs index 58cc0de40..f40894289 100644 --- a/src/middleware/identity.rs +++ b/src/middleware/identity.rs @@ -178,7 +178,7 @@ impl IdentityService { struct IdentityBox(Box); impl> Middleware for IdentityService { - fn start(&mut self, req: &mut HttpRequest) -> Result { + fn start(&self, req: &mut HttpRequest) -> Result { let mut req = req.clone(); let fut = self @@ -195,7 +195,7 @@ impl> Middleware for IdentityService { } fn response( - &mut self, req: &mut HttpRequest, resp: HttpResponse, + &self, req: &mut HttpRequest, resp: HttpResponse, ) -> Result { if let Some(mut id) = req.extensions_mut().remove::() { id.0.write(resp) diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index ab9ae4a02..a731d6955 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -124,14 +124,14 @@ impl Logger { } impl Middleware for Logger { - fn start(&mut self, req: &mut HttpRequest) -> Result { + fn start(&self, req: &mut HttpRequest) -> Result { if !self.exclude.contains(req.path()) { req.extensions_mut().insert(StartTime(time::now())); } Ok(Started::Done) } - fn finish(&mut self, req: &mut HttpRequest, resp: &HttpResponse) -> Finished { + fn finish(&self, req: &mut HttpRequest, resp: &HttpResponse) -> Finished { self.log(req, resp); Finished::Done } @@ -322,7 +322,7 @@ mod tests { #[test] fn test_logger() { - let mut logger = Logger::new("%% %{User-Agent}i %{X-Test}o %{HOME}e %D test"); + let logger = Logger::new("%% %{User-Agent}i %{X-Test}o %{HOME}e %D test"); let mut headers = HeaderMap::new(); headers.insert( diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index 7fd339327..2551ded15 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -51,20 +51,20 @@ pub enum Finished { pub trait Middleware: 'static { /// Method is called when request is ready. It may return /// future, which should resolve before next middleware get called. - fn start(&mut self, req: &mut HttpRequest) -> Result { + fn start(&self, req: &mut HttpRequest) -> Result { Ok(Started::Done) } /// Method is called when handler returns response, /// but before sending http message to peer. fn response( - &mut self, req: &mut HttpRequest, resp: HttpResponse, + &self, req: &mut HttpRequest, resp: HttpResponse, ) -> Result { Ok(Response::Done(resp)) } /// Method is called after body stream get sent to peer. - fn finish(&mut self, req: &mut HttpRequest, resp: &HttpResponse) -> Finished { + fn finish(&self, req: &mut HttpRequest, resp: &HttpResponse) -> Finished { Finished::Done } } diff --git a/src/middleware/session.rs b/src/middleware/session.rs index bb6c82233..bd10b3c23 100644 --- a/src/middleware/session.rs +++ b/src/middleware/session.rs @@ -246,7 +246,7 @@ impl> SessionStorage { } impl> Middleware for SessionStorage { - fn start(&mut self, req: &mut HttpRequest) -> Result { + fn start(&self, req: &mut HttpRequest) -> Result { let mut req = req.clone(); let fut = self.0.from_request(&mut req).then(move |res| match res { @@ -261,7 +261,7 @@ impl> Middleware for SessionStorage { } fn response( - &mut self, req: &mut HttpRequest, resp: HttpResponse, + &self, req: &mut HttpRequest, resp: HttpResponse, ) -> Result { if let Some(s_box) = req.extensions_mut().remove::>() { s_box.0.borrow_mut().write(resp) diff --git a/src/pipeline.rs b/src/pipeline.rs index 2e03c8f62..fe5e1d02a 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -1,4 +1,4 @@ -use std::cell::{RefCell, UnsafeCell}; +use std::cell::UnsafeCell; use std::marker::PhantomData; use std::rc::Rc; use std::{io, mem}; @@ -31,7 +31,7 @@ pub trait PipelineHandler { fn encoding(&self) -> ContentEncoding; fn handle( - &mut self, req: HttpRequest, htype: HandlerType, + &self, req: HttpRequest, htype: HandlerType, ) -> AsyncResult; } @@ -74,7 +74,7 @@ impl> PipelineState { struct PipelineInfo { req: UnsafeCell>, count: u16, - mws: Rc>>>>, + mws: Rc>>>, context: Option>, error: Option, disconnected: Option, @@ -86,7 +86,7 @@ impl PipelineInfo { PipelineInfo { req: UnsafeCell::new(req), count: 0, - mws: Rc::new(RefCell::new(Vec::new())), + mws: Rc::new(Vec::new()), error: None, context: None, disconnected: None, @@ -123,8 +123,8 @@ impl PipelineInfo { impl> Pipeline { pub fn new( - req: HttpRequest, mws: Rc>>>>, - handler: Rc>, htype: HandlerType, + req: HttpRequest, mws: Rc>>>, handler: Rc, + htype: HandlerType, ) -> Pipeline { let mut info = PipelineInfo { mws, @@ -133,7 +133,7 @@ impl> Pipeline { error: None, context: None, disconnected: None, - encoding: handler.borrow().encoding(), + encoding: handler.encoding(), }; let state = StartMiddlewares::init(&mut info, handler, htype); @@ -238,7 +238,7 @@ type Fut = Box, Error = Error>>; /// Middlewares start executor struct StartMiddlewares { - hnd: Rc>, + hnd: Rc, htype: HandlerType, fut: Option, _s: PhantomData, @@ -246,18 +246,17 @@ struct StartMiddlewares { impl> StartMiddlewares { fn init( - info: &mut PipelineInfo, hnd: Rc>, htype: HandlerType, + info: &mut PipelineInfo, hnd: Rc, htype: HandlerType, ) -> PipelineState { // execute middlewares, we need this stage because middlewares could be // non-async and we can move to next state immediately - let len = info.mws.borrow().len() as u16; + let len = info.mws.len() as u16; loop { if info.count == len { - let reply = hnd.borrow_mut().handle(info.req().clone(), htype); + let reply = hnd.handle(info.req().clone(), htype); return WaitingResponse::init(info, reply); } else { - let state = - info.mws.borrow_mut()[info.count as usize].start(info.req_mut()); + let state = info.mws[info.count as usize].start(info.req_mut()); match state { Ok(Started::Done) => info.count += 1, Ok(Started::Response(resp)) => { @@ -278,7 +277,7 @@ impl> StartMiddlewares { } fn poll(&mut self, info: &mut PipelineInfo) -> Option> { - let len = info.mws.borrow().len() as u16; + let len = info.mws.len() as u16; 'outer: loop { match self.fut.as_mut().unwrap().poll() { Ok(Async::NotReady) => return None, @@ -289,14 +288,11 @@ impl> StartMiddlewares { } loop { if info.count == len { - let reply = self - .hnd - .borrow_mut() - .handle(info.req().clone(), self.htype); + let reply = self.hnd.handle(info.req().clone(), self.htype); return Some(WaitingResponse::init(info, reply)); } else { - let state = info.mws.borrow_mut()[info.count as usize] - .start(info.req_mut()); + let state = + info.mws[info.count as usize].start(info.req_mut()); match state { Ok(Started::Done) => info.count += 1, Ok(Started::Response(resp)) => { @@ -366,10 +362,10 @@ impl RunMiddlewares { return ProcessResponse::init(resp); } let mut curr = 0; - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { - let state = info.mws.borrow_mut()[curr].response(info.req_mut(), resp); + let state = info.mws[curr].response(info.req_mut(), resp); resp = match state { Err(err) => { info.count = (curr + 1) as u16; @@ -396,7 +392,7 @@ impl RunMiddlewares { } fn poll(&mut self, info: &mut PipelineInfo) -> Option> { - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { // poll latest fut @@ -413,8 +409,7 @@ impl RunMiddlewares { if self.curr == len { return Some(ProcessResponse::init(resp)); } else { - let state = - info.mws.borrow_mut()[self.curr].response(info.req_mut(), resp); + let state = info.mws[self.curr].response(info.req_mut(), resp); match state { Err(err) => return Some(ProcessResponse::init(err.into())), Ok(Response::Done(r)) => { @@ -739,8 +734,7 @@ impl FinishingMiddlewares { } info.count -= 1; - let state = info.mws.borrow_mut()[info.count as usize] - .finish(info.req_mut(), &self.resp); + let state = info.mws[info.count as usize].finish(info.req_mut(), &self.resp); match state { Finished::Done => { if info.count == 0 { diff --git a/src/resource.rs b/src/resource.rs index 570b79095..2eae570c8 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::marker::PhantomData; use std::rc::Rc; @@ -38,7 +37,7 @@ pub struct ResourceHandler { name: String, state: PhantomData, routes: SmallVec<[Route; 3]>, - middlewares: Rc>>>>, + middlewares: Rc>>>, } impl Default for ResourceHandler { @@ -47,7 +46,7 @@ impl Default for ResourceHandler { name: String::new(), state: PhantomData, routes: SmallVec::new(), - middlewares: Rc::new(RefCell::new(Vec::new())), + middlewares: Rc::new(Vec::new()), } } } @@ -58,7 +57,7 @@ impl ResourceHandler { name: String::new(), state: PhantomData, routes: SmallVec::new(), - middlewares: Rc::new(RefCell::new(Vec::new())), + middlewares: Rc::new(Vec::new()), } } @@ -277,7 +276,6 @@ impl ResourceHandler { pub fn middleware>(&mut self, mw: M) { Rc::get_mut(&mut self.middlewares) .unwrap() - .borrow_mut() .push(Box::new(mw)); } @@ -286,7 +284,7 @@ impl ResourceHandler { ) -> Result, HttpRequest> { for route in &self.routes { if route.check(&mut req) { - return if self.middlewares.borrow().is_empty() { + return if self.middlewares.is_empty() { Ok(route.handle(req)) } else { Ok(route.compose(req, Rc::clone(&self.middlewares))) diff --git a/src/route.rs b/src/route.rs index 42d68f199..80fb17d7c 100644 --- a/src/route.rs +++ b/src/route.rs @@ -1,4 +1,4 @@ -use std::cell::{RefCell, UnsafeCell}; +use std::cell::UnsafeCell; use std::marker::PhantomData; use std::rc::Rc; @@ -55,7 +55,7 @@ impl Route { #[inline] pub(crate) fn compose( - &self, req: HttpRequest, mws: Rc>>>>, + &self, req: HttpRequest, mws: Rc>>>, ) -> AsyncResult { AsyncResult::async(Box::new(Compose::new(req, mws, self.handler.clone()))) } @@ -340,7 +340,7 @@ struct Compose { struct ComposeInfo { count: usize, req: HttpRequest, - mws: Rc>>>>, + mws: Rc>>>, handler: InnerHandler, } @@ -366,8 +366,7 @@ impl ComposeState { impl Compose { fn new( - req: HttpRequest, mws: Rc>>>>, - handler: InnerHandler, + req: HttpRequest, mws: Rc>>>, handler: InnerHandler, ) -> Self { let mut info = ComposeInfo { count: 0, @@ -410,13 +409,13 @@ type Fut = Box, Error = Error>>; impl StartMiddlewares { fn init(info: &mut ComposeInfo) -> ComposeState { - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { if info.count == len { let reply = info.handler.handle(info.req.clone()); return WaitingResponse::init(info, reply); } else { - let state = info.mws.borrow_mut()[info.count].start(&mut info.req); + let state = info.mws[info.count].start(&mut info.req); match state { Ok(MiddlewareStarted::Done) => info.count += 1, Ok(MiddlewareStarted::Response(resp)) => { @@ -435,7 +434,7 @@ impl StartMiddlewares { } fn poll(&mut self, info: &mut ComposeInfo) -> Option> { - let len = info.mws.borrow().len(); + let len = info.mws.len(); 'outer: loop { match self.fut.as_mut().unwrap().poll() { Ok(Async::NotReady) => return None, @@ -449,8 +448,7 @@ impl StartMiddlewares { let reply = info.handler.handle(info.req.clone()); return Some(WaitingResponse::init(info, reply)); } else { - let state = - info.mws.borrow_mut()[info.count].start(&mut info.req); + let state = info.mws[info.count].start(&mut info.req); match state { Ok(MiddlewareStarted::Done) => info.count += 1, Ok(MiddlewareStarted::Response(resp)) => { @@ -513,10 +511,10 @@ struct RunMiddlewares { impl RunMiddlewares { fn init(info: &mut ComposeInfo, mut resp: HttpResponse) -> ComposeState { let mut curr = 0; - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { - let state = info.mws.borrow_mut()[curr].response(&mut info.req, resp); + let state = info.mws[curr].response(&mut info.req, resp); resp = match state { Err(err) => { info.count = curr + 1; @@ -542,7 +540,7 @@ impl RunMiddlewares { } fn poll(&mut self, info: &mut ComposeInfo) -> Option> { - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { // poll latest fut @@ -559,8 +557,7 @@ impl RunMiddlewares { if self.curr == len { return Some(FinishingMiddlewares::init(info, resp)); } else { - let state = - info.mws.borrow_mut()[self.curr].response(&mut info.req, resp); + let state = info.mws[self.curr].response(&mut info.req, resp); match state { Err(err) => { return Some(FinishingMiddlewares::init(info, err.into())) @@ -630,7 +627,7 @@ impl FinishingMiddlewares { info.count -= 1; - let state = info.mws.borrow_mut()[info.count as usize] + let state = info.mws[info.count as usize] .finish(&mut info.req, self.resp.as_ref().unwrap()); match state { MiddlewareFinished::Done => { diff --git a/src/scope.rs b/src/scope.rs index 23e4a7238..a56d753b6 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,4 +1,4 @@ -use std::cell::{RefCell, UnsafeCell}; +use std::cell::UnsafeCell; use std::marker::PhantomData; use std::mem; use std::rc::Rc; @@ -57,7 +57,7 @@ type NestedInfo = (Resource, Route, Vec>>); pub struct Scope { filters: Vec>>, nested: Vec>, - middlewares: Rc>>>>, + middlewares: Rc>>>, default: Option>, resources: ScopeResources, } @@ -71,7 +71,7 @@ impl Scope { filters: Vec::new(), nested: Vec::new(), resources: Rc::new(Vec::new()), - middlewares: Rc::new(RefCell::new(Vec::new())), + middlewares: Rc::new(Vec::new()), default: None, } } @@ -135,7 +135,7 @@ impl Scope { filters: Vec::new(), nested: Vec::new(), resources: Rc::new(Vec::new()), - middlewares: Rc::new(RefCell::new(Vec::new())), + middlewares: Rc::new(Vec::new()), default: None, }; let mut scope = f(scope); @@ -178,7 +178,7 @@ impl Scope { filters: Vec::new(), nested: Vec::new(), resources: Rc::new(Vec::new()), - middlewares: Rc::new(RefCell::new(Vec::new())), + middlewares: Rc::new(Vec::new()), default: None, }; let mut scope = f(scope); @@ -332,7 +332,6 @@ impl Scope { pub fn middleware>(mut self, mw: M) -> Scope { Rc::get_mut(&mut self.middlewares) .expect("Can not use after configuration") - .borrow_mut() .push(Box::new(mw)); self } @@ -345,7 +344,7 @@ impl RouteHandler for Scope { // 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() { + if self.middlewares.is_empty() { return match resource.handle(req) { Ok(result) => result, Err(req) => { @@ -393,7 +392,7 @@ impl RouteHandler for Scope { } // default handler - if self.middlewares.borrow().is_empty() { + if self.middlewares.is_empty() { if let Some(ref default) = self.default { match default.handle(req) { Ok(result) => result, @@ -459,7 +458,7 @@ struct Compose { struct ComposeInfo { count: usize, req: HttpRequest, - mws: Rc>>>>, + mws: Rc>>>, resource: Rc>, } @@ -485,7 +484,7 @@ impl ComposeState { impl Compose { fn new( - req: HttpRequest, mws: Rc>>>>, + req: HttpRequest, mws: Rc>>>, resource: Rc>, ) -> Self { let mut info = ComposeInfo { @@ -529,7 +528,7 @@ type Fut = Box, Error = Error>>; impl StartMiddlewares { fn init(info: &mut ComposeInfo) -> ComposeState { - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { if info.count == len { let reply = { @@ -538,7 +537,7 @@ impl StartMiddlewares { }; return WaitingResponse::init(info, reply); } else { - let state = info.mws.borrow_mut()[info.count].start(&mut info.req); + let state = info.mws[info.count].start(&mut info.req); match state { Ok(MiddlewareStarted::Done) => info.count += 1, Ok(MiddlewareStarted::Response(resp)) => { @@ -557,7 +556,7 @@ impl StartMiddlewares { } fn poll(&mut self, info: &mut ComposeInfo) -> Option> { - let len = info.mws.borrow().len(); + let len = info.mws.len(); 'outer: loop { match self.fut.as_mut().unwrap().poll() { Ok(Async::NotReady) => return None, @@ -574,8 +573,7 @@ impl StartMiddlewares { }; return Some(WaitingResponse::init(info, reply)); } else { - let state = - info.mws.borrow_mut()[info.count].start(&mut info.req); + let state = info.mws[info.count].start(&mut info.req); match state { Ok(MiddlewareStarted::Done) => info.count += 1, Ok(MiddlewareStarted::Response(resp)) => { @@ -638,10 +636,10 @@ struct RunMiddlewares { impl RunMiddlewares { fn init(info: &mut ComposeInfo, mut resp: HttpResponse) -> ComposeState { let mut curr = 0; - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { - let state = info.mws.borrow_mut()[curr].response(&mut info.req, resp); + let state = info.mws[curr].response(&mut info.req, resp); resp = match state { Err(err) => { info.count = curr + 1; @@ -667,7 +665,7 @@ impl RunMiddlewares { } fn poll(&mut self, info: &mut ComposeInfo) -> Option> { - let len = info.mws.borrow().len(); + let len = info.mws.len(); loop { // poll latest fut @@ -684,8 +682,7 @@ impl RunMiddlewares { if self.curr == len { return Some(FinishingMiddlewares::init(info, resp)); } else { - let state = - info.mws.borrow_mut()[self.curr].response(&mut info.req, resp); + let state = info.mws[self.curr].response(&mut info.req, resp); match state { Err(err) => { return Some(FinishingMiddlewares::init(info, err.into())) @@ -754,7 +751,7 @@ impl FinishingMiddlewares { } info.count -= 1; - let state = info.mws.borrow_mut()[info.count as usize] + let state = info.mws[info.count as usize] .finish(&mut info.req, self.resp.as_ref().unwrap()); match state { MiddlewareFinished::Done => { @@ -798,7 +795,7 @@ mod tests { #[test] fn test_scope() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.resource("/path1", |r| r.f(|_| HttpResponse::Ok())) }) @@ -811,7 +808,7 @@ mod tests { #[test] fn test_scope_root() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope .resource("", |r| r.f(|_| HttpResponse::Ok())) @@ -830,7 +827,7 @@ mod tests { #[test] fn test_scope_root2() { - let mut app = App::new() + let app = App::new() .scope("/app/", |scope| { scope.resource("", |r| r.f(|_| HttpResponse::Ok())) }) @@ -847,7 +844,7 @@ mod tests { #[test] fn test_scope_root3() { - let mut app = App::new() + let app = App::new() .scope("/app/", |scope| { scope.resource("/", |r| r.f(|_| HttpResponse::Ok())) }) @@ -864,7 +861,7 @@ mod tests { #[test] fn test_scope_route() { - let mut app = App::new() + let app = App::new() .scope("app", |scope| { scope .route("/path1", Method::GET, |_: HttpRequest<_>| { @@ -895,7 +892,7 @@ mod tests { #[test] fn test_scope_filter() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope .filter(pred::Get()) @@ -918,7 +915,7 @@ mod tests { #[test] fn test_scope_variable_segment() { - let mut app = App::new() + let app = App::new() .scope("/ab-{project}", |scope| { scope.resource("/path1", |r| { r.f(|r| { @@ -950,7 +947,7 @@ mod tests { fn test_scope_with_state() { struct State; - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.with_state("/t1", State, |scope| { scope.resource("/path1", |r| r.f(|_| HttpResponse::Created())) @@ -967,7 +964,7 @@ mod tests { fn test_scope_with_state_root() { struct State; - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.with_state("/t1", State, |scope| { scope @@ -990,7 +987,7 @@ mod tests { fn test_scope_with_state_root2() { struct State; - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.with_state("/t1/", State, |scope| { scope.resource("", |r| r.f(|_| HttpResponse::Ok())) @@ -1011,7 +1008,7 @@ mod tests { fn test_scope_with_state_root3() { struct State; - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.with_state("/t1/", State, |scope| { scope.resource("/", |r| r.f(|_| HttpResponse::Ok())) @@ -1032,7 +1029,7 @@ mod tests { fn test_scope_with_state_filter() { struct State; - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.with_state("/t1", State, |scope| { scope @@ -1057,7 +1054,7 @@ mod tests { #[test] fn test_nested_scope() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.nested("/t1", |scope| { scope.resource("/path1", |r| r.f(|_| HttpResponse::Created())) @@ -1072,7 +1069,7 @@ mod tests { #[test] fn test_nested_scope_root() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.nested("/t1", |scope| { scope @@ -1093,7 +1090,7 @@ mod tests { #[test] fn test_nested_scope_filter() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.nested("/t1", |scope| { scope @@ -1118,7 +1115,7 @@ mod tests { #[test] fn test_nested_scope_with_variable_segment() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.nested("/{project_id}", |scope| { scope.resource("/path1", |r| { @@ -1148,7 +1145,7 @@ mod tests { #[test] fn test_nested2_scope_with_variable_segment() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope.nested("/{project}", |scope| { scope.nested("/{id}", |scope| { @@ -1185,7 +1182,7 @@ mod tests { #[test] fn test_default_resource() { - let mut app = App::new() + let app = App::new() .scope("/app", |scope| { scope .resource("/path1", |r| r.f(|_| HttpResponse::Ok())) @@ -1204,7 +1201,7 @@ mod tests { #[test] fn test_default_resource_propagation() { - let mut app = App::new() + let app = App::new() .scope("/app1", |scope| { scope.default_resource(|r| r.f(|_| HttpResponse::BadRequest())) }) diff --git a/src/test.rs b/src/test.rs index 7bf0f149e..58790f6d4 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, h: H) -> Result { + pub fn run>(self, h: &H) -> Result { let req = self.finish(); let resp = h.handle(req.clone()); diff --git a/tests/test_middleware.rs b/tests/test_middleware.rs index bdcde1482..806211ea8 100644 --- a/tests/test_middleware.rs +++ b/tests/test_middleware.rs @@ -20,23 +20,21 @@ struct MiddlewareTest { } impl middleware::Middleware for MiddlewareTest { - fn start(&mut self, _: &mut HttpRequest) -> Result { + fn start(&self, _: &mut HttpRequest) -> Result { self.start .store(self.start.load(Ordering::Relaxed) + 1, Ordering::Relaxed); Ok(middleware::Started::Done) } fn response( - &mut self, _: &mut HttpRequest, resp: HttpResponse, + &self, _: &mut HttpRequest, resp: HttpResponse, ) -> Result { self.response .store(self.response.load(Ordering::Relaxed) + 1, Ordering::Relaxed); Ok(middleware::Response::Done(resp)) } - fn finish( - &mut self, _: &mut HttpRequest, _: &HttpResponse, - ) -> middleware::Finished { + fn finish(&self, _: &mut HttpRequest, _: &HttpResponse) -> middleware::Finished { self.finish .store(self.finish.load(Ordering::Relaxed) + 1, Ordering::Relaxed); middleware::Finished::Done @@ -434,7 +432,7 @@ struct MiddlewareAsyncTest { } impl middleware::Middleware for MiddlewareAsyncTest { - fn start(&mut self, _: &mut HttpRequest) -> Result { + fn start(&self, _: &mut HttpRequest) -> Result { let to = Delay::new(Instant::now() + Duration::from_millis(10)); let start = Arc::clone(&self.start); @@ -447,7 +445,7 @@ impl middleware::Middleware for MiddlewareAsyncTest { } fn response( - &mut self, _: &mut HttpRequest, resp: HttpResponse, + &self, _: &mut HttpRequest, resp: HttpResponse, ) -> Result { let to = Delay::new(Instant::now() + Duration::from_millis(10)); @@ -460,9 +458,7 @@ impl middleware::Middleware for MiddlewareAsyncTest { ))) } - fn finish( - &mut self, _: &mut HttpRequest, _: &HttpResponse, - ) -> middleware::Finished { + fn finish(&self, _: &mut HttpRequest, _: &HttpResponse) -> middleware::Finished { let to = Delay::new(Instant::now() + Duration::from_millis(10)); let finish = Arc::clone(&self.finish); @@ -797,9 +793,7 @@ fn test_async_sync_resource_middleware_multiple() { struct MiddlewareWithErr; impl middleware::Middleware for MiddlewareWithErr { - fn start( - &mut self, _req: &mut HttpRequest, - ) -> Result { + fn start(&self, _req: &mut HttpRequest) -> Result { Err(ErrorInternalServerError("middleware error")) } } @@ -807,9 +801,7 @@ impl middleware::Middleware for MiddlewareWithErr { struct MiddlewareAsyncWithErr; impl middleware::Middleware for MiddlewareAsyncWithErr { - fn start( - &mut self, _req: &mut HttpRequest, - ) -> Result { + fn start(&self, _req: &mut HttpRequest) -> Result { Ok(middleware::Started::Future(Box::new(future::err( ErrorInternalServerError("middleware error"), ))))