rework resourcedef (#373)

.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"

.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

actix-router/CHANGES.md

@@ -1,15 +1,23 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
-* Resource definitions with unnamed tail segments now correctly interpolate the tail when constructed from an iterator. [#371]
-* Introduce `ResourceDef::resource_path_from_map_with_tail` method to allow building paths in the presence of unnamed tail segments. [#371]
+* 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]
-* Re-work `IntoPatterns` trait. [#372]
+* Fix `ResourceDef` `PartialEq` implementation.
+* 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]
-* Alias `ResourceDef::{resource_path => resource_path_from_iter}` pending eventual deprecation. [#371]
-* Alias `ResourceDef::{resource_path_named => resource_path_from_map}` pending eventual deprecation. [#371]
+* 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
@@ -17,6 +25,7 @@
 [#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

actix-router/examples/flamegraph.rs

@@ -140,7 +140,7 @@ macro_rules! register {
     }};
 }
 
-static PATHS: [&'static str; 5] = [
+static PATHS: [&str; 5] = [
     "/authorizations",
     "/user/repos",
     "/repos/rust-lang/rust/stargazers",

actix-router/src/lib.rs

@@ -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<T: ResourcePath> {
     fn resource_path(&mut self) -> &mut Path<T>;
 }
@@ -41,7 +43,7 @@ impl ResourcePath for bytestring::ByteString {
 }
 
 /// One or many patterns.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum Patterns {
     Single(String),
     List(Vec<String>),
@@ -70,6 +72,12 @@ impl<'a> IntoPatterns for &'a str {
     }
 }
 
+impl IntoPatterns for bytestring::ByteString {
+    fn patterns(&self) -> Patterns {
+        Patterns::Single(self.to_string())
+    }
+}
+
 impl<T: AsRef<str>> IntoPatterns for Vec<T> {
     fn patterns(&self) -> Patterns {
         let mut patterns = self.iter().map(|v| v.as_ref().to_owned());

actix-router/src/path.rs

@@ -4,8 +4,7 @@ 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 {
@@ -120,24 +119,21 @@ impl<T: ResourcePath> Path<T> {
     }
 
     /// Get matched parameter by name without type conversion
-    pub fn get(&self, key: &str) -> Option<&str> {
+    pub fn get(&self, name: &str) -> Option<&str> {
         profile_method!(get);
-        
-        for item in self.segments.iter() {
-            if key == item.0 {
-                return match item.1 {
+
+        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
@@ -150,7 +146,7 @@ impl<T: ResourcePath> Path<T> {
     /// 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 {

actix-router/src/router.rs

@@ -29,10 +29,11 @@ impl<T, U> Router<T, U> {
         profile_method!(recognize);
 
         for item in self.0.iter() {
-            if item.0.match_path(resource.resource_path()) {
+            if item.0.capture_match_info(resource.resource_path()) {
                 return Some((&item.1, ResourceId(item.0.id())));
             }
         }
+
         None
     }
 
@@ -44,18 +45,15 @@ impl<T, U> Router<T, U> {
         profile_method!(recognize_mut);
 
         for item in self.0.iter_mut() {
-            if item.0.match_path(resource.resource_path()) {
+            if item.0.capture_match_info(resource.resource_path()) {
                 return Some((&mut item.1, ResourceId(item.0.id())));
             }
         }
+
         None
     }
 
-    pub fn recognize_checked<R, P, F>(
-        &self,
-        resource: &mut R,
-        check: F,
-    ) -> Option<(&T, ResourceId)>
+    pub fn recognize_fn<R, P, F>(&self, resource: &mut R, check: F) -> Option<(&T, ResourceId)>
     where
         F: Fn(&R, &Option<U>) -> bool,
         R: Resource<P>,
@@ -64,14 +62,15 @@ impl<T, U> Router<T, U> {
         profile_method!(recognize_checked);
 
         for item in self.0.iter() {
-            if item.0.match_path_checked(resource, &check, &item.2) {
+            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<R, P, F>(
+    pub fn recognize_mut_fn<R, P, F>(
         &mut self,
         resource: &mut R,
         check: F,
@@ -84,10 +83,11 @@ impl<T, U> Router<T, U> {
         profile_method!(recognize_mut_checked);
 
         for item in self.0.iter_mut() {
-            if item.0.match_path_checked(resource, &check, &item.2) {
+            if item.0.capture_match_info_fn(resource, &check, &item.2) {
                 return Some((&mut item.1, ResourceId(item.0.id())));
             }
         }
+
         None
     }
 }

actix-router/src/url.rs

@@ -206,7 +206,7 @@ mod tests {
         let re = ResourceDef::new(pattern);
         let uri = Uri::try_from(url.as_ref()).unwrap();
         let mut path = Path::new(Url::new(uri));
-        assert!(re.match_path(&mut path));
+        assert!(re.capture_match_info(&mut path));
         path
     }
 
@@ -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");