From 0d87cc53a1c92ff2c9ca3743816ab6d13453026c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 05:27:52 -0300 Subject: [PATCH 01/12] feat(resources-introspection): add support for resource metadata retrieval --- actix-web/Cargo.toml | 3 + actix-web/src/app_service.rs | 26 +++++ actix-web/src/guard/mod.rs | 98 ++++++++++++++++-- actix-web/src/introspection.rs | 177 +++++++++++++++++++++++++++++++++ actix-web/src/lib.rs | 3 + actix-web/src/resource.rs | 28 ++++++ actix-web/src/rmap.rs | 36 +++++++ actix-web/src/scope.rs | 29 ++++++ 8 files changed, 394 insertions(+), 6 deletions(-) create mode 100644 actix-web/src/introspection.rs diff --git a/actix-web/Cargo.toml b/actix-web/Cargo.toml index d6fd8f5af..219449dc5 100644 --- a/actix-web/Cargo.toml +++ b/actix-web/Cargo.toml @@ -128,6 +128,9 @@ compat = [ # Opt-out forwards-compatibility for handler visibility inheritance fix. compat-routing-macros-force-pub = ["actix-web-codegen?/compat-routing-macros-force-pub"] +# Enabling the retrieval of metadata for initialized resources, including path and HTTP method. +resources-introspection = [] + [dependencies] actix-codec = "0.5" actix-macros = { version = "0.2.3", optional = true } diff --git a/actix-web/src/app_service.rs b/actix-web/src/app_service.rs index 7aa16b790..a935c1354 100644 --- a/actix-web/src/app_service.rs +++ b/actix-web/src/app_service.rs @@ -82,6 +82,9 @@ where let (config, services) = config.into_services(); + #[cfg(feature = "resources-introspection")] + let mut rdef_methods: Vec<(String, Vec)> = Vec::new(); + // complete pipeline creation. *self.factory_ref.borrow_mut() = Some(AppRoutingFactory { default, @@ -89,6 +92,24 @@ where .into_iter() .map(|(mut rdef, srv, guards, nested)| { rmap.add(&mut rdef, nested); + + #[cfg(feature = "resources-introspection")] + { + let http_methods: Vec = + guards.as_ref().map_or_else(Vec::new, |g| { + g.iter() + .flat_map(|g| { + crate::guard::HttpMethodsExtractor::extract_http_methods( + &**g, + ) + }) + .collect::>() + }); + + rdef_methods + .push((rdef.pattern().unwrap_or_default().to_string(), http_methods)); + } + (rdef, srv, RefCell::new(guards)) }) .collect::>() @@ -105,6 +126,11 @@ where let rmap = Rc::new(rmap); ResourceMap::finish(&rmap); + #[cfg(feature = "resources-introspection")] + { + crate::introspection::process_introspection(Rc::clone(&rmap), rdef_methods); + } + // construct all async data factory futures let factory_futs = join_all(self.async_data_factories.iter().map(|f| f())); diff --git a/actix-web/src/guard/mod.rs b/actix-web/src/guard/mod.rs index 41609953a..93aeefe3c 100644 --- a/actix-web/src/guard/mod.rs +++ b/actix-web/src/guard/mod.rs @@ -50,6 +50,7 @@ //! [`Route`]: crate::Route::guard() use std::{ + any::Any, cell::{Ref, RefMut}, rc::Rc, }; @@ -121,7 +122,7 @@ impl<'a> GuardContext<'a> { /// Interface for routing guards. /// /// See [module level documentation](self) for more. -pub trait Guard { +pub trait Guard: AsAny { /// Returns true if predicate condition is met for a given request. fn check(&self, ctx: &GuardContext<'_>) -> bool; } @@ -146,7 +147,7 @@ impl Guard for Rc { /// ``` pub fn fn_guard(f: F) -> impl Guard where - F: Fn(&GuardContext<'_>) -> bool, + F: Fn(&GuardContext<'_>) -> bool + 'static, { FnGuard(f) } @@ -155,7 +156,7 @@ struct FnGuard) -> bool>(F); impl Guard for FnGuard where - F: Fn(&GuardContext<'_>) -> bool, + F: Fn(&GuardContext<'_>) -> bool + 'static, { fn check(&self, ctx: &GuardContext<'_>) -> bool { (self.0)(ctx) @@ -164,7 +165,7 @@ where impl Guard for F where - F: Fn(&GuardContext<'_>) -> bool, + F: for<'a> Fn(&'a GuardContext<'a>) -> bool + 'static, { fn check(&self, ctx: &GuardContext<'_>) -> bool { (self)(ctx) @@ -284,7 +285,7 @@ impl Guard for AllGuard { /// .guard(guard::Not(guard::Get())) /// .to(|| HttpResponse::Ok()); /// ``` -pub struct Not(pub G); +pub struct Not(pub G); impl Guard for Not { #[inline] @@ -322,6 +323,81 @@ impl Guard for MethodGuard { } } +#[cfg(feature = "resources-introspection")] +pub trait HttpMethodsExtractor { + fn extract_http_methods(&self) -> Vec; +} + +#[cfg(feature = "resources-introspection")] +impl HttpMethodsExtractor for dyn Guard { + fn extract_http_methods(&self) -> Vec { + if let Some(method_guard) = self.as_any().downcast_ref::() { + vec![method_guard.0.to_string()] + } else if let Some(any_guard) = self.as_any().downcast_ref::() { + any_guard + .guards + .iter() + .flat_map(|g| g.extract_http_methods()) + .collect() + } else if let Some(all_guard) = self.as_any().downcast_ref::() { + all_guard + .guards + .iter() + .flat_map(|g| g.extract_http_methods()) + .collect() + } else { + vec!["UNKNOWN".to_string()] + } + } +} + +#[cfg(feature = "resources-introspection")] +impl std::fmt::Display for MethodGuard { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "[{}]", self.0) + } +} + +#[cfg(feature = "resources-introspection")] +impl std::fmt::Display for AllGuard { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let methods: Vec = self + .guards + .iter() + .filter_map(|guard| { + guard + .as_any() + .downcast_ref::() + .map(|method_guard| method_guard.0.to_string()) + }) + .collect(); + + write!(f, "[{}]", methods.join(", ")) + } +} + +#[cfg(feature = "resources-introspection")] +impl std::fmt::Display for AnyGuard { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let methods: Vec = self + .guards + .iter() + .map(|guard| { + let guard_ref = &**guard; + if let Some(method_guard) = guard_ref.as_any().downcast_ref::() { + method_guard.0.to_string() + } else if let Some(all_guard) = guard_ref.as_any().downcast_ref::() { + all_guard.to_string() + } else { + "UNKNOWN".to_string() + } + }) + .collect(); + + write!(f, "[{}]", methods.join(", ")) + } +} + macro_rules! method_guard { ($method_fn:ident, $method_const:ident) => { #[doc = concat!("Creates a guard that matches the `", stringify!($method_const), "` request method.")] @@ -384,6 +460,16 @@ impl Guard for HeaderGuard { } } +pub trait AsAny { + fn as_any(&self) -> &dyn Any; +} + +impl AsAny for T { + fn as_any(&self) -> &dyn Any { + self + } +} + #[cfg(test)] mod tests { use actix_http::Method; @@ -495,7 +581,7 @@ mod tests { #[test] fn function_guard() { let domain = "rust-lang.org".to_owned(); - let guard = fn_guard(|ctx| ctx.head().uri.host().unwrap().ends_with(&domain)); + let guard = fn_guard(move |ctx| ctx.head().uri.host().unwrap().ends_with(&domain)); let req = TestRequest::default() .uri("blog.rust-lang.org") diff --git a/actix-web/src/introspection.rs b/actix-web/src/introspection.rs new file mode 100644 index 000000000..d0b439f29 --- /dev/null +++ b/actix-web/src/introspection.rs @@ -0,0 +1,177 @@ +use std::rc::Rc; +use std::sync::{OnceLock, RwLock}; + +use crate::dev::ResourceMap; + +/// Represents an HTTP resource registered for introspection. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ResourceIntrospection { + /// HTTP method (e.g., "GET") + pub method: String, + /// Path (e.g., "/api/v1/test") + pub path: String, +} + +/// Temporary registry for partial routes. +static TEMP_REGISTRY: OnceLock>> = OnceLock::new(); +/// Final registry for complete routes. +static FINAL_REGISTRY: OnceLock>> = OnceLock::new(); + +fn get_temp_registry() -> &'static RwLock> { + TEMP_REGISTRY.get_or_init(|| RwLock::new(Vec::new())) +} + +fn get_final_registry() -> &'static RwLock> { + FINAL_REGISTRY.get_or_init(|| RwLock::new(Vec::new())) +} + +/// Registers a resource. +pub fn register_resource(resource: ResourceIntrospection, is_complete: bool) { + let registry = if is_complete { + get_final_registry() + } else { + get_temp_registry() + }; + let mut reg = registry.write().expect("Failed to acquire lock"); + if !reg.iter().any(|r| r == &resource) { + reg.push(resource); + } +} + +/// Completes (moves to the final registry) partial routes that match the given marker, +/// applying the prefix. Only affects routes whose path contains `marker`. +pub fn complete_partial_routes_with_marker(marker: &str, prefix: &str) { + let temp_registry = get_temp_registry(); + let mut temp = temp_registry + .write() + .expect("Failed to acquire lock TEMP_REGISTRY"); + let final_registry = get_final_registry(); + let mut final_reg = final_registry + .write() + .expect("Failed to acquire lock FINAL_REGISTRY"); + + let mut remaining = Vec::new(); + for resource in temp.drain(..) { + if resource.path.contains(marker) { + // Concatenate the prefix only if it is not already present. + let full_path = if prefix.is_empty() { + resource.path.clone() + } else if prefix.ends_with("/") || resource.path.starts_with("/") { + format!("{}{}", prefix, resource.path) + } else { + format!("{}/{}", prefix, resource.path) + }; + let completed_resource = ResourceIntrospection { + method: resource.method, + path: full_path, + }; + if !final_reg.iter().any(|r| r == &completed_resource) { + final_reg.push(completed_resource); + } + } else { + remaining.push(resource); + } + } + *temp = remaining; +} + +/// Returns the complete list of registered resources. +pub fn get_registered_resources() -> Vec { + let final_registry = get_final_registry(); + let final_reg = final_registry + .read() + .expect("Failed to acquire lock FINAL_REGISTRY"); + final_reg.clone() +} + +/// Processes introspection data for routes and methods. +/// +/// # Parameters +/// - `rmap`: A resource map that can be converted to a vector of full route strings. +/// - `rdef_methods`: A vector of tuples `(sub_path, [methods])`. +/// An entry with an empty methods vector is treated as a level marker, indicating that +/// routes registered in a lower level (in TEMP_REGISTRY) should be "completed" (moved to the final registry) +/// using the deduced prefix. For example, if a marker "/api" is found and the corresponding route is "/api/v1/item/{id}", +/// the deduced prefix will be "" (if the marker starts at the beginning) or a non-empty string indicating a higher level. +/// +/// # Processing Steps +/// 1. **Marker Processing:** +/// For each entry in `rdef_methods` with an empty methods vector, the function: +/// - Searches `rmap_vec` for a route that contains the `sub_path` (the marker). +/// - Deduces the prefix as the portion of the route before the marker. +/// - Calls `complete_partial_routes_with_marker`, moving all partial routes that contain the marker +/// from TEMP_REGISTRY to FINAL_REGISTRY, applying the deduced prefix. +/// +/// 2. **Endpoint Registration:** +/// For each entry in `rdef_methods` with assigned methods: +/// - If `sub_path` is "/", an exact match is used; otherwise, routes ending with `sub_path` are considered. +/// - Among the candidate routes from `rmap_vec`, the candidate that either starts with the deduced prefix +/// (if non-empty) or the shortest candidate (if at root level or no prefix was deduced) is selected. +/// - A single `ResourceIntrospection` is registered with the full route and all methods joined by commas. +/// +/// Note: If multiple markers exist in the same block, only the last one processed (and stored in `deduced_prefix`) +/// is used for selecting endpoint candidates. Consider refactoring if independent processing per level is desired. +pub fn process_introspection(rmap: Rc, rdef_methods: Vec<(String, Vec)>) { + // Convert the ResourceMap to a vector for easier manipulation. + let rmap_vec = rmap.to_vec(); + + // If there are no routes or methods, there is nothing to introspect. + if rmap_vec.is_empty() && rdef_methods.is_empty() { + return; + } + + // Variable to store the deduced prefix for this introspection (if there is a marker) + let mut deduced_prefix: Option = None; + + // First, check the markers (entries with empty methods). + for (sub_path, http_methods) in rdef_methods.iter() { + if http_methods.is_empty() { + if let Some(r) = rmap_vec.iter().find(|r| r.contains(sub_path)) { + if let Some(pos) = r.find(sub_path) { + let prefix = &r[..pos]; + deduced_prefix = Some(prefix.to_string()); + complete_partial_routes_with_marker(sub_path, prefix); + } + } + } + } + + // Then, process the endpoints with assigned methods. + for (sub_path, http_methods) in rdef_methods.iter() { + if !http_methods.is_empty() { + // For the "/" subroute, do an exact match; otherwise, use ends_with. + let candidates: Vec<&String> = if sub_path == "/" { + rmap_vec.iter().filter(|r| r.as_str() == "/").collect() + } else { + rmap_vec.iter().filter(|r| r.ends_with(sub_path)).collect() + }; + + if !candidates.is_empty() { + let chosen = if let Some(prefix) = &deduced_prefix { + if !prefix.is_empty() { + candidates + .iter() + .find(|&&r| r.starts_with(prefix)) + .cloned() + .or_else(|| candidates.iter().min_by_key(|&&r| r.len()).cloned()) + } else { + // Root level: if sub_path is "/" we already filtered by equality. + candidates.iter().min_by_key(|&&r| r.len()).cloned() + } + } else { + candidates.iter().min_by_key(|&&r| r.len()).cloned() + }; + if let Some(full_route) = chosen { + // Register a single entry with all methods joined. + register_resource( + ResourceIntrospection { + method: http_methods.join(","), + path: full_route.clone(), + }, + deduced_prefix.is_some(), // Mark as complete if any marker was detected. + ); + } + } + } + } +} diff --git a/actix-web/src/lib.rs b/actix-web/src/lib.rs index e2a8e2275..e26e76243 100644 --- a/actix-web/src/lib.rs +++ b/actix-web/src/lib.rs @@ -108,6 +108,9 @@ mod thin_data; pub(crate) mod types; pub mod web; +#[cfg(feature = "resources-introspection")] +pub mod introspection; + #[doc(inline)] pub use crate::error::Result; pub use crate::{ diff --git a/actix-web/src/resource.rs b/actix-web/src/resource.rs index aee0dff93..0fb2cd234 100644 --- a/actix-web/src/resource.rs +++ b/actix-web/src/resource.rs @@ -433,6 +433,25 @@ where rdef.set_name(name); } + #[cfg(feature = "resources-introspection")] + let mut rdef_methods: Vec<(String, Vec)> = Vec::new(); + #[cfg(feature = "resources-introspection")] + let mut rmap = crate::rmap::ResourceMap::new(ResourceDef::prefix("")); + + #[cfg(feature = "resources-introspection")] + { + rmap.add(&mut rdef, None); + + self.routes.iter_mut().for_each(|r| { + r.take_guards().iter().for_each(|g| { + let http_methods: Vec = + crate::guard::HttpMethodsExtractor::extract_http_methods(&**g); + rdef_methods + .push((rdef.pattern().unwrap_or_default().to_string(), http_methods)); + }); + }); + } + *self.factory_ref.borrow_mut() = Some(ResourceFactory { routes: self.routes, default: self.default, @@ -451,6 +470,15 @@ where async { Ok(fut.await?.map_into_boxed_body()) } }); + #[cfg(feature = "resources-introspection")] + { + println!("resources"); + crate::introspection::process_introspection( + Rc::clone(&Rc::new(rmap.clone())), + rdef_methods, + ); + } + config.register_service(rdef, guards, endpoint, None) } } diff --git a/actix-web/src/rmap.rs b/actix-web/src/rmap.rs index b445687ac..2e4451b38 100644 --- a/actix-web/src/rmap.rs +++ b/actix-web/src/rmap.rs @@ -38,6 +38,42 @@ impl ResourceMap { } } + #[cfg(feature = "resources-introspection")] + /// Returns a list of all paths in the resource map. + pub fn to_vec(&self) -> Vec { + let mut paths = Vec::new(); + self.collect_full_paths(String::new(), &mut paths); + paths + } + + #[cfg(feature = "resources-introspection")] + /// Recursive function that accumulates the full path and adds it only if the node is an endpoint (leaf). + fn collect_full_paths(&self, current_path: String, paths: &mut Vec) { + // Get the current segment of the pattern + if let Some(segment) = self.pattern.pattern() { + let mut new_path = current_path; + // Add the '/' separator if necessary + if !segment.is_empty() { + if !new_path.ends_with('/') && !new_path.is_empty() && !segment.starts_with('/') { + new_path.push('/'); + } + new_path.push_str(segment); + } + + // If this node is an endpoint (has no children), add the full path + if self.nodes.is_none() { + paths.push(new_path.clone()); + } + + // If it has children, iterate over each one + if let Some(children) = &self.nodes { + for child in children { + child.collect_full_paths(new_path.clone(), paths); + } + } + } + } + /// Format resource map as tree structure (unfinished). #[allow(dead_code)] pub(crate) fn tree(&self) -> String { diff --git a/actix-web/src/scope.rs b/actix-web/src/scope.rs index e317349da..0f2b87c86 100644 --- a/actix-web/src/scope.rs +++ b/actix-web/src/scope.rs @@ -395,6 +395,9 @@ where rmap.add(&mut rdef, None); } + #[cfg(feature = "resources-introspection")] + let mut rdef_methods: Vec<(String, Vec)> = Vec::new(); + // complete scope pipeline creation *self.factory_ref.borrow_mut() = Some(ScopeFactory { default, @@ -404,6 +407,24 @@ where .into_iter() .map(|(mut rdef, srv, guards, nested)| { rmap.add(&mut rdef, nested); + + #[cfg(feature = "resources-introspection")] + { + let http_methods: Vec = + guards.as_ref().map_or_else(Vec::new, |g| { + g.iter() + .flat_map(|g| { + crate::guard::HttpMethodsExtractor::extract_http_methods( + &**g, + ) + }) + .collect::>() + }); + + rdef_methods + .push((rdef.pattern().unwrap_or_default().to_string(), http_methods)); + } + (rdef, srv, RefCell::new(guards)) }) .collect::>() @@ -431,6 +452,14 @@ where async { Ok(fut.await?.map_into_boxed_body()) } }); + #[cfg(feature = "resources-introspection")] + { + crate::introspection::process_introspection( + Rc::clone(&Rc::new(rmap.clone())), + rdef_methods, + ); + } + // register final service config.register_service( ResourceDef::root_prefix(&self.rdef), From 041322ef9cae0ebeca64fa093ce23ec87945d8c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 06:10:45 -0300 Subject: [PATCH 02/12] misc: remove debug print --- actix-web/src/resource.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/actix-web/src/resource.rs b/actix-web/src/resource.rs index 0fb2cd234..104adabf5 100644 --- a/actix-web/src/resource.rs +++ b/actix-web/src/resource.rs @@ -472,7 +472,6 @@ where #[cfg(feature = "resources-introspection")] { - println!("resources"); crate::introspection::process_introspection( Rc::clone(&Rc::new(rmap.clone())), rdef_methods, From 819ee93e9b81c2f4a71ea637df1125682704a0e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 06:46:52 -0300 Subject: [PATCH 03/12] style: cargo fmt --- actix-web/src/introspection.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actix-web/src/introspection.rs b/actix-web/src/introspection.rs index d0b439f29..f683f61ed 100644 --- a/actix-web/src/introspection.rs +++ b/actix-web/src/introspection.rs @@ -1,5 +1,7 @@ -use std::rc::Rc; -use std::sync::{OnceLock, RwLock}; +use std::{ + rc::Rc, + sync::{OnceLock, RwLock}, +}; use crate::dev::ResourceMap; From 91fa813f0e316448eb9fc5afec1946ceb2205a0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 15:31:20 -0300 Subject: [PATCH 04/12] fix(guards): replace take_guards with get_guards to prevent guard removal and fix test failures --- actix-web/src/guard/mod.rs | 2 +- actix-web/src/resource.rs | 4 ++-- actix-web/src/route.rs | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/actix-web/src/guard/mod.rs b/actix-web/src/guard/mod.rs index 93aeefe3c..1f660d174 100644 --- a/actix-web/src/guard/mod.rs +++ b/actix-web/src/guard/mod.rs @@ -11,7 +11,7 @@ //! or handler. This interface is defined by the [`Guard`] trait. //! //! Commonly-used guards are provided in this module as well as a way of creating a guard from a -//! closure ([`fn_guard`]). The [`Not`], [`Any`], and [`All`] guards are noteworthy, as they can be +//! closure ([`fn_guard`]). The [`Not`], [`Any()`], and [`All`] guards are noteworthy, as they can be //! used to compose other guards in a more flexible and semantic way than calling `.guard(...)` on //! services multiple times (which might have different combining behavior than you want). //! diff --git a/actix-web/src/resource.rs b/actix-web/src/resource.rs index 104adabf5..e5016d240 100644 --- a/actix-web/src/resource.rs +++ b/actix-web/src/resource.rs @@ -442,8 +442,8 @@ where { rmap.add(&mut rdef, None); - self.routes.iter_mut().for_each(|r| { - r.take_guards().iter().for_each(|g| { + self.routes.iter().for_each(|r| { + r.get_guards().iter().for_each(|g| { let http_methods: Vec = crate::guard::HttpMethodsExtractor::extract_http_methods(&**g); rdef_methods diff --git a/actix-web/src/route.rs b/actix-web/src/route.rs index e05e6be52..bcbfb9042 100644 --- a/actix-web/src/route.rs +++ b/actix-web/src/route.rs @@ -65,6 +65,11 @@ impl Route { pub(crate) fn take_guards(&mut self) -> Vec> { mem::take(Rc::get_mut(&mut self.guards).unwrap()) } + + #[cfg(feature = "resources-introspection")] + pub(crate) fn get_guards(&self) -> &Vec> { + &self.guards + } } impl ServiceFactory for Route { From 176ea5da779d5fb48957bfbc96bb324e9ed4016f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 16:05:01 -0300 Subject: [PATCH 05/12] ci: downgrade for msrv litemap to version 0.7.4 in justfile --- justfile | 1 + 1 file changed, 1 insertion(+) diff --git a/justfile b/justfile index 3a5e417fd..7e63e037c 100644 --- a/justfile +++ b/justfile @@ -13,6 +13,7 @@ downgrade-for-msrv: cargo update -p=parse-size --precise=1.0.0 cargo update -p=clap --precise=4.4.18 cargo update -p=divan --precise=0.1.15 + cargo update -p=litemap --precise=0.7.4 msrv := ``` cargo metadata --format-version=1 \ From acd7c58097b932c4e5b0f9994c15edf91aa0f285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 16:12:31 -0300 Subject: [PATCH 06/12] chore: update changelog and fix docs for CI --- actix-web/CHANGES.md | 1 + actix-web/src/guard/mod.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index edbdc6982..030c71d1d 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased +- Add `resources-introspection` feature for retrieving configured route paths and HTTP methods. - Implement `Responder` for `Result<(), E: Into>`. Returning `Ok(())` responds with HTTP 204 No Content. - On Windows, an error is now returned from `HttpServer::bind()` (or TLS variants) when binding to a socket that's already in use. - Update `brotli` dependency to `7`. diff --git a/actix-web/src/guard/mod.rs b/actix-web/src/guard/mod.rs index 1f660d174..8f1021f76 100644 --- a/actix-web/src/guard/mod.rs +++ b/actix-web/src/guard/mod.rs @@ -196,7 +196,7 @@ pub fn Any(guard: F) -> AnyGuard { /// /// That is, only one contained guard needs to match in order for the aggregate guard to match. /// -/// Construct an `AnyGuard` using [`Any`]. +/// Construct an `AnyGuard` using [`Any()`]. pub struct AnyGuard { guards: Vec>, } From c4be19942b2c2e41813242f1b42b2fa908ef991f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Mon, 3 Mar 2025 16:25:50 -0300 Subject: [PATCH 07/12] ci: downgrade for msrv zerofrom to version 0.1.5 in justfile --- justfile | 1 + 1 file changed, 1 insertion(+) diff --git a/justfile b/justfile index 7e63e037c..2b6d04d18 100644 --- a/justfile +++ b/justfile @@ -14,6 +14,7 @@ downgrade-for-msrv: cargo update -p=clap --precise=4.4.18 cargo update -p=divan --precise=0.1.15 cargo update -p=litemap --precise=0.7.4 + cargo update -p=zerofrom --precise=0.1.5 msrv := ``` cargo metadata --format-version=1 \ From a79dc9dc79370e766d4f7a4fb3bc673d9b07c784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Tue, 4 Mar 2025 05:09:18 -0300 Subject: [PATCH 08/12] refactor: improve thread safety and add unit tests for introspection process --- actix-web/src/introspection.rs | 486 +++++++++++++++++++++++++++------ 1 file changed, 395 insertions(+), 91 deletions(-) diff --git a/actix-web/src/introspection.rs b/actix-web/src/introspection.rs index f683f61ed..75f5dd395 100644 --- a/actix-web/src/introspection.rs +++ b/actix-web/src/introspection.rs @@ -1,153 +1,142 @@ use std::{ rc::Rc, sync::{OnceLock, RwLock}, + thread, }; -use crate::dev::ResourceMap; +use crate::rmap::ResourceMap; /// Represents an HTTP resource registered for introspection. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ResourceIntrospection { - /// HTTP method (e.g., "GET") + /// HTTP method (e.g., "GET"). pub method: String, - /// Path (e.g., "/api/v1/test") + /// Route path (e.g., "/api/v1/test"). pub path: String, } -/// Temporary registry for partial routes. -static TEMP_REGISTRY: OnceLock>> = OnceLock::new(); -/// Final registry for complete routes. -static FINAL_REGISTRY: OnceLock>> = OnceLock::new(); +/// A global registry of listed resources for introspection. +/// Only the designated thread can modify it. +static RESOURCE_REGISTRY: RwLock> = RwLock::new(Vec::new()); -fn get_temp_registry() -> &'static RwLock> { - TEMP_REGISTRY.get_or_init(|| RwLock::new(Vec::new())) -} +/// Stores the thread ID of the designated thread (the first to call `process_introspection`). +/// Any other thread will immediately return without updating the global registry. +static DESIGNATED_THREAD: OnceLock = OnceLock::new(); -fn get_final_registry() -> &'static RwLock> { - FINAL_REGISTRY.get_or_init(|| RwLock::new(Vec::new())) -} - -/// Registers a resource. -pub fn register_resource(resource: ResourceIntrospection, is_complete: bool) { - let registry = if is_complete { - get_final_registry() - } else { - get_temp_registry() - }; - let mut reg = registry.write().expect("Failed to acquire lock"); - if !reg.iter().any(|r| r == &resource) { - reg.push(resource); +/// Inserts a resource into the global registry, avoiding duplicates. +pub fn register_resource(resource: ResourceIntrospection) { + let mut global = RESOURCE_REGISTRY.write().unwrap(); + if !global.iter().any(|r| r == &resource) { + global.push(resource); } } -/// Completes (moves to the final registry) partial routes that match the given marker, -/// applying the prefix. Only affects routes whose path contains `marker`. +/// Completes (updates) partial routes in the global registry whose path contains `marker`, +/// by applying the specified `prefix`. pub fn complete_partial_routes_with_marker(marker: &str, prefix: &str) { - let temp_registry = get_temp_registry(); - let mut temp = temp_registry - .write() - .expect("Failed to acquire lock TEMP_REGISTRY"); - let final_registry = get_final_registry(); - let mut final_reg = final_registry - .write() - .expect("Failed to acquire lock FINAL_REGISTRY"); + let mut global = RESOURCE_REGISTRY.write().unwrap(); + let mut updated = Vec::new(); let mut remaining = Vec::new(); - for resource in temp.drain(..) { + + // Move all items out of the current registry. + for resource in global.drain(..) { if resource.path.contains(marker) { - // Concatenate the prefix only if it is not already present. + // Build the full path by applying the prefix if needed. let full_path = if prefix.is_empty() { resource.path.clone() - } else if prefix.ends_with("/") || resource.path.starts_with("/") { + } else if prefix.ends_with('/') || resource.path.starts_with('/') { format!("{}{}", prefix, resource.path) } else { format!("{}/{}", prefix, resource.path) }; - let completed_resource = ResourceIntrospection { + + let completed = ResourceIntrospection { method: resource.method, path: full_path, }; - if !final_reg.iter().any(|r| r == &completed_resource) { - final_reg.push(completed_resource); + + // Add to `updated` if it's not already in there. + if !updated.iter().any(|r| r == &completed) { + updated.push(completed); } } else { + // Keep this resource as-is. remaining.push(resource); } } - *temp = remaining; + + // Merge updated items back with the remaining ones. + remaining.extend(updated); + *global = remaining; } -/// Returns the complete list of registered resources. +/// Returns a **copy** of the global registry (safe to call from any thread). pub fn get_registered_resources() -> Vec { - let final_registry = get_final_registry(); - let final_reg = final_registry - .read() - .expect("Failed to acquire lock FINAL_REGISTRY"); - final_reg.clone() + RESOURCE_REGISTRY.read().unwrap().clone() } /// Processes introspection data for routes and methods. +/// Only the **first thread** that calls this function (the "designated" one) may update +/// the global resource registry. Any other thread will immediately return without updating it. /// /// # Parameters -/// - `rmap`: A resource map that can be converted to a vector of full route strings. -/// - `rdef_methods`: A vector of tuples `(sub_path, [methods])`. -/// An entry with an empty methods vector is treated as a level marker, indicating that -/// routes registered in a lower level (in TEMP_REGISTRY) should be "completed" (moved to the final registry) -/// using the deduced prefix. For example, if a marker "/api" is found and the corresponding route is "/api/v1/item/{id}", -/// the deduced prefix will be "" (if the marker starts at the beginning) or a non-empty string indicating a higher level. -/// -/// # Processing Steps -/// 1. **Marker Processing:** -/// For each entry in `rdef_methods` with an empty methods vector, the function: -/// - Searches `rmap_vec` for a route that contains the `sub_path` (the marker). -/// - Deduces the prefix as the portion of the route before the marker. -/// - Calls `complete_partial_routes_with_marker`, moving all partial routes that contain the marker -/// from TEMP_REGISTRY to FINAL_REGISTRY, applying the deduced prefix. -/// -/// 2. **Endpoint Registration:** -/// For each entry in `rdef_methods` with assigned methods: -/// - If `sub_path` is "/", an exact match is used; otherwise, routes ending with `sub_path` are considered. -/// - Among the candidate routes from `rmap_vec`, the candidate that either starts with the deduced prefix -/// (if non-empty) or the shortest candidate (if at root level or no prefix was deduced) is selected. -/// - A single `ResourceIntrospection` is registered with the full route and all methods joined by commas. -/// -/// Note: If multiple markers exist in the same block, only the last one processed (and stored in `deduced_prefix`) -/// is used for selecting endpoint candidates. Consider refactoring if independent processing per level is desired. +/// - `rmap`: A resource map convertible to a vector of route strings. +/// - `rdef_methods`: A vector of `(sub_path, [methods])`. +/// - A tuple with an **empty** methods vector is treated as a "marker" (a partial route) +/// for which we try to deduce a prefix by finding `sub_path` in a route, then calling +/// `complete_partial_routes_with_marker`. +/// - A tuple with one or more methods registers a resource with `register_resource`. pub fn process_introspection(rmap: Rc, rdef_methods: Vec<(String, Vec)>) { - // Convert the ResourceMap to a vector for easier manipulation. + // Determine the designated thread: if none is set yet, assign the current thread's ID. + // This ensures that the first thread to call this function becomes the designated thread. + let current_id = thread::current().id(); + DESIGNATED_THREAD.get_or_init(|| current_id); + + // If the current thread is not the designated one, return immediately. + // This ensures that only the designated thread updates the global registry, + // avoiding any interleaving or inconsistent updates from other threads. + if *DESIGNATED_THREAD.get().unwrap() != current_id { + return; + } + let rmap_vec = rmap.to_vec(); - // If there are no routes or methods, there is nothing to introspect. + // If there is no data, nothing to process. + // Avoid unnecessary work. if rmap_vec.is_empty() && rdef_methods.is_empty() { return; } - // Variable to store the deduced prefix for this introspection (if there is a marker) + // Keep track of the deduced prefix for partial routes. let mut deduced_prefix: Option = None; - // First, check the markers (entries with empty methods). - for (sub_path, http_methods) in rdef_methods.iter() { + // 1. Handle "marker" entries (where methods is empty). + for (sub_path, http_methods) in &rdef_methods { if http_methods.is_empty() { - if let Some(r) = rmap_vec.iter().find(|r| r.contains(sub_path)) { - if let Some(pos) = r.find(sub_path) { - let prefix = &r[..pos]; + // Find any route that contains sub_path and use it to deduce a prefix. + if let Some(route) = rmap_vec.iter().find(|r| r.contains(sub_path)) { + if let Some(pos) = route.find(sub_path) { + let prefix = &route[..pos]; deduced_prefix = Some(prefix.to_string()); + // Complete partial routes in the global registry using this prefix. complete_partial_routes_with_marker(sub_path, prefix); } } } } - // Then, process the endpoints with assigned methods. - for (sub_path, http_methods) in rdef_methods.iter() { + // 2. Handle endpoint entries (where methods is non-empty). + for (sub_path, http_methods) in &rdef_methods { if !http_methods.is_empty() { - // For the "/" subroute, do an exact match; otherwise, use ends_with. + // Identify candidate routes that end with sub_path (or exactly match "/" if sub_path == "/"). let candidates: Vec<&String> = if sub_path == "/" { rmap_vec.iter().filter(|r| r.as_str() == "/").collect() } else { rmap_vec.iter().filter(|r| r.ends_with(sub_path)).collect() }; + // If we found any candidates, pick the best match. if !candidates.is_empty() { let chosen = if let Some(prefix) = &deduced_prefix { if !prefix.is_empty() { @@ -157,23 +146,338 @@ pub fn process_introspection(rmap: Rc, rdef_methods: Vec<(String, V .cloned() .or_else(|| candidates.iter().min_by_key(|&&r| r.len()).cloned()) } else { - // Root level: if sub_path is "/" we already filtered by equality. candidates.iter().min_by_key(|&&r| r.len()).cloned() } } else { candidates.iter().min_by_key(|&&r| r.len()).cloned() }; + if let Some(full_route) = chosen { - // Register a single entry with all methods joined. - register_resource( - ResourceIntrospection { - method: http_methods.join(","), - path: full_route.clone(), - }, - deduced_prefix.is_some(), // Mark as complete if any marker was detected. - ); + // Register the endpoint in the global resource registry. + register_resource(ResourceIntrospection { + method: http_methods.join(","), + path: full_route.clone(), + }); } } } } } + +#[cfg(test)] +mod tests { + use std::{num::NonZeroUsize, rc::Rc}; + + use actix_router::ResourceDef; + use tokio::sync::oneshot; + + use super::*; + use crate::rmap::ResourceMap; + + /// Helper function to create a ResourceMap from a list of route strings. + /// It creates a root ResourceMap with an empty prefix and adds each route as a leaf. + fn create_resource_map(routes: Vec<&str>) -> Rc { + // Create a root node with an empty prefix. + let mut root = ResourceMap::new(ResourceDef::root_prefix("")); + // For each route, create a ResourceDef and add it as a leaf (nested = None). + for route in routes { + let mut def = ResourceDef::new(route); + root.add(&mut def, None); + } + Rc::new(root) + } + + // Helper function to run the full introspection flow. + // It processes introspection data for multiple blocks, each with a different set of routes and methods. + fn run_full_introspection_flow() { + // Block 1: + // rmap_vec: ["/item/{id}"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/item/{id}"]), vec![]); + + // Block 2: + // rmap_vec: ["/info"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/info"]), vec![]); + + // Block 3: + // rmap_vec: ["/guarded"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/guarded"]), vec![]); + + // Block 4: + // rmap_vec: ["/v1/item/{id}", "/v1/info", "/v1/guarded"] + // rdef_methods: [("/item/{id}", ["GET"]), ("/info", ["POST"]), ("/guarded", ["UNKNOWN"])] + process_introspection( + create_resource_map(vec!["/v1/item/{id}", "/v1/info", "/v1/guarded"]), + vec![ + ("/item/{id}".to_string(), vec!["GET".to_string()]), + ("/info".to_string(), vec!["POST".to_string()]), + ("/guarded".to_string(), vec!["UNKNOWN".to_string()]), + ], + ); + + // Block 5: + // rmap_vec: ["/hello"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/hello"]), vec![]); + + // Block 6: + // rmap_vec: ["/v2/hello"] + // rdef_methods: [("/hello", ["GET"])] + process_introspection( + create_resource_map(vec!["/v2/hello"]), + vec![("/hello".to_string(), vec!["GET".to_string()])], + ); + + // Block 7: + // rmap_vec: ["/api/v1/item/{id}", "/api/v1/info", "/api/v1/guarded", "/api/v2/hello"] + // rdef_methods: [("/v1", []), ("/v2", [])] + process_introspection( + create_resource_map(vec![ + "/api/v1/item/{id}", + "/api/v1/info", + "/api/v1/guarded", + "/api/v2/hello", + ]), + vec![("/v1".to_string(), vec![]), ("/v2".to_string(), vec![])], + ); + + // Block 8: + // rmap_vec: ["/dashboard"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/dashboard"]), vec![]); + + // Block 9: + // rmap_vec: ["/settings"] + // rdef_methods: [("/settings", ["GET"]), ("/settings", ["POST"])] + process_introspection( + create_resource_map(vec!["/settings"]), + vec![ + ("/settings".to_string(), vec!["GET".to_string()]), + ("/settings".to_string(), vec!["POST".to_string()]), + ], + ); + + // Block 10: + // rmap_vec: ["/admin/dashboard", "/admin/settings"] + // rdef_methods: [("/dashboard", ["GET"]), ("/settings", [])] + process_introspection( + create_resource_map(vec!["/admin/dashboard", "/admin/settings"]), + vec![ + ("/dashboard".to_string(), vec!["GET".to_string()]), + ("/settings".to_string(), vec![]), + ], + ); + + // Block 11: + // rmap_vec: ["/"] + // rdef_methods: [("/", ["GET"]), ("/", ["POST"])] + process_introspection( + create_resource_map(vec!["/"]), + vec![ + ("/".to_string(), vec!["GET".to_string()]), + ("/".to_string(), vec!["POST".to_string()]), + ], + ); + + // Block 12: + // rmap_vec: ["/ping"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/ping"]), vec![]); + + // Block 13: + // rmap_vec: ["/multi"] + // rdef_methods: [("/multi", ["GET"]), ("/multi", ["POST"])] + process_introspection( + create_resource_map(vec!["/multi"]), + vec![ + ("/multi".to_string(), vec!["GET".to_string()]), + ("/multi".to_string(), vec!["POST".to_string()]), + ], + ); + + // Block 14: + // rmap_vec: ["/extra/ping", "/extra/multi"] + // rdef_methods: [("/ping", ["GET"]), ("/multi", [])] + process_introspection( + create_resource_map(vec!["/extra/ping", "/extra/multi"]), + vec![ + ("/ping".to_string(), vec!["GET".to_string()]), + ("/multi".to_string(), vec![]), + ], + ); + + // Block 15: + // rmap_vec: ["/other_guard"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/other_guard"]), vec![]); + + // Block 16: + // rmap_vec: ["/all_guard"] + // rdef_methods: [] + process_introspection(create_resource_map(vec!["/all_guard"]), vec![]); + + // Block 17: + // rmap_vec: ["/api/v1/item/{id}", "/api/v1/info", "/api/v1/guarded", "/api/v2/hello", + // "/admin/dashboard", "/admin/settings", "/", "/extra/ping", "/extra/multi", + // "/other_guard", "/all_guard"] + // rdef_methods: [("/api", []), ("/admin", []), ("/", []), ("/extra", []), + // ("/other_guard", ["UNKNOWN"]), ("/all_guard", ["GET", "UNKNOWN", "POST"])] + process_introspection( + create_resource_map(vec![ + "/api/v1/item/{id}", + "/api/v1/info", + "/api/v1/guarded", + "/api/v2/hello", + "/admin/dashboard", + "/admin/settings", + "/", + "/extra/ping", + "/extra/multi", + "/other_guard", + "/all_guard", + ]), + vec![ + ("/api".to_string(), vec![]), + ("/admin".to_string(), vec![]), + ("/".to_string(), vec![]), + ("/extra".to_string(), vec![]), + ("/other_guard".to_string(), vec!["UNKNOWN".to_string()]), + ( + "/all_guard".to_string(), + vec!["GET".to_string(), "UNKNOWN".to_string(), "POST".to_string()], + ), + ], + ); + } + + /// This test spawns multiple tasks that run the full introspection flow concurrently. + /// Only the designated task (the first one to call process_introspection) updates the global registry, + /// ensuring that the internal order remains consistent. Finally, we verify that get_registered_resources() + /// returns the expected set of listed resources. + /// Using a dedicated arbiter for each task ensures that the global registry is thread-safe. + #[actix_rt::test] + async fn test_introspection() { + // Number of tasks to spawn. + const NUM_TASKS: usize = 4; + let mut completion_receivers = Vec::with_capacity(NUM_TASKS); + + // Check that the registry is initially empty. + let registered_resources = get_registered_resources(); + + assert_eq!( + registered_resources.len(), + 0, + "Expected 0 registered resources, found: {:?}", + registered_resources + ); + + // Determine parallelism and max blocking threads. + let parallelism = std::thread::available_parallelism().map_or(2, NonZeroUsize::get); + let max_blocking_threads = std::cmp::max(512 / parallelism, 1); + + // Spawn tasks on the arbiter. Each task runs the full introspection flow and then signals completion. + for _ in 0..NUM_TASKS { + let (tx, rx) = oneshot::channel(); + // Create an Arbiter with a dedicated Tokio runtime. + let arbiter = actix_rt::Arbiter::with_tokio_rt(move || { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .max_blocking_threads(max_blocking_threads) + .build() + .unwrap() + }); + // Spawn the task on the arbiter. + arbiter.spawn(async move { + run_full_introspection_flow(); + // Signal that this task has finished. + let _ = tx.send(()); + }); + completion_receivers.push(rx); + } + + // Wait for all spawned tasks to complete. + for rx in completion_receivers { + let _ = rx.await; + } + + // After all blocks, we expect the final registry to contain 14 entries. + let registered_resources = get_registered_resources(); + + assert_eq!( + registered_resources.len(), + 14, + "Expected 14 registered resources, found: {:?}", + registered_resources + ); + + // List of expected resources + let expected_resources = vec![ + ResourceIntrospection { + method: "GET".to_string(), + path: "/api/v1/item/{id}".to_string(), + }, + ResourceIntrospection { + method: "POST".to_string(), + path: "/api/v1/info".to_string(), + }, + ResourceIntrospection { + method: "UNKNOWN".to_string(), + path: "/api/v1/guarded".to_string(), + }, + ResourceIntrospection { + method: "GET".to_string(), + path: "/api/v2/hello".to_string(), + }, + ResourceIntrospection { + method: "GET".to_string(), + path: "/admin/settings".to_string(), + }, + ResourceIntrospection { + method: "POST".to_string(), + path: "/admin/settings".to_string(), + }, + ResourceIntrospection { + method: "GET".to_string(), + path: "/admin/dashboard".to_string(), + }, + ResourceIntrospection { + method: "GET".to_string(), + path: "/extra/multi".to_string(), + }, + ResourceIntrospection { + method: "POST".to_string(), + path: "/extra/multi".to_string(), + }, + ResourceIntrospection { + method: "GET".to_string(), + path: "/extra/ping".to_string(), + }, + ResourceIntrospection { + method: "GET".to_string(), + path: "/".to_string(), + }, + ResourceIntrospection { + method: "POST".to_string(), + path: "/".to_string(), + }, + ResourceIntrospection { + method: "UNKNOWN".to_string(), + path: "/other_guard".to_string(), + }, + ResourceIntrospection { + method: "GET,UNKNOWN,POST".to_string(), + path: "/all_guard".to_string(), + }, + ]; + + for exp in expected_resources { + assert!( + registered_resources.contains(&exp), + "Expected resource not found: {:?}", + exp + ); + } + } +} From 2bb774a52983db3466ed344e7be6ec8bb08eae97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Tue, 4 Mar 2025 05:30:49 -0300 Subject: [PATCH 09/12] fix(introspection): add conditional arbiter creation for io-uring support --- actix-web/src/introspection.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/actix-web/src/introspection.rs b/actix-web/src/introspection.rs index 75f5dd395..7fb0fb4e4 100644 --- a/actix-web/src/introspection.rs +++ b/actix-web/src/introspection.rs @@ -380,14 +380,25 @@ mod tests { // Spawn tasks on the arbiter. Each task runs the full introspection flow and then signals completion. for _ in 0..NUM_TASKS { let (tx, rx) = oneshot::channel(); - // Create an Arbiter with a dedicated Tokio runtime. + + #[cfg(all(target_os = "linux", feature = "experimental-io-uring"))] + let arbiter = { + // TODO: pass max blocking thread config when tokio-uring enable configuration + // on building runtime. + let _ = config.max_blocking_threads; + Arbiter::new() + }; + + #[cfg(not(all(target_os = "linux", feature = "experimental-io-uring")))] let arbiter = actix_rt::Arbiter::with_tokio_rt(move || { + // Create an Arbiter with a dedicated Tokio runtime. tokio::runtime::Builder::new_current_thread() .enable_all() .max_blocking_threads(max_blocking_threads) .build() .unwrap() }); + // Spawn the task on the arbiter. arbiter.spawn(async move { run_full_introspection_flow(); From 3b99f86e89fb120f9cf46d59ad637ff66b38d098 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Tue, 4 Mar 2025 05:30:49 -0300 Subject: [PATCH 10/12] fix(introspection): add conditional arbiter creation for io-uring support --- actix-web/src/introspection.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/actix-web/src/introspection.rs b/actix-web/src/introspection.rs index 75f5dd395..6074f2691 100644 --- a/actix-web/src/introspection.rs +++ b/actix-web/src/introspection.rs @@ -380,14 +380,25 @@ mod tests { // Spawn tasks on the arbiter. Each task runs the full introspection flow and then signals completion. for _ in 0..NUM_TASKS { let (tx, rx) = oneshot::channel(); - // Create an Arbiter with a dedicated Tokio runtime. + + #[cfg(all(target_os = "linux", feature = "experimental-io-uring"))] + let arbiter = { + // TODO: pass max blocking thread config when tokio-uring enable configuration + // on building runtime. + let _ = config.max_blocking_threads; + actix_rt::Arbiter::new() + }; + + #[cfg(not(all(target_os = "linux", feature = "experimental-io-uring")))] let arbiter = actix_rt::Arbiter::with_tokio_rt(move || { + // Create an Arbiter with a dedicated Tokio runtime. tokio::runtime::Builder::new_current_thread() .enable_all() .max_blocking_threads(max_blocking_threads) .build() .unwrap() }); + // Spawn the task on the arbiter. arbiter.spawn(async move { run_full_introspection_flow(); From ae08dcf6dc578d8ca15d69a84470dedb603147f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Wed, 5 Mar 2025 02:34:39 -0300 Subject: [PATCH 11/12] refactor(introspection): add GuardDetail enum and remove downcast_ref usage - Added `GuardDetail` enum to encapsulate various introspection details of a guard. - Refactored `HttpMethodsExtractor` implementation to use `GuardDetail` instead of `downcast_ref`. --- actix-web/src/guard/mod.rs | 177 +++++++++++++++++++++---------------- 1 file changed, 102 insertions(+), 75 deletions(-) diff --git a/actix-web/src/guard/mod.rs b/actix-web/src/guard/mod.rs index 8f1021f76..03a8f370d 100644 --- a/actix-web/src/guard/mod.rs +++ b/actix-web/src/guard/mod.rs @@ -11,7 +11,7 @@ //! or handler. This interface is defined by the [`Guard`] trait. //! //! Commonly-used guards are provided in this module as well as a way of creating a guard from a -//! closure ([`fn_guard`]). The [`Not`], [`Any()`], and [`All`] guards are noteworthy, as they can be +//! closure ([`fn_guard`]). The [`Not()`], [`Any()`], and [`All()`] guards are noteworthy, as they can be //! used to compose other guards in a more flexible and semantic way than calling `.guard(...)` on //! services multiple times (which might have different combining behavior than you want). //! @@ -50,7 +50,6 @@ //! [`Route`]: crate::Route::guard() use std::{ - any::Any, cell::{Ref, RefMut}, rc::Rc, }; @@ -67,6 +66,17 @@ pub use self::{ host::{Host, HostGuard}, }; +/// Enum to encapsulate various introspection details of a Guard. +#[derive(Debug, Clone)] +pub enum GuardDetail { + /// Detail associated with HTTP methods. + HttpMethods(Vec), + /// Detail associated with headers (header, value). + Headers(Vec<(String, String)>), + /// Generic detail. + Generic(String), +} + /// Provides access to request parts that are useful during routing. #[derive(Debug)] pub struct GuardContext<'a> { @@ -122,15 +132,31 @@ impl<'a> GuardContext<'a> { /// Interface for routing guards. /// /// See [module level documentation](self) for more. -pub trait Guard: AsAny { +pub trait Guard { /// Returns true if predicate condition is met for a given request. fn check(&self, ctx: &GuardContext<'_>) -> bool; + + /// Returns a nominal representation of the guard. + fn name(&self) -> String { + std::any::type_name::().to_string() + } + + /// Returns detailed introspection information. + fn details(&self) -> Option> { + None + } } impl Guard for Rc { fn check(&self, ctx: &GuardContext<'_>) -> bool { (**self).check(ctx) } + fn name(&self) -> String { + (**self).name() + } + fn details(&self) -> Option> { + (**self).details() + } } /// Creates a guard using the given function. @@ -147,7 +173,7 @@ impl Guard for Rc { /// ``` pub fn fn_guard(f: F) -> impl Guard where - F: Fn(&GuardContext<'_>) -> bool + 'static, + F: Fn(&GuardContext<'_>) -> bool, { FnGuard(f) } @@ -156,7 +182,7 @@ struct FnGuard) -> bool>(F); impl Guard for FnGuard where - F: Fn(&GuardContext<'_>) -> bool + 'static, + F: Fn(&GuardContext<'_>) -> bool, { fn check(&self, ctx: &GuardContext<'_>) -> bool { (self.0)(ctx) @@ -165,7 +191,7 @@ where impl Guard for F where - F: for<'a> Fn(&'a GuardContext<'a>) -> bool + 'static, + F: Fn(&GuardContext<'_>) -> bool, { fn check(&self, ctx: &GuardContext<'_>) -> bool { (self)(ctx) @@ -220,6 +246,24 @@ impl Guard for AnyGuard { false } + fn name(&self) -> String { + format!( + "AnyGuard({})", + self.guards + .iter() + .map(|g| g.name()) + .collect::>() + .join(", ") + ) + } + fn details(&self) -> Option> { + Some( + self.guards + .iter() + .flat_map(|g| g.details().unwrap_or_default()) + .collect(), + ) + } } /// Creates a guard that matches if all added guards match. @@ -248,7 +292,7 @@ pub fn All(guard: F) -> AllGuard { /// /// That is, **all** contained guard needs to match in order for the aggregate guard to match. /// -/// Construct an `AllGuard` using [`All`]. +/// Construct an `AllGuard` using [`All()`]. pub struct AllGuard { guards: Vec>, } @@ -272,6 +316,24 @@ impl Guard for AllGuard { true } + fn name(&self) -> String { + format!( + "AllGuard({})", + self.guards + .iter() + .map(|g| g.name()) + .collect::>() + .join(", ") + ) + } + fn details(&self) -> Option> { + Some( + self.guards + .iter() + .flat_map(|g| g.details().unwrap_or_default()) + .collect(), + ) + } } /// Wraps a guard and inverts the outcome of its `Guard` implementation. @@ -285,13 +347,19 @@ impl Guard for AllGuard { /// .guard(guard::Not(guard::Get())) /// .to(|| HttpResponse::Ok()); /// ``` -pub struct Not(pub G); +pub struct Not(pub G); impl Guard for Not { #[inline] fn check(&self, ctx: &GuardContext<'_>) -> bool { !self.0.check(ctx) } + fn name(&self) -> String { + format!("Not({})", self.0.name()) + } + fn details(&self) -> Option> { + self.0.details() + } } /// Creates a guard that matches a specified HTTP method. @@ -321,6 +389,12 @@ impl Guard for MethodGuard { ctx.head().method == self.0 } + fn name(&self) -> String { + self.0.to_string() + } + fn details(&self) -> Option> { + Some(vec![GuardDetail::HttpMethods(vec![self.0.to_string()])]) + } } #[cfg(feature = "resources-introspection")] @@ -331,70 +405,24 @@ pub trait HttpMethodsExtractor { #[cfg(feature = "resources-introspection")] impl HttpMethodsExtractor for dyn Guard { fn extract_http_methods(&self) -> Vec { - if let Some(method_guard) = self.as_any().downcast_ref::() { - vec![method_guard.0.to_string()] - } else if let Some(any_guard) = self.as_any().downcast_ref::() { - any_guard - .guards - .iter() - .flat_map(|g| g.extract_http_methods()) - .collect() - } else if let Some(all_guard) = self.as_any().downcast_ref::() { - all_guard - .guards - .iter() - .flat_map(|g| g.extract_http_methods()) - .collect() - } else { - vec!["UNKNOWN".to_string()] - } - } -} - -#[cfg(feature = "resources-introspection")] -impl std::fmt::Display for MethodGuard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "[{}]", self.0) - } -} - -#[cfg(feature = "resources-introspection")] -impl std::fmt::Display for AllGuard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let methods: Vec = self - .guards + .details() + .unwrap_or_default() .iter() - .filter_map(|guard| { - guard - .as_any() - .downcast_ref::() - .map(|method_guard| method_guard.0.to_string()) - }) - .collect(); - - write!(f, "[{}]", methods.join(", ")) - } -} - -#[cfg(feature = "resources-introspection")] -impl std::fmt::Display for AnyGuard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let methods: Vec = self - .guards - .iter() - .map(|guard| { - let guard_ref = &**guard; - if let Some(method_guard) = guard_ref.as_any().downcast_ref::() { - method_guard.0.to_string() - } else if let Some(all_guard) = guard_ref.as_any().downcast_ref::() { - all_guard.to_string() + .flat_map(|detail| { + if let GuardDetail::HttpMethods(methods) = detail { + methods.clone() } else { - "UNKNOWN".to_string() + vec!["UNKNOWN".to_string()] } }) .collect(); - write!(f, "[{}]", methods.join(", ")) + if methods.is_empty() { + vec!["UNKNOWN".to_string()] + } else { + methods + } } } @@ -458,15 +486,14 @@ impl Guard for HeaderGuard { false } -} - -pub trait AsAny { - fn as_any(&self) -> &dyn Any; -} - -impl AsAny for T { - fn as_any(&self) -> &dyn Any { - self + fn name(&self) -> String { + format!("Header({}, {})", self.0, self.1.to_str().unwrap_or("")) + } + fn details(&self) -> Option> { + Some(vec![GuardDetail::Headers(vec![( + self.0.to_string(), + self.1.to_str().unwrap_or("").to_string(), + )])]) } } @@ -581,7 +608,7 @@ mod tests { #[test] fn function_guard() { let domain = "rust-lang.org".to_owned(); - let guard = fn_guard(move |ctx| ctx.head().uri.host().unwrap().ends_with(&domain)); + let guard = fn_guard(|ctx| ctx.head().uri.host().unwrap().ends_with(&domain)); let req = TestRequest::default() .uri("blog.rust-lang.org") From aebab17c1ecee50f8dc6f65b93258189538630ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20C=C3=A9spedes=20Tab=C3=A1rez?= <1295883+dertin@users.noreply.github.com> Date: Wed, 5 Mar 2025 02:34:39 -0300 Subject: [PATCH 12/12] refactor(introspection): add GuardDetail enum and remove downcast_ref usage - Added `GuardDetail` enum to encapsulate various introspection details of a guard. - Refactored `HttpMethodsExtractor` implementation to use `GuardDetail` instead of `downcast_ref`. --- actix-web/src/guard/mod.rs | 177 +++++++++++++++++++++---------------- 1 file changed, 102 insertions(+), 75 deletions(-) diff --git a/actix-web/src/guard/mod.rs b/actix-web/src/guard/mod.rs index 8f1021f76..acdd86e81 100644 --- a/actix-web/src/guard/mod.rs +++ b/actix-web/src/guard/mod.rs @@ -11,7 +11,7 @@ //! or handler. This interface is defined by the [`Guard`] trait. //! //! Commonly-used guards are provided in this module as well as a way of creating a guard from a -//! closure ([`fn_guard`]). The [`Not`], [`Any()`], and [`All`] guards are noteworthy, as they can be +//! closure ([`fn_guard`]). The [`Not`], [`Any()`], and [`All()`] guards are noteworthy, as they can be //! used to compose other guards in a more flexible and semantic way than calling `.guard(...)` on //! services multiple times (which might have different combining behavior than you want). //! @@ -50,7 +50,6 @@ //! [`Route`]: crate::Route::guard() use std::{ - any::Any, cell::{Ref, RefMut}, rc::Rc, }; @@ -67,6 +66,17 @@ pub use self::{ host::{Host, HostGuard}, }; +/// Enum to encapsulate various introspection details of a Guard. +#[derive(Debug, Clone)] +pub enum GuardDetail { + /// Detail associated with HTTP methods. + HttpMethods(Vec), + /// Detail associated with headers (header, value). + Headers(Vec<(String, String)>), + /// Generic detail. + Generic(String), +} + /// Provides access to request parts that are useful during routing. #[derive(Debug)] pub struct GuardContext<'a> { @@ -122,15 +132,31 @@ impl<'a> GuardContext<'a> { /// Interface for routing guards. /// /// See [module level documentation](self) for more. -pub trait Guard: AsAny { +pub trait Guard { /// Returns true if predicate condition is met for a given request. fn check(&self, ctx: &GuardContext<'_>) -> bool; + + /// Returns a nominal representation of the guard. + fn name(&self) -> String { + std::any::type_name::().to_string() + } + + /// Returns detailed introspection information. + fn details(&self) -> Option> { + None + } } impl Guard for Rc { fn check(&self, ctx: &GuardContext<'_>) -> bool { (**self).check(ctx) } + fn name(&self) -> String { + (**self).name() + } + fn details(&self) -> Option> { + (**self).details() + } } /// Creates a guard using the given function. @@ -147,7 +173,7 @@ impl Guard for Rc { /// ``` pub fn fn_guard(f: F) -> impl Guard where - F: Fn(&GuardContext<'_>) -> bool + 'static, + F: Fn(&GuardContext<'_>) -> bool, { FnGuard(f) } @@ -156,7 +182,7 @@ struct FnGuard) -> bool>(F); impl Guard for FnGuard where - F: Fn(&GuardContext<'_>) -> bool + 'static, + F: Fn(&GuardContext<'_>) -> bool, { fn check(&self, ctx: &GuardContext<'_>) -> bool { (self.0)(ctx) @@ -165,7 +191,7 @@ where impl Guard for F where - F: for<'a> Fn(&'a GuardContext<'a>) -> bool + 'static, + F: Fn(&GuardContext<'_>) -> bool, { fn check(&self, ctx: &GuardContext<'_>) -> bool { (self)(ctx) @@ -220,6 +246,24 @@ impl Guard for AnyGuard { false } + fn name(&self) -> String { + format!( + "AnyGuard({})", + self.guards + .iter() + .map(|g| g.name()) + .collect::>() + .join(", ") + ) + } + fn details(&self) -> Option> { + Some( + self.guards + .iter() + .flat_map(|g| g.details().unwrap_or_default()) + .collect(), + ) + } } /// Creates a guard that matches if all added guards match. @@ -248,7 +292,7 @@ pub fn All(guard: F) -> AllGuard { /// /// That is, **all** contained guard needs to match in order for the aggregate guard to match. /// -/// Construct an `AllGuard` using [`All`]. +/// Construct an `AllGuard` using [`All()`]. pub struct AllGuard { guards: Vec>, } @@ -272,6 +316,24 @@ impl Guard for AllGuard { true } + fn name(&self) -> String { + format!( + "AllGuard({})", + self.guards + .iter() + .map(|g| g.name()) + .collect::>() + .join(", ") + ) + } + fn details(&self) -> Option> { + Some( + self.guards + .iter() + .flat_map(|g| g.details().unwrap_or_default()) + .collect(), + ) + } } /// Wraps a guard and inverts the outcome of its `Guard` implementation. @@ -285,13 +347,19 @@ impl Guard for AllGuard { /// .guard(guard::Not(guard::Get())) /// .to(|| HttpResponse::Ok()); /// ``` -pub struct Not(pub G); +pub struct Not(pub G); impl Guard for Not { #[inline] fn check(&self, ctx: &GuardContext<'_>) -> bool { !self.0.check(ctx) } + fn name(&self) -> String { + format!("Not({})", self.0.name()) + } + fn details(&self) -> Option> { + self.0.details() + } } /// Creates a guard that matches a specified HTTP method. @@ -321,6 +389,12 @@ impl Guard for MethodGuard { ctx.head().method == self.0 } + fn name(&self) -> String { + self.0.to_string() + } + fn details(&self) -> Option> { + Some(vec![GuardDetail::HttpMethods(vec![self.0.to_string()])]) + } } #[cfg(feature = "resources-introspection")] @@ -331,70 +405,24 @@ pub trait HttpMethodsExtractor { #[cfg(feature = "resources-introspection")] impl HttpMethodsExtractor for dyn Guard { fn extract_http_methods(&self) -> Vec { - if let Some(method_guard) = self.as_any().downcast_ref::() { - vec![method_guard.0.to_string()] - } else if let Some(any_guard) = self.as_any().downcast_ref::() { - any_guard - .guards - .iter() - .flat_map(|g| g.extract_http_methods()) - .collect() - } else if let Some(all_guard) = self.as_any().downcast_ref::() { - all_guard - .guards - .iter() - .flat_map(|g| g.extract_http_methods()) - .collect() - } else { - vec!["UNKNOWN".to_string()] - } - } -} - -#[cfg(feature = "resources-introspection")] -impl std::fmt::Display for MethodGuard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "[{}]", self.0) - } -} - -#[cfg(feature = "resources-introspection")] -impl std::fmt::Display for AllGuard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let methods: Vec = self - .guards + .details() + .unwrap_or_default() .iter() - .filter_map(|guard| { - guard - .as_any() - .downcast_ref::() - .map(|method_guard| method_guard.0.to_string()) - }) - .collect(); - - write!(f, "[{}]", methods.join(", ")) - } -} - -#[cfg(feature = "resources-introspection")] -impl std::fmt::Display for AnyGuard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let methods: Vec = self - .guards - .iter() - .map(|guard| { - let guard_ref = &**guard; - if let Some(method_guard) = guard_ref.as_any().downcast_ref::() { - method_guard.0.to_string() - } else if let Some(all_guard) = guard_ref.as_any().downcast_ref::() { - all_guard.to_string() + .flat_map(|detail| { + if let GuardDetail::HttpMethods(methods) = detail { + methods.clone() } else { - "UNKNOWN".to_string() + vec!["UNKNOWN".to_string()] } }) .collect(); - write!(f, "[{}]", methods.join(", ")) + if methods.is_empty() { + vec!["UNKNOWN".to_string()] + } else { + methods + } } } @@ -458,15 +486,14 @@ impl Guard for HeaderGuard { false } -} - -pub trait AsAny { - fn as_any(&self) -> &dyn Any; -} - -impl AsAny for T { - fn as_any(&self) -> &dyn Any { - self + fn name(&self) -> String { + format!("Header({}, {})", self.0, self.1.to_str().unwrap_or("")) + } + fn details(&self) -> Option> { + Some(vec![GuardDetail::Headers(vec![( + self.0.to_string(), + self.1.to_str().unwrap_or("").to_string(), + )])]) } } @@ -581,7 +608,7 @@ mod tests { #[test] fn function_guard() { let domain = "rust-lang.org".to_owned(); - let guard = fn_guard(move |ctx| ctx.head().uri.host().unwrap().ends_with(&domain)); + let guard = fn_guard(|ctx| ctx.head().uri.host().unwrap().ends_with(&domain)); let req = TestRequest::default() .uri("blog.rust-lang.org")