diff --git a/.cargo/config.toml b/.cargo/config.toml index 40fe3e57..75362685 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,3 @@ [alias] -chk = "hack check --workspace --all-features --tests --examples" -lint = "hack --clean-per-run clippy --workspace --tests --examples" +chk = "check --workspace --all-features --tests --examples --bins" +lint = "clippy --workspace --all-features --tests --examples --bins -- -Dclippy::todo" diff --git a/.github/workflows/clippy-fmt.yml b/.github/workflows/clippy-fmt.yml index 3bef81db..ca637beb 100644 --- a/.github/workflows/clippy-fmt.yml +++ b/.github/workflows/clippy-fmt.yml @@ -39,4 +39,4 @@ jobs: uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: --workspace --tests --all-features + args: --workspace --all-features --tests --examples --bins -- -Dclippy::todo diff --git a/Cargo.toml b/Cargo.toml index 5bf72300..6ed9b50a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,3 +27,8 @@ actix-utils = { path = "actix-utils" } bytestring = { path = "bytestring" } local-channel = { path = "local-channel" } local-waker = { path = "local-waker" } + +[profile.release] +lto = true +opt-level = 3 +codegen-units = 1 diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index bc22ee7e..0ace6c8f 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -1,12 +1,37 @@ # Changes ## Unreleased - 2021-xx-xx +* Introduce `ResourceDef::join`. [#380] + +[#380]: https://github.com/actix/actix-net/pull/380 + + +## 0.5.0-beta.1 - 2021-07-20 +* Fix a bug in multi-patterns where static patterns are interpreted as regex. [#366] +* Introduce `ResourceDef::pattern_iter` to get an iterator over all patterns in a multi-pattern resource. [#373] * Fix segment interpolation leaving `Path` in unintended state after matching. [#368] -* Path tail pattern now works as expected after a dynamic segment (e.g. `/user/{uid}/*`). [#366] -* Fixed a bug where, in multi-patterns, static patterns are interpreted as regex. [#366] +* Fix `ResourceDef` `PartialEq` implementation. [#373] +* Re-work `IntoPatterns` trait, adding a `Patterns` enum. [#372] +* Implement `IntoPatterns` for `bytestring::ByteString`. [#372] +* Rename `Path::{len => segment_count}` to be more descriptive of it's purpose. [#370] +* Rename `ResourceDef::{resource_path => resource_path_from_iter}`. [#371] +* `ResourceDef::resource_path_from_iter` now takes an `IntoIterator`. [#373] +* Rename `ResourceDef::{resource_path_named => resource_path_from_map}`. [#371] +* Rename `ResourceDef::{is_prefix_match => find_match}`. [#373] +* Rename `ResourceDef::{match_path => capture_match_info}`. [#373] +* Rename `ResourceDef::{match_path_checked => capture_match_info_fn}`. [#373] +* Remove `ResourceDef::name_mut` and introduce `ResourceDef::set_name`. [#373] +* Rename `Router::{*_checked => *_fn}`. [#373] +* Return type of `ResourceDef::name` is now `Option<&str>`. [#373] +* Return type of `ResourceDef::pattern` is now `Option<&str>`. [#373] [#368]: https://github.com/actix/actix-net/pull/368 [#366]: https://github.com/actix/actix-net/pull/366 +[#368]: https://github.com/actix/actix-net/pull/368 +[#370]: https://github.com/actix/actix-net/pull/370 +[#371]: https://github.com/actix/actix-net/pull/371 +[#372]: https://github.com/actix/actix-net/pull/372 +[#373]: https://github.com/actix/actix-net/pull/373 ## 0.4.0 - 2021-06-06 diff --git a/actix-router/Cargo.toml b/actix-router/Cargo.toml index 5f0c22e2..2a2ce1cc 100644 --- a/actix-router/Cargo.toml +++ b/actix-router/Cargo.toml @@ -1,10 +1,14 @@ [package] name = "actix-router" -version = "0.4.0" -authors = ["Nikolay Kim "] -description = "Resource path matching library" +version = "0.5.0-beta.1" +authors = [ + "Nikolay Kim ", + "Ali MJ Al-Nasrawy ", + "Rob Ede ", +] +description = "Resource path matching and router" keywords = ["actix", "router", "routing"] -repository = "https://github.com/actix/actix-net" +repository = "https://github.com/actix/actix-net.git" license = "MIT OR Apache-2.0" edition = "2018" @@ -16,12 +20,19 @@ path = "src/lib.rs" default = ["http"] [dependencies] -regex = "1.3.1" -serde = "1.0.104" bytestring = ">=0.1.5, <2" -log = "0.4.8" -http = { version = "0.2.2", optional = true } +firestorm = "0.4" +http = { version = "0.2.3", optional = true } +log = "0.4" +regex = "1.5" +serde = "1" [dev-dependencies] -http = "0.2.2" -serde_derive = "1.0" +criterion = { version = "0.3", features = ["html_reports"] } +firestorm = { version = "0.4", features = ["enable_system_time"] } +http = "0.2.3" +serde = { version = "1", features = ["derive"] } + +[[bench]] +name = "router" +harness = false diff --git a/actix-router/benches/router.rs b/actix-router/benches/router.rs new file mode 100644 index 00000000..a428b9f1 --- /dev/null +++ b/actix-router/benches/router.rs @@ -0,0 +1,194 @@ +//! Based on https://github.com/ibraheemdev/matchit/blob/master/benches/bench.rs + +use criterion::{black_box, criterion_group, criterion_main, Criterion}; + +macro_rules! register { + (colon) => {{ + register!(finish => ":p1", ":p2", ":p3", ":p4") + }}; + (brackets) => {{ + register!(finish => "{p1}", "{p2}", "{p3}", "{p4}") + }}; + (regex) => {{ + register!(finish => "(.*)", "(.*)", "(.*)", "(.*)") + }}; + (finish => $p1:literal, $p2:literal, $p3:literal, $p4:literal) => {{ + let arr = [ + concat!("/authorizations"), + concat!("/authorizations/", $p1), + concat!("/applications/", $p1, "/tokens/", $p2), + concat!("/events"), + concat!("/repos/", $p1, "/", $p2, "/events"), + concat!("/networks/", $p1, "/", $p2, "/events"), + concat!("/orgs/", $p1, "/events"), + concat!("/users/", $p1, "/received_events"), + concat!("/users/", $p1, "/received_events/public"), + concat!("/users/", $p1, "/events"), + concat!("/users/", $p1, "/events/public"), + concat!("/users/", $p1, "/events/orgs/", $p2), + concat!("/feeds"), + concat!("/notifications"), + concat!("/repos/", $p1, "/", $p2, "/notifications"), + concat!("/notifications/threads/", $p1), + concat!("/notifications/threads/", $p1, "/subscription"), + concat!("/repos/", $p1, "/", $p2, "/stargazers"), + concat!("/users/", $p1, "/starred"), + concat!("/user/starred"), + concat!("/user/starred/", $p1, "/", $p2), + concat!("/repos/", $p1, "/", $p2, "/subscribers"), + concat!("/users/", $p1, "/subscriptions"), + concat!("/user/subscriptions"), + concat!("/repos/", $p1, "/", $p2, "/subscription"), + concat!("/user/subscriptions/", $p1, "/", $p2), + concat!("/users/", $p1, "/gists"), + concat!("/gists"), + concat!("/gists/", $p1), + concat!("/gists/", $p1, "/star"), + concat!("/repos/", $p1, "/", $p2, "/git/blobs/", $p3), + concat!("/repos/", $p1, "/", $p2, "/git/commits/", $p3), + concat!("/repos/", $p1, "/", $p2, "/git/refs"), + concat!("/repos/", $p1, "/", $p2, "/git/tags/", $p3), + concat!("/repos/", $p1, "/", $p2, "/git/trees/", $p3), + concat!("/issues"), + concat!("/user/issues"), + concat!("/orgs/", $p1, "/issues"), + concat!("/repos/", $p1, "/", $p2, "/issues"), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3), + concat!("/repos/", $p1, "/", $p2, "/assignees"), + concat!("/repos/", $p1, "/", $p2, "/assignees/", $p3), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/comments"), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/events"), + concat!("/repos/", $p1, "/", $p2, "/labels"), + concat!("/repos/", $p1, "/", $p2, "/labels/", $p3), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/labels"), + concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3, "/labels"), + concat!("/repos/", $p1, "/", $p2, "/milestones/"), + concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3), + concat!("/emojis"), + concat!("/gitignore/templates"), + concat!("/gitignore/templates/", $p1), + concat!("/meta"), + concat!("/rate_limit"), + concat!("/users/", $p1, "/orgs"), + concat!("/user/orgs"), + concat!("/orgs/", $p1), + concat!("/orgs/", $p1, "/members"), + concat!("/orgs/", $p1, "/members", $p2), + concat!("/orgs/", $p1, "/public_members"), + concat!("/orgs/", $p1, "/public_members/", $p2), + concat!("/orgs/", $p1, "/teams"), + concat!("/teams/", $p1), + concat!("/teams/", $p1, "/members"), + concat!("/teams/", $p1, "/members", $p2), + concat!("/teams/", $p1, "/repos"), + concat!("/teams/", $p1, "/repos/", $p2, "/", $p3), + concat!("/user/teams"), + concat!("/repos/", $p1, "/", $p2, "/pulls"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/commits"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/files"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/merge"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/comments"), + concat!("/user/repos"), + concat!("/users/", $p1, "/repos"), + concat!("/orgs/", $p1, "/repos"), + concat!("/repositories"), + concat!("/repos/", $p1, "/", $p2), + concat!("/repos/", $p1, "/", $p2, "/contributors"), + concat!("/repos/", $p1, "/", $p2, "/languages"), + concat!("/repos/", $p1, "/", $p2, "/teams"), + concat!("/repos/", $p1, "/", $p2, "/tags"), + concat!("/repos/", $p1, "/", $p2, "/branches"), + concat!("/repos/", $p1, "/", $p2, "/branches/", $p3), + concat!("/repos/", $p1, "/", $p2, "/collaborators"), + concat!("/repos/", $p1, "/", $p2, "/collaborators/", $p3), + concat!("/repos/", $p1, "/", $p2, "/comments"), + concat!("/repos/", $p1, "/", $p2, "/commits/", $p3, "/comments"), + concat!("/repos/", $p1, "/", $p2, "/commits"), + concat!("/repos/", $p1, "/", $p2, "/commits/", $p3), + concat!("/repos/", $p1, "/", $p2, "/readme"), + concat!("/repos/", $p1, "/", $p2, "/keys"), + concat!("/repos/", $p1, "/", $p2, "/keys", $p3), + concat!("/repos/", $p1, "/", $p2, "/downloads"), + concat!("/repos/", $p1, "/", $p2, "/downloads", $p3), + concat!("/repos/", $p1, "/", $p2, "/forks"), + concat!("/repos/", $p1, "/", $p2, "/hooks"), + concat!("/repos/", $p1, "/", $p2, "/hooks", $p3), + concat!("/repos/", $p1, "/", $p2, "/releases"), + concat!("/repos/", $p1, "/", $p2, "/releases/", $p3), + concat!("/repos/", $p1, "/", $p2, "/releases/", $p3, "/assets"), + concat!("/repos/", $p1, "/", $p2, "/stats/contributors"), + concat!("/repos/", $p1, "/", $p2, "/stats/commit_activity"), + concat!("/repos/", $p1, "/", $p2, "/stats/code_frequency"), + concat!("/repos/", $p1, "/", $p2, "/stats/participation"), + concat!("/repos/", $p1, "/", $p2, "/stats/punch_card"), + concat!("/repos/", $p1, "/", $p2, "/statuses/", $p3), + concat!("/search/repositories"), + concat!("/search/code"), + concat!("/search/issues"), + concat!("/search/users"), + concat!("/legacy/issues/search/", $p1, "/", $p2, "/", $p3, "/", $p4), + concat!("/legacy/repos/search/", $p1), + concat!("/legacy/user/search/", $p1), + concat!("/legacy/user/email/", $p1), + concat!("/users/", $p1), + concat!("/user"), + concat!("/users"), + concat!("/user/emails"), + concat!("/users/", $p1, "/followers"), + concat!("/user/followers"), + concat!("/users/", $p1, "/following"), + concat!("/user/following"), + concat!("/user/following/", $p1), + concat!("/users/", $p1, "/following", $p2), + concat!("/users/", $p1, "/keys"), + concat!("/user/keys"), + concat!("/user/keys/", $p1), + ]; + std::array::IntoIter::new(arr) + }}; +} + +fn call() -> impl Iterator { + let arr = [ + "/authorizations", + "/user/repos", + "/repos/rust-lang/rust/stargazers", + "/orgs/rust-lang/public_members/nikomatsakis", + "/repos/rust-lang/rust/releases/1.51.0", + ]; + + std::array::IntoIter::new(arr) +} + +fn compare_routers(c: &mut Criterion) { + let mut group = c.benchmark_group("Compare Routers"); + + let mut actix = actix_router::Router::::build(); + for route in register!(brackets) { + actix.path(route, true); + } + let actix = actix.finish(); + group.bench_function("actix", |b| { + b.iter(|| { + for route in call() { + let mut path = actix_router::Path::new(route); + black_box(actix.recognize(&mut path).unwrap()); + } + }); + }); + + let regex_set = regex::RegexSet::new(register!(regex)).unwrap(); + group.bench_function("regex", |b| { + b.iter(|| { + for route in call() { + black_box(regex_set.matches(route)); + } + }); + }); + + group.finish(); +} + +criterion_group!(benches, compare_routers); +criterion_main!(benches); diff --git a/actix-router/examples/flamegraph.rs b/actix-router/examples/flamegraph.rs new file mode 100644 index 00000000..798cc22d --- /dev/null +++ b/actix-router/examples/flamegraph.rs @@ -0,0 +1,169 @@ +macro_rules! register { + (brackets) => {{ + register!(finish => "{p1}", "{p2}", "{p3}", "{p4}") + }}; + (finish => $p1:literal, $p2:literal, $p3:literal, $p4:literal) => {{ + let arr = [ + concat!("/authorizations"), + concat!("/authorizations/", $p1), + concat!("/applications/", $p1, "/tokens/", $p2), + concat!("/events"), + concat!("/repos/", $p1, "/", $p2, "/events"), + concat!("/networks/", $p1, "/", $p2, "/events"), + concat!("/orgs/", $p1, "/events"), + concat!("/users/", $p1, "/received_events"), + concat!("/users/", $p1, "/received_events/public"), + concat!("/users/", $p1, "/events"), + concat!("/users/", $p1, "/events/public"), + concat!("/users/", $p1, "/events/orgs/", $p2), + concat!("/feeds"), + concat!("/notifications"), + concat!("/repos/", $p1, "/", $p2, "/notifications"), + concat!("/notifications/threads/", $p1), + concat!("/notifications/threads/", $p1, "/subscription"), + concat!("/repos/", $p1, "/", $p2, "/stargazers"), + concat!("/users/", $p1, "/starred"), + concat!("/user/starred"), + concat!("/user/starred/", $p1, "/", $p2), + concat!("/repos/", $p1, "/", $p2, "/subscribers"), + concat!("/users/", $p1, "/subscriptions"), + concat!("/user/subscriptions"), + concat!("/repos/", $p1, "/", $p2, "/subscription"), + concat!("/user/subscriptions/", $p1, "/", $p2), + concat!("/users/", $p1, "/gists"), + concat!("/gists"), + concat!("/gists/", $p1), + concat!("/gists/", $p1, "/star"), + concat!("/repos/", $p1, "/", $p2, "/git/blobs/", $p3), + concat!("/repos/", $p1, "/", $p2, "/git/commits/", $p3), + concat!("/repos/", $p1, "/", $p2, "/git/refs"), + concat!("/repos/", $p1, "/", $p2, "/git/tags/", $p3), + concat!("/repos/", $p1, "/", $p2, "/git/trees/", $p3), + concat!("/issues"), + concat!("/user/issues"), + concat!("/orgs/", $p1, "/issues"), + concat!("/repos/", $p1, "/", $p2, "/issues"), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3), + concat!("/repos/", $p1, "/", $p2, "/assignees"), + concat!("/repos/", $p1, "/", $p2, "/assignees/", $p3), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/comments"), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/events"), + concat!("/repos/", $p1, "/", $p2, "/labels"), + concat!("/repos/", $p1, "/", $p2, "/labels/", $p3), + concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/labels"), + concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3, "/labels"), + concat!("/repos/", $p1, "/", $p2, "/milestones/"), + concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3), + concat!("/emojis"), + concat!("/gitignore/templates"), + concat!("/gitignore/templates/", $p1), + concat!("/meta"), + concat!("/rate_limit"), + concat!("/users/", $p1, "/orgs"), + concat!("/user/orgs"), + concat!("/orgs/", $p1), + concat!("/orgs/", $p1, "/members"), + concat!("/orgs/", $p1, "/members", $p2), + concat!("/orgs/", $p1, "/public_members"), + concat!("/orgs/", $p1, "/public_members/", $p2), + concat!("/orgs/", $p1, "/teams"), + concat!("/teams/", $p1), + concat!("/teams/", $p1, "/members"), + concat!("/teams/", $p1, "/members", $p2), + concat!("/teams/", $p1, "/repos"), + concat!("/teams/", $p1, "/repos/", $p2, "/", $p3), + concat!("/user/teams"), + concat!("/repos/", $p1, "/", $p2, "/pulls"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/commits"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/files"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/merge"), + concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/comments"), + concat!("/user/repos"), + concat!("/users/", $p1, "/repos"), + concat!("/orgs/", $p1, "/repos"), + concat!("/repositories"), + concat!("/repos/", $p1, "/", $p2), + concat!("/repos/", $p1, "/", $p2, "/contributors"), + concat!("/repos/", $p1, "/", $p2, "/languages"), + concat!("/repos/", $p1, "/", $p2, "/teams"), + concat!("/repos/", $p1, "/", $p2, "/tags"), + concat!("/repos/", $p1, "/", $p2, "/branches"), + concat!("/repos/", $p1, "/", $p2, "/branches/", $p3), + concat!("/repos/", $p1, "/", $p2, "/collaborators"), + concat!("/repos/", $p1, "/", $p2, "/collaborators/", $p3), + concat!("/repos/", $p1, "/", $p2, "/comments"), + concat!("/repos/", $p1, "/", $p2, "/commits/", $p3, "/comments"), + concat!("/repos/", $p1, "/", $p2, "/commits"), + concat!("/repos/", $p1, "/", $p2, "/commits/", $p3), + concat!("/repos/", $p1, "/", $p2, "/readme"), + concat!("/repos/", $p1, "/", $p2, "/keys"), + concat!("/repos/", $p1, "/", $p2, "/keys", $p3), + concat!("/repos/", $p1, "/", $p2, "/downloads"), + concat!("/repos/", $p1, "/", $p2, "/downloads", $p3), + concat!("/repos/", $p1, "/", $p2, "/forks"), + concat!("/repos/", $p1, "/", $p2, "/hooks"), + concat!("/repos/", $p1, "/", $p2, "/hooks", $p3), + concat!("/repos/", $p1, "/", $p2, "/releases"), + concat!("/repos/", $p1, "/", $p2, "/releases/", $p3), + concat!("/repos/", $p1, "/", $p2, "/releases/", $p3, "/assets"), + concat!("/repos/", $p1, "/", $p2, "/stats/contributors"), + concat!("/repos/", $p1, "/", $p2, "/stats/commit_activity"), + concat!("/repos/", $p1, "/", $p2, "/stats/code_frequency"), + concat!("/repos/", $p1, "/", $p2, "/stats/participation"), + concat!("/repos/", $p1, "/", $p2, "/stats/punch_card"), + concat!("/repos/", $p1, "/", $p2, "/statuses/", $p3), + concat!("/search/repositories"), + concat!("/search/code"), + concat!("/search/issues"), + concat!("/search/users"), + concat!("/legacy/issues/search/", $p1, "/", $p2, "/", $p3, "/", $p4), + concat!("/legacy/repos/search/", $p1), + concat!("/legacy/user/search/", $p1), + concat!("/legacy/user/email/", $p1), + concat!("/users/", $p1), + concat!("/user"), + concat!("/users"), + concat!("/user/emails"), + concat!("/users/", $p1, "/followers"), + concat!("/user/followers"), + concat!("/users/", $p1, "/following"), + concat!("/user/following"), + concat!("/user/following/", $p1), + concat!("/users/", $p1, "/following", $p2), + concat!("/users/", $p1, "/keys"), + concat!("/user/keys"), + concat!("/user/keys/", $p1), + ]; + + arr.to_vec() + }}; +} + +static PATHS: [&str; 5] = [ + "/authorizations", + "/user/repos", + "/repos/rust-lang/rust/stargazers", + "/orgs/rust-lang/public_members/nikomatsakis", + "/repos/rust-lang/rust/releases/1.51.0", +]; + +fn main() { + let mut router = actix_router::Router::::build(); + + for route in register!(brackets) { + router.path(route, true); + } + + let actix = router.finish(); + + if firestorm::enabled() { + firestorm::bench("target", || { + for &route in &PATHS { + let mut path = actix_router::Path::new(route); + actix.recognize(&mut path).unwrap(); + } + }) + .unwrap(); + } +} diff --git a/actix-router/src/de.rs b/actix-router/src/de.rs index 81796348..775c48b8 100644 --- a/actix-router/src/de.rs +++ b/actix-router/src/de.rs @@ -24,10 +24,13 @@ macro_rules! parse_single_value { where V: Visitor<'de>, { - if self.path.len() != 1 { + if self.path.segment_count() != 1 { Err(de::value::Error::custom( - format!("wrong number of parameters: {} expected 1", self.path.len()) - .as_str(), + format!( + "wrong number of parameters: {} expected 1", + self.path.segment_count() + ) + .as_str(), )) } else { let v = self.path[0].parse().map_err(|_| { @@ -110,11 +113,11 @@ impl<'de, T: ResourcePath + 'de> Deserializer<'de> for PathDeserializer<'de, T> where V: Visitor<'de>, { - if self.path.len() < len { + if self.path.segment_count() < len { Err(de::value::Error::custom( format!( "wrong number of parameters: {} expected {}", - self.path.len(), + self.path.segment_count(), len ) .as_str(), @@ -135,11 +138,11 @@ impl<'de, T: ResourcePath + 'de> Deserializer<'de> for PathDeserializer<'de, T> where V: Visitor<'de>, { - if self.path.len() < len { + if self.path.segment_count() < len { Err(de::value::Error::custom( format!( "wrong number of parameters: {} expected {}", - self.path.len(), + self.path.segment_count(), len ) .as_str(), @@ -173,9 +176,13 @@ impl<'de, T: ResourcePath + 'de> Deserializer<'de> for PathDeserializer<'de, T> where V: Visitor<'de>, { - if self.path.len() != 1 { + if self.path.segment_count() != 1 { Err(de::value::Error::custom( - format!("wrong number of parameters: {} expected 1", self.path.len()).as_str(), + format!( + "wrong number of parameters: {} expected 1", + self.path.segment_count() + ) + .as_str(), )) } else { visitor.visit_str(&self.path[0]) @@ -485,8 +492,7 @@ impl<'de> de::VariantAccess<'de> for UnitVariant { #[cfg(test)] mod tests { - use serde::de; - use serde_derive::Deserialize; + use serde::{de, Deserialize}; use super::*; use crate::path::Path; diff --git a/actix-router/src/lib.rs b/actix-router/src/lib.rs index 5850b103..463e59e4 100644 --- a/actix-router/src/lib.rs +++ b/actix-router/src/lib.rs @@ -1,4 +1,4 @@ -//! Resource path matching library. +//! Resource path matching and router. #![deny(rust_2018_idioms, nonstandard_style)] #![doc(html_logo_url = "https://actix.rs/img/logo.png")] @@ -14,6 +14,8 @@ pub use self::path::Path; pub use self::resource::ResourceDef; pub use self::router::{ResourceInfo, Router, RouterBuilder}; +// TODO: this trait is necessary, document it +// see impl Resource for ServiceRequest pub trait Resource { fn resource_path(&mut self) -> &mut Path; } @@ -40,98 +42,92 @@ impl ResourcePath for bytestring::ByteString { } } -/// Helper trait for type that could be converted to path pattern -pub trait IntoPattern { - fn is_single(&self) -> bool; - - fn patterns(&self) -> Vec; +/// One or many patterns. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum Patterns { + Single(String), + List(Vec), } -impl IntoPattern for String { - fn is_single(&self) -> bool { - true - } - - fn patterns(&self) -> Vec { - vec![self.clone()] - } -} - -impl<'a> IntoPattern for &'a String { - fn is_single(&self) -> bool { - true - } - - fn patterns(&self) -> Vec { - vec![self.as_str().to_string()] - } -} - -impl<'a> IntoPattern for &'a str { - fn is_single(&self) -> bool { - true - } - - fn patterns(&self) -> Vec { - vec![(*self).to_string()] - } -} - -impl> IntoPattern for Vec { - fn is_single(&self) -> bool { - self.len() == 1 - } - - fn patterns(&self) -> Vec { - self.iter().map(|v| v.as_ref().to_string()).collect() - } -} - -macro_rules! array_patterns (($tp:ty, $num:tt) => { - impl IntoPattern for [$tp; $num] { - fn is_single(&self) -> bool { - $num == 1 +impl Patterns { + pub fn is_empty(&self) -> bool { + match self { + Patterns::Single(_) => false, + Patterns::List(pats) => pats.is_empty(), } + } +} - fn patterns(&self) -> Vec { - self.iter().map(|v| v.to_string()).collect() +/// Helper trait for type that could be converted to one or more path pattern. +pub trait IntoPatterns { + fn patterns(&self) -> Patterns; +} + +impl IntoPatterns for String { + fn patterns(&self) -> Patterns { + Patterns::Single(self.clone()) + } +} + +impl<'a> IntoPatterns for &'a String { + fn patterns(&self) -> Patterns { + Patterns::Single((*self).clone()) + } +} + +impl<'a> IntoPatterns for &'a str { + fn patterns(&self) -> Patterns { + Patterns::Single((*self).to_owned()) + } +} + +impl IntoPatterns for bytestring::ByteString { + fn patterns(&self) -> Patterns { + Patterns::Single(self.to_string()) + } +} + +impl IntoPatterns for Patterns { + fn patterns(&self) -> Patterns { + self.clone() + } +} + +impl> IntoPatterns for Vec { + fn patterns(&self) -> Patterns { + let mut patterns = self.iter().map(|v| v.as_ref().to_owned()); + + match patterns.size_hint() { + (1, _) => Patterns::Single(patterns.next().unwrap()), + _ => Patterns::List(patterns.collect()), + } + } +} + +macro_rules! array_patterns_single (($tp:ty) => { + impl IntoPatterns for [$tp; 1] { + fn patterns(&self) -> Patterns { + Patterns::Single(self[0].to_owned()) } } }); -array_patterns!(&str, 1); -array_patterns!(&str, 2); -array_patterns!(&str, 3); -array_patterns!(&str, 4); -array_patterns!(&str, 5); -array_patterns!(&str, 6); -array_patterns!(&str, 7); -array_patterns!(&str, 8); -array_patterns!(&str, 9); -array_patterns!(&str, 10); -array_patterns!(&str, 11); -array_patterns!(&str, 12); -array_patterns!(&str, 13); -array_patterns!(&str, 14); -array_patterns!(&str, 15); -array_patterns!(&str, 16); +macro_rules! array_patterns_multiple (($tp:ty, $str_fn:expr, $($num:tt) +) => { + // for each array length specified in $num + $( + impl IntoPatterns for [$tp; $num] { + fn patterns(&self) -> Patterns { + Patterns::List(self.iter().map($str_fn).collect()) + } + } + )+ +}); -array_patterns!(String, 1); -array_patterns!(String, 2); -array_patterns!(String, 3); -array_patterns!(String, 4); -array_patterns!(String, 5); -array_patterns!(String, 6); -array_patterns!(String, 7); -array_patterns!(String, 8); -array_patterns!(String, 9); -array_patterns!(String, 10); -array_patterns!(String, 11); -array_patterns!(String, 12); -array_patterns!(String, 13); -array_patterns!(String, 14); -array_patterns!(String, 15); -array_patterns!(String, 16); +array_patterns_single!(&str); +array_patterns_multiple!(&str, |&v| v.to_owned(), 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16); + +array_patterns_single!(String); +array_patterns_multiple!(String, |v| v.clone(), 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16); #[cfg(feature = "http")] mod url; @@ -140,10 +136,11 @@ mod url; pub use self::url::{Quoter, Url}; #[cfg(feature = "http")] -mod http_support { - use super::ResourcePath; +mod http_impls { use http::Uri; + use super::ResourcePath; + impl ResourcePath for Uri { fn path(&self) -> &str { self.path() diff --git a/actix-router/src/path.rs b/actix-router/src/path.rs index 6e4e2fdf..e29591f9 100644 --- a/actix-router/src/path.rs +++ b/actix-router/src/path.rs @@ -1,10 +1,10 @@ use std::borrow::Cow; use std::ops::Index; +use firestorm::profile_method; use serde::de; -use crate::de::PathDeserializer; -use crate::{Resource, ResourcePath}; +use crate::{de::PathDeserializer, Resource, ResourcePath}; #[derive(Debug, Clone)] pub(crate) enum PathItem { @@ -18,36 +18,16 @@ impl Default for PathItem { } } -/// Resource path match information +/// Resource path match information. /// /// If resource path contains variable patterns, `Path` stores them. -#[derive(Debug)] +#[derive(Debug, Clone, Default)] pub struct Path { path: T, pub(crate) skip: u16, pub(crate) segments: Vec<(Cow<'static, str>, PathItem)>, } -impl Default for Path { - fn default() -> Self { - Path { - path: T::default(), - skip: 0, - segments: Vec::new(), - } - } -} - -impl Clone for Path { - fn clone(&self) -> Self { - Path { - path: self.path.clone(), - skip: self.skip, - segments: self.segments.clone(), - } - } -} - impl Path { pub fn new(path: T) -> Path { Path { @@ -57,21 +37,23 @@ impl Path { } } - /// Get reference to inner path instance + /// Get reference to inner path instance. #[inline] pub fn get_ref(&self) -> &T { &self.path } - /// Get mutable reference to inner path instance + /// Get mutable reference to inner path instance. #[inline] pub fn get_mut(&mut self) -> &mut T { &mut self.path } - /// Path + /// Path. #[inline] pub fn path(&self) -> &str { + profile_method!(path); + let skip = self.skip as usize; let path = self.path.path(); if skip <= path.len() { @@ -81,7 +63,7 @@ impl Path { } } - /// Set new path + /// Set new path. #[inline] pub fn set(&mut self, path: T) { self.skip = 0; @@ -89,20 +71,22 @@ impl Path { self.segments.clear(); } - /// Reset state + /// Reset state. #[inline] pub fn reset(&mut self) { self.skip = 0; self.segments.clear(); } - /// Skip first `n` chars in path + /// Skip first `n` chars in path. #[inline] pub fn skip(&mut self, n: u16) { self.skip += n; } pub(crate) fn add(&mut self, name: impl Into>, value: PathItem) { + profile_method!(add); + match value { PathItem::Static(s) => self.segments.push((name.into(), PathItem::Static(s))), PathItem::Segment(begin, end) => self.segments.push(( @@ -122,35 +106,34 @@ impl Path { .push((name.into(), PathItem::Static(value.into()))); } - /// Check if there are any matched patterns + /// Check if there are any matched patterns. #[inline] pub fn is_empty(&self) -> bool { self.segments.is_empty() } - /// Check number of extracted parameters + /// Returns number of interpolated segments. #[inline] - pub fn len(&self) -> usize { + pub fn segment_count(&self) -> usize { self.segments.len() } /// Get matched parameter by name without type conversion - pub fn get(&self, key: &str) -> Option<&str> { - for item in self.segments.iter() { - if key == item.0 { - return match item.1 { + pub fn get(&self, name: &str) -> Option<&str> { + profile_method!(get); + + for (seg_name, val) in self.segments.iter() { + if name == seg_name { + return match val { PathItem::Static(ref s) => Some(&s), PathItem::Segment(s, e) => { - Some(&self.path.path()[(s as usize)..(e as usize)]) + Some(&self.path.path()[(*s as usize)..(*e as usize)]) } }; } } - if key == "tail" { - Some(&self.path.path()[(self.skip as usize)..]) - } else { - None - } + + None } /// Get unprocessed part of the path @@ -160,9 +143,10 @@ impl Path { /// Get matched parameter by name. /// - /// If keyed parameter is not available empty string is used as default - /// value. + /// If keyed parameter is not available empty string is used as default value. pub fn query(&self, key: &str) -> &str { + profile_method!(query); + if let Some(s) = self.get(key) { s } else { @@ -170,7 +154,7 @@ impl Path { } } - /// Return iterator to items in parameter container + /// Return iterator to items in parameter container. pub fn iter(&self) -> PathIter<'_, T> { PathIter { idx: 0, @@ -180,6 +164,7 @@ impl Path { /// Try to deserialize matching parameters to a specified type `U` pub fn load<'de, U: serde::Deserialize<'de>>(&'de self) -> Result { + profile_method!(load); de::Deserialize::deserialize(PathDeserializer::new(self)) } } @@ -195,7 +180,7 @@ impl<'a, T: ResourcePath> Iterator for PathIter<'a, T> { #[inline] fn next(&mut self) -> Option<(&'a str, &'a str)> { - if self.idx < self.params.len() { + if self.idx < self.params.segment_count() { let idx = self.idx; let res = match self.params.segments[idx].1 { PathItem::Static(ref s) => &s, diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 65cb5cf1..61ff587a 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -1,179 +1,682 @@ -use std::cmp::min; -use std::collections::HashMap; -use std::hash::{Hash, Hasher}; -use std::mem; +use std::{ + borrow::{Borrow, Cow}, + collections::HashMap, + hash::{BuildHasher, Hash, Hasher}, + mem, +}; +use firestorm::{profile_fn, profile_method, profile_section}; use regex::{escape, Regex, RegexSet}; -use crate::path::{Path, PathItem}; -use crate::{IntoPattern, Resource, ResourcePath}; +use crate::{ + path::{Path, PathItem}, + IntoPatterns, Patterns, Resource, ResourcePath, +}; const MAX_DYNAMIC_SEGMENTS: usize = 16; /// Regex flags to allow '.' in regex to match '\n' /// -/// See the docs under: https://docs.rs/regex/1.5.4/regex/#grouping-and-flags +/// See the docs under: https://docs.rs/regex/1/regex/#grouping-and-flags const REGEX_FLAGS: &str = "(?s-m)"; -/// ResourceDef describes an entry in resources table +/// Describes the set of paths that match to a resource. /// -/// Resource definition can contain only 16 dynamic segments +/// `ResourceDef`s are effectively a way to transform the a custom resource pattern syntax into +/// suitable regular expressions from which to check matches with paths and capture portions of a +/// matched path into variables. Common cases are on a fast path that avoids going through the +/// regex engine. +/// +/// +/// # Static Resources +/// A static resource is the most basic type of definition. Pass a regular string to +/// [new][Self::new]. Conforming paths must match the string exactly. +/// +/// ## Examples +/// ``` +/// # use actix_router::ResourceDef; +/// let resource = ResourceDef::new("/home"); +/// +/// assert!(resource.is_match("/home")); +/// +/// assert!(!resource.is_match("/home/new")); +/// assert!(!resource.is_match("/homes")); +/// assert!(!resource.is_match("/search")); +/// ``` +/// +/// +/// # Dynamic Segments +/// Also known as "path parameters". Resources can define sections of a pattern that be extracted +/// from a conforming path, if it conforms to (one of) the resource pattern(s). +/// +/// The marker for a dynamic segment is curly braces wrapping an identifier. For example, +/// `/user/{id}` would match paths like `/user/123` or `/user/james` and be able to extract the user +/// IDs "123" and "james", respectively. +/// +/// However, this resource pattern (`/user/{id}`) would, not cover `/user/123/stars` (unless +/// constructed as a prefix; see next section) since the default pattern for segments matches all +/// characters until it finds a `/` character (or the end of the path). Custom segment patterns are +/// covered further down. +/// +/// Dynamic segments do not need to be delimited by `/` characters, they can be defined within a +/// path segment. For example, `/rust-is-{opinion}` can match the paths `/rust-is-cool` and +/// `/rust-is-hard`. +/// +/// For information on capturing segment values from paths or other custom resource types, +/// see [`capture_match_info`][Self::capture_match_info] +/// and [`capture_match_info_fn`][Self::capture_match_info_fn]. +/// +/// A resource definition can contain at most 16 dynamic segments. +/// +/// ## Examples +/// ``` +/// use actix_router::{Path, ResourceDef}; +/// +/// let resource = ResourceDef::prefix("/user/{id}"); +/// +/// assert!(resource.is_match("/user/123")); +/// assert!(!resource.is_match("/user")); +/// assert!(!resource.is_match("/user/")); +/// +/// let mut path = Path::new("/user/123"); +/// resource.capture_match_info(&mut path); +/// assert_eq!(path.get("id").unwrap(), "123"); +/// ``` +/// +/// +/// # Prefix Resources +/// A prefix resource is defined as pattern that can match just the start of a path. +/// +/// This library chooses to restrict that definition slightly. In particular, when matching, the +/// prefix must be separated from the remaining part of the path by a `/` character, either at the +/// end of the prefix pattern or at the start of the the remaining slice. In practice, this is not +/// much of a limitation. +/// +/// Prefix resources can contain dynamic segments. +/// +/// ## Examples +/// ``` +/// # use actix_router::ResourceDef; +/// let resource = ResourceDef::prefix("/home"); +/// assert!(resource.is_match("/home")); +/// assert!(resource.is_match("/home/new")); +/// assert!(!resource.is_match("/homes")); +/// +/// let resource = ResourceDef::prefix("/user/{id}/"); +/// assert!(resource.is_match("/user/123/")); +/// assert!(resource.is_match("/user/123/stars")); +/// ``` +/// +/// +/// # Custom Regex Segments +/// Dynamic segments can be customised to only match a specific regular expression. It can be +/// helpful to do this if resource definitions would otherwise conflict and cause one to +/// be inaccessible. +/// +/// The regex used when capturing segment values can be specified explicitly using this syntax: +/// `{name:regex}`. For example, `/user/{id:\d+}` will only match paths where the user ID +/// is numeric. +/// +/// By default, dynamic segments use this regex: `[^/]+`. This shows why it is the case, as shown in +/// the earlier section, that segments capture a slice of the path up to the next `/` character. +/// +/// Custom regex segments can be used in static and prefix resource definition variants. +/// +/// ## Examples +/// ``` +/// # use actix_router::ResourceDef; +/// let resource = ResourceDef::new(r"/user/{id:\d+}"); +/// assert!(resource.is_match("/user/123")); +/// assert!(resource.is_match("/user/314159")); +/// assert!(!resource.is_match("/user/abc")); +/// ``` +/// +/// +/// # Tail Segments +/// As a shortcut to defining a custom regex for matching _all_ remaining characters (not just those +/// up until a `/` character), there is a special pattern to match (and capture) the remaining +/// path portion. +/// +/// To do this, use the segment pattern: `{name}*`. Since a tail segment also has a name, values are +/// extracted in the same way as non-tail dynamic segments. +/// +/// ## Examples +/// ```rust +/// # use actix_router::{Path, ResourceDef}; +/// let resource = ResourceDef::new("/blob/{tail}*"); +/// assert!(resource.is_match("/blob/HEAD/Cargo.toml")); +/// assert!(resource.is_match("/blob/HEAD/README.md")); +/// +/// let mut path = Path::new("/blob/main/LICENSE"); +/// resource.capture_match_info(&mut path); +/// assert_eq!(path.get("tail").unwrap(), "main/LICENSE"); +/// ``` +/// +/// +/// # Multi-Pattern Resources +/// For resources that can map to multiple distinct paths, it may be suitable to use +/// multi-pattern resources by passing an array/vec to [`new`][Self::new]. They will be combined +/// into a regex set which is usually quicker to check matches on than checking each +/// pattern individually. +/// +/// Multi-pattern resources can contain dynamic segments just like single pattern ones. +/// However, take care to use consistent and semantically-equivalent segment names; it could affect +/// expectations in the router using these definitions and cause runtime panics. +/// +/// ## Examples +/// ```rust +/// # use actix_router::ResourceDef; +/// let resource = ResourceDef::new(["/home", "/index"]); +/// assert!(resource.is_match("/home")); +/// assert!(resource.is_match("/index")); +/// ``` +/// +/// +/// # Trailing Slashes +/// It should be noted that this library takes no steps to normalize intra-path or trailing slashes. +/// As such, all resource definitions implicitly expect a pre-processing step to normalize paths if +/// they you wish to accommodate "recoverable" path errors. Below are several examples of +/// resource-path pairs that would not be compatible. +/// +/// ## Examples +/// ```rust +/// # use actix_router::ResourceDef; +/// assert!(!ResourceDef::new("/root").is_match("/root/")); +/// assert!(!ResourceDef::new("/root/").is_match("/root")); +/// assert!(!ResourceDef::prefix("/root/").is_match("/root")); +/// ``` #[derive(Clone, Debug)] pub struct ResourceDef { id: u16, - tp: PatternType, - name: String, - pattern: String, - elements: Option>, + + /// Optional name of resource. + name: Option, + + /// Pattern that generated the resource definition. + /// + /// `None` when pattern type is `DynamicSet`. + patterns: Patterns, + + /// Pattern type. + pat_type: PatternType, + + /// List of segments that compose the pattern, in order. + /// + /// `None` when pattern type is `DynamicSet`. + segments: Option>, } #[derive(Debug, Clone, PartialEq)] -enum PatternElement { +enum PatternSegment { + /// Literal slice of pattern. Const(String), + + /// Name of dynamic segment. Var(String), } #[derive(Clone, Debug)] #[allow(clippy::large_enum_variant)] enum PatternType { + /// Single constant/literal segment. Static(String), + + /// Single constant/literal prefix segment. Prefix(String), + + /// Single regular expression and list of dynamic segment names. Dynamic(Regex, Vec<&'static str>), + + /// Regular expression set and list of component expressions plus dynamic segment names. DynamicSet(RegexSet, Vec<(Regex, Vec<&'static str>)>), } impl ResourceDef { - /// Parse path pattern and create new `Pattern` instance. + /// Constructs a new resource definition from patterns. /// + /// Multi-pattern resources can be constructed by providing a slice (or vec) of patterns. + /// + /// # Panics /// Panics if path pattern is malformed. - pub fn new(path: T) -> Self { - if path.is_single() { - ResourceDef::from_single_pattern(&path.patterns()[0], false) - } else { - let mut data = Vec::new(); - let mut re_set = Vec::new(); + /// + /// # Examples + /// ``` + /// use actix_router::ResourceDef; + /// + /// let resource = ResourceDef::new("/user/{id}"); + /// assert!(resource.is_match("/user/123")); + /// assert!(!resource.is_match("/user/123/stars")); + /// assert!(!resource.is_match("user/1234")); + /// assert!(!resource.is_match("/foo")); + /// + /// let resource = ResourceDef::new(["/profile", "/user/{id}"]); + /// assert!(resource.is_match("/profile")); + /// assert!(resource.is_match("/user/123")); + /// assert!(!resource.is_match("user/123")); + /// assert!(!resource.is_match("/foo")); + /// ``` + pub fn new(paths: T) -> Self { + profile_method!(new); - for pattern in path.patterns() { - match ResourceDef::parse(&pattern, false, true) { - (PatternType::Dynamic(re, names), _) => { - re_set.push(re.as_str().to_owned()); - data.push((re, names)); - } - _ => unreachable!(), - } - } + match paths.patterns() { + Patterns::Single(pattern) => ResourceDef::from_single_pattern(&pattern, false), - ResourceDef { + // since zero length pattern sets are possible + // just return a useless `ResourceDef` + Patterns::List(patterns) if patterns.is_empty() => ResourceDef { id: 0, - tp: PatternType::DynamicSet(RegexSet::new(re_set).unwrap(), data), - elements: None, - name: String::new(), - pattern: "".to_owned(), + name: None, + patterns: Patterns::List(patterns), + pat_type: PatternType::DynamicSet(RegexSet::empty(), Vec::new()), + segments: None, + }, + + Patterns::List(patterns) => { + let mut re_set = Vec::with_capacity(patterns.len()); + let mut pattern_data = Vec::new(); + + for pattern in &patterns { + match ResourceDef::parse(&pattern, false, true) { + (PatternType::Dynamic(re, names), _) => { + re_set.push(re.as_str().to_owned()); + pattern_data.push((re, names)); + } + _ => unreachable!(), + } + } + + let pattern_re_set = RegexSet::new(re_set).unwrap(); + + ResourceDef { + id: 0, + name: None, + patterns: Patterns::List(patterns), + pat_type: PatternType::DynamicSet(pattern_re_set, pattern_data), + segments: None, + } } } } - /// Parse path pattern and create new `Pattern` instance. + /// Constructs a new resource definition using a string pattern that performs prefix matching. /// - /// Use `prefix` type instead of `static`. + /// More specifically, the regular expressions generated for matching are different when using + /// this method vs using `new`; they will not be appended with the `$` meta-character that + /// matches the end of an input. /// + /// Although it will compile and run correctly, it is meaningless to construct a prefix + /// resource definition with a tail segment; use [`new`][Self::new] in this case. + /// + /// # Panics /// Panics if path regex pattern is malformed. + /// + /// # Examples + /// ``` + /// use actix_router::ResourceDef; + /// + /// let resource = ResourceDef::prefix("/user/{id}"); + /// assert!(resource.is_match("/user/123")); + /// assert!(resource.is_match("/user/123/stars")); + /// assert!(!resource.is_match("user/123")); + /// assert!(!resource.is_match("user/123/stars")); + /// assert!(!resource.is_match("/foo")); + /// + /// let resource = ResourceDef::prefix("user/{id}"); + /// assert!(resource.is_match("user/123")); + /// assert!(resource.is_match("user/123/stars")); + /// assert!(!resource.is_match("/user/123")); + /// assert!(!resource.is_match("/user/123/stars")); + /// assert!(!resource.is_match("foo")); + /// ``` pub fn prefix(path: &str) -> Self { + profile_method!(prefix); ResourceDef::from_single_pattern(path, true) } - /// Parse path pattern and create new `Pattern` instance. - /// Inserts `/` to begging of the pattern. - /// - /// - /// Use `prefix` type instead of `static`. + /// Constructs a new resource definition using a string pattern that performs prefix matching, + /// inserting a `/` to beginning of the pattern if absent and pattern is not empty. /// + /// # Panics /// Panics if path regex pattern is malformed. + /// + /// # Examples + /// ``` + /// use actix_router::ResourceDef; + /// + /// let resource = ResourceDef::root_prefix("user/{id}"); + /// + /// assert_eq!(&resource, &ResourceDef::prefix("/user/{id}")); + /// assert_eq!(&resource, &ResourceDef::root_prefix("/user/{id}")); + /// assert_ne!(&resource, &ResourceDef::new("user/{id}")); + /// assert_ne!(&resource, &ResourceDef::new("/user/{id}")); + /// + /// assert!(resource.is_match("/user/123")); + /// assert!(!resource.is_match("user/123")); + /// ``` pub fn root_prefix(path: &str) -> Self { - ResourceDef::from_single_pattern(&insert_slash(path), true) + profile_method!(root_prefix); + ResourceDef::prefix(&insert_slash(path)) } - /// Resource id + /// Returns a numeric resource ID. + /// + /// If not explicitly set using [`set_id`][Self::set_id], this will return `0`. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut resource = ResourceDef::new("/root"); + /// assert_eq!(resource.id(), 0); + /// + /// resource.set_id(42); + /// assert_eq!(resource.id(), 42); + /// ``` pub fn id(&self) -> u16 { self.id } - /// Set resource id + /// Set numeric resource ID. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut resource = ResourceDef::new("/root"); + /// resource.set_id(42); + /// assert_eq!(resource.id(), 42); + /// ``` pub fn set_id(&mut self, id: u16) { self.id = id; } - /// Parse path pattern and create a new instance - fn from_single_pattern(pattern: &str, for_prefix: bool) -> Self { - let pattern = pattern.to_owned(); - let (tp, elements) = ResourceDef::parse(&pattern, for_prefix, false); + /// Returns resource definition name, if set. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut resource = ResourceDef::new("/root"); + /// assert!(resource.name().is_none()); + /// + /// resource.set_name("root"); + /// assert_eq!(resource.name().unwrap(), "root"); + pub fn name(&self) -> Option<&str> { + self.name.as_deref() + } - ResourceDef { - tp, - pattern, - elements: Some(elements), - id: 0, - name: String::new(), + /// Assigns a new name to the resource. + /// + /// # Panics + /// Panics if `name` is an empty string. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut resource = ResourceDef::new("/root"); + /// resource.set_name("root"); + /// assert_eq!(resource.name().unwrap(), "root"); + /// ``` + pub fn set_name(&mut self, name: impl Into) { + let name = name.into(); + + if name.is_empty() { + panic!("resource name should not be empty"); + } + + self.name = Some(name) + } + + /// Returns `true` if pattern type is prefix. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// assert!(ResourceDef::prefix("/user").is_prefix()); + /// assert!(!ResourceDef::new("/user").is_prefix()); + /// ``` + pub fn is_prefix(&self) -> bool { + match &self.pat_type { + PatternType::Prefix(_) => true, + PatternType::Dynamic(re, _) if !re.as_str().ends_with('$') => true, + _ => false, } } - /// Resource pattern name - pub fn name(&self) -> &str { - &self.name + /// Returns the pattern string that generated the resource definition. + /// + /// Returns `None` if definition was constructed with multiple patterns. + /// See [`patterns_iter`][Self::pattern_iter]. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut resource = ResourceDef::new("/user/{id}"); + /// assert_eq!(resource.pattern().unwrap(), "/user/{id}"); + /// + /// let mut resource = ResourceDef::new(["/profile", "/user/{id}"]); + /// assert!(resource.pattern().is_none()); + pub fn pattern(&self) -> Option<&str> { + match &self.patterns { + Patterns::Single(pattern) => Some(pattern.as_str()), + Patterns::List(_) => None, + } } - /// Mutable reference to a name of a resource definition. - pub fn name_mut(&mut self) -> &mut String { - &mut self.name + /// Returns iterator of pattern strings that generated the resource definition. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut resource = ResourceDef::new("/root"); + /// let mut iter = resource.pattern_iter(); + /// assert_eq!(iter.next().unwrap(), "/root"); + /// assert!(iter.next().is_none()); + /// + /// let mut resource = ResourceDef::new(["/root", "/backup"]); + /// let mut iter = resource.pattern_iter(); + /// assert_eq!(iter.next().unwrap(), "/root"); + /// assert_eq!(iter.next().unwrap(), "/backup"); + /// assert!(iter.next().is_none()); + pub fn pattern_iter(&self) -> impl Iterator { + struct PatternIter<'a> { + patterns: &'a Patterns, + list_idx: usize, + done: bool, + } + + impl<'a> Iterator for PatternIter<'a> { + type Item = &'a str; + + fn next(&mut self) -> Option { + match &self.patterns { + Patterns::Single(pattern) => { + if self.done { + return None; + } + + self.done = true; + Some(pattern.as_str()) + } + Patterns::List(patterns) if patterns.is_empty() => None, + Patterns::List(patterns) => match patterns.get(self.list_idx) { + Some(pattern) => { + self.list_idx += 1; + Some(pattern.as_str()) + } + None => { + // fast path future call + self.done = true; + None + } + }, + } + } + + fn size_hint(&self) -> (usize, Option) { + match &self.patterns { + Patterns::Single(_) => (1, Some(1)), + Patterns::List(patterns) => (patterns.len(), Some(patterns.len())), + } + } + } + + PatternIter { + patterns: &self.patterns, + list_idx: 0, + done: false, + } } - /// Path pattern of the resource - pub fn pattern(&self) -> &str { - &self.pattern + /// Joins two resources. + /// + /// Resulting resource is prefix if `other` is prefix. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let joined = ResourceDef::prefix("/root").join(&ResourceDef::prefix("/seg")); + /// assert_eq!(joined, ResourceDef::prefix("/root/seg")); + /// ``` + pub fn join(&self, other: &ResourceDef) -> ResourceDef { + let patterns = self + .pattern_iter() + .flat_map(move |this| other.pattern_iter().map(move |other| (this, other))) + .map(|(this, other)| [this, other].join("")) + .collect::>(); + + match patterns.len() { + 1 => ResourceDef::from_single_pattern(&patterns[0], other.is_prefix()), + _ => ResourceDef::new(patterns), + } } - /// Check if path matches this pattern. + /// Returns `true` if `path` matches this resource. + /// + /// The behavior of this method depends on how the `ResourceDef` was constructed. For example, + /// static resources will not be able to match as many paths as dynamic and prefix resources. + /// See [`ResourceDef`] struct docs for details on resource definition types. + /// + /// This method will always agree with [`find_match`][Self::find_match] on whether the path + /// matches or not. + /// + /// # Examples + /// ``` + /// use actix_router::ResourceDef; + /// + /// // static resource + /// let resource = ResourceDef::new("/user"); + /// assert!(resource.is_match("/user")); + /// assert!(!resource.is_match("/users")); + /// assert!(!resource.is_match("/user/123")); + /// assert!(!resource.is_match("/foo")); + /// + /// // dynamic resource + /// let resource = ResourceDef::new("/user/{user_id}"); + /// assert!(resource.is_match("/user/123")); + /// assert!(!resource.is_match("/user/123/stars")); + /// + /// // prefix resource + /// let resource = ResourceDef::prefix("/root"); + /// assert!(resource.is_match("/root")); + /// assert!(resource.is_match("/root/leaf")); + /// assert!(!resource.is_match("/roots")); + /// + /// // more examples are shown in the `ResourceDef` struct docs + /// ``` #[inline] pub fn is_match(&self, path: &str) -> bool { - match self.tp { + profile_method!(is_match); + + // this function could be expressed as: + // `self.find_match(path).is_some()` + // but this skips some checks and uses potentially faster regex methods + + match self.pat_type { PatternType::Static(ref s) => s == path, - PatternType::Prefix(ref s) => path.starts_with(s), + + PatternType::Prefix(ref prefix) if prefix == path => true, + PatternType::Prefix(ref prefix) => is_strict_prefix(prefix, path), + + // dynamic prefix + PatternType::Dynamic(ref re, _) if !re.as_str().ends_with('$') => { + match re.find(path) { + // prefix matches exactly + Some(m) if m.end() == path.len() => true, + + // prefix matches part + Some(m) => is_strict_prefix(m.as_str(), path), + + // prefix does not match + None => false, + } + } + PatternType::Dynamic(ref re, _) => re.is_match(path), PatternType::DynamicSet(ref re, _) => re.is_match(path), } } - /// Is prefix path a match against this resource. - pub fn is_prefix_match(&self, path: &str) -> Option { - let p_len = path.len(); - let path = if path.is_empty() { "/" } else { path }; + /// Tries to match `path` to this resource, returning the position in the path where the + /// match ends. + /// + /// This method will always agree with [`is_match`][Self::is_match] on whether the path matches + /// or not. + /// + /// # Examples + /// ``` + /// use actix_router::ResourceDef; + /// + /// // static resource + /// let resource = ResourceDef::new("/user"); + /// assert_eq!(resource.find_match("/user"), Some(5)); + /// assert!(resource.find_match("/user/").is_none()); + /// assert!(resource.find_match("/user/123").is_none()); + /// assert!(resource.find_match("/foo").is_none()); + /// + /// // constant prefix resource + /// let resource = ResourceDef::prefix("/user"); + /// assert_eq!(resource.find_match("/user"), Some(5)); + /// assert_eq!(resource.find_match("/user/"), Some(5)); + /// assert_eq!(resource.find_match("/user/123"), Some(5)); + /// + /// // dynamic prefix resource + /// let resource = ResourceDef::prefix("/user/{id}"); + /// assert_eq!(resource.find_match("/user/123"), Some(9)); + /// assert_eq!(resource.find_match("/user/1234/"), Some(10)); + /// assert_eq!(resource.find_match("/user/12345/stars"), Some(11)); + /// assert!(resource.find_match("/user/").is_none()); + /// + /// // multi-pattern resource + /// let resource = ResourceDef::new(["/user/{id}", "/profile/{id}"]); + /// assert_eq!(resource.find_match("/user/123"), Some(9)); + /// assert_eq!(resource.find_match("/profile/1234"), Some(13)); + /// ``` + pub fn find_match(&self, path: &str) -> Option { + profile_method!(find_match); - match self.tp { - PatternType::Static(ref s) => { - if s == path { - Some(p_len) - } else { - None + match &self.pat_type { + PatternType::Static(segment) if path == segment => Some(segment.len()), + PatternType::Static(_) => None, + + PatternType::Prefix(prefix) if path == prefix => Some(prefix.len()), + PatternType::Prefix(prefix) if is_strict_prefix(prefix, path) => Some(prefix.len()), + PatternType::Prefix(_) => None, + + // dynamic prefix + PatternType::Dynamic(ref re, _) if !re.as_str().ends_with('$') => { + match re.find(path) { + // prefix matches exactly + Some(m) if m.end() == path.len() => Some(m.end()), + + // prefix matches part + Some(m) if is_strict_prefix(m.as_str(), path) => Some(m.end()), + + // prefix does not match + _ => None, } } - PatternType::Dynamic(ref re, _) => re.find(path).map(|m| m.end()), - PatternType::Prefix(ref s) => { - let len = if path == s { - s.len() - } else if path.starts_with(s) - && (s.ends_with('/') || path.split_at(s.len()).1.starts_with('/')) - { - if s.ends_with('/') { - s.len() - 1 - } else { - s.len() - } - } else { - return None; - }; - Some(min(p_len, len)) - } - PatternType::DynamicSet(ref re, ref params) => { + + PatternType::Dynamic(re, _) => re.find(path).map(|m| m.end()), + + PatternType::DynamicSet(re, params) => { let idx = re.matches(path).into_iter().next()?; let (ref pattern, _) = params[idx]; pattern.find(path).map(|m| m.end()) @@ -181,77 +684,141 @@ impl ResourceDef { } } - /// Is the given path and parameters a match against this pattern. - pub fn match_path(&self, path: &mut Path) -> bool { - self.match_path_checked(path, &|_, _| true, &Some(())) + /// Collects dynamic segment values into `path`. + /// + /// Returns `true` if `path` matches this resource. + /// + /// # Examples + /// ``` + /// use actix_router::{Path, ResourceDef}; + /// + /// let resource = ResourceDef::prefix("/user/{id}"); + /// let mut path = Path::new("/user/123/stars"); + /// assert!(resource.capture_match_info(&mut path)); + /// assert_eq!(path.get("id").unwrap(), "123"); + /// assert_eq!(path.unprocessed(), "/stars"); + /// + /// let resource = ResourceDef::new("/blob/{path}*"); + /// let mut path = Path::new("/blob/HEAD/Cargo.toml"); + /// assert!(resource.capture_match_info(&mut path)); + /// assert_eq!(path.get("path").unwrap(), "HEAD/Cargo.toml"); + /// assert_eq!(path.unprocessed(), ""); + /// ``` + pub fn capture_match_info(&self, path: &mut Path) -> bool { + profile_method!(capture_match_info); + self.capture_match_info_fn(path, |_, _| true, ()) } - /// Is the given path and parameters a match against this pattern? - pub fn match_path_checked( + /// Collects dynamic segment values into `resource` after matching paths and executing + /// check function. + /// + /// The check function is given a reference to the passed resource and optional arbitrary data. + /// This is useful if you want to conditionally match on some non-path related aspect of the + /// resource type. + /// + /// Returns `true` if resource path matches this resource definition _and_ satisfies the + /// given check function. + /// + /// # Examples + /// ``` + /// use actix_router::{Path, ResourceDef}; + /// + /// fn try_match(resource: &ResourceDef, path: &mut Path<&str>) -> bool { + /// let admin_allowed = std::env::var("ADMIN_ALLOWED").ok(); + /// + /// resource.capture_match_info_fn( + /// path, + /// // when env var is not set, reject when path contains "admin" + /// |res, admin_allowed| !res.path().contains("admin"), + /// &admin_allowed + /// ) + /// } + /// + /// let resource = ResourceDef::prefix("/user/{id}"); + /// + /// // path matches; segment values are collected into path + /// let mut path = Path::new("/user/james/stars"); + /// assert!(try_match(&resource, &mut path)); + /// assert_eq!(path.get("id").unwrap(), "james"); + /// assert_eq!(path.unprocessed(), "/stars"); + /// + /// // path matches but fails check function; no segments are collected + /// let mut path = Path::new("/user/admin/stars"); + /// assert!(!try_match(&resource, &mut path)); + /// assert_eq!(path.unprocessed(), "/user/admin/stars"); + /// ``` + pub fn capture_match_info_fn( &self, - res: &mut R, - check: &F, - user_data: &Option, + resource: &mut R, + check_fn: F, + user_data: U, ) -> bool where - T: ResourcePath, R: Resource, - F: Fn(&R, &Option) -> bool, + T: ResourcePath, + F: FnOnce(&R, U) -> bool, { - let mut segments: [PathItem; MAX_DYNAMIC_SEGMENTS] = Default::default(); - let path = res.resource_path(); + profile_method!(capture_match_info_fn); - let (matched_len, matched_vars) = match self.tp { - PatternType::Static(ref s) => { - if s != path.path() { - return false; + let mut segments = <[PathItem; MAX_DYNAMIC_SEGMENTS]>::default(); + let path = resource.resource_path(); + let path_str = path.path(); + + let (matched_len, matched_vars) = match &self.pat_type { + PatternType::Static(_) | PatternType::Prefix(_) => { + profile_section!(pattern_static_or_prefix); + + match self.find_match(path_str) { + Some(len) => (len, None), + None => return false, } - (path.path().len(), None) } - PatternType::Prefix(ref s) => { - let len = { - let r_path = path.path(); - if s == r_path { - s.len() - } else if r_path.starts_with(s) - && (s.ends_with('/') || r_path.split_at(s.len()).1.starts_with('/')) - { - if s.ends_with('/') { - s.len() - 1 + + PatternType::Dynamic(re, names) => { + profile_section!(pattern_dynamic); + + let captures = { + profile_section!(pattern_dynamic_regex_exec); + + match re.captures(path.path()) { + Some(captures) => captures, + _ => return false, + } + }; + + { + profile_section!(pattern_dynamic_extract_captures); + + for (no, name) in names.iter().enumerate() { + if let Some(m) = captures.name(&name) { + segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); } else { - s.len() + log::error!( + "Dynamic path match but not all segments found: {}", + name + ); + return false; } - } else { - return false; } }; - (min(path.path().len(), len), None) - } - PatternType::Dynamic(ref re, ref names) => { - let captures = match re.captures(path.path()) { - Some(captures) => captures, - _ => return false, - }; - for (no, name) in names.iter().enumerate() { - if let Some(m) = captures.name(&name) { - segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); - } else { - log::error!("Dynamic path match but not all segments found: {}", name); - return false; - } - } + (captures[0].len(), Some(names)) } - PatternType::DynamicSet(ref re, ref params) => { + + PatternType::DynamicSet(re, params) => { + profile_section!(pattern_dynamic_set); + let path = path.path(); let (pattern, names) = match re.matches(path).into_iter().next() { Some(idx) => ¶ms[idx], _ => return false, }; + let captures = match pattern.captures(path.path()) { Some(captures) => captures, _ => return false, }; + for (no, name) in names.iter().enumerate() { if let Some(m) = captures.name(&name) { segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); @@ -260,73 +827,144 @@ impl ResourceDef { return false; } } + (captures[0].len(), Some(names)) } }; - if !check(res, user_data) { + if !check_fn(resource, user_data) { return false; } // Modify `path` to skip matched part and store matched segments - let path = res.resource_path(); + let path = resource.resource_path(); + if let Some(vars) = matched_vars { for i in 0..vars.len() { path.add(vars[i], mem::take(&mut segments[i])); } } + path.skip(matched_len as u16); true } - /// Build resource path with a closure that maps variable elements' names to values. + /// Assembles resource path using a closure that maps variable segment names to values. fn build_resource_path(&self, path: &mut String, mut vars: F) -> bool where F: FnMut(&str) -> Option, I: AsRef, { - for el in match self.elements { - Some(ref elements) => elements, + for el in match self.segments { + Some(ref segments) => segments, None => return false, } { match *el { - PatternElement::Const(ref val) => path.push_str(val), - PatternElement::Var(ref name) => match vars(name) { + PatternSegment::Const(ref val) => path.push_str(val), + PatternSegment::Var(ref name) => match vars(name) { Some(val) => path.push_str(val.as_ref()), _ => return false, }, } } + true } - /// Build resource path from elements. Returns `true` on success. - pub fn resource_path(&self, path: &mut String, elements: &mut U) -> bool + /// Assembles full resource path from iterator of dynamic segment values. + /// + /// Returns `true` on success. + /// + /// Resource paths can not be built from multi-pattern resources; this call will always return + /// false and will not add anything to the string buffer. + /// + /// # Examples + /// ``` + /// # use actix_router::ResourceDef; + /// let mut s = String::new(); + /// let resource = ResourceDef::new("/user/{id}/post/{title}"); + /// + /// assert!(resource.resource_path_from_iter(&mut s, &["123", "my-post"])); + /// assert_eq!(s, "/user/123/post/my-post"); + /// ``` + pub fn resource_path_from_iter(&self, path: &mut String, values: I) -> bool where - U: Iterator, - I: AsRef, + I: IntoIterator, + I::Item: AsRef, { - self.build_resource_path(path, |_| elements.next()) + profile_method!(resource_path_from_iter); + let mut iter = values.into_iter(); + self.build_resource_path(path, |_| iter.next()) } - /// Build resource path from elements. Returns `true` on success. - pub fn resource_path_named( + /// Assembles resource path from map of dynamic segment values. + /// + /// Returns `true` on success. + /// + /// Resource paths can not be built from multi-pattern resources; this call will always return + /// false and will not add anything to the string buffer. + /// + /// # Examples + /// ``` + /// # use std::collections::HashMap; + /// # use actix_router::ResourceDef; + /// let mut s = String::new(); + /// let resource = ResourceDef::new("/user/{id}/post/{title}"); + /// + /// let mut map = HashMap::new(); + /// map.insert("id", "123"); + /// map.insert("title", "my-post"); + /// + /// assert!(resource.resource_path_from_map(&mut s, &map)); + /// assert_eq!(s, "/user/123/post/my-post"); + /// ``` + pub fn resource_path_from_map( &self, path: &mut String, - elements: &HashMap, + values: &HashMap, ) -> bool where - K: std::borrow::Borrow + Eq + Hash, + K: Borrow + Eq + Hash, V: AsRef, - S: std::hash::BuildHasher, + S: BuildHasher, { - self.build_resource_path(path, |name| elements.get(name)) + profile_method!(resource_path_from_map); + self.build_resource_path(path, |name| values.get(name).map(AsRef::::as_ref)) } - fn parse_param(pattern: &str) -> (PatternElement, String, &str, bool) { + /// Parse path pattern and create a new instance. + fn from_single_pattern(pattern: &str, is_prefix: bool) -> Self { + profile_method!(from_single_pattern); + + let pattern = pattern.to_owned(); + let (pat_type, segments) = ResourceDef::parse(&pattern, is_prefix, false); + + ResourceDef { + id: 0, + name: None, + patterns: Patterns::Single(pattern), + pat_type, + segments: Some(segments), + } + } + + /// Parses a dynamic segment definition from a pattern. + /// + /// The returned tuple includes: + /// - the segment descriptor, either `Var` or `Tail` + /// - the segment's regex to check values against + /// - the remaining, unprocessed string slice + /// - whether the parsed parameter represents a tail pattern + /// + /// # Panics + /// Panics if given patterns does not contain a dynamic segment. + fn parse_param(pattern: &str) -> (PatternSegment, String, &str, bool) { + profile_method!(parse_param); + const DEFAULT_PATTERN: &str = "[^/]+"; const DEFAULT_PATTERN_TAIL: &str = ".*"; + let mut params_nesting = 0usize; let close_idx = pattern .find(|c| match c { @@ -340,103 +978,159 @@ impl ResourceDef { } _ => false, }) - .expect("malformed dynamic segment"); - let (mut param, mut rem) = pattern.split_at(close_idx + 1); - param = ¶m[1..param.len() - 1]; // Remove outer brackets - let tail = rem == "*"; + .unwrap_or_else(|| { + panic!(r#"path "{}" contains malformed dynamic segment"#, pattern) + }); + + let (mut param, mut unprocessed) = pattern.split_at(close_idx + 1); + + // remove outer curly brackets + param = ¶m[1..param.len() - 1]; + + let tail = unprocessed == "*"; let (name, pattern) = match param.find(':') { Some(idx) => { if tail { - panic!("Custom regex is not supported for remainder match"); + panic!("custom regex is not supported for tail match"); } + let (name, pattern) = param.split_at(idx); (name, &pattern[1..]) } None => ( param, if tail { - rem = &rem[1..]; + unprocessed = &unprocessed[1..]; DEFAULT_PATTERN_TAIL } else { DEFAULT_PATTERN }, ), }; - ( - PatternElement::Var(name.to_string()), - format!(r"(?P<{}>{})", &name, &pattern), - rem, - tail, - ) + + let segment = PatternSegment::Var(name.to_string()); + let regex = format!(r"(?P<{}>{})", &name, &pattern); + + (segment, regex, unprocessed, tail) } + /// Parse `pattern` using `is_prefix` and `force_dynamic` flags. + /// + /// Parameters: + /// - `is_prefix`: Use `true` if `pattern` should be treated as a prefix; i.e., a conforming + /// path will be a match even if it has parts remaining to process + /// - `force_dynamic`: Use `true` to disallow the return of static and prefix segments. + /// + /// The returned tuple includes: + /// - the pattern type detected, either `Static`, `Prefix`, or `Dynamic` + /// - a list of segment descriptors from the pattern fn parse( - mut pattern: &str, - mut for_prefix: bool, + pattern: &str, + is_prefix: bool, force_dynamic: bool, - ) -> (PatternType, Vec) { - if !force_dynamic && pattern.find('{').is_none() && !pattern.ends_with('*') { - let tp = if for_prefix { - PatternType::Prefix(String::from(pattern)) + ) -> (PatternType, Vec) { + profile_method!(parse); + + let mut unprocessed = pattern; + + if !force_dynamic && unprocessed.find('{').is_none() && !unprocessed.ends_with('*') { + // pattern is static + + let tp = if is_prefix { + PatternType::Prefix(unprocessed.to_owned()) } else { - PatternType::Static(String::from(pattern)) + PatternType::Static(unprocessed.to_owned()) }; - return (tp, vec![PatternElement::Const(String::from(pattern))]); + + return (tp, vec![PatternSegment::Const(unprocessed.to_owned())]); } - let pattern_orig = pattern; - let mut elements = Vec::new(); + let mut segments = Vec::new(); let mut re = format!("{}^", REGEX_FLAGS); - let mut dyn_elements = 0; + let mut dyn_segment_count = 0; + let mut has_tail_segment = false; - while let Some(idx) = pattern.find('{') { - let (prefix, rem) = pattern.split_at(idx); - elements.push(PatternElement::Const(String::from(prefix))); + while let Some(idx) = unprocessed.find('{') { + let (prefix, rem) = unprocessed.split_at(idx); + + segments.push(PatternSegment::Const(prefix.to_owned())); re.push_str(&escape(prefix)); + let (param_pattern, re_part, rem, tail) = Self::parse_param(rem); + if tail { - for_prefix = true; + has_tail_segment = true; } - elements.push(param_pattern); + segments.push(param_pattern); re.push_str(&re_part); - pattern = rem; - dyn_elements += 1; + + unprocessed = rem; + dyn_segment_count += 1; } - if let Some(path) = pattern.strip_suffix('*') { - elements.push(PatternElement::Const(String::from(path))); - re.push_str(&escape(path)); - re.push_str("(.*)"); - pattern = ""; + if is_prefix && has_tail_segment { + // tail segments in prefixes have no defined semantics + + #[cfg(not(test))] + log::warn!( + "Prefix resources should not have tail segments. \ + Use `ResourceDef::new` constructor. \ + This may become a panic in the future." + ); + + // panic in tests to make this case detectable + #[cfg(test)] + panic!("prefix resource definitions should not have tail segments"); } - elements.push(PatternElement::Const(String::from(pattern))); - re.push_str(&escape(pattern)); + if unprocessed.ends_with('*') { + // unnamed tail segment - if dyn_elements > MAX_DYNAMIC_SEGMENTS { + #[cfg(not(test))] + log::warn!( + "Tail segments must have names. \ + Consider `.../{{tail}}*`. \ + This may become a panic in the future." + ); + + // panic in tests to make this case detectable + #[cfg(test)] + panic!("tail segments must have names"); + } else if !has_tail_segment && !unprocessed.is_empty() { + // prevent `Const("")` element from being added after last dynamic segment + + segments.push(PatternSegment::Const(unprocessed.to_owned())); + re.push_str(&escape(unprocessed)); + } + + if dyn_segment_count > MAX_DYNAMIC_SEGMENTS { panic!( "Only {} dynamic segments are allowed, provided: {}", - MAX_DYNAMIC_SEGMENTS, dyn_elements + MAX_DYNAMIC_SEGMENTS, dyn_segment_count ); } - if !for_prefix { + if !is_prefix && !has_tail_segment { re.push('$'); } let re = match Regex::new(&re) { Ok(re) => re, - Err(err) => panic!("Wrong path pattern: \"{}\" {}", pattern_orig, err), + Err(err) => panic!("Wrong path pattern: \"{}\" {}", pattern, err), }; - // actix creates one router per thread + + // `Bok::leak(Box::new(name))` is an intentional memory leak. In typical applications the + // routing table is only constructed once (per worker) so leak is bounded. If you are + // constructing `ResourceDef`s more than once in your application's lifecycle you would + // expect a linear increase in leaked memory over time. let names = re .capture_names() .filter_map(|name| name.map(|name| Box::leak(Box::new(name.to_owned())).as_str())) .collect(); - (PatternType::Dynamic(re, names), elements) + (PatternType::Dynamic(re, names), segments) } } @@ -444,13 +1138,24 @@ impl Eq for ResourceDef {} impl PartialEq for ResourceDef { fn eq(&self, other: &ResourceDef) -> bool { - self.pattern == other.pattern + self.patterns == other.patterns + && match &self.pat_type { + PatternType::Static(_) => matches!(&other.pat_type, PatternType::Static(_)), + PatternType::Prefix(_) => matches!(&other.pat_type, PatternType::Prefix(_)), + PatternType::Dynamic(re, _) => match &other.pat_type { + PatternType::Dynamic(other_re, _) => re.as_str() == other_re.as_str(), + _ => false, + }, + PatternType::DynamicSet(_, _) => { + matches!(&other.pat_type, PatternType::DynamicSet(..)) + } + } } } impl Hash for ResourceDef { fn hash(&self, state: &mut H) { - self.pattern.hash(state); + self.patterns.hash(state); } } @@ -466,12 +1171,24 @@ impl From for ResourceDef { } } -pub(crate) fn insert_slash(path: &str) -> String { - let mut path = path.to_owned(); +pub(crate) fn insert_slash(path: &str) -> Cow<'_, str> { + profile_fn!(insert_slash); + if !path.is_empty() && !path.starts_with('/') { - path.insert(0, '/'); - }; - path + let mut new_path = String::with_capacity(path.len() + 1); + new_path.push('/'); + new_path.push_str(path); + Cow::Owned(new_path) + } else { + Cow::Borrowed(path) + } +} + +/// Returns true if `prefix` acts as a proper prefix (i.e., separated by a slash) in `path`. +/// +/// The `strict` refers to the fact that this will return `false` if `prefix == path`. +fn is_strict_prefix(prefix: &str, path: &str) -> bool { + path.starts_with(prefix) && (prefix.ends_with('/') || path[prefix.len()..].starts_with('/')) } #[cfg(test)] @@ -479,10 +1196,47 @@ mod tests { use super::*; #[test] - fn test_parse_static() { + fn equivalence() { + assert_eq!( + ResourceDef::root_prefix("/root"), + ResourceDef::prefix("/root") + ); + assert_eq!( + ResourceDef::root_prefix("root"), + ResourceDef::prefix("/root") + ); + assert_eq!( + ResourceDef::root_prefix("/{id}"), + ResourceDef::prefix("/{id}") + ); + assert_eq!( + ResourceDef::root_prefix("{id}"), + ResourceDef::prefix("/{id}") + ); + + assert_eq!(ResourceDef::new("/"), ResourceDef::new(["/"])); + assert_eq!(ResourceDef::new("/"), ResourceDef::new(vec!["/"])); + + assert_ne!(ResourceDef::new(""), ResourceDef::prefix("")); + assert_ne!(ResourceDef::new("/"), ResourceDef::prefix("/")); + assert_ne!(ResourceDef::new("/{id}"), ResourceDef::prefix("/{id}")); + } + + #[test] + fn parse_static() { + let re = ResourceDef::new(""); + + assert!(!re.is_prefix()); + + assert!(re.is_match("")); + assert!(!re.is_match("/")); + assert_eq!(re.find_match(""), Some(0)); + assert_eq!(re.find_match("/"), None); + let re = ResourceDef::new("/"); assert!(re.is_match("/")); - assert!(!re.is_match("/a")); + assert!(!re.is_match("")); + assert!(!re.is_match("/foo")); let re = ResourceDef::new("/name"); assert!(re.is_match("/name")); @@ -491,13 +1245,13 @@ mod tests { assert!(!re.is_match("/name~")); let mut path = Path::new("/name"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.unprocessed(), ""); - assert_eq!(re.is_prefix_match("/name"), Some(5)); - assert_eq!(re.is_prefix_match("/name1"), None); - assert_eq!(re.is_prefix_match("/name/"), None); - assert_eq!(re.is_prefix_match("/name~"), None); + assert_eq!(re.find_match("/name"), Some(5)); + assert_eq!(re.find_match("/name1"), None); + assert_eq!(re.find_match("/name/"), None); + assert_eq!(re.find_match("/name~"), None); let re = ResourceDef::new("/name/"); assert!(re.is_match("/name/")); @@ -509,12 +1263,12 @@ mod tests { assert!(!re.is_match("/user/profile/profile")); let mut path = Path::new("/user/profile"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.unprocessed(), ""); } #[test] - fn test_parse_param() { + fn parse_param() { let re = ResourceDef::new("/user/{id}"); assert!(re.is_match("/user/profile")); assert!(re.is_match("/user/2345")); @@ -522,12 +1276,12 @@ mod tests { assert!(!re.is_match("/user/2345/sdg")); let mut path = Path::new("/user/profile"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "profile"); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/user/1245125"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "1245125"); assert_eq!(path.unprocessed(), ""); @@ -537,7 +1291,7 @@ mod tests { assert!(!re.is_match("/resource")); let mut path = Path::new("/v151/resource/adage32"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("version").unwrap(), "151"); assert_eq!(path.get("id").unwrap(), "adage32"); assert_eq!(path.unprocessed(), ""); @@ -549,14 +1303,14 @@ mod tests { assert!(!re.is_match("/XXXXXX")); let mut path = Path::new("/012345"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "012345"); assert_eq!(path.unprocessed(), ""); } #[allow(clippy::cognitive_complexity)] #[test] - fn test_dynamic_set() { + fn dynamic_set() { let re = ResourceDef::new(vec![ "/user/{id}", "/v{version}/resource/{id}", @@ -569,12 +1323,12 @@ mod tests { assert!(!re.is_match("/user/2345/sdg")); let mut path = Path::new("/user/profile"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "profile"); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/user/1245125"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "1245125"); assert_eq!(path.unprocessed(), ""); @@ -583,7 +1337,7 @@ mod tests { assert!(!re.is_match("/resource")); let mut path = Path::new("/v151/resource/adage32"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("version").unwrap(), "151"); assert_eq!(path.get("id").unwrap(), "adage32"); @@ -597,7 +1351,7 @@ mod tests { assert!(!re.is_match("/static/a")); let mut path = Path::new("/012345"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "012345"); let re = ResourceDef::new([ @@ -622,199 +1376,428 @@ mod tests { } #[test] - fn test_parse_tail() { + fn parse_tail() { let re = ResourceDef::new("/user/-{id}*"); let mut path = Path::new("/user/-profile"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "profile"); let mut path = Path::new("/user/-2345"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "2345"); let mut path = Path::new("/user/-2345/"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "2345/"); let mut path = Path::new("/user/-2345/sdg"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "2345/sdg"); } #[test] - fn test_static_tail() { - let re = ResourceDef::new("/user*"); + fn static_tail() { + let re = ResourceDef::new("/user{tail}*"); + assert!(re.is_match("/users")); + assert!(re.is_match("/user-foo")); assert!(re.is_match("/user/profile")); assert!(re.is_match("/user/2345")); assert!(re.is_match("/user/2345/")); assert!(re.is_match("/user/2345/sdg")); + assert!(!re.is_match("/foo/profile")); - let re = ResourceDef::new("/user/*"); + let re = ResourceDef::new("/user/{tail}*"); assert!(re.is_match("/user/profile")); assert!(re.is_match("/user/2345")); assert!(re.is_match("/user/2345/")); assert!(re.is_match("/user/2345/sdg")); - - let re = ResourceDef::new("/user/{id}/*"); - assert!(!re.is_match("/user/2345")); - let mut path = Path::new("/user/2345/sdg"); - assert!(re.match_path(&mut path)); - assert_eq!(path.get("id").unwrap(), "2345"); + assert!(!re.is_match("/foo/profile")); } #[test] - fn test_newline() { + fn dynamic_tail() { + let re = ResourceDef::new("/user/{id}/{tail}*"); + assert!(!re.is_match("/user/2345")); + let mut path = Path::new("/user/2345/sdg"); + assert!(re.capture_match_info(&mut path)); + assert_eq!(path.get("id").unwrap(), "2345"); + assert_eq!(path.get("tail").unwrap(), "sdg"); + assert_eq!(path.unprocessed(), ""); + } + + #[test] + fn newline_patterns_and_paths() { let re = ResourceDef::new("/user/a\nb"); assert!(re.is_match("/user/a\nb")); assert!(!re.is_match("/user/a\nb/profile")); let re = ResourceDef::new("/a{x}b/test/a{y}b"); let mut path = Path::new("/a\nb/test/a\nb"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("x").unwrap(), "\n"); assert_eq!(path.get("y").unwrap(), "\n"); - let re = ResourceDef::new("/user/*"); + let re = ResourceDef::new("/user/{tail}*"); assert!(re.is_match("/user/a\nb/")); let re = ResourceDef::new("/user/{id}*"); let mut path = Path::new("/user/a\nb/a\nb"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "a\nb/a\nb"); let re = ResourceDef::new("/user/{id:.*}"); let mut path = Path::new("/user/a\nb/a\nb"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "a\nb/a\nb"); } #[cfg(feature = "http")] #[test] - fn test_parse_urlencoded_param() { + fn parse_urlencoded_param() { use std::convert::TryFrom; let re = ResourceDef::new("/user/{id}/test"); let mut path = Path::new("/user/2345/test"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "2345"); let mut path = Path::new("/user/qwe%25/test"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "qwe%25"); let uri = http::Uri::try_from("/user/qwe%25/test").unwrap(); let mut path = Path::new(uri); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.get("id").unwrap(), "qwe%25"); } #[test] - fn test_resource_prefix() { + fn prefix_static() { let re = ResourceDef::prefix("/name"); + + assert!(re.is_prefix()); + assert!(re.is_match("/name")); assert!(re.is_match("/name/")); assert!(re.is_match("/name/test/test")); - assert!(re.is_match("/name1")); - assert!(re.is_match("/name~")); + assert!(!re.is_match("/name1")); + assert!(!re.is_match("/name~")); let mut path = Path::new("/name"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/name/test"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(path.unprocessed(), "/test"); - assert_eq!(re.is_prefix_match("/name"), Some(5)); - assert_eq!(re.is_prefix_match("/name/"), Some(5)); - assert_eq!(re.is_prefix_match("/name/test/test"), Some(5)); - assert_eq!(re.is_prefix_match("/name1"), None); - assert_eq!(re.is_prefix_match("/name~"), None); + assert_eq!(re.find_match("/name"), Some(5)); + assert_eq!(re.find_match("/name/"), Some(5)); + assert_eq!(re.find_match("/name/test/test"), Some(5)); + assert_eq!(re.find_match("/name1"), None); + assert_eq!(re.find_match("/name~"), None); let re = ResourceDef::prefix("/name/"); assert!(re.is_match("/name/")); assert!(re.is_match("/name/gs")); assert!(!re.is_match("/name")); + let mut path = Path::new("/name/gs"); + assert!(re.capture_match_info(&mut path)); + assert_eq!(path.unprocessed(), "gs"); + let re = ResourceDef::root_prefix("name/"); assert!(re.is_match("/name/")); assert!(re.is_match("/name/gs")); assert!(!re.is_match("/name")); let mut path = Path::new("/name/gs"); - assert!(re.match_path(&mut path)); - assert_eq!(path.unprocessed(), "/gs"); + assert!(re.capture_match_info(&mut path)); + assert_eq!(path.unprocessed(), "gs"); } #[test] - fn test_resource_prefix_dynamic() { + fn prefix_dynamic() { let re = ResourceDef::prefix("/{name}/"); + + assert!(re.is_prefix()); + assert!(re.is_match("/name/")); assert!(re.is_match("/name/gs")); assert!(!re.is_match("/name")); - assert_eq!(re.is_prefix_match("/name/"), Some(6)); - assert_eq!(re.is_prefix_match("/name/gs"), Some(6)); - assert_eq!(re.is_prefix_match("/name"), None); + assert_eq!(re.find_match("/name/"), Some(6)); + assert_eq!(re.find_match("/name/gs"), Some(6)); + assert_eq!(re.find_match("/name"), None); let mut path = Path::new("/test2/"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(&path["name"], "test2"); assert_eq!(&path[0], "test2"); assert_eq!(path.unprocessed(), ""); let mut path = Path::new("/test2/subpath1/subpath2/index.html"); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); assert_eq!(&path["name"], "test2"); assert_eq!(&path[0], "test2"); assert_eq!(path.unprocessed(), "subpath1/subpath2/index.html"); + + let resource = ResourceDef::prefix("/user"); + // input string shorter than prefix + assert!(resource.find_match("/foo").is_none()); } #[test] - fn test_resource_path() { + fn build_path_list() { let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/test"); - assert!(resource.resource_path(&mut s, &mut (&["user1"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut (&["user1"]).iter())); assert_eq!(s, "/user/user1/test"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}/test"); - assert!(resource.resource_path(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); assert_eq!(s, "/user/item/item2/test"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}"); - assert!(resource.resource_path(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); assert_eq!(s, "/user/item/item2"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}/"); - assert!(resource.resource_path(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); assert_eq!(s, "/user/item/item2/"); let mut s = String::new(); - assert!(!resource.resource_path(&mut s, &mut (&["item"]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut (&["item"]).iter())); let mut s = String::new(); - assert!(resource.resource_path(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); assert_eq!(s, "/user/item/item2/"); - assert!(!resource.resource_path(&mut s, &mut (&["item"]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut (&["item"]).iter())); let mut s = String::new(); - assert!(resource.resource_path(&mut s, &mut vec!["item", "item2"].into_iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut vec!["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/"); + } + + #[test] + fn multi_pattern_cannot_build_path() { + let resource = ResourceDef::new(["/user/{id}", "/profile/{id}"]); + let mut s = String::new(); + assert!(!resource.resource_path_from_iter(&mut s, &mut ["123"].iter())); + } + + #[test] + fn multi_pattern_capture_segment_values() { + let resource = ResourceDef::new(["/user/{id}", "/profile/{id}"]); + + let mut path = Path::new("/user/123"); + assert!(resource.capture_match_info(&mut path)); + assert!(path.get("id").is_some()); + + let mut path = Path::new("/profile/123"); + assert!(resource.capture_match_info(&mut path)); + assert!(path.get("id").is_some()); + + let resource = ResourceDef::new(["/user/{id}", "/profile/{uid}"]); + + let mut path = Path::new("/user/123"); + assert!(resource.capture_match_info(&mut path)); + assert!(path.get("id").is_some()); + assert!(path.get("uid").is_none()); + + let mut path = Path::new("/profile/123"); + assert!(resource.capture_match_info(&mut path)); + assert!(path.get("id").is_none()); + assert!(path.get("uid").is_some()); + } + + #[test] + fn dynamic_prefix_proper_segmentation() { + let resource = ResourceDef::prefix(r"/id/{id:\d{3}}"); + + assert!(resource.is_match("/id/123")); + assert!(resource.is_match("/id/123/foo")); + assert!(!resource.is_match("/id/1234")); + assert!(!resource.is_match("/id/123a")); + + assert_eq!(resource.find_match("/id/123"), Some(7)); + assert_eq!(resource.find_match("/id/123/foo"), Some(7)); + assert_eq!(resource.find_match("/id/1234"), None); + assert_eq!(resource.find_match("/id/123a"), None); + } + + #[test] + fn build_path_map() { + let resource = ResourceDef::new("/user/{item1}/{item2}/"); let mut map = HashMap::new(); map.insert("item1", "item"); let mut s = String::new(); - assert!(!resource.resource_path_named(&mut s, &map)); + assert!(!resource.resource_path_from_map(&mut s, &map)); + + map.insert("item2", "item2"); let mut s = String::new(); - map.insert("item2", "item2"); - assert!(resource.resource_path_named(&mut s, &map)); + assert!(resource.resource_path_from_map(&mut s, &map)); assert_eq!(s, "/user/item/item2/"); } + + #[test] + fn build_path_tail() { + let resource = ResourceDef::new("/user/{item1}*"); + + let mut s = String::new(); + assert!(!resource.resource_path_from_iter(&mut s, &mut (&[""; 0]).iter())); + + let mut s = String::new(); + assert!(resource.resource_path_from_iter(&mut s, &mut (&["user1"]).iter())); + assert_eq!(s, "/user/user1"); + + let mut s = String::new(); + let mut map = HashMap::new(); + map.insert("item1", "item"); + assert!(resource.resource_path_from_map(&mut s, &map)); + assert_eq!(s, "/user/item"); + } + + #[test] + fn consistent_match_length() { + let result = Some(5); + + let re = ResourceDef::prefix("/abc/"); + assert_eq!(re.find_match("/abc/def"), result); + + let re = ResourceDef::prefix("/{id}/"); + assert_eq!(re.find_match("/abc/def"), result); + } + + #[test] + fn join() { + // test joined defs match the same paths as each component separately + + fn seq_find_match(re1: &ResourceDef, re2: &ResourceDef, path: &str) -> Option { + let len1 = re1.find_match(path)?; + let len2 = re2.find_match(&path[len1..])?; + Some(len1 + len2) + } + + macro_rules! join_test { + ($pat1:expr, $pat2:expr => $($test:expr),+) => {{ + let pat1 = $pat1; + let pat2 = $pat2; + $({ + let _path = $test; + let (re1, re2) = (ResourceDef::prefix(pat1), ResourceDef::new(pat2)); + let _seq = seq_find_match(&re1, &re2, _path); + let _join = re1.join(&re2).find_match(_path); + assert_eq!( + _seq, _join, + "patterns: prefix {:?}, {:?}; mismatch on \"{}\"; seq={:?}; join={:?}", + pat1, pat2, _path, _seq, _join + ); + assert!(!re1.join(&re2).is_prefix()); + + let (re1, re2) = (ResourceDef::prefix(pat1), ResourceDef::prefix(pat2)); + let _seq = seq_find_match(&re1, &re2, _path); + let _join = re1.join(&re2).find_match(_path); + assert_eq!( + _seq, _join, + "patterns: prefix {:?}, prefix {:?}; mismatch on \"{}\"; seq={:?}; join={:?}", + pat1, pat2, _path, _seq, _join + ); + assert!(re1.join(&re2).is_prefix()); + })+ + }} + } + + join_test!("", "" => "", "/hello", "/"); + join_test!("/user", "" => "", "/user", "/user/123", "/user11", "user", "user/123"); + join_test!("", "/user"=> "", "/user", "foo", "/user11", "user", "user/123"); + join_test!("/user", "/xx"=> "", "", "/", "/user", "/xx", "/userxx", "/user/xx"); + } + + #[test] + fn match_methods_agree() { + macro_rules! match_methods_agree { + ($pat:expr => $($test:expr),+) => {{ + match_methods_agree!(finish $pat, ResourceDef::new($pat), $($test),+); + }}; + (prefix $pat:expr => $($test:expr),+) => {{ + match_methods_agree!(finish $pat, ResourceDef::prefix($pat), $($test),+); + }}; + (finish $pat:expr, $re:expr, $($test:expr),+) => {{ + let re = $re; + $({ + let _is = re.is_match($test); + let _find = re.find_match($test).is_some(); + assert_eq!( + _is, _find, + "pattern: {:?}; mismatch on \"{}\"; is={}; find={}", + $pat, $test, _is, _find + ); + })+ + }} + } + + match_methods_agree!("" => "", "/", "/foo"); + match_methods_agree!("/" => "", "/", "/foo"); + match_methods_agree!("/user" => "user", "/user", "/users", "/user/123", "/foo"); + match_methods_agree!("/v{v}" => "v", "/v", "/v1", "/v222", "/foo"); + match_methods_agree!(["/v{v}", "/version/{v}"] => "/v", "/v1", "/version", "/version/1", "/foo"); + + match_methods_agree!("/path{tail}*" => "/path", "/path1", "/path/123"); + match_methods_agree!("/path/{tail}*" => "/path", "/path1", "/path/123"); + + match_methods_agree!(prefix "" => "", "/", "/foo"); + match_methods_agree!(prefix "/user" => "user", "/user", "/users", "/user/123", "/foo"); + match_methods_agree!(prefix r"/id/{id:\d{3}}" => "/id/123", "/id/1234"); + } + + #[test] + #[should_panic] + fn invalid_dynamic_segment_delimiter() { + ResourceDef::new("/user/{username"); + } + + #[test] + #[should_panic] + fn invalid_dynamic_segment_name() { + ResourceDef::new("/user/{}"); + } + + #[test] + #[should_panic] + fn invalid_too_many_dynamic_segments() { + // valid + ResourceDef::new("/{a}/{b}/{c}/{d}/{e}/{f}/{g}/{h}/{i}/{j}/{k}/{l}/{m}/{n}/{o}/{p}"); + + // panics + ResourceDef::new( + "/{a}/{b}/{c}/{d}/{e}/{f}/{g}/{h}/{i}/{j}/{k}/{l}/{m}/{n}/{o}/{p}/{q}", + ); + } + + #[test] + #[should_panic] + fn invalid_custom_regex_for_tail() { + ResourceDef::new(r"/{tail:\d+}*"); + } + + #[test] + #[should_panic] + fn invalid_unnamed_tail_segment() { + ResourceDef::new("/*"); + } + + #[test] + #[should_panic] + fn prefix_plus_tail_match_is_allowed() { + ResourceDef::prefix("/user/{id}*"); + } } diff --git a/actix-router/src/router.rs b/actix-router/src/router.rs index aeb1aa36..f5deb858 100644 --- a/actix-router/src/router.rs +++ b/actix-router/src/router.rs @@ -1,4 +1,6 @@ -use crate::{IntoPattern, Resource, ResourceDef, ResourcePath}; +use firestorm::profile_method; + +use crate::{IntoPatterns, Resource, ResourceDef, ResourcePath}; #[derive(Debug, Copy, Clone, PartialEq)] pub struct ResourceId(pub u16); @@ -10,7 +12,11 @@ pub struct ResourceInfo { } /// Resource router. -pub struct Router(Vec<(ResourceDef, T, Option)>); +// T is the resource itself +// U is any other data needed for routing like method guards +pub struct Router { + routes: Vec<(ResourceDef, T, Option)>, +} impl Router { pub fn build() -> RouterBuilder { @@ -24,11 +30,14 @@ impl Router { R: Resource

, P: ResourcePath, { - for item in self.0.iter() { - if item.0.match_path(resource.resource_path()) { + profile_method!(recognize); + + for item in self.routes.iter() { + if item.0.capture_match_info(resource.resource_path()) { return Some((&item.1, ResourceId(item.0.id()))); } } + None } @@ -37,33 +46,35 @@ impl Router { R: Resource

, P: ResourcePath, { - for item in self.0.iter_mut() { - if item.0.match_path(resource.resource_path()) { + profile_method!(recognize_mut); + + for item in self.routes.iter_mut() { + if item.0.capture_match_info(resource.resource_path()) { return Some((&mut item.1, ResourceId(item.0.id()))); } } + None } - pub fn recognize_checked( - &self, - resource: &mut R, - check: F, - ) -> Option<(&T, ResourceId)> + pub fn recognize_fn(&self, resource: &mut R, check: F) -> Option<(&T, ResourceId)> where F: Fn(&R, &Option) -> bool, R: Resource

, P: ResourcePath, { - for item in self.0.iter() { - if item.0.match_path_checked(resource, &check, &item.2) { + profile_method!(recognize_checked); + + for item in self.routes.iter() { + if item.0.capture_match_info_fn(resource, &check, &item.2) { return Some((&item.1, ResourceId(item.0.id()))); } } + None } - pub fn recognize_mut_checked( + pub fn recognize_mut_fn( &mut self, resource: &mut R, check: F, @@ -73,11 +84,14 @@ impl Router { R: Resource

, P: ResourcePath, { - for item in self.0.iter_mut() { - if item.0.match_path_checked(resource, &check, &item.2) { + profile_method!(recognize_mut_checked); + + for item in self.routes.iter_mut() { + if item.0.capture_match_info_fn(resource, &check, &item.2) { return Some((&mut item.1, ResourceId(item.0.id()))); } } + None } } @@ -88,11 +102,13 @@ pub struct RouterBuilder { impl RouterBuilder { /// Register resource for specified path. - pub fn path( + pub fn path( &mut self, path: P, resource: T, ) -> &mut (ResourceDef, T, Option) { + profile_method!(path); + self.resources .push((ResourceDef::new(path), resource, None)); self.resources.last_mut().unwrap() @@ -100,6 +116,8 @@ impl RouterBuilder { /// Register resource for specified path prefix. pub fn prefix(&mut self, prefix: &str, resource: T) -> &mut (ResourceDef, T, Option) { + profile_method!(prefix); + self.resources .push((ResourceDef::prefix(prefix), resource, None)); self.resources.last_mut().unwrap() @@ -107,13 +125,17 @@ impl RouterBuilder { /// Register resource for ResourceDef pub fn rdef(&mut self, rdef: ResourceDef, resource: T) -> &mut (ResourceDef, T, Option) { + profile_method!(rdef); + self.resources.push((rdef, resource, None)); self.resources.last_mut().unwrap() } /// Finish configuration and create router instance. pub fn finish(self) -> Router { - Router(self.resources) + Router { + routes: self.resources, + } } } diff --git a/actix-router/src/url.rs b/actix-router/src/url.rs index 130ac76f..e08a7171 100644 --- a/actix-router/src/url.rs +++ b/actix-router/src/url.rs @@ -206,12 +206,12 @@ mod tests { let re = ResourceDef::new(pattern); let uri = Uri::try_from(url.as_ref()).unwrap(); let mut path = Path::new(Url::new(uri)); - assert!(re.match_path(&mut path)); + assert!(re.capture_match_info(&mut path)); path } fn percent_encode(data: &[u8]) -> String { - data.into_iter().map(|c| format!("%{:02X}", c)).collect() + data.iter().map(|c| format!("%{:02X}", c)).collect() } #[test] @@ -221,7 +221,7 @@ mod tests { let path = match_url(re, "/user/2345/test"); assert_eq!(path.get("id").unwrap(), "2345"); - // "%25" should never be decoded into '%' to gurantee the output is a valid + // "%25" should never be decoded into '%' to guarantee the output is a valid // percent-encoded format let path = match_url(re, "/user/qwe%25/test"); assert_eq!(path.get("id").unwrap(), "qwe%25"); diff --git a/actix-rt/tests/tests.rs b/actix-rt/tests/tests.rs index b82a4380..285c4074 100644 --- a/actix-rt/tests/tests.rs +++ b/actix-rt/tests/tests.rs @@ -287,7 +287,7 @@ fn new_arbiter_with_tokio() { arb.join().unwrap(); - assert_eq!(false, counter.load(Ordering::SeqCst)); + assert!(!counter.load(Ordering::SeqCst)); } #[test] diff --git a/actix-server/src/worker.rs b/actix-server/src/worker.rs index 79f15b16..a974522a 100644 --- a/actix-server/src/worker.rs +++ b/actix-server/src/worker.rs @@ -73,7 +73,7 @@ fn handle_pair( /// On reaching counter limit worker would use `mio::Waker` and `WakerQueue` to wake up `Accept` /// and notify it to update cached `Availability` again to mark worker as able to accept work again. /// -/// Hense a wake up would only happen after `Accept` increment it to limit. +/// Hence, a wake up would only happen after `Accept` increment it to limit. /// And a decrement to limit always wake up `Accept`. #[derive(Clone)] pub(crate) struct Counter { @@ -179,8 +179,9 @@ impl WorkerHandleAccept { /// Handle to worker than can send stop message to worker. /// /// Held by [ServerBuilder](crate::builder::ServerBuilder). +#[derive(Debug)] pub(crate) struct WorkerHandleServer { - pub idx: usize, + idx: usize, tx: UnboundedSender, } diff --git a/actix-service/src/macros.rs b/actix-service/src/macros.rs index e23b2960..6cf3ef08 100644 --- a/actix-service/src/macros.rs +++ b/actix-service/src/macros.rs @@ -1,5 +1,9 @@ /// An implementation of [`poll_ready`]() that always signals readiness. /// +/// This should only be used for basic leaf services that have no concept of un-readiness. +/// For wrapper or other serivice types, use [`forward_ready!`] for simple cases or write a bespoke +/// `poll_ready` implementation. +/// /// [`poll_ready`]: crate::Service::poll_ready /// /// # Examples