Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic node removal #49

Merged
merged 24 commits into from
Apr 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
b2b4bc0
feat(tree): wip remove node
Totodore Jan 13, 2024
edc4c40
feat(tree): node removal
Totodore Jan 14, 2024
2e2e205
feat(tree): node removal
Totodore Jan 14, 2024
ac97272
feat: add specific behavior for root removal
Totodore Jan 15, 2024
1f7bcc7
test: gate `Display` impl for `Node` for test only
Totodore Jan 15, 2024
53f6a55
feat: add `remove` fn to router && docs
Totodore Jan 15, 2024
3367a04
fmt
Totodore Jan 15, 2024
f3b9c0e
chore(clippy): apply clippy lints
Totodore Jan 15, 2024
6fb68a8
test: add testing
Totodore Jan 19, 2024
49e6354
feat(tree): wip remove node
Totodore Jan 13, 2024
0ece4a4
feat(tree): node removal
Totodore Jan 14, 2024
43969a2
feat: add specific behavior for root removal
Totodore Jan 15, 2024
24a6356
test: gate `Display` impl for `Node` for test only
Totodore Jan 15, 2024
fa6d159
feat: add `remove` fn to router && docs
Totodore Jan 15, 2024
819fc0a
fmt
Totodore Jan 15, 2024
6a740d3
chore(clippy): apply clippy lints
Totodore Jan 15, 2024
e63e332
feat(tree): wip remove node
Totodore Mar 12, 2024
737528e
Merge branch 'ft-remove-node' of https://github.com/Totodore/matchit …
Totodore Mar 12, 2024
1af61c0
Merge remote-tracking branch 'origin/master' into ft-remove-node
Totodore Mar 12, 2024
ba55e99
Merge branch 'temp' into ft-remove-node
Totodore Mar 12, 2024
c94713e
fix: remove display impl for `Node`
Totodore Mar 12, 2024
7ab94bc
feat: check param remapping before dropping child
Totodore Mar 18, 2024
37d4722
doc(router): add doc about incorrect routes.
Totodore Mar 18, 2024
3f39176
fix(tree/remove): duplicated code typo
Totodore Apr 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,40 @@ impl<T> Router<T> {
}
}

/// Remove a given route from the router.
///
/// Returns the value stored under the route if it was found.
///
/// If the route was not found or if the route is incorrect, `None` is returned.
///
/// # Examples
///
/// ```rust
/// # use matchit::Router;
/// let mut router = Router::new();
/// router.insert("/home", "Welcome!");
///
/// assert_eq!(router.remove("/home"), Some("Welcome!"));
/// assert_eq!(router.remove("/home"), None);
///
/// router.insert("/home/{id}/", "Hello!");
/// assert_eq!(router.remove("/home/{id}/"), Some("Hello!"));
/// assert_eq!(router.remove("/home/{id}/"), None);
///
/// router.insert("/home/{id}/", "Hello!");
/// // Bad route
/// assert_eq!(router.remove("/home/{user}"), None);
/// assert_eq!(router.remove("/home/{id}/"), Some("Hello!"));
///
/// router.insert("/home/{id}/", "Hello!");
/// // Ill-formed route
/// assert_eq!(router.remove("/home/{id"), None);
/// assert_eq!(router.remove("/home/{id}/"), Some("Hello!"));
/// ```
pub fn remove(&mut self, path: impl Into<String>) -> Option<T> {
self.root.remove(path)
}

#[cfg(feature = "__test_helpers")]
pub fn check_priorities(&self) -> Result<u32, (u32, u32)> {
self.root.check_priorities()
Expand Down
100 changes: 100 additions & 0 deletions src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,106 @@
}
}

/// Removes a route from the tree, returning the value if the route existed.
/// The provided path should be the same as the one used to insert the route (including wildcards).
pub fn remove(&mut self, full_path: impl Into<String>) -> Option<T> {
let mut current = self;
let unescaped = UnescapedRoute::new(full_path.into().into_bytes());
let (full_path, param_remapping) = normalize_params(unescaped).ok()?;
let full_path = full_path.into_inner();
let mut path: &[u8] = full_path.as_ref();

let drop_child = |node: &mut Node<T>, i: usize| -> Option<T> {
if node.children[i].param_remapping != param_remapping {
return None;
}
// if the node we are dropping doesn't have any children, we can remove it
let val = if node.children[i].children.is_empty() {
// if the parent node only has one child there are no indices
if node.children.len() == 1 && node.indices.is_empty() {
node.wild_child = false;
node.children.remove(0).value.take()
} else {
let child = node.children.remove(i);
// Indices are only used for static nodes
if child.node_type == NodeType::Static {
node.indices.remove(i);
} else {
// It was a dynamic node, we remove the wildcard child flag
node.wild_child = false;
}
child.value
}
} else {
node.children[i].value.take()
};

val.map(UnsafeCell::into_inner)
};

// Specific case if we are removing the root node
if path == current.prefix.inner() {
let val = current.value.take().map(UnsafeCell::into_inner);
// if the root node has no children, we can just reset it
if current.children.is_empty() {
*current = Self::default();
}
return val;
}

'walk: loop {
// the path is longer than this node's prefix, we are expecting a child node
if path.len() > current.prefix.len() {
let (prefix, rest) = path.split_at(current.prefix.len());
// the prefix matches
if prefix == current.prefix.inner() {
let first = rest[0];
path = rest;

// If there is only one child we can continue with the child node
if current.children.len() == 1 {
if current.children[0].prefix.inner() == rest {
return drop_child(current, 0);
} else {

Check warning on line 236 in src/tree.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

redundant else block
current = &mut current.children[0];
continue 'walk;
}
}

// If there are many we get the index of the child matching the first byte
if let Some(i) = current.indices.iter().position(|&c| c == first) {
// continue with the child node
if current.children[i].prefix.inner() == rest {
return drop_child(current, i);
} else {

Check warning on line 247 in src/tree.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

redundant else block
current = &mut current.children[i];
continue 'walk;
}
}

// If this node has a wildcard child and that it matches our standardized path
// we continue with that
if current.wild_child
&& !current.children.is_empty()
&& rest.len() > 2
&& rest[0] == b'{'
&& rest[2] == b'}'
{
// continue with the wildcard child
if current.children.last_mut().unwrap().prefix.inner() == rest {
return drop_child(current, current.children.len() - 1);
} else {

Check warning on line 264 in src/tree.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

redundant else block
current = current.children.last_mut().unwrap();
continue 'walk;
}
}
}
}

return None;
}
}

// add a child node, keeping wildcards at the end
fn add_child(&mut self, child: Node<T>) -> usize {
let len = self.children.len();
Expand Down
248 changes: 248 additions & 0 deletions tests/remove.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
use matchit::Router;

struct RemoveTest {
routes: Vec<&'static str>,
ops: Vec<(Operation, &'static str, Option<&'static str>)>,
remaining: Vec<&'static str>,
}

enum Operation {
Insert,
Remove,
}

use Operation::*;

impl RemoveTest {
fn run(self) {
let mut router = Router::new();

for route in self.routes.iter() {
assert_eq!(router.insert(*route, route.to_owned()), Ok(()), "{route}");
}

for (op, route, expected) in self.ops.iter() {
match op {
Insert => {
assert_eq!(router.insert(*route, route), Ok(()), "{route}")
}
Remove => {
assert_eq!(router.remove(*route), *expected, "removing {route}",)
}
}
}

for route in self.remaining {
assert!(matches!(router.at(route), Ok(_)), "remaining {route}");
}
}
}

#[test]
fn normalized() {
RemoveTest {
routes: vec![
"/x/{foo}/bar",
"/x/{bar}/baz",
"/{foo}/{baz}/bax",
"/{foo}/{bar}/baz",
"/{fod}/{baz}/{bax}/foo",
"/{fod}/baz/bax/foo",
"/{foo}/baz/bax",
"/{bar}/{bay}/bay",
"/s",
"/s/s",
"/s/s/s",
"/s/s/s/s",
"/s/s/{s}/x",
"/s/s/{y}/d",
],
ops: vec![
(Remove, "/x/{foo}/bar", Some("/x/{foo}/bar")),
(Remove, "/x/{bar}/baz", Some("/x/{bar}/baz")),
(Remove, "/{foo}/{baz}/bax", Some("/{foo}/{baz}/bax")),
(Remove, "/{foo}/{bar}/baz", Some("/{foo}/{bar}/baz")),
(
Remove,
"/{fod}/{baz}/{bax}/foo",
Some("/{fod}/{baz}/{bax}/foo"),
),
(Remove, "/{fod}/baz/bax/foo", Some("/{fod}/baz/bax/foo")),
(Remove, "/{foo}/baz/bax", Some("/{foo}/baz/bax")),
(Remove, "/{bar}/{bay}/bay", Some("/{bar}/{bay}/bay")),
(Remove, "/s", Some("/s")),
(Remove, "/s/s", Some("/s/s")),
(Remove, "/s/s/s", Some("/s/s/s")),
(Remove, "/s/s/s/s", Some("/s/s/s/s")),
(Remove, "/s/s/{s}/x", Some("/s/s/{s}/x")),
(Remove, "/s/s/{y}/d", Some("/s/s/{y}/d")),
],
remaining: vec![],
}
.run();
}

#[test]
fn test() {
RemoveTest {
routes: vec!["/home", "/home/{id}"],
ops: vec![
(Remove, "/home", Some("/home")),
(Remove, "/home", None),
(Remove, "/home/{id}", Some("/home/{id}")),
(Remove, "/home/{id}", None),
],
remaining: vec![],
}
.run();
}

#[test]
fn blog() {
RemoveTest {
routes: vec![
"/{page}",
"/posts/{year}/{month}/{post}",
"/posts/{year}/{month}/index",
"/posts/{year}/top",
"/static/{*path}",
"/favicon.ico",
],
ops: vec![
(Remove, "/{page}", Some("/{page}")),
(
Remove,
"/posts/{year}/{month}/{post}",
Some("/posts/{year}/{month}/{post}"),
),
(
Remove,
"/posts/{year}/{month}/index",
Some("/posts/{year}/{month}/index"),
),
(Remove, "/posts/{year}/top", Some("/posts/{year}/top")),
(Remove, "/static/{*path}", Some("/static/{*path}")),
(Remove, "/favicon.ico", Some("/favicon.ico")),
],
remaining: vec![],
}
.run()
}

#[test]
fn catchall() {
RemoveTest {
routes: vec!["/foo/{*catchall}", "/bar", "/bar/", "/bar/{*catchall}"],
ops: vec![
(Remove, "/foo/{*catchall}", Some("/foo/{*catchall}")),
(Remove, "/bar/", Some("/bar/")),
(Insert, "/foo/*catchall", Some("/foo/*catchall")),
(Remove, "/bar/{*catchall}", Some("/bar/{*catchall}")),
],
remaining: vec!["/bar", "/foo/*catchall"],
}
.run();
}

#[test]
fn overlapping_routes() {
RemoveTest {
routes: vec![
"/home",
"/home/{id}",
"/users",
"/users/{id}",
"/users/{id}/posts",
"/users/{id}/posts/{post_id}",
"/articles",
"/articles/{category}",
"/articles/{category}/{id}",
],
ops: vec![
(Remove, "/home", Some("/home")),
(Insert, "/home", Some("/home")),
(Remove, "/home/{id}", Some("/home/{id}")),
(Insert, "/home/{id}", Some("/home/{id}")),
(Remove, "/users", Some("/users")),
(Insert, "/users", Some("/users")),
(Remove, "/users/{id}", Some("/users/{id}")),
(Insert, "/users/{id}", Some("/users/{id}")),
(Remove, "/users/{id}/posts", Some("/users/{id}/posts")),
(Insert, "/users/{id}/posts", Some("/users/{id}/posts")),
(
Remove,
"/users/{id}/posts/{post_id}",
Some("/users/{id}/posts/{post_id}"),
),
(
Insert,
"/users/{id}/posts/{post_id}",
Some("/users/{id}/posts/{post_id}"),
),
(Remove, "/articles", Some("/articles")),
(Insert, "/articles", Some("/articles")),
(Remove, "/articles/{category}", Some("/articles/{category}")),
(Insert, "/articles/{category}", Some("/articles/{category}")),
(
Remove,
"/articles/{category}/{id}",
Some("/articles/{category}/{id}"),
),
(
Insert,
"/articles/{category}/{id}",
Some("/articles/{category}/{id}"),
),
],
remaining: vec![
"/home",
"/home/{id}",
"/users",
"/users/{id}",
"/users/{id}/posts",
"/users/{id}/posts/{post_id}",
"/articles",
"/articles/{category}",
"/articles/{category}/{id}",
],
}
.run();
}

#[test]
fn remove_root() {
RemoveTest {
routes: vec!["/"],
ops: vec![(Remove, "/", Some("/"))],
remaining: vec![],
}
.run();
}

#[test]
fn check_escaped_params() {
RemoveTest {
routes: vec![
"/foo/{id}",
"/foo/{id}/bar",
"/bar/{user}/{id}",
"/bar/{user}/{id}/baz",
"/baz/{product}/{user}/{id}",
],
ops: vec![
(Remove, "/foo/{a}", None),
(Remove, "/foo/{a}/bar", None),
(Remove, "/bar/{a}/{b}", None),
(Remove, "/bar/{a}/{b}/baz", None),
(Remove, "/baz/{a}/{b}/{c}", None),
],
remaining: vec![
"/foo/{id}",
"/foo/{id}/bar",
"/bar/{user}/{id}",
"/bar/{user}/{id}/baz",
"/baz/{product}/{user}/{id}",
],
}
.run();
}
Loading