diff --git a/Cargo.lock b/Cargo.lock index 17f81f3b..df79303a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -444,7 +444,7 @@ dependencies = [ "hyper", "hyper-util", "itoa", - "matchit", + "matchit 0.8.4", "memchr", "mime", "percent-encoding", @@ -940,6 +940,7 @@ dependencies = [ "insta", "is_terminal_polyfill", "lettre", + "matchit 0.9.2", "mime", "mime_guess", "mockall", @@ -2660,6 +2661,12 @@ version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47e1ffaa40ddd1f3ed91f717a33c8c0ee23fff369e3aa8772b9605cc1d22f4c3" +[[package]] +name = "matchit" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8863b587001c1b9a8a4e36008cebc6b3612cb1226fe2de94858e06092687b608" + [[package]] name = "md-5" version = "0.11.0" diff --git a/Cargo.toml b/Cargo.toml index b61a9952..90ab7281 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,6 +115,7 @@ insta-cmd = "0.7" is_terminal_polyfill = "1.70" lettre = { version = "0.11.22", default-features = false } libtest-mimic = "0.8" +matchit = "0.9.2" mime = "0.3" mime_guess = { version = "2", default-features = false } mockall = "0.15" diff --git a/cot/Cargo.toml b/cot/Cargo.toml index 345dac81..0ef4ab23 100644 --- a/cot/Cargo.toml +++ b/cot/Cargo.toml @@ -44,6 +44,7 @@ idna = { workspace = true, optional = true } indexmap.workspace = true is_terminal_polyfill.workspace = true lettre = { workspace = true, features = ["builder", "sendmail-transport", "smtp-transport", "tokio1", "tokio1-rustls", "ring", "rustls-platform-verifier"], optional = true } +matchit.workspace = true mime.workspace = true mime_guess.workspace = true multer.workspace = true diff --git a/cot/src/error_page.rs b/cot/src/error_page.rs index 16e2eec4..2ec29ba6 100644 --- a/cot/src/error_page.rs +++ b/cot/src/error_page.rs @@ -8,6 +8,7 @@ use tracing::{Level, error, warn}; use crate::config::ProjectConfig; use crate::error::NotFound; use crate::router::Router; +use crate::router::path::AbsolutePath; use crate::{Error, Result, StatusCode, Template}; #[derive(Debug)] @@ -123,7 +124,12 @@ impl ErrorPageTemplateBuilder { fn diagnostics(&mut self, diagnostics: &Diagnostics) -> &mut Self { self.project_config = format!("{:#?}", diagnostics.project_config); self.route_data.clear(); - Self::build_route_data(&mut self.route_data, &diagnostics.router, "", ""); + Self::build_route_data( + &mut self.route_data, + &diagnostics.router, + &AbsolutePath::root(), + "", + ); self.request_data = diagnostics .request_head .as_ref() @@ -133,14 +139,16 @@ impl ErrorPageTemplateBuilder { fn build_route_data( route_data: &mut Vec, - router: &Router, - url_prefix: &str, + router: &Arc, + url_prefix: &AbsolutePath, index_prefix: &str, ) { for (index, route) in router.routes().iter().enumerate() { + let full_path = url_prefix.join(&AbsolutePath::new(route.url())); + route_data.push(RouteData { index: format!("{index_prefix}{index}"), - path: format!("{url_prefix}{}", route.url()), + path: full_path.to_string(), kind: match route.kind() { crate::router::RouteKind::Router => if route_data.is_empty() { "Root Router" @@ -151,13 +159,14 @@ impl ErrorPageTemplateBuilder { crate::router::RouteKind::Handler => "View".to_owned(), }, name: route.name().unwrap_or_default().to_owned(), + app: route.app_name().map(|a| a.0.clone()).unwrap_or_default(), }); if let Some(inner_router) = route.router() { Self::build_route_data( route_data, - inner_router, - &format!("{}{}", url_prefix, route.url()), + &inner_router, + &full_path, &format!("{index_prefix}{index}."), ); } @@ -242,6 +251,7 @@ struct RouteData { path: String, kind: String, name: String, + app: String, } #[derive(Debug, Clone)] @@ -453,6 +463,10 @@ mod tests { use std::panic; use std::sync::Arc; + use cot_core::handler::RequestHandler; + use cot_core::html::Html; + use cot_core::request::Request; + use cot_core::response::{IntoResponse, Response}; use tracing_test::traced_test; use super::*; @@ -468,6 +482,14 @@ mod tests { } } + struct MockHandler; + + impl RequestHandler for MockHandler { + fn handle(&self, _request: Request) -> impl Future> { + std::future::ready(Html::new("OK").into_response()) + } + } + #[test] #[traced_test] fn test_log_error() { @@ -588,9 +610,16 @@ mod tests { let mut route_data = Vec::new(); let sub_sub_router = Router::with_urls(vec![]); let sub_router = Router::with_urls(vec![Route::with_router("/bar", sub_sub_router)]); - let router = Router::with_urls(vec![Route::with_router("/foo", sub_router)]); - - ErrorPageTemplateBuilder::build_route_data(&mut route_data, &router, "", ""); + let router = Arc::new(Router::with_urls(vec![Route::with_router( + "/foo", sub_router, + )])); + + ErrorPageTemplateBuilder::build_route_data( + &mut route_data, + &router, + &AbsolutePath::root(), + "", + ); assert_eq!( route_data, @@ -599,18 +628,61 @@ mod tests { index: "0".to_string(), path: "/foo".to_string(), kind: "Root Router".to_string(), - name: String::new() + name: String::new(), + app: String::new() }, RouteData { index: "0.0".to_string(), path: "/foo/bar".to_string(), kind: "Router".to_string(), - name: String::new() + name: String::new(), + app: String::new() } ] ); } + #[test] + fn build_route_data_root_mount_no_double_slash() { + let mut route_data = Vec::new(); + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "index", + )]); + let router = Arc::new(Router::with_urls(vec![Route::with_router("/", sub_router)])); + + ErrorPageTemplateBuilder::build_route_data( + &mut route_data, + &router, + &AbsolutePath::root(), + "", + ); + + assert_eq!(route_data[0].path, "/"); + assert_eq!(route_data[1].path, "/"); + } + + #[test] + fn build_route_data_root_mount_with_nested_static_route() { + let mut route_data = Vec::new(); + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/nested", + MockHandler, + "nested", + )]); + let router = Arc::new(Router::with_urls(vec![Route::with_router("/", sub_router)])); + + ErrorPageTemplateBuilder::build_route_data( + &mut route_data, + &router, + &AbsolutePath::root(), + "", + ); + + assert_eq!(route_data[1].path, "/nested"); + } + #[test] fn test_build_cot_failure_page() { let response = build_cot_failure_page(); diff --git a/cot/src/openapi.rs b/cot/src/openapi.rs index 28947c3c..31ff42c3 100644 --- a/cot/src/openapi.rs +++ b/cot/src/openapi.rs @@ -86,7 +86,7 @@ //! } //! //! fn register_apps(&self, apps: &mut AppBuilder, _context: &RegisterAppsContext) { -//! apps.register_with_views(SwaggerUi::new(), "/swagger"); +//! apps.register_with_views(SwaggerUi::new(), "/swagger/"); //! apps.register_with_views(AddApp, ""); //! } //! } diff --git a/cot/src/router.rs b/cot/src/router.rs index 48711d6d..b26d27d8 100644 --- a/cot/src/router.rs +++ b/cot/src/router.rs @@ -27,6 +27,7 @@ use std::pin::Pin; use std::sync::Arc; use std::task::{Context, Poll}; +use cot::router::path::AbsolutePath; use cot_core::error::impl_into_cot_error; use cot_core::handler::{BoxRequestHandler, RequestHandler, into_box_request_handler}; use cot_core::request::{AppName, RouteName}; @@ -36,11 +37,15 @@ use tracing::debug; use crate::error::NotFound; use crate::request::{PathParams, Request, RequestExt, RequestHead}; use crate::response::Response; -use crate::router::path::{CaptureResult, PathMatcher, ReverseParamMap}; +use crate::router::path::{PathMatcher, ReverseParamMap}; +use crate::router::tree::{Entry, MatchitPattern, RouteTrie}; use crate::{Error, ProjectContext, Result}; pub mod method; pub mod path; +mod tree; + +type RouteMap = HashMap, Arc)>>; /// A router that can be used to route requests to their respective views. /// @@ -65,7 +70,8 @@ pub mod path; pub struct Router { app_name: Option, urls: Vec, - names: HashMap>, + names: RouteMap, + route_tree: RouteTrie, } impl Router { @@ -100,22 +106,139 @@ impl Router { /// /// let router = Router::with_urls([Route::with_handler_and_name("/", home, "home")]); /// ``` + /// + /// # Panics + /// + /// Panics when a url string could not be parsed into a [`Route`] #[must_use] pub fn with_urls>>(urls: T) -> Self { - let urls = urls.into(); - let mut names = HashMap::new(); + Self::try_with_urls(urls).unwrap_or_else(|err| panic!("{err}")) + } + + /// Create a router with the given routes. This is a fallible version + /// of [`Self::with_urls`] + /// + /// # Examples + /// + /// ``` + /// use cot::request::Request; + /// use cot::response::Response; + /// use cot::router::{Route, Router}; + /// + /// async fn home(request: Request) -> cot::Result { + /// unimplemented!() + /// } + /// + /// let router = Router::try_with_urls([Route::with_handler_and_name("/", home, "home")]).unwrap(); + /// ``` + /// + /// # Errors + /// + /// This method fails when the underlying trie fails to build. + pub fn try_with_urls>>(urls: T) -> Result { + let urls = Self::merge_conflicting_routers(urls.into())?; + let mut names: RouteMap = HashMap::new(); for url in &urls { if let Some(name) = &url.name { - names.insert(name.clone(), url.url.clone()); + let requested = url.app_name.as_ref().map(|a| a.0.as_str()); + let bucket = names.entry(name.clone()).or_default(); + if let Some((_, existing_url)) = bucket + .iter() + .find(|(app, _)| app.as_ref().map(|a| a.0.as_str()) == requested) + { + // we found another route in the same app with the same + // name. This is unacceptable + return Err(RouteConflictError::DuplicateRouteName { + name: name.0.clone(), + existing: existing_url.to_string(), + new: url.url(), + } + .into()); + } + bucket.push((url.app_name.clone(), url.url.clone())); } } - - Self { + let route_tree = RouteTrie::build(&urls)?; + Ok(Self { app_name: None, urls, names, + route_tree, + }) + } + + fn merge_conflicting_routers(urls: Vec) -> Result> { + let mut merged: Vec = Vec::with_capacity(urls.len()); + let mut positions: HashMap = HashMap::new(); + + for route in urls { + if route.kind() != RouteKind::Router { + merged.push(route); + continue; + } + // we want to merge/fold routers that have the same mount + // point(pattern). This can be useful especially in cases + // where multiple apps are registered with the same pattern + // as shown in the example below: + // + // fn register_apps( + // &self, apps: &mut AppBuilder, + // _context: &RegisterAppsContext + // ) { + // apps.register_with_views(App1, "/foo"); + // apps.register_with_views(App2, "/foo"); + // } + + let pattern = tree::router_mount_pattern(&route); + if let Some(&pos) = positions.get(&pattern) { + let existing = merged[pos].clone(); + merged[pos] = Self::merge_router_routes(&existing, &route)?; + } else { + positions.insert(pattern, merged.len()); + merged.push(route); + } } + + Ok(merged) + } + + fn merge_router_routes(existing: &Route, new: &Route) -> Result { + let existing_router = existing + .router() + .expect("existing route should be a nested router"); + let new_router = new.router().expect("new route should be a nested router"); + + debug!( + path = %existing.url(), + existing_app = ?existing_router.app_name, + new_app = ?new_router.app_name, + "merging nested routers mounted at the same path", + ); + + let mut combined = Vec::with_capacity(existing_router.urls.len() + new_router.urls.len()); + combined.extend( + existing_router + .urls + .iter() + .cloned() + .map(|r| r.with_app_name_if_unset(existing_router.app_name.clone())), + ); + + combined.extend( + new_router + .urls + .iter() + .cloned() + .map(|r| r.with_app_name_if_unset(new_router.app_name.clone())), + ); + + let merged_router = Router::try_with_urls(combined)?; + + // setting the router's url to that of the existing route is done purely + // for deterministic purposes. This should have no side-effect since we + // match against the normalized url + Ok(Route::with_router(&existing.url(), merged_router)) } pub(crate) fn set_app_name(&mut self, app_name: AppName) { @@ -127,7 +250,7 @@ impl Router { if let Some(result) = self.get_handler(request_path) { let mut path_params = PathParams::new(); - for (key, value) in result.params.iter().rev() { + for (key, value) in &result.params { path_params.insert(key.clone(), value.clone()); } request.extensions_mut().insert(path_params); @@ -145,64 +268,72 @@ impl Router { } fn get_handler(&self, request_path: &str) -> Option> { - for route in &self.urls { - if let Some(matches) = route.url.capture(request_path) { - let matches_fully = matches.matches_fully(); - - match &route.view { - RouteInner::Handler(handler) => { - if matches_fully { - return Some(HandlerFound { - handler: &**handler, - app_name: self.app_name.clone(), - name: route.name.clone(), - params: Self::matches_to_path_params(&matches, Vec::new()), - }); - } - } - RouteInner::Router(router) => { - if let Some(result) = router.get_handler(matches.remaining_path) { - return Some(HandlerFound { - handler: result.handler, - app_name: result.app_name.or_else(|| self.app_name.clone()), - name: result.name, - params: Self::matches_to_path_params(&matches, result.params), - }); - } - } - #[cfg(feature = "openapi")] - RouteInner::ApiHandler(handler) => { - if matches_fully { - let handler: &(dyn BoxRequestHandler + Send + Sync) = &**handler; - return Some(HandlerFound { - handler, - app_name: self.app_name.clone(), - name: route.name.clone(), - params: Self::matches_to_path_params(&matches, Vec::new()), - }); - } - } - } + let m = self.route_tree.at(request_path)?; + + let (route_index, remaining_path) = match m.value { + Entry::Handler(idx) => (*idx, String::new()), + Entry::Router(idx) => { + let remaining = match m.params.get(tree::NESTED_ROUTER_PARAM) { + Some(rest) => AbsolutePath::new(rest), + None => AbsolutePath::root(), + }; + (*idx, remaining.into()) } - } + }; - None - } + let params: Vec<(String, String)> = m + .params + .iter() + .filter(|(key, _)| *key != tree::NESTED_ROUTER_PARAM) + .map(|(k, v)| (k.to_owned(), v.to_owned())) + .collect(); - pub(crate) fn has_route(&self, request_path: &str) -> bool { - self.get_handler(request_path).is_some() + Self::route_to_handler(self, route_index, &remaining_path, ¶ms) } - fn matches_to_path_params( - matches: &CaptureResult<'_, '_>, - mut path_params: Vec<(String, String)>, - ) -> Vec<(String, String)> { - // Adding in reverse order, since we're doing this from the bottom up - // (we're going to reverse the order before running the handler) - for param in matches.params.iter().rev() { - path_params.push((param.name.to_owned(), param.value.clone())); + fn route_to_handler<'a>( + router: &'a Router, + route_index: usize, + remaining_path: &str, + params: &[(String, String)], + ) -> Option> { + let route = &router.urls[route_index]; + + match &route.view { + RouteInner::Handler(handler) => Some(HandlerFound { + handler: &**handler, + app_name: route.app_name.clone().or_else(|| router.app_name.clone()), + name: route.name.clone(), + params: params.to_vec(), + }), + RouteInner::Router(nested_router) => { + nested_router.get_handler(remaining_path).map(|mut found| { + found.app_name = found + .app_name + .or_else(|| route.app_name.clone()) + .or_else(|| router.app_name.clone()); + + let mut combined = params.to_vec(); + combined.extend(found.params); + found.params = combined; + found + }) + } + #[cfg(feature = "openapi")] + RouteInner::ApiHandler(handler) => { + let handler: &(dyn BoxRequestHandler + Send + Sync) = &**handler; + Some(HandlerFound { + handler, + app_name: route.app_name.clone().or_else(|| router.app_name.clone()), + name: route.name.clone(), + params: params.to_vec(), + }) + } } - path_params + } + + pub(crate) fn has_route(&self, request_path: &str) -> bool { + self.get_handler(request_path).is_some() } /// Handle a request. @@ -282,24 +413,52 @@ impl Router { name: &str, params: &ReverseParamMap, ) -> Result> { - let url = self - .names - .get(&RouteName(String::from(name))) - .map(|matcher| matcher.reverse(params)); - if let Some(url) = url { - return Ok(Some(url?)); + if let Some(candidates) = self.names.get(&RouteName(String::from(name))) { + let matched = candidates + .iter() + .find(|(candidate_app, _)| candidate_app.as_ref().map(|a| a.0.as_str()) == app_name) + .or_else(|| { + candidates + .iter() + .find(|(candidate_app, _)| candidate_app.is_none()) + }); + if let Some((_, matcher)) = matched { + // fast path: we found the route with the provided name in this + // router + return Ok(Some(matcher.reverse(params)?)); + } } + // slow path: the route may exist in the nested routers. we search + // through recursively to find it. for route in &self.urls { if let RouteInner::Router(router) = &route.view + && Self::app_name_matches(app_name, route.app_name.as_ref()) && let Some(url) = router.reverse_option(app_name, name, params)? { - return Ok(Some(route.url.reverse(params)? + &url)); + let prefix = AbsolutePath::new(route.url.reverse(params)?); + let suffix = AbsolutePath::new(url); + + // we are in a sub-router, and if its parent does not end in a + // trailing slash (eg. `foo`) and the found route is the + // sub-router's root (`/`), then we can safely assume that + // the trailing-slash version (eg. `foo/`) does not exist. + // We return its parent and must not join + let combined = if !prefix.as_str().ends_with('/') && suffix.as_str() == "/" { + prefix + } else { + prefix.join(&suffix) + }; + return Ok(Some(combined.into())); } } Ok(None) } + fn app_name_matches(requested: Option<&str>, candidate: Option<&AppName>) -> bool { + requested.is_none() || candidate.is_none() || candidate.map(|a| a.0.as_str()) == requested + } + /// Get the routes in this router. /// /// # Examples @@ -363,7 +522,12 @@ impl Router { let mut schema_generator = schemars::SchemaGenerator::new(schemars::generate::SchemaSettings::openapi3()); - self.as_openapi_impl("", &[], &mut paths, &mut schema_generator); + self.as_openapi_impl( + &AbsolutePath::root(), + &[], + &mut paths, + &mut schema_generator, + ); let component_schemas = schema_generator .take_definitions(true) @@ -394,7 +558,7 @@ impl Router { #[cfg(feature = "openapi")] fn as_openapi_impl( &self, - url: &str, + url: &AbsolutePath, param_names: &[&str], paths: &mut aide::openapi::Paths, schema_generator: &mut schemars::SchemaGenerator, @@ -410,14 +574,14 @@ impl Router { param_names: &[&str], paths: &mut aide::openapi::Paths, schema_generator: &mut schemars::SchemaGenerator, - url: &str, + url: &AbsolutePath, ) { match &route.view { RouteInner::Router(router) => { let mut params = Vec::from(param_names); params.extend(route.url.param_names()); - let url = format!("{url}{}", route.url); + let url = url.join(&AbsolutePath::new(route.url())); router.as_openapi_impl(&url, ¶ms, paths, schema_generator); } @@ -425,13 +589,13 @@ impl Router { let mut params = Vec::from(param_names); params.extend(route.url.param_names()); - let url = format!("{url}{}", route.url); + let url = url.join(&AbsolutePath::new(route.url())); let mut route_context = crate::openapi::RouteContext::new(); route_context.param_names = ¶ms; paths.paths.insert( - url, + url.into(), aide::openapi::ReferenceOr::Item( handler.as_api_route(&route_context, schema_generator), ), @@ -456,6 +620,64 @@ struct NoViewToReverse { } impl_into_cot_error!(NoViewToReverse); +const ERROR_PREFIX: &str = "route conflict error:"; +#[derive(Debug, thiserror::Error)] +enum RouteConflictError { + #[error( + "{ERROR_PREFIX} duplicate route: `{new}` conflicts with an already registered handler route `{existing}` \ + (both fully match the same path)" + )] + DuplicateHandler { existing: String, new: String }, + + #[error( + "{ERROR_PREFIX} duplicate nested router: `{new}` conflicts with an already registered \ + nested router mounted at `{existing}`" + )] + DuplicateRouter { existing: String, new: String }, + + #[error( + "{ERROR_PREFIX} duplicate route name: `{name}` is registered at both `{existing}` and `{new}`; route names \ + must be unique within the same app" + )] + DuplicateRouteName { + name: String, + existing: String, + new: String, + }, + + #[error( + "{ERROR_PREFIX} conflicting route parameters: `{existing}` uses `{{{existing_name}}}` but `{new}` uses \ + `{{{new_name}}}` at the same position in the path; both routes must bind the same \ + parameter name there, since only one value can be captured at that position" + )] + ConflictingParamName { + existing: String, + existing_name: String, + new: String, + new_name: String, + }, + + #[error( + "{ERROR_PREFIX} conflicting wildcard parameters: `{existing}` uses `{{*{existing_name}}}` but `{new}` \ + uses `{{*{new_name}}}` at the same position in the path" + )] + ConflictingWildcardName { + existing: String, + existing_name: String, + new: String, + new_name: String, + }, + + #[error( + "{ERROR_PREFIX} duplicate wildcard route: `{new}` conflicts with an already-registered \ + wildcard route `{existing}`" + )] + DuplicateWildcard { existing: String, new: String }, + #[error("{ERROR_PREFIX} error while inserting route")] + RouteInsert(#[from] matchit::InsertError), +} +impl_into_cot_error!(RouteConflictError); + #[derive(Debug)] struct HandlerFound<'a> { #[debug("handler(...)")] @@ -552,6 +774,7 @@ pub struct Route { url: Arc, view: RouteInner, name: Option, + app_name: Option, } impl Route { @@ -582,6 +805,7 @@ impl Route { url: Arc::new(PathMatcher::new(url)), view: RouteInner::Handler(Arc::new(into_box_request_handler(handler))), name: None, + app_name: None, } } @@ -619,6 +843,7 @@ impl Route { crate::openapi::into_box_api_endpoint_request_handler(handler), )), name: None, + app_name: None, } } @@ -650,6 +875,7 @@ impl Route { url: Arc::new(PathMatcher::new(url)), view: RouteInner::Handler(Arc::new(into_box_request_handler(handler))), name: Some(RouteName(name.into())), + app_name: None, } } @@ -688,6 +914,7 @@ impl Route { crate::openapi::into_box_api_endpoint_request_handler(handler), )), name: Some(RouteName(name.into())), + app_name: None, } } @@ -711,9 +938,18 @@ impl Route { pub fn with_router(url: &str, router: Router) -> Self { Self { url: Arc::new(PathMatcher::new(url)), - view: RouteInner::Router(router), + view: RouteInner::Router(Arc::new(router)), name: None, + app_name: None, + } + } + + #[must_use] + fn with_app_name_if_unset(mut self, app_name: Option) -> Self { + if self.app_name.is_none() { + self.app_name = app_name; } + self } /// Get the URL for this route. @@ -759,6 +995,11 @@ impl Route { self.name.as_ref().map(|name| name.0.as_str()) } + #[must_use] + pub(crate) fn app_name(&self) -> Option<&AppName> { + self.app_name.as_ref() + } + #[must_use] pub(crate) fn kind(&self) -> RouteKind { match &self.view { @@ -770,9 +1011,9 @@ impl Route { } #[must_use] - pub(crate) fn router(&self) -> Option<&Router> { + pub(crate) fn router(&self) -> Option> { match &self.view { - RouteInner::Router(router) => Some(router), + RouteInner::Router(router) => Some(router.clone()), RouteInner::Handler(_) => None, #[cfg(feature = "openapi")] RouteInner::ApiHandler(_) => None, @@ -789,7 +1030,7 @@ pub(crate) enum RouteKind { #[derive(Clone)] enum RouteInner { Handler(Arc), - Router(Router), + Router(Arc), #[cfg(feature = "openapi")] ApiHandler(Arc), } @@ -1084,8 +1325,17 @@ mod tests { } } + fn assert_params(mut actual: Vec<(String, String)>, expected: &[(&str, &str)]) { + let mut expected = expected + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect::>(); + actual.sort(); + expected.sort(); + assert_eq!(actual, expected); + } + #[test] - #[cfg(feature = "openapi")] fn route_inner_debug() { let route = Route::with_handler("/test", MockHandler); assert!(format!("{route:?}").contains("Handler(\"handler(...)\")")); @@ -1093,12 +1343,14 @@ mod tests { let route = Route::with_router("/test", Router::empty()); assert!(format!("{route:?}").contains("Router(Router {")); - let route = Route::with_api_handler("/test", MockHandler); - assert!(format!("{route:?}").contains("ApiHandler(\"handler(...)\")")); + #[cfg(feature = "openapi")] + { + let route = Route::with_api_handler("/test", MockHandler); + assert!(format!("{route:?}").contains("ApiHandler(\"handler(...)\")")); + } } #[test] - #[cfg(feature = "openapi")] fn route_kind() { let handler_route = Route::with_handler("/test", MockHandler); assert_eq!(handler_route.kind(), RouteKind::Handler); @@ -1106,12 +1358,14 @@ mod tests { let router_route = Route::with_router("/test", Router::empty()); assert_eq!(router_route.kind(), RouteKind::Router); - let api_route = Route::with_api_handler("/test", MockHandler); - assert_eq!(api_route.kind(), RouteKind::Handler); + #[cfg(feature = "openapi")] + { + let api_route = Route::with_api_handler("/test", MockHandler); + assert_eq!(api_route.kind(), RouteKind::Handler); + } } #[test] - #[cfg(feature = "openapi")] fn route_router() { let router = Router::empty(); let route = Route::with_router("/test", router.clone()); @@ -1120,17 +1374,59 @@ mod tests { let route = Route::with_handler("/test", MockHandler); assert!(route.router().is_none()); - let route = Route::with_api_handler("/test", MockHandler); - assert!(route.router().is_none()); + #[cfg(feature = "openapi")] + { + let route = Route::with_api_handler("/test", MockHandler); + assert!(route.router().is_none()); + } + } + + #[test] + fn route_with_handler() { + let route = Route::with_handler("/test", MockHandler); + assert_eq!(route.url.to_string(), "/test"); + } + + #[test] + fn route_with_handler_and_params() { + let route = Route::with_handler("/test/{id}", MockHandler); + assert_eq!(route.url.to_string(), "/test/{id}"); + } + + #[test] + fn route_with_handler_and_name() { + let route = Route::with_handler_and_name("/test", MockHandler, "test"); + assert_eq!(route.url.to_string(), "/test"); + assert_eq!(route.name, Some(RouteName("test".to_string()))); + } + + #[test] + fn route_with_router() { + let sub_route = Route::with_handler("/sub", MockHandler); + let sub_router = Router::with_urls(vec![sub_route]); + let route = Route::with_router("/test", sub_router); + assert_eq!(route.url.to_string(), "/test"); + } + + #[test] + fn router_is_empty() { + let router = Router::with_urls(vec![]); + assert!(router.is_empty()); } #[test] - fn router_with_urls() { + fn router_routes() { let route = Route::with_handler("/test", MockHandler); let router = Router::with_urls(vec![route.clone()]); assert_eq!(router.routes().len(), 1); } + #[test] + fn router_empty_returns_no_handler() { + let router = Router::empty(); + assert!(router.get_handler("/").is_none()); + } + #[cot::test] async fn router_route() { let route = Route::with_handler("/test", MockHandler); @@ -1181,108 +1477,1210 @@ mod tests { } #[test] - fn router_reverse() { - let route = Route::with_handler_and_name("/test", MockHandler, "test"); - let router = Router::with_urls(vec![route.clone()]); - let params = ReverseParamMap::new(); - let url = router.reverse(None, "test", ¶ms).unwrap(); - assert_eq!(url, "/test"); + fn router_no_param_route_matches_exact_path() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users", + MockHandler, + "users", + )]); + + let found = router.get_handler("/users").unwrap(); + + assert_eq!(found.name, Some(RouteName("users".to_string()))); + assert!(found.params.is_empty()); } #[test] - fn router_reverse_with_param() { - let route = Route::with_handler_and_name("/test/{id}", MockHandler, "test"); - let router = Router::with_urls(vec![route.clone()]); - let mut params = ReverseParamMap::new(); - params.insert("id", "123"); - let url = router.reverse(None, "test", ¶ms).unwrap(); - assert_eq!(url, "/test/123"); + fn router_no_param_route_rejects_different_path() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users", + MockHandler, + "users", + )]); + + assert!(router.get_handler("/test").is_none()); } #[test] - fn router_reverse_app_name() { - let route = Route::with_handler_and_name("/test", MockHandler, "test"); - let mut router_1 = Router::with_urls(vec![route.clone()]); - router_1.set_app_name(AppName("app_1".to_string())); - let mut router_2 = Router::with_urls(vec![route.clone()]); - router_2.set_app_name(AppName("app_2".to_string())); - let root_router = Router::with_urls(vec![ - Route::with_router("/", router_1), - Route::with_router("/sub", router_2), + fn router_routes_with_common_static_prefixes_match_independently() { + let router = Router::with_urls(vec![ + Route::with_handler_and_name("/car", MockHandler, "car"), + Route::with_handler_and_name("/cart", MockHandler, "cart"), + Route::with_handler_and_name("/catalog", MockHandler, "catalog"), ]); - let params = ReverseParamMap::new(); - let url = root_router.reverse(Some("app_2"), "test", ¶ms).unwrap(); + assert_eq!( + router.get_handler("/car").unwrap().name, + Some(RouteName("car".to_string())) + ); + assert_eq!( + router.get_handler("/cart").unwrap().name, + Some(RouteName("cart".to_string())) + ); + assert_eq!( + router.get_handler("/catalog").unwrap().name, + Some(RouteName("catalog".to_string())) + ); + assert!(router.get_handler("/cartographer").is_none()); + } - assert_eq!(url, "/sub/test"); + #[test] + fn router_param_route_captures_single_segment() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/{id}", + MockHandler, + "user_detail", + )]); + + let found = router.get_handler("/users/123").unwrap(); + + assert_eq!(found.name, Some(RouteName("user_detail".to_string()))); + assert_params(found.params, &[("id", "123")]); } #[test] - fn router_reverse_app_name_nested() { - let route = Route::with_handler_and_name("/test", MockHandler, "test"); - let router = Router::with_urls(vec![route.clone()]); - let sub_router = Router::with_urls(vec![Route::with_router("/sub", router)]); - let mut root_router = Router::with_urls(vec![Route::with_router("/subsub", sub_router)]); - root_router.set_app_name(AppName("app_root".to_string())); + fn router_param_route_rejects_empty_segment() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/{id}", + MockHandler, + "user_detail", + )]); + + assert!(router.get_handler("/users/").is_none()); + } - let params = ReverseParamMap::new(); - let url = root_router - .reverse(Some("app_root"), "test", ¶ms) - .unwrap(); + #[test] + fn router_param_route_rejects_extra_path_for_handler() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/{id}", + MockHandler, + "user_detail", + )]); + + assert!(router.get_handler("/users/123/abc").is_none()); + } - assert_eq!(url, "/subsub/sub/test"); + #[test] + fn router_multiple_param_route_captures_all_params() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/{id}/posts/{post_id}", + MockHandler, + "post_detail", + )]); + + let found = router.get_handler("/users/123/posts/456").unwrap(); + + assert_eq!(found.name, Some(RouteName("post_detail".to_string()))); + assert_params(found.params, &[("id", "123"), ("post_id", "456")]); } #[test] - fn router_reverse_option() { - let route = Route::with_handler_and_name("/test", MockHandler, "test"); - let router = Router::with_urls(vec![route.clone()]); - let params = ReverseParamMap::new(); - let url = router - .reverse_option(None, "test", ¶ms) - .unwrap() - .unwrap(); - assert_eq!(url, "/test"); + fn router_static_route_takes_priority_over_dynamic_route() { + let router = Router::with_urls(vec![ + Route::with_handler_and_name("/users/{id}", MockHandler, "dynamic"), + Route::with_handler_and_name("/users/new", MockHandler, "static"), + ]); + + let found = router.get_handler("/users/new").unwrap(); + + assert_eq!(found.name, Some(RouteName("static".to_string()))); } #[test] - fn router_routes() { - let route = Route::with_handler("/test", MockHandler); - let router = Router::with_urls(vec![route.clone()]); - assert_eq!(router.routes().len(), 1); + fn router_single_pattern_multi_param_order_preserved() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/{model_name}/{pk}/edit/", + MockHandler, + "edit", + )]); + + let found = router.get_handler("/database_user/1/edit/").unwrap(); + + assert_eq!( + found.params, + vec![ + ("model_name".to_string(), "database_user".to_string()), + ("pk".to_string(), "1".to_string()), + ] + ); } #[test] - fn router_is_empty() { - let router = Router::with_urls(vec![]); - assert!(router.is_empty()); + fn router_wildcard_root() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*path}", + MockHandler, + "users", + )]); + + let found = router.get_handler("/foo/bar").unwrap(); + + assert_eq!(found.name, Some(RouteName("users".to_string()))); + assert_eq!( + found.params, + vec![("path".to_string(), "foo/bar".to_string())] + ); } #[test] - fn route_with_handler() { - let route = Route::with_handler("/test", MockHandler); - assert_eq!(route.url.to_string(), "/test"); + fn router_wildcard_single_segment() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/rand/{*path}", + MockHandler, + "users", + )]); + + let found = router.get_handler("/users/rand/foo").unwrap(); + + assert_eq!(found.name, Some(RouteName("users".to_string()))); + assert_eq!(found.params, vec![("path".to_string(), "foo".to_string())]); } #[test] - fn route_with_handler_and_params() { - let route = Route::with_handler("/test/{id}", MockHandler); - assert_eq!(route.url.to_string(), "/test/{id}"); + fn router_wildcard_multi_segment() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/rand/{*path}", + MockHandler, + "users", + )]); + + let found = router.get_handler("/users/rand/foo/bar").unwrap(); + + assert_eq!(found.name, Some(RouteName("users".to_string()))); + assert_eq!( + found.params, + vec![("path".to_string(), "foo/bar".to_string())] + ); } #[test] - fn route_with_handler_and_name() { - let route = Route::with_handler_and_name("/test", MockHandler, "test"); - assert_eq!(route.url.to_string(), "/test"); - assert_eq!(route.name, Some(RouteName("test".to_string()))); + fn router_wildcard_empty_not_allowed() { + let router = Router::with_urls(vec![Route::with_handler_and_name( + "/users/rand/{*path}", + MockHandler, + "users", + )]); + + assert!(router.get_handler("/users/rand").is_none()); } #[test] - fn route_with_router() { - let sub_route = Route::with_handler("/sub", MockHandler); - let sub_router = Router::with_urls(vec![sub_route]); - let route = Route::with_router("/test", sub_router); - assert_eq!(route.url.to_string(), "/test"); + fn router_wildcard_route_is_lower_priority_than_static_route() { + let router = Router::with_urls(vec![ + Route::with_handler_and_name("/static/{*path}", MockHandler, "wildcard"), + Route::with_handler_and_name("/static/index.html", MockHandler, "static"), + ]); + + let found = router.get_handler("/static/index.html").unwrap(); + + assert_eq!(found.name, Some(RouteName("static".to_string()))); + } + + #[test] + fn router_root_mount_matches_root_path() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "index", + )]); + let router = Router::with_urls(vec![Route::with_router("", sub_router)]); + + let found = router.get_handler("/").unwrap(); + + assert_eq!(found.name, Some(RouteName("index".to_string()))); + } + + #[test] + fn router_exact_mount_match_routes_to_nested_root_not_empty_path() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "sub_index", + )]); + let router = Router::with_urls(vec![Route::with_router("/api", sub_router)]); + + let found = router.get_handler("/api").unwrap(); + + assert_eq!(found.name, Some(RouteName("sub_index".to_string()))); + } + + #[test] + fn router_root_mounted_nested_router() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/inner", + MockHandler, + "inner", + )]); + let router = Router::with_urls(vec![Route::with_router("/", sub_router)]); + + let found = router.get_handler("/inner").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + } + + #[test] + fn router_root_mounted_nested_router_empty() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "inner")]); + let router = Router::with_urls(vec![Route::with_router("/outer", sub_router)]); + + let found = router.get_handler("/outer").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + assert!(router.get_handler("/outer/").is_none()); + assert!(router.get_handler("outer/").is_none()); + assert!(router.get_handler("outer").is_none()); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/outer"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_root_mounted_nested_router_empty_and_root_without_slash() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "inner")]); + // this should normalize to `/outer` + let router = Router::with_urls(vec![Route::with_router("outer", sub_router)]); + + let found = router.get_handler("/outer").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + assert!(router.get_handler("/outer/").is_none()); + assert!(router.get_handler("outer/").is_none()); + assert!(router.get_handler("outer").is_none()); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/outer"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_root_mounted_nested_router_empty_root() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "inner", + MockHandler, + "inner", + )]); + let router = Router::with_urls(vec![Route::with_router("", sub_router)]); + + let found = router.get_handler("/inner").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + // remaining path becomes "/inner/", sub-router only registered "/inner" + assert!(router.get_handler("/inner/").is_none()); + assert!(router.get_handler("inner").is_none()); + assert!(router.get_handler("inner/").is_none()); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/inner"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_root_mounted_nested_router_empty_root_empty_nested() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "inner")]); + let router = Router::with_urls(vec![Route::with_router("", sub_router)]); + + // exact match at the mount point, remaining defaults to root "/" + let found = router.get_handler("/").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + // wildcard sentinel capturing a literal "/", so this is legal + let found = router.get_handler("//").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_root_mounted_nested_router_slash_root_slash_nested() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "inner", + )]); + let router = Router::with_urls(vec![Route::with_router("/", sub_router)]); + + let found = router.get_handler("/").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let found = router.get_handler("//").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_root_mounted_nested_router_slash_root_empty_nested() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "inner")]); + let router = Router::with_urls(vec![Route::with_router("/", sub_router)]); + + let found = router.get_handler("/").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let found = router.get_handler("//").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_root_mounted_nested_router_empty_root_slash_nested() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "inner", + )]); + let router = Router::with_urls(vec![Route::with_router("", sub_router)]); + + let found = router.get_handler("/").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let found = router.get_handler("//").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_nested_router_trailing_slash_prefix() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/inner", + MockHandler, + "inner", + )]); + let router = Router::with_urls(vec![Route::with_router("/api/", sub_router)]); + + let found = router.get_handler("/api/inner").unwrap(); + assert_eq!(found.name, Some(RouteName("inner".to_string()))); + } + + #[test] + fn router_nested_router_consumes_remaining_path() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/posts/{post_id}", + MockHandler, + "post_detail", + )]); + let router = Router::with_urls(vec![Route::with_router("/users/{id}", sub_router)]); + + let found = router.get_handler("/users/123/posts/456").unwrap(); + + assert_eq!(found.name, Some(RouteName("post_detail".to_string()))); + assert_params(found.params, &[("id", "123"), ("post_id", "456")]); + } + + #[test] + fn router_param_mount_param_nested_captures_both_in_order() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{sub_id}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/{id}", sub_router)]); + + let found = router.get_handler("/123/456").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_eq!( + found.params, + vec![ + ("id".to_string(), "123".to_string()), + ("sub_id".to_string(), "456".to_string()), + ] + ); + } + + #[test] + fn router_param_mount_wildcard_nested_exact_match_fails_deep_match_works() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/{id}", sub_router)]); + + assert!(router.get_handler("/123").is_none()); + + let found = router.get_handler("/123/a/b").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_params(found.params, &[("id", "123"), ("rest", "a/b")]); + } + + #[test] + fn router_param_mount_trailing_slash_empty_nested_captures_param() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "leaf")]); + let router = Router::with_urls(vec![Route::with_router("/{id}/", sub_router)]); + + let found = router.get_handler("/123/").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_eq!(found.params, vec![("id".to_string(), "123".to_string())]); + } + + #[test] + fn router_param_mount_trailing_slash_bare_path_fails() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "leaf")]); + let router = Router::with_urls(vec![Route::with_router("/{id}/", sub_router)]); + assert!(router.get_handler("/123").is_none()); + } + + #[test] + fn router_duplicate_param_name_across_nesting_levels_allowed() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{id}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/{id}", sub_router)]); + + let found = router.get_handler("/1/2").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_eq!( + found.params, + vec![ + ("id".to_string(), "1".to_string()), + ("id".to_string(), "2".to_string()), + ] + ); + } + + #[test] + fn router_bare_mount_match_fails_when_nested_is_wildcard_only() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/api", sub_router)]); + + assert!(router.get_handler("/api").is_none()); + + let found = router.get_handler("/api/x/y").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_params(found.params, &[("rest", "x/y")]); + } + + #[test] + fn router_multi_segment_mount_with_wildcard_nested() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/a/b", sub_router)]); + + assert!(router.get_handler("/a/b").is_none()); + let found = router.get_handler("/a/b/c/d").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_params(found.params, &[("rest", "c/d")]); + } + + #[test] + fn router_slash_mount_wildcard_nested_bare_slash_fails() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*path}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/files/", sub_router)]); + + assert!(router.get_handler("/files/").is_none()); + assert!(router.get_handler("/files").is_none()); + + let found = router.get_handler("/files/x").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_params(found.params, &[("path", "x")]); + } + + #[test] + fn router_multi_segment_slash_mount_param_nested() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{id}", + MockHandler, + "leaf", + )]); + let router = Router::with_urls(vec![Route::with_router("/a/b/", sub_router)]); + + assert!(router.get_handler("/a/b").is_none()); + assert!(router.get_handler("/a/b/").is_none()); + + let found = router.get_handler("/a/b/42").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_params(found.params, &[("id", "42")]); + } + + #[test] + fn router_handler_takes_priority_over_nested_router_at_same_path() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "nested", + )]); + let router = Router::with_urls(vec![ + Route::with_router("/users", sub_router), + Route::with_handler_and_name("/users", MockHandler, "handler"), + ]); + + let found = router.get_handler("/users").unwrap(); + + assert_eq!(found.name, Some(RouteName("handler".to_string()))); + } + + #[test] + fn router_static_nested_mount_priority_over_sibling_wildcard_mount() { + let generic_sub = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "generic", + )]); + let specific_sub = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "specific", + )]); + let router = Router::with_urls(vec![ + Route::with_router("/admin", generic_sub), + Route::with_router("/admin/extra", specific_sub), + ]); + + let found = router.get_handler("/admin/extra/more").unwrap(); + assert_eq!(found.name, Some(RouteName("specific".to_string()))); + assert_params(found.params, &[("rest", "more")]); + + let found = router.get_handler("/admin/other/thing").unwrap(); + assert_eq!(found.name, Some(RouteName("generic".to_string()))); + assert_params(found.params, &[("rest", "other/thing")]); + } + + #[test] + fn router_handler_priority_swallows_routers_own_trailing_slash_exact_entry() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "sub_catch_all", + )]); + let router = Router::with_urls(vec![ + Route::with_handler_and_name("/users", MockHandler, "handler"), + Route::with_router("/users/", sub_router), + ]); + + let found = router.get_handler("/users").unwrap(); + assert_eq!(found.name, Some(RouteName("handler".to_string()))); + assert!(router.get_handler("/users/").is_none()); + + let found = router.get_handler("/users/anything").unwrap(); + assert_eq!(found.name, Some(RouteName("sub_catch_all".to_string()))); + assert_params(found.params, &[("rest", "anything")]); + } + + #[test] + fn router_triple_nested_all_empty_mounts_reachable_via_single_slash() { + let leaf = Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "leaf")]); + let mid = Router::with_urls(vec![Route::with_router("", leaf)]); + let router = Router::with_urls(vec![Route::with_router("", mid)]); + + let found = router.get_handler("/").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + } + + #[test] + fn router_triple_nested_param_then_empty_then_param() { + let leaf = Router::with_urls(vec![Route::with_handler_and_name( + "/{b}", + MockHandler, + "leaf", + )]); + let mid = Router::with_urls(vec![Route::with_router("", leaf)]); + let router = Router::with_urls(vec![Route::with_router("/{a}", mid)]); + + let found = router.get_handler("/1/2").unwrap(); + assert_eq!(found.name, Some(RouteName("leaf".to_string()))); + assert_eq!( + found.params, + vec![ + ("a".to_string(), "1".to_string()), + ("b".to_string(), "2".to_string()), + ] + ); + } + + #[test] + #[should_panic( + expected = "route conflict error: duplicate route: `/users` conflicts with an already registered handler route `/users` (both fully match the same path)" + )] + fn router_duplicate_handler_routes_panic() { + let _ = Router::with_urls(vec![ + Route::with_handler("/users", MockHandler), + Route::with_handler("/users", MockHandler), + ]); + } + + #[test] + #[should_panic( + expected = "route conflict error: duplicate route: `/users/` conflicts with an already registered handler route `/users/` (both fully match the same path)" + )] + fn router_duplicate_handler_routes_with_trailing_slash_panic() { + let _ = Router::with_urls(vec![ + Route::with_handler("/users/", MockHandler), + Route::with_handler("/users/", MockHandler), + ]); + } + + #[test] + #[should_panic( + expected = "route conflict error: duplicate route name: `home` is registered at both `/a` and `/b`; route names must be unique within the same app" + )] + fn router_duplicate_route_name_same_app_panics() { + let _ = Router::with_urls(vec![ + Route::with_handler_and_name("/a", MockHandler, "home"), + Route::with_handler_and_name("/b", MockHandler, "home"), + ]); + } + + #[test] + fn router_duplicate_nested_router_routes_merge() { + let router1 = Router::with_urls(vec![Route::with_handler_and_name("/a", MockHandler, "a")]); + let router2 = Router::with_urls(vec![Route::with_handler_and_name("/b", MockHandler, "b")]); + + let router = Router::with_urls(vec![ + Route::with_router("/users", router1), + Route::with_router("/users", router2), + ]); + + assert_eq!( + router.get_handler("/users/a").unwrap().name, + Some(RouteName("a".to_string())) + ); + assert_eq!( + router.get_handler("/users/b").unwrap().name, + Some(RouteName("b".to_string())) + ); + } + + #[test] + fn router_duplicate_nested_router_routes_merge_with_slash() { + let router1 = Router::with_urls(vec![Route::with_handler_and_name("/a", MockHandler, "a")]); + let router2 = Router::with_urls(vec![Route::with_handler_and_name("/b", MockHandler, "b")]); + + let router = Router::with_urls(vec![ + Route::with_router("/users/", router1), + Route::with_router("/users/", router2), + ]); + + assert_eq!( + router.get_handler("/users/a").unwrap().name, + Some(RouteName("a".to_string())) + ); + assert_eq!( + router.get_handler("/users/b").unwrap().name, + Some(RouteName("b".to_string())) + ); + } + + #[test] + fn router_duplicate_nested_router_routes_merge_slash_diff() { + let router1 = Router::with_urls(vec![Route::with_handler_and_name("/a", MockHandler, "a")]); + let router2 = Router::with_urls(vec![Route::with_handler_and_name("/b", MockHandler, "b")]); + + let router = Router::with_urls(vec![ + Route::with_router("/users/", router1), + Route::with_router("/users", router2), + ]); + + assert_eq!( + router.get_handler("/users/a").unwrap().name, + Some(RouteName("a".to_string())) + ); + assert_eq!( + router.get_handler("/users/b").unwrap().name, + Some(RouteName("b".to_string())) + ); + } + + #[test] + #[should_panic(expected = "route conflict error: duplicate route")] + fn router_merged_routers_reject_leaf_conflicts() { + let router1 = Router::with_urls(vec![Route::with_handler("/health", MockHandler)]); + let router2 = Router::with_urls(vec![Route::with_handler("/health", MockHandler)]); + + let _ = Router::with_urls(vec![ + Route::with_router("/api", router1), + Route::with_router("/api", router2), + ]); + } + + #[test] + #[should_panic(expected = "route conflict error: duplicate route name")] + fn router_merged_routers_both_unscoped_same_name_panics() { + let router1 = Router::with_urls(vec![Route::with_handler_and_name( + "/a", + MockHandler, + "home", + )]); + let router2 = Router::with_urls(vec![Route::with_handler_and_name( + "/b", + MockHandler, + "home", + )]); + + let _ = Router::with_urls(vec![ + Route::with_router("/shared", router1), + Route::with_router("/shared", router2), + ]); + } + + #[test] + fn router_merged_routers_reverse_scoped_by_app_name() { + let mut router1 = Router::with_urls(vec![Route::with_handler_and_name( + "/one", + MockHandler, + "index", + )]); + router1.set_app_name(AppName("app1".to_string())); + + let mut router2 = Router::with_urls(vec![Route::with_handler_and_name( + "/two", + MockHandler, + "index", + )]); + router2.set_app_name(AppName("app2".to_string())); + + let router = Router::with_urls(vec![ + Route::with_router("/shared", router1), + Route::with_router("/shared", router2), + ]); + + assert_eq!( + router + .reverse(Some("app1"), "index", &ReverseParamMap::new()) + .unwrap(), + "/shared/one" + ); + assert_eq!( + router + .reverse(Some("app2"), "index", &ReverseParamMap::new()) + .unwrap(), + "/shared/two" + ); + } + + #[test] + fn router_reverse_prefers_exact_app_scope_over_unscoped_candidate() { + // router with no name set + let unscoped_router = Router::with_urls(vec![Route::with_handler_and_name( + "/unscoped", + MockHandler, + "dup", + )]); + + // router with name set + let mut scoped_router = Router::with_urls(vec![Route::with_handler_and_name( + "/scoped", + MockHandler, + "dup", + )]); + scoped_router.set_app_name(AppName("app1".to_string())); + + let router = Router::with_urls(vec![ + Route::with_router("/a", unscoped_router), + Route::with_router("/a", scoped_router), + ]); + + let url = router + .reverse(Some("app1"), "dup", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/a/scoped"); + } + + #[test] + fn router_reverse_no_app_name_returns_none() { + let mut router1 = Router::with_urls(vec![Route::with_handler_and_name( + "/one", + MockHandler, + "home", + )]); + router1.set_app_name(AppName("app1".to_string())); + let mut router2 = Router::with_urls(vec![Route::with_handler_and_name( + "/two", + MockHandler, + "home", + )]); + router2.set_app_name(AppName("app2".to_string())); + + let router = Router::with_urls(vec![ + Route::with_router("/shared", router1), + Route::with_router("/shared", router2), + ]); + + let result = router + .reverse_option(None, "home", &ReverseParamMap::new()) + .unwrap(); + assert!(result.is_none()); + } + + #[test] + #[should_panic( + expected = "route conflict error: conflicting route parameters: `/foo/{bar}` uses `{bar}` but `/foo/{baz}` uses `{baz}` at the same position in the path; both routes must bind the same parameter name there, since only one value can be captured at that position" + )] + fn router_conflicting_param_names_panic() { + let _ = Router::with_urls(vec![ + Route::with_handler("/foo/{bar}", MockHandler), + Route::with_handler("/foo/{baz}", MockHandler), + ]); + } + + #[test] + fn router_same_path_with_trailing_lash_diff() { + // this should not fail + let _ = Router::with_urls(vec![ + Route::with_handler("/foo/{bar}/", MockHandler), + Route::with_handler("/foo/{baz}", MockHandler), + ]); + } + + #[test] + #[should_panic( + expected = "route conflict error: duplicate route: `/static/{*path}` conflicts with an already registered handler route `/static/{*path}` (both fully match the same path)" + )] + fn router_duplicate_wildcard_routes_panic() { + let _ = Router::with_urls(vec![ + Route::with_handler("/static/{*path}", MockHandler), + Route::with_handler("/static/{*path}", MockHandler), + ]); + } + + #[test] + #[should_panic( + expected = "route conflict error: conflicting wildcard parameters: `/static/{*path}` uses `{*path}` but `/static/{*file_path}` uses `{*file_path}` at the same position in the path" + )] + fn router_conflicting_wildcard_names_panic() { + let _ = Router::with_urls(vec![ + Route::with_handler("/static/{*path}", MockHandler), + Route::with_handler("/static/{*file_path}", MockHandler), + ]); + } + + #[test] + #[should_panic( + expected = "route conflict error: duplicate route: `/static/{*file_path}` conflicts with an already registered handler route `/static/{path}` (both fully match the same path)" + )] + fn router_wildcard_and_param_at_same_segment_conflict() { + let _ = Router::with_urls(vec![ + Route::with_handler("/static/{path}", MockHandler), + Route::with_handler("/static/{*file_path}", MockHandler), + ]); + } + + #[test] + #[should_panic(expected = "route conflict error")] + fn router_wildcard_mount_with_static_nested_errors() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/leaf", + MockHandler, + "leaf", + )]); + let _ = Router::with_urls(vec![Route::with_router("/{*rest}", sub_router)]); + } + + #[test] + #[should_panic(expected = "route conflict error")] + fn router_wildcard_mount_with_param_nested_errors() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{id}", + MockHandler, + "leaf", + )]); + let _ = Router::with_urls(vec![Route::with_router("/{*rest}", sub_router)]); + } + + #[test] + #[should_panic(expected = "route conflict error")] + fn router_wildcard_mount_with_empty_nested_errors() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "leaf")]); + let _ = Router::with_urls(vec![Route::with_router("/{*rest}", sub_router)]); + } + + #[test] + #[should_panic(expected = "route conflict error")] + fn router_wildcard_mount_with_wildcard_nested_errors() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{*rest}", + MockHandler, + "leaf", + )]); + let _ = Router::with_urls(vec![Route::with_router("/{*outer}", sub_router)]); + } + + #[test] + #[should_panic(expected = "route conflict error")] + fn router_prefixed_wildcard_mount_errors() { + let sub_router = + Router::with_urls(vec![Route::with_handler_and_name("", MockHandler, "leaf")]); + let _ = Router::with_urls(vec![Route::with_router("/files/{*path}", sub_router)]); + } + + #[test] + fn router_reverse() { + let route = Route::with_handler_and_name("/test", MockHandler, "test"); + let router = Router::with_urls(vec![route.clone()]); + let params = ReverseParamMap::new(); + let url = router.reverse(None, "test", ¶ms).unwrap(); + assert_eq!(url, "/test"); + } + + #[test] + fn router_reverse_with_param() { + let route = Route::with_handler_and_name("/test/{id}", MockHandler, "test"); + let router = Router::with_urls(vec![route.clone()]); + let mut params = ReverseParamMap::new(); + params.insert("id", "123"); + let url = router.reverse(None, "test", ¶ms).unwrap(); + assert_eq!(url, "/test/123"); + } + + #[test] + fn router_reverse_option() { + let route = Route::with_handler_and_name("/test", MockHandler, "test"); + let router = Router::with_urls(vec![route.clone()]); + let params = ReverseParamMap::new(); + let url = router + .reverse_option(None, "test", ¶ms) + .unwrap() + .unwrap(); + assert_eq!(url, "/test"); + } + + #[test] + fn router_reverse_option_wrong_app_name_returns_none() { + let route = Route::with_handler_and_name("/test", MockHandler, "test"); + let mut router = Router::with_urls(vec![route]); + router.set_app_name(AppName("app_1".to_string())); + + let result = router + .reverse_option(Some("app_2"), "test", &ReverseParamMap::new()) + .unwrap(); + + assert!(result.is_none()); + } + + #[test] + fn router_reverse_missing_view_returns_error() { + let router = Router::empty(); + + let result = router.reverse(None, "missing", &ReverseParamMap::new()); + assert!(result.is_err()); + } + + #[test] + fn router_reverse_of_nested_index_uses_bare_mount_path() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "index", + )]); + let router = Router::with_urls(vec![Route::with_router("/admin", sub_router)]); + + let url = router + .reverse(None, "index", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/admin"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_reverse_root_mount_no_double_slash() { + let route = Route::with_handler_and_name("/", MockHandler, "index"); + let sub_router = Router::with_urls(vec![route]); + let router = Router::with_urls(vec![Route::with_router("/", sub_router)]); + + let url = router + .reverse(None, "index", &ReverseParamMap::new()) + .unwrap(); + + assert_eq!(url, "/"); + } + + #[test] + fn router_reverse_nested_under_root_mount_no_double_slash() { + let route = Route::with_handler_and_name("/inner", MockHandler, "inner"); + let sub_router = Router::with_urls(vec![route]); + let router = Router::with_urls(vec![Route::with_router("/", sub_router)]); + + let url = router + .reverse(None, "inner", &ReverseParamMap::new()) + .unwrap(); + + assert_eq!(url, "/inner"); + } + + #[test] + fn router_reverse_deeply_nested_root_mounts_no_double_slash() { + let route = Route::with_handler_and_name("/leaf", MockHandler, "leaf"); + let inner_router = Router::with_urls(vec![route]); + let mid_router = Router::with_urls(vec![Route::with_router("/", inner_router)]); + let router = Router::with_urls(vec![Route::with_router("/", mid_router)]); + + let url = router + .reverse(None, "leaf", &ReverseParamMap::new()) + .unwrap(); + + assert_eq!(url, "/leaf"); + } + + #[test] + fn router_reverse_slash_mounted_root_route_keeps_slash() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/", + MockHandler, + "index", + )]); + let router = Router::with_urls(vec![Route::with_router("/admin/", sub_router)]); + + let url = router + .reverse(None, "index", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/admin/"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_reverse_slash_mounted_non_root_route_unaffected() { + let nested_sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/bar/{buz}", + MockHandler, + "biz", + )]); + let sub_router = Router::with_urls(vec![ + Route::with_handler_and_name("/foo", MockHandler, "foo"), + Route::with_router("/fab", nested_sub_router), + Route::with_handler_and_name("/bar/", MockHandler, "bar"), + ]); + let router = Router::with_urls(vec![Route::with_router("/admin/", sub_router)]); + + let url = router + .reverse(None, "foo", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/admin/foo"); + assert!(router.has_route(&url)); + + let mut params = ReverseParamMap::new(); + params.insert("buz", "random"); + let url = router.reverse(None, "biz", ¶ms).unwrap(); + assert_eq!(url, "/admin/fab/bar/random"); + + let url = router + .reverse(None, "bar", &ReverseParamMap::new()) + .unwrap(); + assert_eq!(url, "/admin/bar/"); + assert!(router.has_route(&url)); + } + + #[test] + fn router_reverse_app_name() { + let route = Route::with_handler_and_name("/test", MockHandler, "test"); + let mut router_1 = Router::with_urls(vec![route.clone()]); + router_1.set_app_name(AppName("app_1".to_string())); + let mut router_2 = Router::with_urls(vec![route.clone()]); + router_2.set_app_name(AppName("app_2".to_string())); + let root_router = Router::with_urls(vec![ + Route::with_router("/", router_1), + Route::with_router("/sub", router_2), + ]); + + let params = ReverseParamMap::new(); + let url = root_router.reverse(Some("app_2"), "test", ¶ms).unwrap(); + + assert_eq!(url, "/sub/test"); + } + + #[test] + fn router_reverse_app_name_nested() { + let route = Route::with_handler_and_name("/test", MockHandler, "test"); + let router = Router::with_urls(vec![route.clone()]); + let sub_router = Router::with_urls(vec![Route::with_router("/sub", router)]); + let mut root_router = Router::with_urls(vec![Route::with_router("/subsub", sub_router)]); + root_router.set_app_name(AppName("app_root".to_string())); + + let params = ReverseParamMap::new(); + let url = root_router + .reverse(Some("app_root"), "test", ¶ms) + .unwrap(); + + assert_eq!(url, "/subsub/sub/test"); + } + + #[test] + fn router_nested_mount_and_leaf_param_order_preserved() { + let sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/{model_name}/{pk}/edit/", + MockHandler, + "edit", + )]); + let router = Router::with_urls(vec![Route::with_router("/admin", sub_router)]); + + let found = router.get_handler("/admin/database_user/1/edit/").unwrap(); + + assert_eq!( + found.params, + vec![ + ("model_name".to_string(), "database_user".to_string()), + ("pk".to_string(), "1".to_string()), + ] + ); + } + + #[test] + fn router_very_nested_mount_and_leaf_param_order_preserved() { + let nested_sub_router = Router::with_urls(vec![Route::with_handler_and_name( + "/foo/{bar}/{*baz}", + MockHandler, + "edit", + )]); + + let sub_router = Router::with_urls(vec![Route::with_router( + "/{model_name}/{pk}/edit/", + nested_sub_router, + )]); + let router = Router::with_urls(vec![Route::with_router("/admin", sub_router)]); + + let found = router + .get_handler("/admin/database_user/1/edit/foo/jon/2/doe") + .unwrap(); + + assert_eq!( + found.params, + vec![ + ("model_name".to_string(), "database_user".to_string()), + ("pk".to_string(), "1".to_string()), + ("bar".to_string(), "jon".to_string()), + ("baz".to_string(), "2/doe".to_string()) + ] + ); } #[test] diff --git a/cot/src/router/path.rs b/cot/src/router/path.rs index 398791c1..c8092bea 100644 --- a/cot/src/router/path.rs +++ b/cot/src/router/path.rs @@ -5,11 +5,109 @@ //! form given a set of parameters. use std::collections::HashMap; -use std::fmt::Display; +use std::fmt::{Display, Write}; +use std::sync::Arc; +use cot::router::tree::MatchitPattern; use cot_core::error::impl_into_cot_error; use thiserror::Error; -use tracing::debug; + +const PATH_MATCHER_ERROR_PREFIX: &str = "invalid route pattern:"; +/// An error produced when parsing a route path pattern fails. +#[derive(Debug, Error)] +#[non_exhaustive] +pub(super) enum PathMatcherError { + /// Two parameters appear consecutively with no literal text between them, + #[error( + "{PATH_MATCHER_ERROR_PREFIX} consecutive parameters are not allowed in pattern `{pattern}`" + )] + #[non_exhaustive] + ConsecutiveParams { pattern: String }, + /// A `{` was opened but never closed with a matching `}`. + #[error( + "{PATH_MATCHER_ERROR_PREFIX} unclosed parameter `{{{name}` in pattern `{pattern}`; expected a closing `}}`" + )] + #[non_exhaustive] + UnclosedParam { pattern: String, name: String }, + /// A `}` appeared without a preceding `{` to open it. + #[error( + "{PATH_MATCHER_ERROR_PREFIX} closing brace `}}` without a matching opening `{{` in pattern `{pattern}`" + )] + #[non_exhaustive] + UnmatchedClosingBrace { pattern: String }, + /// A parameter name is empty or contains characters other than + /// alphanumerics/underscore, or starts with a digit. + #[error( + "{PATH_MATCHER_ERROR_PREFIX} invalid parameter name `{name}` in pattern `{pattern}`; parameter names must start \ + with a letter or underscore and contain only letters, digits, or underscores" + )] + #[non_exhaustive] + InvalidParamName { pattern: String, name: String }, + /// Same as [`PathMatcherError::InvalidParamName`], but for the name + /// following a `*` in a wildcard segment. + #[error( + "{PATH_MATCHER_ERROR_PREFIX} invalid wildcard name `{name}` in pattern `{pattern}`; wildcard names must start \ + with a letter or underscore and contain only letters, digits, or underscores" + )] + #[non_exhaustive] + InvalidWildcardName { pattern: String, name: String }, + /// A wildcard segment was followed by more path segments, + #[error( + "{PATH_MATCHER_ERROR_PREFIX} wildcard parameter `{{*{name}}}` must be the last segment of pattern `{pattern}`; \ + a wildcard consumes the rest of the path, so nothing can follow it" + )] + #[non_exhaustive] + WildcardNotAtEnd { pattern: String, name: String }, + #[error("{PATH_MATCHER_ERROR_PREFIX} unsupported brace in {pattern}")] + UnsupportedLiteralBrace { pattern: String }, +} +impl_into_cot_error!(PathMatcherError); + +/// An absolute route path. +/// +/// The path is normalized to always begin with `/` and allows paths to be +/// joined without introducing duplicate `/` separators. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct AbsolutePath(String); + +impl AbsolutePath { + #[must_use] + pub(crate) fn new>(s: S) -> Self { + let mut s = s.into(); + if !s.starts_with('/') { + s.insert(0, '/'); + } + Self(s) + } + + #[must_use] + pub(crate) fn root() -> Self { + Self(String::from("/")) + } + + #[must_use] + pub(crate) fn as_str(&self) -> &str { + self.0.as_str() + } + + #[must_use] + pub(crate) fn join(&self, suffix: &AbsolutePath) -> AbsolutePath { + let trimmed = self.0.strip_suffix('/').unwrap_or(&self.0); + AbsolutePath(format!("{trimmed}{}", suffix.0)) + } +} + +impl Display for AbsolutePath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl From for String { + fn from(value: AbsolutePath) -> Self { + value.0 + } +} #[derive(Debug, Clone)] pub(super) struct PathMatcher { @@ -19,6 +117,10 @@ pub(super) struct PathMatcher { impl PathMatcher { #[must_use] pub(crate) fn new>(path_pattern: T) -> Self { + Self::try_new(path_pattern).unwrap_or_else(|err| panic!("{err}")) + } + + pub(crate) fn try_new>(path_pattern: T) -> Result { #[derive(Debug, Copy, Clone)] enum State { Literal { start: usize }, @@ -26,7 +128,7 @@ impl PathMatcher { } let mut path_pattern = path_pattern.into(); - if !path_pattern.is_empty() && !path_pattern.starts_with('/') { + if !path_pattern.starts_with('/') { path_pattern.insert(0, '/'); } @@ -43,10 +145,11 @@ impl PathMatcher { (Some('{') | None, State::Literal { start }) => { let literal = &path_pattern[start..index]; if literal.is_empty() { - assert!( - index == 0 || ch.is_none(), - "Consecutive parameters are not allowed" - ); + if index != 0 && ch.is_some() { + return Err(PathMatcherError::ConsecutiveParams { + pattern: path_pattern.clone(), + }); + } } else { parts.push(PathPart::Literal(literal.to_string())); } @@ -57,7 +160,10 @@ impl PathMatcher { // escaped `{` state = State::Literal { start: index }; } else { - panic!("Unclosed parameter: `{}`", &path_pattern[start..index]); + return Err(PathMatcherError::UnclosedParam { + pattern: path_pattern.clone(), + name: path_pattern[start..index].to_string(), + }); } } (Some('}'), State::Literal { start }) => { @@ -71,32 +177,40 @@ impl PathMatcher { char_iter.next(); state = State::Literal { start: index + 2 }; } else { - panic!("Closing brace encountered without opening brace"); + return Err(PathMatcherError::UnmatchedClosingBrace { + pattern: path_pattern.clone(), + }); } } (Some('}'), State::Param { start }) => { let param_name = &path_pattern[start..index].trim(); if let Some(wildcard_name) = param_name.strip_prefix('*') { - assert!( - Self::is_param_name_valid(wildcard_name), - "Invalid wildcard parameter name: `{wildcard_name}`" - ); + if !Self::is_param_name_valid(wildcard_name) { + return Err(PathMatcherError::InvalidWildcardName { + pattern: path_pattern.clone(), + name: wildcard_name.to_string(), + }); + } let next_char = char_iter.peek().map(|(_, ch)| *ch).unwrap_or_default(); - assert!( - next_char.is_none(), - "Wildcard must be the last part of the path: `{path_pattern}`" - ); + if next_char.is_some() { + return Err(PathMatcherError::WildcardNotAtEnd { + pattern: path_pattern.clone(), + name: wildcard_name.to_string(), + }); + } parts.push(PathPart::Wildcard { name: wildcard_name.to_string(), }); } else { - assert!( - Self::is_param_name_valid(param_name), - "Invalid parameter name: `{param_name}`" - ); + if !Self::is_param_name_valid(param_name) { + return Err(PathMatcherError::InvalidParamName { + pattern: path_pattern.clone(), + name: param_name.to_string(), + }); + } parts.push(PathPart::Param { name: param_name.to_string(), @@ -105,13 +219,16 @@ impl PathMatcher { state = State::Literal { start: index + 1 }; } (Some('/') | None, State::Param { start }) => { - panic!("Unclosed parameter: `{}`", &path_pattern[start..index]); + return Err(PathMatcherError::UnclosedParam { + pattern: path_pattern.clone(), + name: path_pattern[start..index].to_string(), + }); } _ => {} } } - Self { parts } + Ok(Self { parts }) } fn is_param_name_valid(name: &str) -> bool { @@ -130,49 +247,6 @@ impl PathMatcher { true } - #[must_use] - pub(crate) fn capture<'matcher, 'path>( - &'matcher self, - path: &'path str, - ) -> Option> { - debug!("Matching path `{}` against pattern `{}`", path, self); - - let mut current_path = path; - let mut params = Vec::with_capacity(self.param_len()); - for part in &self.parts { - match part { - PathPart::Literal(s) => { - if !current_path.starts_with(s) { - return None; - } - current_path = ¤t_path[s.len()..]; - } - PathPart::Wildcard { name } => { - if current_path.is_empty() { - return None; - } - params.push(PathParam::new(name, current_path)); - current_path = ""; - } - PathPart::Param { name } => { - let next_slash = current_path.find('/'); - let value = if let Some(next_slash) = next_slash { - ¤t_path[..next_slash] - } else { - current_path - }; - if value.is_empty() { - return None; - } - params.push(PathParam::new(name, value)); - current_path = ¤t_path[value.len()..]; - } - } - } - - Some(CaptureResult::new(params, current_path)) - } - pub(crate) fn reverse(&self, params: &ReverseParamMap) -> Result { let mut result = String::new(); @@ -191,17 +265,43 @@ impl PathMatcher { Ok(result) } - #[must_use] - fn param_len(&self) -> usize { - self.param_names().count() - } - + #[cfg(feature = "openapi")] pub(super) fn param_names(&self) -> impl Iterator { self.parts.iter().filter_map(|part| match part { PathPart::Literal(..) => None, PathPart::Param { name } | PathPart::Wildcard { name } => Some(name.as_str()), }) } + + pub(super) fn parts(&self) -> &[PathPart] { + &self.parts + } +} + +impl TryFrom> for MatchitPattern { + type Error = PathMatcherError; + + fn try_from(value: Arc) -> Result { + let mut pattern = String::new(); + for part in &value.parts { + match part { + PathPart::Literal(s) if s.contains(['{', '}']) => { + return Err(PathMatcherError::UnsupportedLiteralBrace { + pattern: value.to_string(), + }); + } + PathPart::Literal(s) => pattern.push_str(s), + PathPart::Param { name } => { + let _ = write!(pattern, "{{{name}}}"); + } + PathPart::Wildcard { name } => { + let _ = write!(pattern, "{{*{name}}}"); + } + } + } + + Ok(MatchitPattern::new(pattern)) + } } impl Display for PathMatcher { @@ -290,41 +390,20 @@ macro_rules! reverse_param_map { }}; } -const ERROR_PREFIX: &str = "failed to reverse route:"; +const REVERSE_ERROR_PREFIX: &str = "failed to reverse route:"; /// An error that occurs when reversing a path with missing parameters. #[derive(Debug, Error)] #[non_exhaustive] pub enum ReverseError { /// A parameter is missing for the reverse operation. - #[error("{ERROR_PREFIX} missing parameter for reverse: `{0}`")] + #[error("{REVERSE_ERROR_PREFIX} missing parameter for reverse: `{0}`")] #[non_exhaustive] MissingParam(String), } impl_into_cot_error!(ReverseError); -#[derive(Debug, PartialEq, Eq)] -pub(super) struct CaptureResult<'matcher, 'path> { - pub(super) params: Vec>, - pub(super) remaining_path: &'path str, -} - -impl<'matcher, 'path> CaptureResult<'matcher, 'path> { - #[must_use] - fn new(params: Vec>, remaining_path: &'path str) -> Self { - Self { - params, - remaining_path, - } - } - - #[must_use] - pub(crate) fn matches_fully(&self) -> bool { - self.remaining_path.is_empty() - } -} - #[derive(Debug, Clone)] -enum PathPart { +pub(super) enum PathPart { Literal(String), Param { name: String }, Wildcard { name: String }, @@ -343,22 +422,6 @@ impl Display for PathPart { } } -#[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct PathParam<'a> { - pub(super) name: &'a str, - pub(super) value: String, -} - -impl<'a> PathParam<'a> { - #[must_use] - pub(crate) fn new(name: &'a str, value: &str) -> Self { - Self { - name, - value: value.to_string(), - } - } -} - #[cfg(test)] mod tests { use super::*; @@ -372,11 +435,11 @@ mod tests { #[test] fn path_parser_no_params() { let path_parser = PathMatcher::new("/users"); + assert_eq!(path_parser.to_string(), "/users"); assert_eq!( - path_parser.capture("/users"), - Some(CaptureResult::new(vec![], "")) + path_parser.param_names().collect::>(), + Vec::<&str>::new() ); - assert_eq!(path_parser.capture("/test"), None); } #[test] @@ -385,129 +448,113 @@ mod tests { let mut params = ReverseParamMap::new(); params.insert("id", "123"); - assert_eq!( - path_parser.capture("/users/123"), - Some(CaptureResult::new(vec![PathParam::new("id", "123")], "")) - ); assert_eq!(path_parser.reverse(¶ms).unwrap(), "/users/123"); assert_eq!(path_parser.to_string(), "/users/{id}"); + assert_eq!(path_parser.param_names().collect::>(), vec!["id"]); } #[test] fn path_parser_escaped() { let path_parser = PathMatcher::new("/users/{{{{{{escaped}}}}}}"); + assert_eq!(path_parser.to_string(), "/users/{{{{{{escaped}}}}}}"); assert_eq!( - path_parser.capture("/users/{{{escaped}}}"), - Some(CaptureResult::new(vec![], "")) + path_parser.reverse(&ReverseParamMap::new()).unwrap(), + "/users/{{{escaped}}}" ); } #[test] fn path_parser_single_param() { let path_parser = PathMatcher::new("/users/{id}"); - assert_eq!( - path_parser.capture("/users/123"), - Some(CaptureResult::new(vec![PathParam::new("id", "123")], "")) - ); - assert_eq!( - path_parser.capture("/users/123/"), - Some(CaptureResult::new(vec![PathParam::new("id", "123")], "/")) - ); - assert_eq!( - path_parser.capture("/users/123/abc"), - Some(CaptureResult::new( - vec![PathParam::new("id", "123")], - "/abc" - )) - ); - assert_eq!(path_parser.capture("/users/"), None); + assert_eq!(path_parser.to_string(), "/users/{id}"); + assert_eq!(path_parser.param_names().collect::>(), vec!["id"]); } #[test] fn path_parser_param_whitespace() { let path_parser = PathMatcher::new("/users/{ id }"); - assert_eq!( - path_parser.capture("/users/123"), - Some(CaptureResult::new(vec![PathParam::new("id", "123")], "")) - ); + assert_eq!(path_parser.to_string(), "/users/{id}"); + assert_eq!(path_parser.param_names().collect::>(), vec!["id"]); } #[test] fn path_parser_multiple_params() { let path_parser = PathMatcher::new("/users/{id}/posts/{post_id}"); assert_eq!( - path_parser.capture("/users/123/posts/456"), - Some(CaptureResult::new( - vec![ - PathParam::new("id", "123"), - PathParam::new("post_id", "456"), - ], - "" - )) - ); - assert_eq!( - path_parser.capture("/users/123/posts/456/abc"), - Some(CaptureResult::new( - vec![ - PathParam::new("id", "123"), - PathParam::new("post_id", "456"), - ], - "/abc" - )) + path_parser.param_names().collect::>(), + vec!["id", "post_id"] ); } #[test] - #[should_panic(expected = "Consecutive parameters are not allowed")] + #[should_panic( + expected = "invalid route pattern: consecutive parameters are not allowed in pattern `/users/{id}{post_id}`" + )] fn path_parser_consecutive_params() { let _ = PathMatcher::new("/users/{id}{post_id}"); } #[test] - #[should_panic(expected = "Invalid parameter name: ``")] + #[should_panic( + expected = "invalid route pattern: invalid parameter name `` in pattern `/users/{}`; parameter names must start with a letter or underscore and contain only letters, digits, or underscores" + )] fn path_parser_invalid_name_empty() { let _ = PathMatcher::new("/users/{}"); } #[test] - #[should_panic(expected = "Invalid parameter name: `123`")] + #[should_panic( + expected = "invalid route pattern: invalid parameter name `123` in pattern `/users/{123}`; parameter names must start with a letter or underscore and contain only letters, digits, or underscores" + )] fn path_parser_invalid_name_numeric() { let _ = PathMatcher::new("/users/{123}"); } #[test] - #[should_panic(expected = "Invalid parameter name: `abc#$%`")] + #[should_panic( + expected = "invalid route pattern: invalid parameter name `abc#$%` in pattern `/users/{abc#$%}`; parameter names must start with a letter or underscore and contain only letters, digits, or underscores" + )] fn path_parser_invalid_name_non_alphanumeric() { let _ = PathMatcher::new("/users/{abc#$%}"); } #[test] - #[should_panic(expected = "Unclosed parameter: `foo`")] + #[should_panic( + expected = "invalid route pattern: unclosed parameter `{foo` in pattern `/users/{foo`; expected a closing `}`" + )] fn path_parser_unclosed() { let _ = PathMatcher::new("/users/{foo"); } #[test] - #[should_panic(expected = "Closing brace encountered without opening brace")] + #[should_panic( + expected = "invalid route pattern: closing brace `}` without a matching opening `{` in pattern `/users/foo}`" + )] fn path_parser_missing_opening_brace() { let _ = PathMatcher::new("/users/foo}"); } #[test] - #[should_panic(expected = "Unclosed parameter: `foo`")] + #[should_panic( + expected = "invalid route pattern: unclosed parameter `{foo` in pattern `/users/{foo/bar`; expected a closing `}`" + )] fn path_parser_unclosed_slash() { let _ = PathMatcher::new("/users/{foo/bar"); } #[test] - #[should_panic(expected = "Unclosed parameter: `foo`")] + #[should_panic( + expected = "invalid route pattern: unclosed parameter `{foo` in pattern `/users/{foo{bar`; expected a closing `}`" + )] fn path_parser_unclosed_double() { let _ = PathMatcher::new("/users/{foo{bar"); } #[test] - #[should_panic(expected = "Closing brace encountered without opening brace")] + #[should_panic( + expected = "invalid route pattern: closing brace `}` without a matching opening `{` in pattern `/users/{{{foo}}/bar`" + )] fn path_parser_escaping_unclosed() { let _ = PathMatcher::new("/users/{{{foo}}/bar"); } @@ -572,53 +619,75 @@ mod tests { } #[test] - fn path_parser_wildcard_root() { - let path_parser = PathMatcher::new("/{*path}"); - assert_eq!( - path_parser.capture("/foo/bar"), - Some(CaptureResult::new( - vec![PathParam::new("path", "foo/bar")], - "" - )) - ); + fn path_parser_wildcard() { + let path_parser = PathMatcher::new("/static/{*path}"); + assert_eq!(path_parser.to_string(), "/static/{*path}"); + assert_eq!(path_parser.param_names().collect::>(), vec!["path"]); } #[test] - fn path_parser_wildcard_single_segment() { - let path_parser = PathMatcher::new("/users/rand/{*path}"); - assert_eq!( - path_parser.capture("/users/rand/foo"), - Some(CaptureResult::new(vec![PathParam::new("path", "foo")], "")) - ); + fn reverse_with_wildcard() { + let path_parser = PathMatcher::new("/static/{*path}"); + let mut params = ReverseParamMap::new(); + params.insert("path", "css/app.css"); + + assert_eq!(path_parser.reverse(¶ms).unwrap(), "/static/css/app.css"); } #[test] - fn path_parser_wildcard_multi_segment() { - let path_parser = PathMatcher::new("/users/rand/{*path}"); + #[should_panic( + expected = "invalid route pattern: wildcard parameter `{*rest}` must be the last segment of pattern `/users/{*rest}/edit`; a wildcard consumes the rest of the path, so nothing can follow it" + )] + fn path_parser_no_path_allowed_after_wildcard() { + let _ = PathMatcher::new("/users/{*rest}/edit"); + } + + #[test] + #[should_panic( + expected = "invalid route pattern: wildcard parameter `{*rest}` must be the last segment of pattern `/users/{*rest}/`; a wildcard consumes the rest of the path, so nothing can follow it" + )] + fn path_parser_trail_slash_not_allowed_after_wildcard() { + let _ = PathMatcher::new("/users/{*rest}/"); + } + + #[test] + #[should_panic( + expected = "invalid route pattern: invalid wildcard name `` in pattern `/users/{*}`; wildcard names must start with a letter or underscore and contain only letters, digits, or underscores" + )] + fn path_parser_invalid_wildcard_name_empty() { + let _ = PathMatcher::new("/users/{*}"); + } + + #[test] + fn absolute_path_join_root_with_root() { assert_eq!( - path_parser.capture("/users/rand/foo/bar"), - Some(CaptureResult::new( - vec![PathParam::new("path", "foo/bar")], - "" - )) + AbsolutePath::root().join(&AbsolutePath::root()).as_str(), + "/" ); } #[test] - fn path_parser_wildcard_no_match() { - let path_parser = PathMatcher::new("/prefix/{*path}"); - assert_eq!(path_parser.capture("/other/foo"), None); + fn absolute_path_join_root_is_identity() { + let x = AbsolutePath::new("/foo/bar"); + assert_eq!(AbsolutePath::root().join(&x), x); } #[test] - fn path_parser_wildcard_empty_not_allowed() { - let path_parser = PathMatcher::new("/users/rand/{*path}"); - assert_eq!(path_parser.capture("/users/rand/"), None); + fn absolute_path_join_trims_doubled_slash() { + let prefix = AbsolutePath::new("/api/"); + let suffix = AbsolutePath::new("/inner"); + assert_eq!(prefix.join(&suffix).as_str(), "/api/inner"); } #[test] - #[should_panic(expected = "Wildcard must be the last part of the path: `/users/{*rest}/`")] - fn path_parser_no_path_allowed_after_wildcard() { - let _ = PathMatcher::new("/users/{*rest}/"); + fn absolute_path_join_no_trailing_slash_on_prefix() { + let prefix = AbsolutePath::new("/api"); + let suffix = AbsolutePath::new("/inner"); + assert_eq!(prefix.join(&suffix).as_str(), "/api/inner"); + } + + #[test] + fn absolute_path_new_normalizes_missing_leading_slash() { + assert_eq!(AbsolutePath::new("foo").as_str(), "/foo"); } } diff --git a/cot/src/router/tree.rs b/cot/src/router/tree.rs new file mode 100644 index 00000000..ebf99d8a --- /dev/null +++ b/cot/src/router/tree.rs @@ -0,0 +1,421 @@ +use std::collections::HashMap; + +use cot::router::{Route, RouteKind}; +use matchit::{Match, Router as MatchitRouter}; + +use crate::router::RouteConflictError; +use crate::router::path::{AbsolutePath, PathPart}; + +pub(super) const NESTED_ROUTER_PARAM: &str = "__cot_nested_router__"; + +#[derive(Debug, Clone, Hash, Eq, PartialEq)] +pub(super) struct MatchitPattern(String); + +impl MatchitPattern { + #[must_use] + pub(super) fn new>(pattern: T) -> Self { + Self(pattern.into()) + } + + #[must_use] + pub(super) fn as_str(&self) -> &str { + self.0.as_str() + } +} + +impl From for String { + fn from(value: MatchitPattern) -> Self { + value.0 + } +} + +#[derive(Debug, Clone, Copy)] +pub(super) enum Entry { + Handler(usize), + Router(usize), +} + +#[derive(Debug, Clone)] +pub(super) struct RouteTrie { + inner: MatchitRouter, +} + +impl RouteTrie { + pub(super) fn build(routes: &[Route]) -> super::Result { + let mut inner = MatchitRouter::new(); + + let mut pattern_map: HashMap, Option)> = + HashMap::new(); + for (i, route) in routes.iter().enumerate() { + let pattern = if route.kind() == RouteKind::Router { + // normalize path of sub-routers since we will attach an + // internal wildcard sentinel. This should also + // allow us reject routes for + // routers(sub-routers) who's version without a trailing slash + // already exist. (eg. `foo` and `foo/`cannot overlap as + // sub-routers) + router_mount_pattern(route) + } else { + MatchitPattern::try_from(route.url.clone())? + }; + let entry = pattern_map.entry(pattern).or_default(); + match route.kind() { + RouteKind::Handler => { + if let Some(existing) = entry.0 { + return Err(RouteConflictError::DuplicateHandler { + existing: routes[existing].url(), + new: route.url(), + } + .into()); + } + entry.0 = Some(i); + } + RouteKind::Router => { + if let Some(existing) = entry.1 { + return Err(RouteConflictError::DuplicateRouter { + existing: routes[existing].url(), + new: route.url(), + } + .into()); + } + entry.1 = Some(i); + } + } + } + + let mut entries: Vec<_> = pattern_map.into_iter().collect(); + // sort for deterministic insertion behavior + entries.sort_by_key(|(_, (handler_idx, router_idx))| { + handler_idx + .or(*router_idx) + .expect("route index should exist") + }); + + for (_, (handler_idx, router_idx)) in entries { + let value = match (handler_idx, router_idx) { + (Some(h), None) => Entry::Handler(h), + (None, Some(r)) => Entry::Router(r), + // for cases where a handler overlaps a router for the same route/path, the handler + // takes precedence. + (Some(h), Some(_r)) => Entry::Handler(h), + (None, None) => unreachable!("there should always be a route or handler or both"), + }; + + let route_idx = handler_idx + .or(router_idx) + .expect("route index should exist"); + + // we insert the original path, not the (possibly trimmed) deduped + // route so that routers(sub-routers) that were + // mounted/declared with trailing slashes still match. + let insertion_pattern = MatchitPattern::try_from(routes[route_idx].url.clone())?; + Self::insert_or_diagnose( + &mut inner, + insertion_pattern, + value, + &routes[route_idx], + routes, + )?; + + // when a nested router is provided, we treat it as a "false" + // wildcard segment and keep a sentinel there so we can + // use that to find what sub router to search at lookup + // time. + if let Some(r) = router_idx { + let prefix = AbsolutePath::new(routes[route_idx].url()); + let wildcard_suffix = AbsolutePath::new(format!("{{*{NESTED_ROUTER_PARAM}}}")); + let wildcard = prefix.join(&wildcard_suffix); + + Self::insert_or_diagnose( + &mut inner, + MatchitPattern::new(wildcard.as_str()), + Entry::Router(r), + &routes[r], + routes, + )?; + } + } + + Ok(Self { inner }) + } + + fn insert_or_diagnose( + trie: &mut MatchitRouter, + pattern: MatchitPattern, + value: Entry, + new_route: &Route, + routes: &[Route], + ) -> super::Result<()> { + trie.insert(pattern, value) + .map_err(|err| Self::diagnose(new_route, err, routes).into()) + } + + fn diagnose( + new_route: &Route, + err: matchit::InsertError, + routes: &[Route], + ) -> RouteConflictError { + match err { + matchit::InsertError::Conflict { with } => { + let existing_route = routes.iter().find(|r| { + MatchitPattern::try_from(r.url.clone()).is_ok_and(|p| p.as_str() == with) + }); + + if let Some(existing_route) = existing_route { + Self::classify(existing_route, new_route) + } else { + RouteConflictError::RouteInsert(matchit::InsertError::Conflict { with }) + } + } + + other => RouteConflictError::RouteInsert(other), + } + } + + fn classify(existing_route: &Route, new_route: &Route) -> RouteConflictError { + for (existing_part, new_part) in + existing_route.url.parts().iter().zip(new_route.url.parts()) + { + match (existing_part, new_part) { + (PathPart::Param { name: a }, PathPart::Param { name: b }) if a != b => { + return RouteConflictError::ConflictingParamName { + existing: existing_route.url(), + existing_name: a.clone(), + new: new_route.url(), + new_name: b.clone(), + }; + } + (PathPart::Wildcard { name: a }, PathPart::Wildcard { name: b }) if a != b => { + return RouteConflictError::ConflictingWildcardName { + existing: existing_route.url(), + existing_name: a.clone(), + new: new_route.url(), + new_name: b.clone(), + }; + } + (PathPart::Wildcard { .. }, PathPart::Wildcard { .. }) => { + return RouteConflictError::DuplicateWildcard { + existing: existing_route.url(), + new: new_route.url(), + }; + } + _ => continue, + } + } + + // Every segment matched so this is a duplicate + RouteConflictError::DuplicateHandler { + existing: existing_route.url(), + new: new_route.url(), + } + } + + pub(super) fn at<'a>(&'a self, path: &'a str) -> Option> { + self.inner.at(path).ok() + } +} + +pub(super) fn router_mount_pattern(route: &Route) -> MatchitPattern { + let url = route.url(); + let trimmed = url + .strip_suffix('/') + .filter(|s| !s.is_empty()) + .unwrap_or(&url); + MatchitPattern::new(trimmed) +} + +#[cfg(test)] +mod tests { + use cot::router::Route; + + use super::*; + use crate::html::Html; + use crate::router::Router; + + async fn handler() -> Html { + Html::new("ok") + } + + fn route(url: &str) -> Route { + Route::with_handler(url, handler) + } + + #[test] + fn build_single_handler_route() { + let routes = vec![route("/users")]; + let trie = RouteTrie::build(&routes).unwrap(); + + let m = trie.at("/users").unwrap(); + assert!(matches!(m.value, Entry::Handler(0))); + } + + #[test] + fn build_no_match_returns_none() { + let routes = vec![route("/users")]; + let trie = RouteTrie::build(&routes).unwrap(); + + assert!(trie.at("/other").is_none()); + } + + #[test] + fn build_root_path_matches() { + let routes = vec![route("/")]; + let trie = RouteTrie::build(&routes).unwrap(); + + assert!(matches!(trie.at("/").unwrap().value, Entry::Handler(0))); + } + + #[test] + fn build_param_route_captures_value() { + let routes = vec![route("/users/{id}")]; + let trie = RouteTrie::build(&routes).unwrap(); + + let m = trie.at("/users/42").unwrap(); + assert!(matches!(m.value, Entry::Handler(0))); + assert_eq!(m.params.get("id"), Some("42")); + } + + #[test] + fn build_wildcard_route_captures_remaining_path() { + let routes = vec![route("/static/{*path}")]; + let trie = RouteTrie::build(&routes).unwrap(); + + let m = trie.at("/static/css/app.css").unwrap(); + assert!(matches!(m.value, Entry::Handler(0))); + assert_eq!(m.params.get("path"), Some("css/app.css")); + } + + #[test] + fn build_router_route_inserts_wildcard_sentinel() { + let sub_router = Router::with_urls(vec![route("/inner")]); + let routes = vec![Route::with_router("/api", sub_router)]; + let trie = RouteTrie::build(&routes).unwrap(); + + assert!(matches!(trie.at("/api").unwrap().value, Entry::Router(0))); + + let m = trie.at("/api/inner").unwrap(); + assert!(matches!(m.value, Entry::Router(0))); + assert_eq!(m.params.get(NESTED_ROUTER_PARAM), Some("inner")); + } + + #[test] + fn build_router_trailing_slash_prefix_does_not_double_slash() { + let sub_router = Router::with_urls(vec![route("/inner")]); + let routes = vec![Route::with_router("/api/", sub_router)]; + let trie = RouteTrie::build(&routes).unwrap(); + + let m = trie.at("/api/inner").unwrap(); + assert_eq!(m.params.get(NESTED_ROUTER_PARAM), Some("inner")); + } + + #[test] + fn build_combined_handler_and_router_same_path() { + let sub_router = Router::with_urls(vec![route("/inner")]); + let routes = vec![Route::with_router("/api", sub_router), route("/api")]; + let trie = RouteTrie::build(&routes).unwrap(); + + assert!(matches!(trie.at("/api").unwrap().value, Entry::Handler(1))); + } + + #[test] + fn static_route_priority_over_param_route() { + let routes = vec![route("/users/{id}"), route("/users/new")]; + let trie = RouteTrie::build(&routes).unwrap(); + + assert!(matches!( + trie.at("/users/new").unwrap().value, + Entry::Handler(1) + )); + } + + #[test] + fn build_duplicate_handler_errors() { + let routes = vec![route("/users"), route("/users")]; + let err = RouteTrie::build(&routes).unwrap_err(); + assert!(err.to_string().contains("duplicate route")); + } + + #[test] + fn build_duplicate_router_errors() { + let routes = vec![ + Route::with_router("/users", Router::empty()), + Route::with_router("/users", Router::empty()), + ]; + let err = RouteTrie::build(&routes).unwrap_err(); + assert!(err.to_string().contains("duplicate nested router")); + } + + #[test] + fn build_conflicting_param_names_errors() { + let routes = vec![route("/foo/{bar}/"), route("/foo/{baz}/")]; + let err = RouteTrie::build(&routes).unwrap_err(); + assert!(err.to_string().contains("conflicting route parameters")); + } + + #[test] + fn build_conflicting_wildcard_names_errors() { + let routes = vec![route("/static/{*path}"), route("/static/{*file_path}")]; + let err = RouteTrie::build(&routes).unwrap_err(); + assert!(err.to_string().contains("conflicting wildcard parameters")); + } + + #[test] + fn build_duplicate_wildcard_errors() { + let routes = vec![route("/static/{*path}"), route("/static/{*path}")]; + let err = RouteTrie::build(&routes).unwrap_err(); + assert!(err.to_string().contains("duplicate route")); + } + + #[test] + fn build_root_mounted_router_matches_root_path() { + let sub_router = Router::with_urls(vec![route("/")]); + let routes = vec![Route::with_router("", sub_router)]; + let trie = RouteTrie::build(&routes).unwrap(); + + let m = trie.at("/").unwrap(); + assert!(matches!(m.value, Entry::Router(0))); + } + + #[test] + fn build_root_mounted_router_exact_match_has_no_wildcard_capture() { + let sub_router = Router::with_urls(vec![route("/")]); + let routes = vec![Route::with_router("/", sub_router)]; + let trie = RouteTrie::build(&routes).unwrap(); + + let m = trie.at("/").unwrap(); + assert!(m.params.get(NESTED_ROUTER_PARAM).is_none()); + } + + #[test] + fn matchit_pattern_new_and_as_str() { + let pattern = MatchitPattern::new("/users/{id}"); + assert_eq!(pattern.as_str(), "/users/{id}"); + } + + #[test] + fn matchit_pattern_into_string() { + let pattern = MatchitPattern::new("/users"); + let s: String = pattern.into(); + assert_eq!(s, "/users"); + } + + #[test] + fn build_root_mounted_router_pattern_not_trimmed_to_empty() { + let sub_router = Router::with_urls(vec![route("/inner")]); + let routes = vec![Route::with_router("/", sub_router)]; + let trie = RouteTrie::build(&routes).unwrap(); + assert!(trie.at("/inner").is_some()); + } + + #[test] + fn build_router_mount_slash_and_no_slash_variants_conflict_with_clear_error() { + let router1 = Router::with_urls(vec![route("/foo")]); + let router2 = Router::with_urls(vec![route("/bar")]); + let routes = vec![ + Route::with_router("/admin", router1), + Route::with_router("/admin/", router2), + ]; + let err = RouteTrie::build(&routes).unwrap_err(); + assert!(err.to_string().contains("duplicate nested router")); + } +} diff --git a/cot/templates/error.html b/cot/templates/error.html index 825732ff..db1dcd2c 100644 --- a/cot/templates/error.html +++ b/cot/templates/error.html @@ -92,6 +92,7 @@

Routes

URL Type Name + App @@ -113,6 +114,13 @@

Routes

{{ route.name }} {% endif %} + + {% if route.app.is_empty() %} + <none> + {% else %} + {{ route.app }} + {% endif %} + {% endfor %} diff --git a/cot/tests/admin.rs b/cot/tests/admin.rs index 46867a0a..afe7b3cd 100644 --- a/cot/tests/admin.rs +++ b/cot/tests/admin.rs @@ -53,7 +53,7 @@ impl Project for AdminProject { fn register_apps(&self, apps: &mut AppBuilder, _context: &RegisterAppsContext) { apps.register(DatabaseUserApp::new()); - apps.register_with_views(AdminApp::new(), "/admin"); + apps.register_with_views(AdminApp::new(), "/admin/"); apps.register(HelloApp); } diff --git a/cot/tests/router.rs b/cot/tests/router.rs index cd598cf4..49653a10 100644 --- a/cot/tests/router.rs +++ b/cot/tests/router.rs @@ -13,10 +13,25 @@ async fn index() -> Html { async fn parameterized(request: Request) -> Html { let name = request.path_params().get("name").unwrap().to_owned(); - Html::new(name) } +async fn multi_param(request: Request) -> Html { + let id = request.path_params().get("id").unwrap().to_owned(); + let post_id = request.path_params().get("post_id").unwrap().to_owned(); + Html::new(format!("{id}/{post_id}")) +} + +async fn catch_all(request: Request) -> Html { + let path = request.path_params().get("path").unwrap().to_owned(); + Html::new(path) +} + +async fn nested(request: Request) -> Html { + let id = request.path_params().get("id").unwrap().to_owned(); + Html::new(format!("nested/{id}")) +} + #[cot::test] #[cfg_attr( miri, @@ -49,6 +64,82 @@ async fn path_params() { ); } +#[cot::test] +#[cfg_attr( + miri, + ignore = "unsupported operation: can't call foreign function `sqlite3_open_v2`" +)] +async fn multi_path_params() { + let client = Client::new(project()); + + let response = client.await.get("/multi/1/posts/2").await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.into_body().into_bytes().await.unwrap(), + Bytes::from("1/2") + ); +} + +#[cot::test] +#[cfg_attr( + miri, + ignore = "unsupported operation: can't call foreign function `sqlite3_open_v2`" +)] +async fn wildcard_catch_all() { + let client = Client::new(project()); + + let response = client.await.get("/static/css/app.css").await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.into_body().into_bytes().await.unwrap(), + Bytes::from("css/app.css") + ); +} + +#[cot::test] +#[cfg_attr( + miri, + ignore = "unsupported operation: can't call foreign function `sqlite3_open_v2`" +)] +async fn nested_router() { + let client = Client::new(project()); + + let response = client.await.get("/nested/inner/42").await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.into_body().into_bytes().await.unwrap(), + Bytes::from("nested/42") + ); +} + +#[cot::test] +#[cfg_attr( + miri, + ignore = "unsupported operation: can't call foreign function `sqlite3_open_v2`" +)] +async fn unmatched_path_returns_404() { + let client = Client::new(project()); + + let response = client.await.get("/does-not-exist").await.unwrap(); + assert_eq!(response.status(), StatusCode::NOT_FOUND); +} + +#[cot::test] +#[cfg_attr( + miri, + ignore = "unsupported operation: can't call foreign function `sqlite3_open_v2`" +)] +async fn static_route_priority_over_dynamic() { + let client = Client::new(project()); + + let response = client.await.get("/get/new").await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.into_body().into_bytes().await.unwrap(), + Bytes::from("new") + ); +} + #[must_use] fn project() -> impl Project { struct RouterApp; @@ -58,9 +149,22 @@ fn project() -> impl Project { } fn router(&self) -> Router { + let nested_router = Router::with_urls([Route::with_handler_and_name( + "/inner/{id}", + nested, + "nested", + )]); + Router::with_urls([ Route::with_handler_and_name("/", index, "index"), Route::with_handler_and_name("/get/{name}", parameterized, "parameterized"), + Route::with_handler_and_name( + "/multi/{id}/posts/{post_id}", + multi_param, + "multi_param", + ), + Route::with_handler_and_name("/static/{*path}", catch_all, "catch_all"), + Route::with_router("/nested", nested_router), ]) } }