From dba20e24d6656389d09d93b05c8ff9c89333f399 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 19 Jul 2021 17:10:50 +0100 Subject: [PATCH] fix partialeq implementation --- actix-router/CHANGES.md | 3 +- actix-router/src/lib.rs | 6 ++ actix-router/src/resource.rs | 162 +++++++++++++++++++++++++++++------ actix-router/src/router.rs | 6 +- 4 files changed, 143 insertions(+), 34 deletions(-) diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index 6312e22d..27637128 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -6,7 +6,8 @@ * 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 in multi-patterns where static patterns are interpreted as regex. [#366] +* Fix a bug in multi-patterns where static patterns are interpreted as regex. [#366] +* Fix `ResourceDef` `PartialEq` implementation. * Re-work `IntoPatterns` trait. [#372] * Rename `Path::{len => segment_count}` to be more descriptive of it's purpose. [#370] * Rename `ResourceDef::{resource_path => resource_path_from_iter}`. [#371] diff --git a/actix-router/src/lib.rs b/actix-router/src/lib.rs index 3f54b135..6082c1a3 100644 --- a/actix-router/src/lib.rs +++ b/actix-router/src/lib.rs @@ -72,6 +72,12 @@ impl<'a> IntoPatterns for &'a str { } } +impl IntoPatterns for bytestring::ByteString { + fn patterns(&self) -> Patterns { + Patterns::Single(self.to_string()) + } +} + impl> IntoPatterns for Vec { fn patterns(&self) -> Patterns { let mut patterns = self.iter().map(|v| v.as_ref().to_owned()); diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index b2815be0..ad0a5f49 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -20,18 +20,78 @@ const MAX_DYNAMIC_SEGMENTS: usize = 16; /// See the docs under: https://docs.rs/regex/1.5.4/regex/#grouping-and-flags const REGEX_FLAGS: &str = "(?s-m)"; -/// Describes an entry in a resource table. -/// -/// # Dynamic Segments -/// TODO -/// -/// Resource definition can contain at most 16 dynamic segments. -/// -/// # Tail Segments -/// TODO -/// -/// # Multi-Pattern Resources -/// TODO +/** +Describes an entry in a resource table. + +# Static Resources +A static resource is the most basic type of definition. + +## 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")); +``` + +# Prefix Resources +TODO + +## 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")); +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 with [prefix][Self::prefix]) since the default pattern for segments only matches up +to the next `/` character. See the next section for more on custom segment patterns. + +A resource definition can contain at most 16 dynamic segments. + +## Examples +``` +# use actix_router::ResourceDef; +let resource = ResourceDef::prefix("/user/{id}"); + +assert!(resource.is_match("/user/123")); + +assert!(!resource.is_match("/user")); +assert!(!resource.is_match("/homes")); +assert!(!resource.is_match("/search")); +``` + +# Custom Regex Segments +TODO + +# Tail Segments +TODO + +# Multi-Pattern Resources +TODO + +# Trailing Slashes +basically they matter, be consistent in definitions or try to normalize +TODO +*/ #[derive(Clone, Debug)] pub struct ResourceDef { id: u16, @@ -187,19 +247,15 @@ impl ResourceDef { /// ``` /// use actix_router::ResourceDef; /// - /// let resource = ResourceDef::root_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::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/stars")); /// assert!(!resource.is_match("user/123")); - /// assert!(!resource.is_match("user/123/stars")); - /// assert!(!resource.is_match("foo")); /// ``` pub fn root_prefix(path: &str) -> Self { profile_method!(root_prefix); @@ -279,8 +335,13 @@ impl ResourceDef { /// # use actix_router::ResourceDef; /// assert!(ResourceDef::prefix("/user").is_prefix()); /// assert!(!ResourceDef::new("/user").is_prefix()); + /// ``` pub fn is_prefix(&self) -> bool { - matches!(&self.pat_type, &PatternType::Prefix(_)) + match &self.pat_type { + PatternType::Prefix(_) => true, + PatternType::Dynamic(re, _) if !re.as_str().ends_with('$') => true, + _ => false, + } } /// Returns the pattern string that generated the resource definition. @@ -639,10 +700,6 @@ impl ResourceDef { /// Assembles resource path from map of dynamic segment values. /// /// Returns `true` on success. - /// - /// If resource pattern has an unnamed tail segment, path building will fail. - /// See [`resource_path_from_map_with_tail`][Self::resource_path_from_map_with_tail] for a - /// variant of this function that accepts a tail parameter. pub fn resource_path_from_map( &self, path: &mut String, @@ -842,6 +899,17 @@ impl Eq for ResourceDef {} impl PartialEq for ResourceDef { fn eq(&self, other: &ResourceDef) -> bool { 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(..)) + } + } } } @@ -887,9 +955,35 @@ fn is_strict_prefix(prefix: &str, path: &str) -> bool { mod tests { use super::*; + #[test] + 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_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)); @@ -1087,7 +1181,7 @@ mod tests { } #[test] - fn newline() { + 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")); @@ -1137,6 +1231,8 @@ mod tests { 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")); @@ -1180,6 +1276,8 @@ mod tests { 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")); @@ -1200,6 +1298,10 @@ mod tests { assert_eq!(&path[0], "test2"); assert_eq!(path.unprocessed(), "subpath1/subpath2/index.html"); + let resource = ResourceDef::prefix(r"/id/{id:\d{3}}"); + assert!(resource.is_match("/id/1234")); + assert_eq!(resource.find_match("/id/1234"), Some(7)); + let resource = ResourceDef::prefix("/user"); // input string shorter than prefix assert!(resource.find_match("/foo").is_none()); @@ -1317,8 +1419,12 @@ mod tests { 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] diff --git a/actix-router/src/router.rs b/actix-router/src/router.rs index 73f4d2c7..057f7e17 100644 --- a/actix-router/src/router.rs +++ b/actix-router/src/router.rs @@ -53,11 +53,7 @@ impl Router { None } - pub fn recognize_fn( - &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

,