Skip to content

Commit

Permalink
Fix same-branch matching to prioritize depth
Browse files Browse the repository at this point in the history
  • Loading branch information
CathalMullan committed Sep 10, 2024
1 parent 391a88d commit e914b37
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 18 deletions.
8 changes: 8 additions & 0 deletions src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ pub enum NodeData<T> {
Reference(Arc<str>),
}

impl<T> NodeData<T> {
const fn route(&self) -> &Arc<str> {
match self {
Self::Inline { route, .. } | Self::Reference(route) => route,
}
}
}

/// Represents a node in the tree structure.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Node<T> {
Expand Down
38 changes: 22 additions & 16 deletions src/node/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ pub struct Match<'router, 'path, T> {
///
/// The key of the parameter is tied to the lifetime of the router, since it is a ref to the prefix of a given node.
/// Meanwhile, the value is extracted from the path.
///
/// TODO: Consider simply storing these as Strings.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Parameter<'router, 'path> {
pub key: &'router str,
Expand Down Expand Up @@ -77,10 +75,8 @@ impl<T> Node<T> {
&& static_child.prefix.iter().zip(path).all(|(a, b)| a == b)
{
let remaining_path = &path[static_child.prefix.len()..];
if let Some(node_data) =
static_child.search(remaining_path, parameters, constraints)?
{
return Ok(Some(node_data));
if let Some(node) = static_child.search(remaining_path, parameters, constraints)? {
return Ok(Some(node));
}
}
}
Expand Down Expand Up @@ -112,7 +108,7 @@ impl<T> Node<T> {
for dynamic_child in &self.dynamic_children {
let mut consumed = 0;

let mut last_match = None;
let mut last_match: Option<&Self> = None;
let mut last_match_parameters = vec![];

while consumed < path.len() {
Expand Down Expand Up @@ -141,17 +137,27 @@ impl<T> Node<T> {
})?,
});

if let Some(node_data) =
if let Some(node) =
dynamic_child.search(&path[consumed..], &mut current_parameters, constraints)?
{
last_match = Some(node_data);
last_match_parameters = current_parameters;
// FIXME: Rather than compare lengths of routes, we should probably calculate some kind of 'depth' up-front, at insert time.
if let Some(old) = &last_match {
if node.data.as_ref().unwrap().route().len()
>= old.data.as_ref().unwrap().route().len()
{
last_match = Some(node);
last_match_parameters = current_parameters;
}
} else {
last_match = Some(node);
last_match_parameters = current_parameters;
}
}
}

if let Some(node_data) = last_match {
if let Some(node) = last_match {
*parameters = last_match_parameters;
return Ok(Some(node_data));
return Ok(Some(node));
}
}

Expand Down Expand Up @@ -186,10 +192,10 @@ impl<T> Node<T> {
})?,
});

if let Some(node_data) =
if let Some(node) =
dynamic_child.search(&path[segment_end..], parameters, constraints)?
{
return Ok(Some(node_data));
return Ok(Some(node));
}

parameters.pop();
Expand Down Expand Up @@ -250,12 +256,12 @@ impl<T> Node<T> {
})?,
});

if let Some(node_data) = wildcard_child.search(
if let Some(node) = wildcard_child.search(
&remaining_path[segment_end..],
parameters,
constraints,
)? {
return Ok(Some(node_data));
return Ok(Some(node));
}

parameters.pop();
Expand Down
4 changes: 2 additions & 2 deletions tests/edge_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use wayfind::Router;
mod common;

#[test]
fn test_same_branch_matching_simple() -> Result<(), Box<dyn Error>> {
fn test_depth_matching_simple() -> Result<(), Box<dyn Error>> {
let mut router = Router::new();
router.insert("/{file}", 1)?;
router.insert("/{file}.{extension}", 1)?;
Expand Down Expand Up @@ -40,7 +40,7 @@ fn test_same_branch_matching_simple() -> Result<(), Box<dyn Error>> {
}

#[test]
fn test_same_branch_matching_complex() -> Result<(), Box<dyn Error>> {
fn test_depth_matching_complex() -> Result<(), Box<dyn Error>> {
let mut router = Router::new();
router.insert("/{year}", 1)?;
router.insert("/{year}-{month}", 1)?;
Expand Down

0 comments on commit e914b37

Please sign in to comment.