Refactor priority logic.

DuskSystems · Jan 5, 2025 · 01889ce · 01889ce
1 parent 31ec191
commit 01889ce
Show file tree

Hide file tree

Showing 27 changed files with 993 additions and 393 deletions.
diff --git a/README.md b/README.md
@@ -26,6 +26,8 @@ Real-world projects often need fancy routing capabilities, such as projects port
 
 The goal of `wayfind` is to remain competitive with the fastest libraries, while offering advanced routing features when needed. Unused features shouldn't impact performance - you only pay for what you use.
 
+TODO: Mention downsides of this approach.
+
 ## Features
 
 ### Dynamic Routing
@@ -49,12 +51,12 @@ fn main() -> Result<(), Box<dyn Error>> {
     router.insert("/users/{id}", 1)?;
     router.insert("/users/{id}/files/{filename}.{extension}", 2)?;
 
-    let search = router.search("/users/123")?.unwrap();
+    let search = router.search("/users/123").unwrap();
     assert_eq!(*search.data, 1);
     assert_eq!(search.template, "/users/{id}");
     assert_eq!(search.parameters[0], ("id", "123"));
 
-    let search = router.search("/users/123/files/my.document.pdf")?.unwrap();
+    let search = router.search("/users/123/files/my.document.pdf").unwrap();
     assert_eq!(*search.data, 2);
     assert_eq!(search.template, "/users/{id}/files/{filename}.{extension}");
     assert_eq!(search.parameters[0], ("id", "123"));
@@ -87,12 +89,12 @@ fn main() -> Result<(), Box<dyn Error>> {
     router.insert("/files/{*slug}/delete", 1)?;
     router.insert("/{*catch_all}", 2)?;
 
-    let search = router.search("/files/documents/reports/annual.pdf/delete")?.unwrap();
+    let search = router.search("/files/documents/reports/annual.pdf/delete").unwrap();
     assert_eq!(*search.data, 1);
     assert_eq!(search.template, "/files/{*slug}/delete");
     assert_eq!(search.parameters[0], ("slug", "documents/reports/annual.pdf"));
 
-    let search = router.search("/any/other/path")?.unwrap();
+    let search = router.search("/any/other/path").unwrap();
     assert_eq!(*search.data, 2);
     assert_eq!(search.template, "/{*catch_all}");
     assert_eq!(search.parameters[0], ("catch_all", "any/other/path"));
@@ -130,26 +132,26 @@ fn main() -> Result<(), Box<dyn Error>> {
     router.insert("/users(/{id})", 1)?;
     router.insert("/files/{*slug}/{file}(.{extension})", 2)?;
 
-    let search = router.search("/users")?.unwrap();
+    let search = router.search("/users").unwrap();
     assert_eq!(*search.data, 1);
     assert_eq!(search.template, "/users(/{id})");
     assert_eq!(search.expanded, Some("/users"));
 
-    let search = router.search("/users/123")?.unwrap();
+    let search = router.search("/users/123").unwrap();
     assert_eq!(*search.data, 1);
     assert_eq!(search.template, "/users(/{id})");
     assert_eq!(search.expanded, Some("/users/{id}"));
     assert_eq!(search.parameters[0], ("id", "123"));
 
-    let search = router.search("/files/documents/folder/report.pdf")?.unwrap();
+    let search = router.search("/files/documents/folder/report.pdf").unwrap();
     assert_eq!(*search.data, 2);
     assert_eq!(search.template, "/files/{*slug}/{file}(.{extension})");
     assert_eq!(search.expanded, Some("/files/{*slug}/{file}.{extension}"));
     assert_eq!(search.parameters[0], ("slug", "documents/folder"));
     assert_eq!(search.parameters[1], ("file", "report"));
     assert_eq!(search.parameters[2], ("extension", "pdf"));
 
-    let search = router.search("/files/documents/folder/readme")?.unwrap();
+    let search = router.search("/files/documents/folder/readme").unwrap();
     assert_eq!(*search.data, 2);
     assert_eq!(search.template, "/files/{*slug}/{file}(.{extension})");
     assert_eq!(search.expanded, Some("/files/{*slug}/{file}"));
@@ -232,18 +234,18 @@ fn main() -> Result<(), Box<dyn Error>> {
     router.insert("/v2", 1)?;
     router.insert("/v2/{*name:namespace}/blobs/{type}:{digest}", 2)?;
 
-    let search = router.search("/v2")?.unwrap();
+    let search = router.search("/v2").unwrap();
     assert_eq!(*search.data, 1);
     assert_eq!(search.template, "/v2");
 
-    let search = router.search("/v2/my-org/my-repo/blobs/sha256:1234567890")?.unwrap();
+    let search = router.search("/v2/my-org/my-repo/blobs/sha256:1234567890").unwrap();
     assert_eq!(*search.data, 2);
     assert_eq!(search.template, "/v2/{*name:namespace}/blobs/{type}:{digest}");
     assert_eq!(search.parameters[0], ("name", "my-org/my-repo"));
     assert_eq!(search.parameters[1], ("type", "sha256"));
     assert_eq!(search.parameters[2], ("digest", "1234567890"));
 
-    let search = router.search("/v2/invalid repo/blobs/uploads")?;
+    let search = router.search("/v2/invalid repo/blobs/uploads");
     assert!(search.is_none());
 
     Ok(())

diff --git a/benches/matchit_criterion.rs b/benches/matchit_criterion.rs
@@ -24,7 +24,7 @@ fn matchit_benchmark(criterion: &mut Criterion) {
 
         bencher.iter(|| {
             for path in black_box(paths()) {
-                let output = black_box(router.search(black_box(path)).unwrap().unwrap());
+                let output = black_box(router.search(black_box(path)).unwrap());
                 let _parameters: Vec<(&str, &str)> =
                     black_box(output.parameters.iter().map(|p| (p.0, p.1)).collect());
             }

diff --git a/benches/matchit_divan.rs b/benches/matchit_divan.rs
@@ -23,7 +23,7 @@ fn wayfind(bencher: divan::Bencher<'_, '_>) {
 
     bencher.bench(|| {
         for path in black_box(paths()) {
-            let output = black_box(router.search(black_box(path)).unwrap().unwrap());
+            let output = black_box(router.search(black_box(path)).unwrap());
             let _parameters: Vec<(&str, &str)> =
                 black_box(output.parameters.iter().map(|p| (p.0, p.1)).collect());
         }

diff --git a/benches/matchit_routes.rs b/benches/matchit_routes.rs
@@ -4,7 +4,7 @@ pub fn paths() -> impl IntoIterator<Item = &'static str> {
         "/user/repos",
         "/repos/rust-lang/rust/stargazers",
         "/orgs/rust-lang/public_members/nikomatsakis",
-        "/repos/rust-lang/rust/releases/1%2E51%2E0",
+        "/repos/rust-lang/rust/releases/1.51.0",
     ]
 }
 

diff --git a/benches/path_tree_criterion.rs b/benches/path_tree_criterion.rs
@@ -24,7 +24,7 @@ fn path_tree_benchmark(criterion: &mut Criterion) {
 
         bencher.iter(|| {
             for path in black_box(paths()) {
-                let output = black_box(router.search(black_box(path)).unwrap().unwrap());
+                let output = black_box(router.search(black_box(path)).unwrap());
                 let _parameters: Vec<(&str, &str)> =
                     black_box(output.parameters.iter().map(|p| (p.0, p.1)).collect());
             }

diff --git a/benches/path_tree_divan.rs b/benches/path_tree_divan.rs
@@ -23,7 +23,7 @@ fn wayfind(bencher: divan::Bencher<'_, '_>) {
 
     bencher.bench(|| {
         for path in black_box(paths()) {
-            let output = black_box(router.search(black_box(path)).unwrap().unwrap());
+            let output = black_box(router.search(black_box(path)).unwrap());
             let _parameters: Vec<(&str, &str)> =
                 black_box(output.parameters.iter().map(|p| (p.0, p.1)).collect());
         }

diff --git a/benches/path_tree_routes.rs b/benches/path_tree_routes.rs
@@ -37,7 +37,7 @@ pub fn paths() -> impl IntoIterator<Item = &'static str> {
         "/issues",
         "/legacy/issues/search/rust-lang/rust/987/1597",
         "/legacy/repos/search/1597",
-        "/legacy/user/email/rust%40rust-lang.org",
+        "/legacy/user/email/rust@rust-lang.org",
         "/legacy/user/search/1597",
         "/licenses",
         "/licenses/mit",

diff --git a/examples/oci/src/router.rs b/examples/oci/src/router.rs
@@ -83,7 +83,7 @@ impl<'r> AppRouter<'r> {
             return StatusCode::METHOD_NOT_ALLOWED.into_response();
         };
 
-        let Ok(Some(search)) = router.search(&path) else {
+        let Some(search) = router.search(&path) else {
             return StatusCode::NOT_FOUND.into_response();
         };
 

diff --git a/src/delete.rs b/src/delete.rs
@@ -17,17 +17,20 @@ impl<T, S: NodeState> Node<'_, T, S> {
         if let Some(part) = template.parts.pop() {
             match part {
                 Part::Static { prefix } => self.delete_static(template, &prefix),
-                Part::Dynamic {
-                    name, constraint, ..
-                } => self.delete_dynamic(template, &name, constraint.as_ref()),
-                Part::Wildcard {
-                    name, constraint, ..
-                } if template.parts.is_empty() => {
-                    self.delete_end_wildcard(&name, constraint.as_ref())
+                Part::DynamicConstrained { name, constraint } => {
+                    self.delete_dynamic_constrained(template, &name, &constraint)
                 }
-                Part::Wildcard {
-                    name, constraint, ..
-                } => self.delete_wildcard(template, &name, constraint.as_ref()),
+                Part::Dynamic { name } => self.delete_dynamic(template, &name),
+                Part::WildcardConstrained { name, constraint } if template.parts.is_empty() => {
+                    self.delete_end_wildcard_constrained(&name, &constraint)
+                }
+                Part::Wildcard { name } if template.parts.is_empty() => {
+                    self.delete_end_wildcard(&name)
+                }
+                Part::WildcardConstrained { name, constraint } => {
+                    self.delete_wildcard_constrained(template, &name, &constraint)
+                }
+                Part::Wildcard { name } => self.delete_wildcard(template, &name),
             }
         } else {
             let data = self.data.take()?;
@@ -57,11 +60,9 @@ impl<T, S: NodeState> Node<'_, T, S> {
         };
 
         if child.is_empty() {
-            // Delete empty nodes.
             self.static_children.remove(index);
             self.needs_optimization = true;
         } else if child.is_compressible() {
-            // Compress redundant nodes.
             let merge = child.static_children.remove(0);
 
             let mut prefix = std::mem::take(&mut child.state.prefix);
@@ -77,15 +78,33 @@ impl<T, S: NodeState> Node<'_, T, S> {
         result
     }
 
-    fn delete_dynamic(
+    fn delete_dynamic_constrained(
         &mut self,
         template: &mut Template,
         name: &str,
-        constraint: Option<&String>,
+        constraint: &str,
     ) -> Option<T> {
-        let index = self.dynamic_children.iter().position(|child| {
-            child.state.name == name && child.state.constraint.as_ref() == constraint
-        })?;
+        let index = self
+            .dynamic_constrained_children
+            .iter()
+            .position(|child| child.state.name == name && child.state.constraint == constraint)?;
+
+        let child = &mut self.dynamic_constrained_children[index];
+        let result = child.delete(template);
+
+        if child.is_empty() {
+            self.dynamic_constrained_children.remove(index);
+            self.needs_optimization = true;
+        }
+
+        result
+    }
+
+    fn delete_dynamic(&mut self, template: &mut Template, name: &str) -> Option<T> {
+        let index = self
+            .dynamic_children
+            .iter()
+            .position(|child| child.state.name == name)?;
 
         let child = &mut self.dynamic_children[index];
         let result = child.delete(template);
@@ -98,15 +117,33 @@ impl<T, S: NodeState> Node<'_, T, S> {
         result
     }
 
-    fn delete_wildcard(
+    fn delete_wildcard_constrained(
         &mut self,
         template: &mut Template,
         name: &str,
-        constraint: Option<&String>,
+        constraint: &str,
     ) -> Option<T> {
-        let index = self.wildcard_children.iter().position(|child| {
-            child.state.name == name && child.state.constraint.as_ref() == constraint
-        })?;
+        let index = self
+            .wildcard_constrained_children
+            .iter()
+            .position(|child| child.state.name == name && child.state.constraint == constraint)?;
+
+        let child = &mut self.wildcard_constrained_children[index];
+        let result = child.delete(template);
+
+        if child.is_empty() {
+            self.wildcard_constrained_children.remove(index);
+            self.needs_optimization = true;
+        }
+
+        result
+    }
+
+    fn delete_wildcard(&mut self, template: &mut Template, name: &str) -> Option<T> {
+        let index = self
+            .wildcard_children
+            .iter()
+            .position(|child| child.state.name == name)?;
 
         let child = &mut self.wildcard_children[index];
         let result = child.delete(template);
@@ -119,10 +156,28 @@ impl<T, S: NodeState> Node<'_, T, S> {
         result
     }
 
-    fn delete_end_wildcard(&mut self, name: &str, constraint: Option<&String>) -> Option<T> {
-        let index = self.end_wildcard_children.iter().position(|child| {
-            child.state.name == name && child.state.constraint.as_ref() == constraint
-        })?;
+    fn delete_end_wildcard_constrained(&mut self, name: &str, constraint: &str) -> Option<T> {
+        let index = self
+            .end_wildcard_constrained_children
+            .iter()
+            .position(|child| child.state.name == name && child.state.constraint == constraint)?;
+
+        let mut child = self.end_wildcard_constrained_children.remove(index);
+
+        let data = child.data.take()?;
+        self.needs_optimization = true;
+
+        match data {
+            NodeData::Inline { data, .. } => Some(data),
+            NodeData::Shared { data, .. } => Arc::try_unwrap(data).ok(),
+        }
+    }
+
+    fn delete_end_wildcard(&mut self, name: &str) -> Option<T> {
+        let index = self
+            .end_wildcard_children
+            .iter()
+            .position(|child| child.state.name == name)?;
 
         let mut child = self.end_wildcard_children.remove(index);
 
@@ -138,16 +193,22 @@ impl<T, S: NodeState> Node<'_, T, S> {
     fn is_empty(&self) -> bool {
         self.data.is_none()
             && self.static_children.is_empty()
+            && self.dynamic_constrained_children.is_empty()
             && self.dynamic_children.is_empty()
+            && self.wildcard_constrained_children.is_empty()
             && self.wildcard_children.is_empty()
+            && self.end_wildcard_constrained_children.is_empty()
             && self.end_wildcard_children.is_empty()
     }
 
     fn is_compressible(&self) -> bool {
         self.data.is_none()
             && self.static_children.len() == 1
+            && self.dynamic_constrained_children.is_empty()
             && self.dynamic_children.is_empty()
+            && self.wildcard_constrained_children.is_empty()
             && self.wildcard_children.is_empty()
+            && self.end_wildcard_constrained_children.is_empty()
             && self.end_wildcard_children.is_empty()
     }
 }