diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 52cfc0d8f..480fd2e4b 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -39,7 +39,7 @@ //! ``` //! # use actix_web::HttpResponse; //! # use actix_web_codegen::route; -//! #[route("/test", method="GET", method="HEAD")] +//! #[route("/test", method = "GET", method = "HEAD")] //! async fn get_and_head_handler() -> HttpResponse { //! HttpResponse::Ok().finish() //! } @@ -74,10 +74,12 @@ mod route; /// /// # Attributes /// - `"path"` - Raw literal string with path for which to register handler. -/// - `name="resource_name"` - Specifies resource name for the handler. If not set, the function name of handler is used. -/// - `method="HTTP_METHOD"` - Registers HTTP method to provide guard for. Upper-case string, "GET", "POST" for example. -/// - `guard="function_name"` - Registers function as guard using `actix_web::guard::fn_guard` -/// - `wrap="Middleware"` - Registers a resource middleware. +/// - `name = "resource_name"` - Specifies resource name for the handler. If not set, the function +/// name of handler is used. +/// - `method = "HTTP_METHOD"` - Registers HTTP method to provide guard for. Upper-case string, +/// "GET", "POST" for example. +/// - `guard = "function_name"` - Registers function as guard using `actix_web::guard::fn_guard` +/// - `wrap = "Middleware"` - Registers a resource middleware. /// /// # Notes /// Function name can be specified as any expression that is going to be accessible to the generate diff --git a/src/config.rs b/src/config.rs index 77fba18ed..9fe75df97 100644 --- a/src/config.rs +++ b/src/config.rs @@ -102,8 +102,14 @@ impl AppService { InitError = (), > + 'static, { - self.services - .push((rdef, boxed::factory(factory.into_factory()), guards, nested)); + dbg!(rdef.pattern()); + + self.services.push(( + rdef, + boxed::factory(factory.into_factory()), + guards, + dbg!(nested), + )); } } diff --git a/src/request.rs b/src/request.rs index 63db56fd3..61b820950 100644 --- a/src/request.rs +++ b/src/request.rs @@ -508,10 +508,12 @@ mod tests { use bytes::Bytes; use super::*; - use crate::dev::{ResourceDef, ResourceMap}; - use crate::http::{header, StatusCode}; - use crate::test::{call_service, init_service, read_body, TestRequest}; - use crate::{web, App, HttpResponse}; + use crate::{ + dev::{ResourceDef, ResourceMap}, + http::{header, StatusCode}, + test::{self, call_service, init_service, read_body, TestRequest}, + web, App, HttpResponse, + }; #[test] fn test_debug() { @@ -865,4 +867,46 @@ mod tests { let res = call_service(&srv, req).await; assert_eq!(res.status(), StatusCode::OK); } + + #[actix_rt::test] + async fn url_for_closest_named_resource() { + // we mount the route named 'nested' on 2 different scopes, 'a' and 'b' + let srv = test::init_service( + App::new() + .service( + web::scope("/foo") + .service(web::resource("/nested").name("nested").route(web::get().to( + |req: HttpRequest| { + HttpResponse::Ok() + .body(format!("{}", req.url_for_static("nested").unwrap())) + }, + ))) + .service(web::scope("/baz").service(web::resource("deep"))) + .service(web::resource("{foo_param}")), + ) + .service(web::scope("/bar").service( + web::resource("/nested").name("nested").route(web::get().to( + |req: HttpRequest| { + HttpResponse::Ok() + .body(format!("{}", req.url_for_static("nested").unwrap())) + }, + )), + )), + ) + .await; + + let foo_resp = + test::call_service(&srv, TestRequest::with_uri("/foo/nested").to_request()).await; + assert_eq!(foo_resp.status(), StatusCode::OK); + let body = read_body(foo_resp).await; + // XXX: body equals http://localhost:8080/bar/nested + // because nested from /bar overrides /foo's + assert_eq!(body, "http://localhost:8080/bar/nested"); + + let bar_resp = + test::call_service(&srv, TestRequest::with_uri("/bar/nested").to_request()).await; + assert_eq!(bar_resp.status(), StatusCode::OK); + let body = read_body(bar_resp).await; + assert_eq!(body, "http://localhost:8080/bar/nested"); + } } diff --git a/src/rmap.rs b/src/rmap.rs index 432eaf83c..932f7acde 100644 --- a/src/rmap.rs +++ b/src/rmap.rs @@ -1,6 +1,7 @@ use std::{ borrow::Cow, cell::RefCell, + fmt::Write as _, rc::{Rc, Weak}, }; @@ -10,12 +11,14 @@ use url::Url; use crate::{error::UrlGenerationError, request::HttpRequest}; +const AVG_PATH_LEN: usize = 24; + #[derive(Clone, Debug)] pub struct ResourceMap { pattern: ResourceDef, - /// Named resources within the tree or, for external resources, - /// it points to isolated nodes outside the tree. + /// Named resources within the tree or, for external resources, it points to isolated nodes + /// outside the tree. named: AHashMap>, parent: RefCell>, @@ -35,6 +38,35 @@ impl ResourceMap { } } + /// Format resource map as tree structure (unfinished). + #[allow(dead_code)] + pub(crate) fn tree(&self) -> String { + let mut buf = String::new(); + self._tree(&mut buf, 0); + buf + } + + pub(crate) fn _tree(&self, buf: &mut String, level: usize) { + if let Some(children) = &self.nodes { + for child in children { + writeln!( + buf, + "{}{} {}", + "--".repeat(level), + child.pattern.pattern().unwrap(), + child + .pattern + .name() + .map(|name| format!("({})", name)) + .unwrap_or_else(|| "".to_owned()) + ) + .unwrap(); + + ResourceMap::_tree(child, buf, level + 1); + } + } + } + /// Adds a (possibly nested) resource. /// /// To add a non-prefix pattern, `nested` must be `None`. @@ -44,7 +76,11 @@ impl ResourceMap { pattern.set_id(self.nodes.as_ref().unwrap().len() as u16); if let Some(new_node) = nested { - assert_eq!(&new_node.pattern, pattern, "`patern` and `nested` mismatch"); + debug_assert_eq!( + &new_node.pattern, pattern, + "`pattern` and `nested` mismatch" + ); + // parents absorb references to the named resources of children self.named.extend(new_node.named.clone().into_iter()); self.nodes.as_mut().unwrap().push(new_node); } else { @@ -64,7 +100,7 @@ impl ResourceMap { None => false, }; - // Don't add external resources to the tree + // don't add external resources to the tree if !is_external { self.nodes.as_mut().unwrap().push(new_node); } @@ -78,7 +114,7 @@ impl ResourceMap { } } - /// Generate url for named resource + /// Generate URL for named resource. /// /// Check [`HttpRequest::url_for`] for detailed information. pub fn url_for( @@ -97,7 +133,7 @@ impl ResourceMap { .named .get(name) .ok_or(UrlGenerationError::ResourceNotFound)? - .root_rmap_fn(String::with_capacity(24), |mut acc, node| { + .root_rmap_fn(String::with_capacity(AVG_PATH_LEN), |mut acc, node| { node.pattern .resource_path_from_iter(&mut acc, &mut elements) .then(|| acc) @@ -128,6 +164,7 @@ impl ResourceMap { Ok(url) } + /// Returns true if there is a resource that would match `path`. pub fn has_resource(&self, path: &str) -> bool { self.find_matching_node(path).is_some() } @@ -142,9 +179,10 @@ impl ResourceMap { /// is possible. pub fn match_pattern(&self, path: &str) -> Option { self.find_matching_node(path)?.root_rmap_fn( - String::with_capacity(24), + String::with_capacity(AVG_PATH_LEN), |mut acc, node| { - acc.push_str(node.pattern.pattern()?); + let pattern = node.pattern.pattern()?; + acc.push_str(pattern); Some(acc) }, ) @@ -490,4 +528,33 @@ mod tests { "https://duck.com/abcd" ); } + + #[test] + fn url_for_override_within_map() { + let mut root = ResourceMap::new(ResourceDef::prefix("")); + + let mut foo_rdef = ResourceDef::prefix("/foo"); + let mut foo_map = ResourceMap::new(foo_rdef.clone()); + let mut nested_rdef = ResourceDef::new("/nested"); + nested_rdef.set_name("nested"); + foo_map.add(&mut nested_rdef, None); + root.add(&mut foo_rdef, Some(Rc::new(foo_map))); + + let mut foo_rdef = ResourceDef::prefix("/bar"); + let mut foo_map = ResourceMap::new(foo_rdef.clone()); + let mut nested_rdef = ResourceDef::new("/nested"); + nested_rdef.set_name("nested"); + foo_map.add(&mut nested_rdef, None); + root.add(&mut foo_rdef, Some(Rc::new(foo_map))); + + let rmap = Rc::new(root); + ResourceMap::finish(&rmap); + + let req = crate::test::TestRequest::default().to_http_request(); + + let url = rmap.url_for(&req, "nested", &[""; 0]).unwrap().to_string(); + assert_eq!(url, "http://localhost:8080/bar/nested"); + + assert!(rmap.url_for(&req, "missing", &["u123"]).is_err()); + } } diff --git a/src/service.rs b/src/service.rs index 81424a654..061c3e044 100644 --- a/src/service.rs +++ b/src/service.rs @@ -831,6 +831,7 @@ mod tests { let req = ServiceRequest::from_parts(req, pl); svc.call(req) }) + .route("/", web::get().to(|| async { "" })) .service( web::resource("/resource1/{name}/index.html").route(web::get().to(index)), ),