ResourceDef: cleanup (#365)

actix-router/src/resource.rs

@@ -24,12 +24,12 @@ pub struct ResourceDef {
     tp: PatternType,
     name: String,
     pattern: String,
-    elements: Vec<PatternElement>,
+    elements: Option<Vec<PatternElement>>,
 }
 
 #[derive(Debug, Clone, PartialEq)]
 enum PatternElement {
-    Str(String),
+    Const(String),
     Var(String),
 }
 
@@ -38,8 +38,8 @@ enum PatternElement {
 enum PatternType {
     Static(String),
     Prefix(String),
-    Dynamic(Regex, Vec<&'static str>, usize),
+    Dynamic(Regex, Vec<&'static str>),
-    DynamicSet(RegexSet, Vec<(Regex, Vec<&'static str>, usize)>),
+    DynamicSet(RegexSet, Vec<(Regex, Vec<&'static str>)>),
 }
 
 impl ResourceDef {
@@ -56,7 +56,7 @@ impl ResourceDef {
             let mut re_set = Vec::new();
 
             for path in set {
-                let (pattern, _, _, len) = ResourceDef::parse(&path, false);
+                let (pattern, _, _) = ResourceDef::parse(&path, false);
 
                 let re = match Regex::new(&pattern) {
                     Ok(re) => re,
@@ -69,14 +69,14 @@ impl ResourceDef {
                         name.map(|name| Box::leak(Box::new(name.to_owned())).as_str())
                     })
                     .collect();
-                data.push((re, names, len));
+                data.push((re, names));
                 re_set.push(pattern);
             }
 
             ResourceDef {
                 id: 0,
                 tp: PatternType::DynamicSet(RegexSet::new(re_set).unwrap(), data),
-                elements: Vec::new(),
+                elements: None,
                 name: String::new(),
                 pattern: "".to_owned(),
             }
@@ -116,7 +116,7 @@ impl ResourceDef {
     /// Parse path pattern and create new `Pattern` instance with custom prefix
     fn with_prefix(path: &str, for_prefix: bool) -> Self {
         let path = path.to_owned();
-        let (pattern, elements, is_dynamic, len) = ResourceDef::parse(&path, for_prefix);
+        let (pattern, elements, is_dynamic) = ResourceDef::parse(&path, for_prefix);
 
         let tp = if is_dynamic {
             let re = match Regex::new(&pattern) {
@@ -130,7 +130,7 @@ impl ResourceDef {
                     name.map(|name| Box::leak(Box::new(name.to_owned())).as_str())
                 })
                 .collect();
-            PatternType::Dynamic(re, names, len)
+            PatternType::Dynamic(re, names)
         } else if for_prefix {
             PatternType::Prefix(pattern)
         } else {
@@ -139,7 +139,7 @@ impl ResourceDef {
 
         ResourceDef {
             tp,
-            elements,
+            elements: Some(elements),
             id: 0,
             name: String::new(),
             pattern: path,
@@ -167,7 +167,7 @@ impl ResourceDef {
         match self.tp {
             PatternType::Static(ref s) => s == path,
             PatternType::Prefix(ref s) => path.starts_with(s),
-            PatternType::Dynamic(ref re, _, _) => re.is_match(path),
+            PatternType::Dynamic(ref re, _) => re.is_match(path),
             PatternType::DynamicSet(ref re, _) => re.is_match(path),
         }
     }
@@ -185,25 +185,7 @@ impl ResourceDef {
                     None
                 }
             }
-            PatternType::Dynamic(ref re, _, len) => {
+            PatternType::Dynamic(ref re, _) => re.find(path).map(|m| m.end()),
-                if let Some(captures) = re.captures(path) {
-                    let mut pos = 0;
-                    let mut passed = false;
-                    for capture in captures.iter() {
-                        if let Some(ref m) = capture {
-                            if !passed {
-                                passed = true;
-                                continue;
-                            }
-
-                            pos = m.end();
-                        }
-                    }
-                    Some(pos + len)
-                } else {
-                    None
-                }
-            }
             PatternType::Prefix(ref s) => {
                 let len = if path == s {
                     s.len()
@@ -221,121 +203,16 @@ impl ResourceDef {
                 Some(min(p_len, len))
             }
             PatternType::DynamicSet(ref re, ref params) => {
-                if let Some(idx) = re.matches(path).into_iter().next() {
+                let idx = re.matches(path).into_iter().next()?;
-                    let (ref pattern, _, len) = params[idx];
+                let (ref pattern, _) = params[idx];
-                    if let Some(captures) = pattern.captures(path) {
+                pattern.find(path).map(|m| m.end())
-                        let mut pos = 0;
-                        let mut passed = false;
-                        for capture in captures.iter() {
-                            if let Some(ref m) = capture {
-                                if !passed {
-                                    passed = true;
-                                    continue;
-                                }
-
-                                pos = m.end();
-                            }
-                        }
-                        Some(pos + len)
-                    } else {
-                        None
-                    }
-                } else {
-                    None
-                }
             }
         }
     }
 
     /// Is the given path and parameters a match against this pattern.
     pub fn match_path<T: ResourcePath>(&self, path: &mut Path<T>) -> bool {
-        match self.tp {
+        self.match_path_checked(path, &|_, _| true, &Some(()))
-            PatternType::Static(ref s) => {
-                if s == path.path() {
-                    path.skip(path.len() as u16);
-                    true
-                } else {
-                    false
-                }
-            }
-            PatternType::Prefix(ref s) => {
-                let r_path = path.path();
-                let len = 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
-                    } else {
-                        s.len()
-                    }
-                } else {
-                    return false;
-                };
-                let r_path_len = r_path.len();
-                path.skip(min(r_path_len, len) as u16);
-                true
-            }
-            PatternType::Dynamic(ref re, ref names, len) => {
-                let mut pos = 0;
-                let mut segments: [PathItem; MAX_DYNAMIC_SEGMENTS] = Default::default();
-
-                if let Some(captures) = re.captures(path.path()) {
-                    for (no, name) in names.iter().enumerate() {
-                        if let Some(m) = captures.name(&name) {
-                            pos = m.end();
-                            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;
-                        }
-                    }
-                } else {
-                    return false;
-                }
-                for i in 0..names.len() {
-                    path.add(names[i], mem::take(&mut segments[i]));
-                }
-                path.skip((pos + len) as u16);
-                true
-            }
-            PatternType::DynamicSet(ref re, ref params) => {
-                if let Some(idx) = re.matches(path.path()).into_iter().next() {
-                    let (ref pattern, ref names, len) = params[idx];
-                    let mut pos = 0;
-                    let mut segments: [PathItem; MAX_DYNAMIC_SEGMENTS] = Default::default();
-
-                    if let Some(captures) = pattern.captures(path.path()) {
-                        for (no, name) in names.iter().enumerate() {
-                            if let Some(m) = captures.name(&name) {
-                                pos = m.end();
-                                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;
-                            }
-                        }
-                    } else {
-                        return false;
-                    }
-                    for i in 0..names.len() {
-                        path.add(names[i], mem::take(&mut segments[i]));
-                    }
-                    path.skip((pos + len) as u16);
-                    true
-                } else {
-                    false
-                }
-            }
-        }
     }
 
     /// Is the given path and parameters a match against this pattern?
@@ -350,19 +227,19 @@ impl ResourceDef {
         R: Resource<T>,
         F: Fn(&R, &Option<U>) -> bool,
     {
-        match self.tp {
+        let mut segments: [PathItem; MAX_DYNAMIC_SEGMENTS] = Default::default();
-            PatternType::Static(ref s) => {
-                if s == res.resource_path().path() && check(res, user_data) {
         let path = res.resource_path();
-                    path.skip(path.len() as u16);
+
-                    true
+        let (matched_len, matched_vars) = match self.tp {
-                } else {
+            PatternType::Static(ref s) => {
-                    false
+                if s != path.path() {
+                    return false;
                 }
+                (path.len(), None)
             }
             PatternType::Prefix(ref s) => {
                 let len = {
-                    let r_path = res.resource_path().path();
+                    let r_path = path.path();
                     if s == r_path {
                         s.len()
                     } else if r_path.starts_with(s)
@@ -377,85 +254,80 @@ impl ResourceDef {
                         return false;
                     }
                 };
-                if !check(res, user_data) {
+                (min(path.len(), len), None)
-                    return false;
             }
-                let path = res.resource_path();
+            PatternType::Dynamic(ref re, ref names) => {
-                path.skip(min(path.path().len(), len) as u16);
+                let captures = match re.captures(path.path()) {
-                true
+                    Some(captures) => captures,
-            }
+                    _ => return false,
-            PatternType::Dynamic(ref re, ref names, len) => {
+                };
-                let mut pos = 0;
-                let mut segments: [PathItem; MAX_DYNAMIC_SEGMENTS] = Default::default();
-
-                if let Some(captures) = re.captures(res.resource_path().path()) {
                 for (no, name) in names.iter().enumerate() {
                     if let Some(m) = captures.name(&name) {
-                            pos = m.end();
                         segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16);
                     } else {
-                            log::error!(
+                        log::error!("Dynamic path match but not all segments found: {}", name);
-                                "Dynamic path match but not all segments found: {}",
-                                name
-                            );
                         return false;
                     }
                 }
-                } else {
+                (captures[0].len(), Some(names))
-                    return false;
-                }
-
-                if !check(res, user_data) {
-                    return false;
-                }
-
-                let path = res.resource_path();
-                for i in 0..names.len() {
-                    path.add(names[i], mem::take(&mut segments[i]));
-                }
-                path.skip((pos + len) as u16);
-                true
             }
             PatternType::DynamicSet(ref re, ref params) => {
-                let path = res.resource_path().path();
+                let path = path.path();
-                if let Some(idx) = re.matches(path).into_iter().next() {
+                let (pattern, names) = match re.matches(path).into_iter().next() {
-                    let (ref pattern, ref names, len) = params[idx];
+                    Some(idx) => &params[idx],
-                    let mut pos = 0;
+                    _ => return false,
-                    let mut segments: [PathItem; MAX_DYNAMIC_SEGMENTS] = Default::default();
+                };
-
+                let captures = match pattern.captures(path.path()) {
-                    if let Some(captures) = pattern.captures(path) {
+                    Some(captures) => captures,
+                    _ => return false,
+                };
                 for (no, name) in names.iter().enumerate() {
                     if let Some(m) = captures.name(&name) {
-                                pos = m.end();
+                        segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16);
-                                segments[no] =
-                                    PathItem::Segment(m.start() as u16, m.end() as u16);
                     } else {
-                                log::error!(
+                        log::error!("Dynamic path match but not all segments found: {}", name);
-                                    "Dynamic path match but not all segments found: {}",
-                                    name
-                                );
                         return false;
                     }
                 }
-                    } else {
+                (captures[0].len(), Some(names))
-                        return false;
             }
+        };
 
         if !check(res, user_data) {
             return false;
         }
 
+        // Modify `path` to skip matched part and store matched segments
         let path = res.resource_path();
-                    for i in 0..names.len() {
+        if let Some(vars) = matched_vars {
-                        path.add(names[i], mem::take(&mut segments[i]));
+            for i in 0..vars.len() {
+                path.add(vars[i], mem::take(&mut segments[i]));
             }
-                    path.skip((pos + len) as u16);
+        }
+        path.skip(matched_len as u16);
+
         true
-                } else {
+    }
-                    false
+
-                }
+    /// Build resource path with a closure that maps variable elements' names to values.
+    fn build_resource_path<F, I>(&self, path: &mut String, mut vars: F) -> bool
+    where
+        F: FnMut(&str) -> Option<I>,
+        I: AsRef<str>,
+    {
+        for el in match self.elements {
+            Some(ref elements) => elements,
+            None => return false,
+        } {
+            match *el {
+                PatternElement::Const(ref val) => path.push_str(val),
+                PatternElement::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.
@@ -464,28 +336,7 @@ impl ResourceDef {
         U: Iterator<Item = I>,
         I: AsRef<str>,
     {
-        match self.tp {
+        self.build_resource_path(path, |_| elements.next())
-            PatternType::Prefix(ref p) => path.push_str(p),
-            PatternType::Static(ref p) => path.push_str(p),
-            PatternType::Dynamic(..) => {
-                for el in &self.elements {
-                    match *el {
-                        PatternElement::Str(ref s) => path.push_str(s),
-                        PatternElement::Var(_) => {
-                            if let Some(val) = elements.next() {
-                                path.push_str(val.as_ref())
-                            } else {
-                                return false;
-                            }
-                        }
-                    }
-                }
-            }
-            PatternType::DynamicSet(..) => {
-                return false;
-            }
-        }
-        true
     }
 
     /// Build resource path from elements. Returns `true` on success.
@@ -499,28 +350,7 @@ impl ResourceDef {
         V: AsRef<str>,
         S: std::hash::BuildHasher,
     {
-        match self.tp {
+        self.build_resource_path(path, |name| elements.get(name))
-            PatternType::Prefix(ref p) => path.push_str(p),
-            PatternType::Static(ref p) => path.push_str(p),
-            PatternType::Dynamic(..) => {
-                for el in &self.elements {
-                    match *el {
-                        PatternElement::Str(ref s) => path.push_str(s),
-                        PatternElement::Var(ref name) => {
-                            if let Some(val) = elements.get(name) {
-                                path.push_str(val.as_ref())
-                            } else {
-                                return false;
-                            }
-                        }
-                    }
-                }
-            }
-            PatternType::DynamicSet(..) => {
-                return false;
-            }
-        }
-        true
     }
 
     fn parse_param(pattern: &str) -> (PatternElement, String, &str, bool) {
@@ -570,20 +400,16 @@ impl ResourceDef {
         )
     }
 
-    fn parse(
+    fn parse(mut pattern: &str, mut for_prefix: bool) -> (String, Vec<PatternElement>, bool) {
-        mut pattern: &str,
-        mut for_prefix: bool,
-    ) -> (String, Vec<PatternElement>, bool, usize) {
         if pattern.find('{').is_none() {
             return if let Some(path) = pattern.strip_suffix('*') {
                 let re = format!("{}^{}(.*)", REGEX_FLAGS, path);
-                (re, vec![PatternElement::Str(String::from(path))], true, 0)
+                (re, vec![PatternElement::Const(String::from(path))], true)
             } else {
                 (
                     String::from(pattern),
-                    vec![PatternElement::Str(String::from(pattern))],
+                    vec![PatternElement::Const(String::from(pattern))],
                     false,
-                    pattern.chars().count(),
                 )
             };
         }
@@ -594,7 +420,7 @@ impl ResourceDef {
 
         while let Some(idx) = pattern.find('{') {
             let (prefix, rem) = pattern.split_at(idx);
-            elements.push(PatternElement::Str(String::from(prefix)));
+            elements.push(PatternElement::Const(String::from(prefix)));
             re.push_str(&escape(prefix));
             let (param_pattern, re_part, rem, tail) = Self::parse_param(rem);
             if tail {
@@ -607,7 +433,7 @@ impl ResourceDef {
             dyn_elements += 1;
         }
 
-        elements.push(PatternElement::Str(String::from(pattern)));
+        elements.push(PatternElement::Const(String::from(pattern)));
         re.push_str(&escape(pattern));
 
         if dyn_elements > MAX_DYNAMIC_SEGMENTS {
@@ -620,7 +446,7 @@ impl ResourceDef {
         if !for_prefix {
             re.push('$');
         }
-        (re, elements, true, pattern.chars().count())
+        (re, elements, true)
     }
 }