From ffa95b7f6079f8253345762748c243d9b3f25765 Mon Sep 17 00:00:00 2001 From: manuelgdlvh Date: Wed, 14 May 2025 20:26:28 +0200 Subject: [PATCH] feat: Add improvements in resource path resolve --- actix-router/src/de.rs | 12 +-- actix-router/src/resource.rs | 176 ++++++++++++++++++++--------------- actix-router/src/router.rs | 48 ++++++++-- actix-router/src/url.rs | 2 +- actix-web/src/app_service.rs | 51 +++++----- actix-web/src/scope.rs | 144 ++++++++++++++-------------- actix-web/src/types/path.rs | 10 +- 7 files changed, 254 insertions(+), 189 deletions(-) diff --git a/actix-router/src/de.rs b/actix-router/src/de.rs index 2f50619f8..3237637bd 100644 --- a/actix-router/src/de.rs +++ b/actix-router/src/de.rs @@ -662,13 +662,13 @@ mod tests { let rdef = ResourceDef::new("/{key}"); let mut path = Path::new("/%25"); - rdef.capture_match_info(&mut path); + rdef.resolve_resource_if_matches(&mut path); let de = PathDeserializer::new(&path); let segment: String = serde::Deserialize::deserialize(de).unwrap(); assert_eq!(segment, "%"); let mut path = Path::new("/%2F"); - rdef.capture_match_info(&mut path); + rdef.resolve_resource_if_matches(&mut path); let de = PathDeserializer::new(&path); let segment: String = serde::Deserialize::deserialize(de).unwrap(); assert_eq!(segment, "/") @@ -679,7 +679,7 @@ mod tests { let rdef = ResourceDef::new("/{key}/{value}"); let mut path = Path::new("/%30%25/%30%2F"); - rdef.capture_match_info(&mut path); + rdef.resolve_resource_if_matches(&mut path); let de = PathDeserializer::new(&path); let segment: (String, String) = serde::Deserialize::deserialize(de).unwrap(); assert_eq!(segment.0, "0%"); @@ -697,7 +697,7 @@ mod tests { let rdef = ResourceDef::new("/{key}/{value}"); let mut path = Path::new("/%25/%2F"); - rdef.capture_match_info(&mut path); + rdef.resolve_resource_if_matches(&mut path); let de = PathDeserializer::new(&path); let vals: Vals = serde::Deserialize::deserialize(de).unwrap(); assert_eq!(vals.key, "%"); @@ -714,7 +714,7 @@ mod tests { let rdef = ResourceDef::new("/{val}"); let mut path = Path::new("/X"); - rdef.capture_match_info(&mut path); + rdef.resolve_resource_if_matches(&mut path); let de = PathDeserializer::new(&path); let params: Params<'_> = serde::Deserialize::deserialize(de).unwrap(); assert_eq!(params.val, "X"); @@ -723,7 +723,7 @@ mod tests { assert_eq!(params, "X"); let mut path = Path::new("/%2F"); - rdef.capture_match_info(&mut path); + rdef.resolve_resource_if_matches(&mut path); let de = PathDeserializer::new(&path); assert!( as serde::Deserialize>::deserialize(de).is_err()); let de = PathDeserializer::new(&path); diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index b5ee01958..3882140fb 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -8,9 +8,9 @@ use std::{ use tracing::error; use crate::{ + IntoPatterns, path::PathItem, - regex_set::{escape, Regex, RegexSet}, - IntoPatterns, Patterns, Resource, ResourcePath, + Patterns, regex_set::{escape, Regex, RegexSet}, Resource, ResourcePath, }; const MAX_DYNAMIC_SEGMENTS: usize = 16; @@ -96,7 +96,7 @@ const REGEX_FLAGS: &str = "(?s-m)"; /// assert!(!resource.is_match("/user/")); /// /// let mut path = Path::new("/user/123"); -/// resource.capture_match_info(&mut path); +/// resource.resolve_resource_if_matches(&mut path); /// assert_eq!(path.get("id").unwrap(), "123"); /// ``` /// @@ -171,7 +171,7 @@ const REGEX_FLAGS: &str = "(?s-m)"; /// assert!(resource.is_match("/blob/HEAD/README.md")); /// /// let mut path = Path::new("/blob/main/LICENSE"); -/// resource.capture_match_info(&mut path); +/// resource.resolve_resource_if_matches(&mut path); /// assert_eq!(path.get("tail").unwrap(), "main/LICENSE"); /// ``` /// @@ -249,6 +249,17 @@ enum PatternType { DynamicSet(RegexSet, Vec<(Regex, Vec<&'static str>)>), } +pub enum ResourceMatchInfo<'a> { + Static { + matched_len: u16, + }, + Dynamic { + matched_len: u16, + matched_vars: &'a [&'static str], + segments: [PathItem; MAX_DYNAMIC_SEGMENTS], + }, +} + impl ResourceDef { /// Constructs a new resource definition from patterns. /// @@ -440,7 +451,7 @@ impl ResourceDef { /// assert_eq!(iter.next().unwrap(), "/root"); /// assert_eq!(iter.next().unwrap(), "/backup"); /// assert!(iter.next().is_none()); - pub fn pattern_iter(&self) -> impl Iterator { + pub fn pattern_iter(&self) -> impl Iterator { struct PatternIter<'a> { patterns: &'a Patterns, list_idx: usize, @@ -623,18 +634,24 @@ impl ResourceDef { /// /// let resource = ResourceDef::prefix("/user/{id}"); /// let mut path = Path::new("/user/123/stars"); - /// assert!(resource.capture_match_info(&mut path)); + /// assert!(resource.resolve_resource_if_matches(&mut path)); /// assert_eq!(path.get("id").unwrap(), "123"); /// assert_eq!(path.unprocessed(), "/stars"); /// /// let resource = ResourceDef::new("/blob/{path}*"); /// let mut path = Path::new("/blob/HEAD/Cargo.toml"); - /// assert!(resource.capture_match_info(&mut path)); + /// assert!(resource.resolve_resource_if_matches(&mut path)); /// assert_eq!(path.get("path").unwrap(), "HEAD/Cargo.toml"); /// assert_eq!(path.unprocessed(), ""); /// ``` - pub fn capture_match_info(&self, resource: &mut R) -> bool { - self.capture_match_info_fn(resource, |_| true) + pub fn resolve_resource_if_matches(&self, resource: &mut R) -> bool { + match self.capture_match_info(resource) { + None => { false } + Some(match_info) => { + self.resolve_resource(resource, match_info); + true + } + } } /// Collects dynamic segment values into `resource` after matching paths and executing @@ -674,80 +691,90 @@ impl ResourceDef { /// assert!(!try_match(&resource, &mut path)); /// assert_eq!(path.unprocessed(), "/user/admin/stars"); /// ``` - pub fn capture_match_info_fn(&self, resource: &mut R, check_fn: F) -> bool + pub fn capture_match_info(&self, resource: &mut R) -> Option> where R: Resource, - F: FnOnce(&R) -> bool, { - let mut segments = <[PathItem; MAX_DYNAMIC_SEGMENTS]>::default(); let path = resource.resource_path(); let path_str = path.unprocessed(); - - let (matched_len, matched_vars) = match &self.pat_type { + match &self.pat_type { PatternType::Static(pattern) => match self.static_match(pattern, path_str) { - Some(len) => (len, None), - None => return false, + Some(len) => Some(ResourceMatchInfo::Static { matched_len: len as u16 }), + None => return None, }, PatternType::Dynamic(re, names) => { - let captures = match re.captures(path.unprocessed()) { + let captures = match re.captures(path_str) { Some(captures) => captures, - _ => return false, + _ => return None, }; + let mut segments = <[PathItem; MAX_DYNAMIC_SEGMENTS]>::default(); for (no, name) in names.iter().enumerate() { if let Some(m) = captures.name(name) { segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); } else { error!("Dynamic path match but not all segments found: {}", name); - return false; + return None; } } - (captures[1].len(), Some(names)) + Some(ResourceMatchInfo::Dynamic { + matched_len: captures[1].len() as u16, + matched_vars: names, + segments, + }) } PatternType::DynamicSet(re, params) => { - let path = path.unprocessed(); - let (pattern, names) = match re.first_match_idx(path) { + let (pattern, names) = match re.first_match_idx(path_str) { Some(idx) => ¶ms[idx], - _ => return false, + _ => return None, }; - let captures = match pattern.captures(path.path()) { + let captures = match pattern.captures(path_str) { Some(captures) => captures, - _ => return false, + _ => return None, }; + let mut segments = <[PathItem; MAX_DYNAMIC_SEGMENTS]>::default(); for (no, name) in names.iter().enumerate() { if let Some(m) = captures.name(name) { segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); } else { error!("Dynamic path match but not all segments found: {}", name); - return false; + return None; } } - (captures[1].len(), Some(names)) + Some(ResourceMatchInfo::Dynamic { + matched_len: captures[1].len() as u16, + matched_vars: names, + segments, + }) } - }; - - if !check_fn(resource) { - return false; } + } - // Modify `path` to skip matched part and store matched segments + + /// + pub fn resolve_resource(&self, resource: &mut R, match_info: ResourceMatchInfo<'_>) + where + R: Resource, + { let path = resource.resource_path(); + match match_info { + ResourceMatchInfo::Static { matched_len } => { + path.skip(matched_len); + } + ResourceMatchInfo::Dynamic { matched_len, matched_vars, mut segments } => { + for i in 0..matched_vars.len() { + path.add(matched_vars[i], mem::take(&mut segments[i])); + } - if let Some(vars) = matched_vars { - for i in 0..vars.len() { - path.add(vars[i], mem::take(&mut segments[i])); + path.skip(matched_len); } } - - path.skip(matched_len as u16); - - true } /// Assembles resource path using a closure that maps variable segment names to values. @@ -1118,9 +1145,10 @@ pub(crate) fn insert_slash(path: &str) -> Cow<'_, str> { #[cfg(test)] mod tests { - use super::*; use crate::Path; + use super::*; + #[test] fn equivalence() { assert_eq!( @@ -1171,7 +1199,7 @@ mod tests { assert!(!re.is_match("/name~")); let mut path = Path::new("/name"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.unprocessed(), ""); assert_eq!(re.find_match("/name"), Some(5)); @@ -1189,7 +1217,7 @@ mod tests { assert!(!re.is_match("/user/profile/profile")); let mut path = Path::new("/user/profile"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.unprocessed(), ""); } @@ -1202,12 +1230,12 @@ mod tests { assert!(!re.is_match("/user/2345/sdg")); let mut path = Path::new("/user/profile"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "profile"); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/user/1245125"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "1245125"); assert_eq!(path.unprocessed(), ""); @@ -1217,7 +1245,7 @@ mod tests { assert!(!re.is_match("/resource")); let mut path = Path::new("/v151/resource/adage32"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("version").unwrap(), "151"); assert_eq!(path.get("id").unwrap(), "adage32"); assert_eq!(path.unprocessed(), ""); @@ -1229,7 +1257,7 @@ mod tests { assert!(!re.is_match("/XXXXXX")); let mut path = Path::new("/012345"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "012345"); assert_eq!(path.unprocessed(), ""); } @@ -1249,12 +1277,12 @@ mod tests { assert!(!re.is_match("/user/2345/sdg")); let mut path = Path::new("/user/profile"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "profile"); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/user/1245125"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "1245125"); assert_eq!(path.unprocessed(), ""); @@ -1263,7 +1291,7 @@ mod tests { assert!(!re.is_match("/resource")); let mut path = Path::new("/v151/resource/adage32"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("version").unwrap(), "151"); assert_eq!(path.get("id").unwrap(), "adage32"); @@ -1277,7 +1305,7 @@ mod tests { assert!(!re.is_match("/static/a")); let mut path = Path::new("/012345"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "012345"); let re = ResourceDef::new([ @@ -1314,7 +1342,7 @@ mod tests { assert_eq!(re.find_match("/12345"), None); let mut path = Path::new("/151/res"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "151"); assert_eq!(path.unprocessed(), "/res"); } @@ -1324,19 +1352,19 @@ mod tests { let re = ResourceDef::new("/user/-{id}*"); let mut path = Path::new("/user/-profile"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "profile"); let mut path = Path::new("/user/-2345"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "2345"); let mut path = Path::new("/user/-2345/"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "2345/"); let mut path = Path::new("/user/-2345/sdg"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "2345/sdg"); } @@ -1364,7 +1392,7 @@ mod tests { let re = ResourceDef::new("/user/{id}/{tail}*"); assert!(!re.is_match("/user/2345")); let mut path = Path::new("/user/2345/sdg"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "2345"); assert_eq!(path.get("tail").unwrap(), "sdg"); assert_eq!(path.unprocessed(), ""); @@ -1379,7 +1407,7 @@ mod tests { let re = ResourceDef::new("/a{x}b/test/a{y}b"); let mut path = Path::new("/a\nb/test/a\nb"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("x").unwrap(), "\n"); assert_eq!(path.get("y").unwrap(), "\n"); @@ -1388,12 +1416,12 @@ mod tests { let re = ResourceDef::new("/user/{id}*"); let mut path = Path::new("/user/a\nb/a\nb"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "a\nb/a\nb"); let re = ResourceDef::new("/user/{id:.*}"); let mut path = Path::new("/user/a\nb/a\nb"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "a\nb/a\nb"); } @@ -1403,16 +1431,16 @@ mod tests { let re = ResourceDef::new("/user/{id}/test"); let mut path = Path::new("/user/2345/test"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "2345"); let mut path = Path::new("/user/qwe%25/test"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "qwe%25"); let uri = http::Uri::try_from("/user/qwe%25/test").unwrap(); let mut path = Path::new(uri); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.get("id").unwrap(), "qwe%25"); } @@ -1429,11 +1457,11 @@ mod tests { assert!(!re.is_match("/name~")); let mut path = Path::new("/name"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/name/test"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.unprocessed(), "/test"); assert_eq!(re.find_match("/name"), Some(5)); @@ -1449,10 +1477,10 @@ mod tests { assert!(!re.is_match("/name")); let mut path = Path::new("/name/gs"); - assert!(!re.capture_match_info(&mut path)); + assert!(!re.resolve_resource_if_matches(&mut path)); let mut path = Path::new("/name//gs"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(path.unprocessed(), "/gs"); let re = ResourceDef::root_prefix("name/"); @@ -1462,7 +1490,7 @@ mod tests { assert!(!re.is_match("/name")); let mut path = Path::new("/name/gs"); - assert!(!re.capture_match_info(&mut path)); + assert!(!re.resolve_resource_if_matches(&mut path)); } #[test] @@ -1481,13 +1509,13 @@ mod tests { assert_eq!(re.find_match(""), None); let mut path = Path::new("/test2/"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(&path["name"], "test2"); assert_eq!(&path[0], "test2"); assert_eq!(path.unprocessed(), "/"); let mut path = Path::new("/test2/subpath1/subpath2/index.html"); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); assert_eq!(&path["name"], "test2"); assert_eq!(&path[0], "test2"); assert_eq!(path.unprocessed(), "/subpath1/subpath2/index.html"); @@ -1543,7 +1571,7 @@ mod tests { assert!(resource.resource_path_from_iter( &mut s, #[allow(clippy::useless_vec)] - &mut vec!["item", "item2"].iter() + &mut vec!["item", "item2"].iter(), )); assert_eq!(s, "/user/item/item2/"); } @@ -1561,22 +1589,22 @@ mod tests { let resource = ResourceDef::new(["/user/{id}", "/profile/{id}"]); let mut path = Path::new("/user/123"); - assert!(resource.capture_match_info(&mut path)); + assert!(resource.resolve_resource_if_matches(&mut path)); assert!(path.get("id").is_some()); let mut path = Path::new("/profile/123"); - assert!(resource.capture_match_info(&mut path)); + assert!(resource.resolve_resource_if_matches(&mut path)); assert!(path.get("id").is_some()); let resource = ResourceDef::new(["/user/{id}", "/profile/{uid}"]); let mut path = Path::new("/user/123"); - assert!(resource.capture_match_info(&mut path)); + assert!(resource.resolve_resource_if_matches(&mut path)); assert!(path.get("id").is_some()); assert!(path.get("uid").is_none()); let mut path = Path::new("/profile/123"); - assert!(resource.capture_match_info(&mut path)); + assert!(resource.resolve_resource_if_matches(&mut path)); assert!(path.get("id").is_none()); assert!(path.get("uid").is_some()); } diff --git a/actix-router/src/router.rs b/actix-router/src/router.rs index b20cb7ee3..1557146cf 100644 --- a/actix-router/src/router.rs +++ b/actix-router/src/router.rs @@ -13,12 +13,13 @@ pub struct ResourceId(pub u16); /// not required. pub struct Router { routes: Vec<(ResourceDef, T, U)>, + max_path_conflicts: u16, } impl Router { /// Constructs new `RouterBuilder` with empty route list. pub fn build() -> RouterBuilder { - RouterBuilder { routes: Vec::new() } + RouterBuilder { routes: Vec::new(), path_conflicts: vec![] } } /// Finds the value in the router that matches a given [routing resource](Resource). @@ -46,14 +47,24 @@ impl Router { /// the `check` closure is executed, passing the resource and each route's context data. If the /// closure returns true then the match result is stored into `resource` and a reference to /// the matched _value_ is returned. - pub fn recognize_fn(&self, resource: &mut R, mut check: F) -> Option<(&T, ResourceId)> + pub fn recognize_fn(&self, resource: &mut R, mut check_fn: F) -> Option<(&T, ResourceId)> where R: Resource, F: FnMut(&R, &U) -> bool, { + let mut matches = 0; for (rdef, val, ctx) in self.routes.iter() { - if rdef.capture_match_info_fn(resource, |res| check(res, ctx)) { - return Some((val, ResourceId(rdef.id()))); + match rdef.capture_match_info(resource) { + None => {} + Some(match_info) => { + matches += 1; + if check_fn(resource, ctx) { + rdef.resolve_resource(resource, match_info); + return Some((val, ResourceId(rdef.id()))); + } else if matches == self.max_path_conflicts { + return None; + } + } } } @@ -65,15 +76,25 @@ impl Router { pub fn recognize_mut_fn( &mut self, resource: &mut R, - mut check: F, + mut check_fn: F, ) -> Option<(&mut T, ResourceId)> where R: Resource, F: FnMut(&R, &U) -> bool, { + let mut matches = 0; for (rdef, val, ctx) in self.routes.iter_mut() { - if rdef.capture_match_info_fn(resource, |res| check(res, ctx)) { - return Some((val, ResourceId(rdef.id()))); + match rdef.capture_match_info(resource) { + None => {} + Some(match_info) => { + matches += 1; + if check_fn(resource, ctx) { + rdef.resolve_resource(resource, match_info); + return Some((val, ResourceId(rdef.id()))); + } else if matches == self.max_path_conflicts { + return None; + } + } } } @@ -84,6 +105,7 @@ impl Router { /// Builder for an ordered [routing](Router) list. pub struct RouterBuilder { routes: Vec<(ResourceDef, T, U)>, + path_conflicts: Vec<(ResourceDef, u16)>, } impl RouterBuilder { @@ -96,6 +118,13 @@ impl RouterBuilder { val: T, ctx: U, ) -> (&mut ResourceDef, &mut T, &mut U) { + if let Some((_, path_conflicts)) = self.path_conflicts.iter_mut() + .find(|(current_rdef, _)| rdef.eq(current_rdef)) { + *path_conflicts += 1; + } else { + self.path_conflicts.push((rdef.clone(), 1)); + } + self.routes.push((rdef, val, ctx)); #[allow(clippy::map_identity)] // map is used to distribute &mut-ness to tuple elements self.routes @@ -106,8 +135,13 @@ impl RouterBuilder { /// Finish configuration and create router instance. pub fn finish(self) -> Router { + let max_path_conflicts = self.path_conflicts.iter() + .map(|(_, path_conflicts)| *path_conflicts) + .max() + .unwrap_or(1); Router { routes: self.routes, + max_path_conflicts, } } } diff --git a/actix-router/src/url.rs b/actix-router/src/url.rs index b3d9e1121..cfdb34b60 100644 --- a/actix-router/src/url.rs +++ b/actix-router/src/url.rs @@ -75,7 +75,7 @@ mod tests { let re = ResourceDef::new(pattern); let uri = Uri::try_from(url.as_ref()).unwrap(); let mut path = Path::new(Url::new(uri)); - assert!(re.capture_match_info(&mut path)); + assert!(re.resolve_resource_if_matches(&mut path)); path } diff --git a/actix-web/src/app_service.rs b/actix-web/src/app_service.rs index 7aa16b790..5eff76c95 100644 --- a/actix-web/src/app_service.rs +++ b/actix-web/src/app_service.rs @@ -1,24 +1,25 @@ use std::{cell::RefCell, mem, rc::Rc}; -use actix_http::Request; -use actix_router::{Path, ResourceDef, Router, Url}; use actix_service::{boxed, fn_service, Service, ServiceFactory}; use futures_core::future::LocalBoxFuture; use futures_util::future::join_all; +use actix_http::Request; +use actix_router::{Path, ResourceDef, Router, Url}; + use crate::{ body::BoxBody, config::{AppConfig, AppService}, data::FnDataFactory, dev::Extensions, + Error, guard::Guard, + HttpResponse, request::{HttpRequest, HttpRequestPool}, - rmap::ResourceMap, - service::{ + rmap::ResourceMap, service::{ AppServiceFactory, BoxedHttpService, BoxedHttpServiceFactory, ServiceRequest, ServiceResponse, }, - Error, HttpResponse, }; /// Service factory to convert [`Request`] to a [`ServiceRequest`]. @@ -28,10 +29,10 @@ pub struct AppInit where T: ServiceFactory< ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), + Config=(), + Response=ServiceResponse, + Error=Error, + InitError=(), >, { pub(crate) endpoint: T, @@ -47,10 +48,10 @@ impl ServiceFactory for AppInit where T: ServiceFactory< ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), + Config=(), + Response=ServiceResponse, + Error=Error, + InitError=(), >, T::Future: 'static, { @@ -144,7 +145,7 @@ where /// Wraps a service receiving a [`ServiceRequest`] into one receiving a [`Request`]. pub struct AppInitService where - T: Service, Error = Error>, + T: Service, Error=Error>, { service: T, app_data: Rc, @@ -189,7 +190,7 @@ impl AppInitServiceState { impl Service for AppInitService where - T: Service, Error = Error>, + T: Service, Error=Error>, { type Response = ServiceResponse; type Error = T::Error; @@ -229,7 +230,7 @@ where impl Drop for AppInitService where - T: Service, Error = Error>, + T: Service, Error=Error>, { fn drop(&mut self) { self.app_state.pool().clear(); @@ -306,16 +307,16 @@ impl Service for AppRouting { actix_service::always_ready!(); fn call(&self, mut req: ServiceRequest) -> Self::Future { - let res = self.router.recognize_fn(&mut req, |req, guards| { + let guards_check_fn = |req: &ServiceRequest, guards: &Vec>| { let guard_ctx = req.guard_ctx(); guards.iter().all(|guard| guard.check(&guard_ctx)) - }); + }; + let res = self.router.recognize_fn(&mut req, guards_check_fn); if let Some((srv, _info)) = res { - srv.call(req) - } else { - self.default.call(req) + return srv.call(req); } + self.default.call(req) } } @@ -346,15 +347,15 @@ impl ServiceFactory for AppEntry { #[cfg(test)] mod tests { use std::sync::{ - atomic::{AtomicBool, Ordering}, Arc, + atomic::{AtomicBool, Ordering}, }; use actix_service::Service; use crate::{ - test::{init_service, TestRequest}, - web, App, HttpResponse, + App, + HttpResponse, test::{init_service, TestRequest}, web, }; struct DropData(Arc); @@ -377,7 +378,7 @@ mod tests { .data(DropData(data.clone())) .service(web::resource("/test").to(HttpResponse::Ok)), ) - .await; + .await; let req = TestRequest::with_uri("/test").to_request(); let _ = app.call(req).await.unwrap(); } diff --git a/actix-web/src/scope.rs b/actix-web/src/scope.rs index e317349da..743385cdc 100644 --- a/actix-web/src/scope.rs +++ b/actix-web/src/scope.rs @@ -1,7 +1,5 @@ use std::{cell::RefCell, fmt, future::Future, mem, rc::Rc}; -use actix_http::{body::MessageBody, Extensions}; -use actix_router::{ResourceDef, Router}; use actix_service::{ apply, apply_fn_factory, boxed, IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt, Transform, @@ -9,17 +7,20 @@ use actix_service::{ use futures_core::future::LocalBoxFuture; use futures_util::future::join_all; +use actix_http::{body::MessageBody, Extensions}; +use actix_router::{ResourceDef, Router}; + use crate::{ config::ServiceConfig, data::Data, dev::AppService, + Error, guard::Guard, - rmap::ResourceMap, - service::{ + Resource, + rmap::ResourceMap, Route, service::{ AppServiceFactory, BoxedHttpService, BoxedHttpServiceFactory, HttpServiceFactory, ServiceFactoryWrapper, ServiceRequest, ServiceResponse, }, - Error, Resource, Route, }; type Guards = Vec>; @@ -85,7 +86,7 @@ impl Scope { impl Scope where - T: ServiceFactory, + T: ServiceFactory, { /// Add match guard to a scope. /// @@ -272,8 +273,8 @@ where pub fn default_service(mut self, f: F) -> Self where F: IntoServiceFactory, - U: ServiceFactory - + 'static, + U: ServiceFactory + + 'static, U::InitError: fmt::Debug, { // create and configure default resource @@ -300,20 +301,20 @@ where ) -> Scope< impl ServiceFactory< ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), + Config=(), + Response=ServiceResponse, + Error=Error, + InitError=(), >, > where M: Transform< - T::Service, - ServiceRequest, - Response = ServiceResponse, - Error = Error, - InitError = (), - > + 'static, + T::Service, + ServiceRequest, + Response=ServiceResponse, + Error=Error, + InitError=(), + > + 'static, B: MessageBody, { Scope { @@ -343,15 +344,15 @@ where ) -> Scope< impl ServiceFactory< ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), + Config=(), + Response=ServiceResponse, + Error=Error, + InitError=(), >, > where F: Fn(ServiceRequest, &T::Service) -> R + Clone + 'static, - R: Future, Error>>, + R: Future, Error>>, B: MessageBody, { Scope { @@ -370,12 +371,12 @@ where impl HttpServiceFactory for Scope where T: ServiceFactory< - ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), - > + 'static, + ServiceRequest, + Config=(), + Response=ServiceResponse, + Error=Error, + InitError=(), + > + 'static, B: MessageBody + 'static, { fn register(mut self, config: &mut AppService) { @@ -510,16 +511,16 @@ impl Service for ScopeService { actix_service::always_ready!(); fn call(&self, mut req: ServiceRequest) -> Self::Future { - let res = self.router.recognize_fn(&mut req, |req, guards| { + let guards_check_fn = |req: &ServiceRequest, guards: &Vec>| { let guard_ctx = req.guard_ctx(); guards.iter().all(|guard| guard.check(&guard_ctx)) - }); + }; + let res = self.router.recognize_fn(&mut req, guards_check_fn); if let Some((srv, _info)) = res { - srv.call(req) - } else { - self.default.call(req) + return srv.call(req); } + self.default.call(req) } } @@ -552,18 +553,19 @@ mod tests { use actix_utils::future::ok; use bytes::Bytes; - use super::*; use crate::{ + App, guard, http::{ header::{self, HeaderValue}, Method, StatusCode, }, - middleware::DefaultHeaders, - test::{assert_body_eq, call_service, init_service, read_body, TestRequest}, - web, App, HttpMessage, HttpRequest, HttpResponse, + HttpMessage, + HttpRequest, HttpResponse, middleware::DefaultHeaders, test::{assert_body_eq, call_service, init_service, read_body, TestRequest}, web, }; + use super::*; + #[test] fn can_be_returned_from_fn() { fn my_scope_1() -> Scope { @@ -574,10 +576,10 @@ mod tests { fn my_scope_2() -> Scope< impl ServiceFactory< ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), + Config=(), + Response=ServiceResponse, + Error=Error, + InitError=(), >, > { web::scope("/test-compat") @@ -604,7 +606,7 @@ mod tests { App::new() .service(web::scope("/app").service(web::resource("/path1").to(HttpResponse::Ok))), ) - .await; + .await; let req = TestRequest::with_uri("/app/path1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -620,7 +622,7 @@ mod tests { .service(web::resource("/").to(HttpResponse::Created)), ), ) - .await; + .await; let req = TestRequest::with_uri("/app").to_request(); let resp = srv.call(req).await.unwrap(); @@ -636,7 +638,7 @@ mod tests { let srv = init_service( App::new().service(web::scope("/app/").service(web::resource("").to(HttpResponse::Ok))), ) - .await; + .await; let req = TestRequest::with_uri("/app").to_request(); let resp = srv.call(req).await.unwrap(); @@ -653,7 +655,7 @@ mod tests { App::new() .service(web::scope("/app/").service(web::resource("/").to(HttpResponse::Ok))), ) - .await; + .await; let req = TestRequest::with_uri("/app").to_request(); let resp = srv.call(req).await.unwrap(); @@ -673,7 +675,7 @@ mod tests { .route("/path1", web::delete().to(HttpResponse::Ok)), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/path1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -703,7 +705,7 @@ mod tests { ), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/path1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -731,7 +733,7 @@ mod tests { .service(web::resource("/path1").to(HttpResponse::Ok)), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/path1") .method(Method::POST) @@ -753,7 +755,7 @@ mod tests { HttpResponse::Ok().body(format!("project: {}", &r.match_info()["project"])) }), ))) - .await; + .await; let req = TestRequest::with_uri("/ab-project1/path1").to_request(); let res = srv.call(req).await.unwrap(); @@ -770,7 +772,7 @@ mod tests { let srv = init_service(App::new().service(web::scope("/app").service( web::scope("/t1").service(web::resource("/path1").to(HttpResponse::Created)), ))) - .await; + .await; let req = TestRequest::with_uri("/app/t1/path1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -783,7 +785,7 @@ mod tests { init_service(App::new().service(web::scope("/app").service( web::scope("t1").service(web::resource("/path1").to(HttpResponse::Created)), ))) - .await; + .await; let req = TestRequest::with_uri("/app/t1/path1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -801,7 +803,7 @@ mod tests { ), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/t1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -823,7 +825,7 @@ mod tests { ), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/t1/path1") .method(Method::POST) @@ -845,7 +847,7 @@ mod tests { HttpResponse::Created().body(format!("project: {}", &r.match_info()["project_id"])) })), ))) - .await; + .await; let req = TestRequest::with_uri("/app/project_1/path1").to_request(); let res = srv.call(req).await.unwrap(); @@ -866,7 +868,7 @@ mod tests { }), )), ))) - .await; + .await; let req = TestRequest::with_uri("/app/test/1/path1").to_request(); let res = srv.call(req).await.unwrap(); @@ -889,7 +891,7 @@ mod tests { }), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/path2").to_request(); let resp = srv.call(req).await.unwrap(); @@ -910,7 +912,7 @@ mod tests { ok(r.into_response(HttpResponse::MethodNotAllowed())) }), ) - .await; + .await; let req = TestRequest::with_uri("/non-exist").to_request(); let resp = srv.call(req).await.unwrap(); @@ -937,7 +939,7 @@ mod tests { .service(web::resource("/test").route(web::get().to(HttpResponse::Ok))), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/test").to_request(); let resp = call_service(&srv, req).await; @@ -961,7 +963,7 @@ mod tests { .service(web::resource("/test").route(web::get().to(|| async { "hello" }))), ), ) - .await; + .await; // test if `MessageBody::try_into_bytes()` is preserved across scope layer use actix_http::body::MessageBody as _; @@ -988,7 +990,7 @@ mod tests { .route("/test", web::get().to(HttpResponse::Ok)), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/test").to_request(); let resp = call_service(&srv, req).await; @@ -1023,7 +1025,7 @@ mod tests { }), ), ) - .await; + .await; let req = TestRequest::with_uri("/app/test").to_request(); let resp = call_service(&srv, req).await; @@ -1047,7 +1049,7 @@ mod tests { }), ), )) - .await; + .await; let req = TestRequest::with_uri("/app/t").to_request(); let resp = call_service(&srv, req).await; @@ -1067,7 +1069,7 @@ mod tests { }, )), )) - .await; + .await; let req = TestRequest::with_uri("/app/t").to_request(); let resp = call_service(&srv, req).await; @@ -1085,7 +1087,7 @@ mod tests { }), ), )) - .await; + .await; let req = TestRequest::with_uri("/app/t").to_request(); let resp = call_service(&srv, req).await; @@ -1097,7 +1099,7 @@ mod tests { let srv = init_service(App::new().service(web::scope("/app").configure(|s| { s.route("/path1", web::get().to(HttpResponse::Ok)); }))) - .await; + .await; let req = TestRequest::with_uri("/app/path1").to_request(); let resp = srv.call(req).await.unwrap(); @@ -1111,7 +1113,7 @@ mod tests { s.route("/", web::get().to(HttpResponse::Ok)); })); }))) - .await; + .await; let req = TestRequest::with_uri("/app/v1/").to_request(); let resp = srv.call(req).await.unwrap(); @@ -1132,7 +1134,7 @@ mod tests { ); })); }))) - .await; + .await; let req = TestRequest::with_uri("/app/v1/").to_request(); let resp = srv.call(req).await.unwrap(); @@ -1150,7 +1152,7 @@ mod tests { }, ))), ))) - .await; + .await; let req = TestRequest::with_uri("/a/b/c/test").to_request(); let resp = call_service(&srv, req).await; @@ -1177,7 +1179,7 @@ mod tests { ), ), ) - .await; + .await; // note the unintuitive behavior with trailing slashes on scopes with dynamic segments let req = TestRequest::with_uri("/a//b//c").to_request(); @@ -1209,7 +1211,7 @@ mod tests { ), ), ) - .await; + .await; let req = TestRequest::with_uri("/a/b/c").to_request(); let resp = call_service(&srv, req).await; diff --git a/actix-web/src/types/path.rs b/actix-web/src/types/path.rs index 5f22568cc..15b0c878e 100644 --- a/actix-web/src/types/path.rs +++ b/actix-web/src/types/path.rs @@ -176,7 +176,7 @@ mod tests { let resource = ResourceDef::new("/{value}/"); let mut req = TestRequest::with_uri("/32/").to_srv_request(); - resource.capture_match_info(req.match_info_mut()); + resource.resolve_resource_if_matches(req.match_info_mut()); let (req, mut pl) = req.into_parts(); assert_eq!(*Path::::from_request(&req, &mut pl).await.unwrap(), 32); @@ -189,7 +189,7 @@ mod tests { let resource = ResourceDef::new("/{key}/{value}/"); let mut req = TestRequest::with_uri("/name/user1/?id=test").to_srv_request(); - resource.capture_match_info(req.match_info_mut()); + resource.resolve_resource_if_matches(req.match_info_mut()); let (req, mut pl) = req.into_parts(); let (Path(res),) = <(Path<(String, String)>,)>::from_request(&req, &mut pl) @@ -215,7 +215,7 @@ mod tests { let mut req = TestRequest::with_uri("/name/user1/?id=test").to_srv_request(); let resource = ResourceDef::new("/{key}/{value}/"); - resource.capture_match_info(req.match_info_mut()); + resource.resolve_resource_if_matches(req.match_info_mut()); let (req, mut pl) = req.into_parts(); let mut s = Path::::from_request(&req, &mut pl).await.unwrap(); @@ -238,7 +238,7 @@ mod tests { let mut req = TestRequest::with_uri("/name/32/").to_srv_request(); let resource = ResourceDef::new("/{key}/{value}/"); - resource.capture_match_info(req.match_info_mut()); + resource.resolve_resource_if_matches(req.match_info_mut()); let (req, mut pl) = req.into_parts(); let s = Path::::from_request(&req, &mut pl).await.unwrap(); @@ -262,7 +262,7 @@ mod tests { async fn paths_decoded() { let resource = ResourceDef::new("/{key}/{value}"); let mut req = TestRequest::with_uri("/na%2Bme/us%2Fer%254%32").to_srv_request(); - resource.capture_match_info(req.match_info_mut()); + resource.resolve_resource_if_matches(req.match_info_mut()); let (req, mut pl) = req.into_parts(); let path_items = Path::::from_request(&req, &mut pl).await.unwrap();