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

Add Support for Parameter Suffixes #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/// Represents errors that can occur when inserting a new route.
#[non_exhaustive]
#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub enum InsertError {

Check warning on line 9 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

item name ends with its containing module's name

Check warning on line 9 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

item name ends with its containing module's name
/// Attempted to insert a path that conflicts with an existing route.
Conflict {
/// The existing route that the insertion is conflicting with.
Expand All @@ -14,8 +14,8 @@
},
/// Only one parameter per route segment is allowed.
///
/// Static segments are also allowed before a parameter, but not after it. For example,
/// `/foo-{bar}` is a valid route, but `/{bar}-foo` is not.
/// For example, `/foo-{bar}` and `/{bar}-foo` are valid routes, but `/{foo}-{bar}`
/// is not.
InvalidParamSegment,
/// Parameters must be registered with a valid name and matching braces.
///
Expand All @@ -29,7 +29,7 @@
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Conflict { with } => {
write!(

Check warning on line 32 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

variables can be used directly in the `format!` string

Check warning on line 32 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

variables can be used directly in the `format!` string
f,
"Insertion failed due to conflict with previously registered route: {}",
with
Expand Down Expand Up @@ -63,7 +63,7 @@
// The route is conflicting with the current node.
if prefix.unescaped() == current.prefix.unescaped() {
denormalize_params(&mut route, &current.remapping);
return InsertError::Conflict {

Check warning on line 66 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

unnecessary structure name repetition

Check warning on line 66 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

unnecessary structure name repetition
with: String::from_utf8(route.into_unescaped()).unwrap(),
};
}
Expand Down Expand Up @@ -91,7 +91,7 @@
denormalize_params(&mut route, &last.remapping);

// Return the conflicting route.
InsertError::Conflict {

Check warning on line 94 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

unnecessary structure name repetition

Check warning on line 94 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

unnecessary structure name repetition
with: String::from_utf8(route.into_unescaped()).unwrap(),
}
}
Expand All @@ -113,7 +113,7 @@
/// # Ok(())
/// # }
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum MatchError {

Check warning on line 116 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

item name ends with its containing module's name

Check warning on line 116 in src/error.rs

View workflow job for this annotation

GitHub Actions / Clippy Lints

item name ends with its containing module's name
/// No matching route was found.
NotFound,
}
Expand Down
132 changes: 116 additions & 16 deletions src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ pub(crate) enum NodeType {
Root,
/// A route parameter, e.g. `/{id}`.
Param,
/// A route parameter that is followed by a static suffix
/// before a trailing slash, ex: `/{id}.png`.
ParamSuffix { suffix_start: usize },
/// A catch-all parameter, e.g. `/*file`.
CatchAll,
/// A static prefix, e.g. `/foo`.
Expand Down Expand Up @@ -127,7 +130,11 @@ impl<T> Node<T> {
// After matching against a wildcard the next character is always `/`.
//
// Continue searching in the child node if it already exists.
if current.node_type == NodeType::Param && current.children.len() == 1 {
if matches!(
current.node_type,
NodeType::Param | NodeType::ParamSuffix { .. }
) && current.children.len() == 1
{
debug_assert_eq!(next, b'/');
current = &mut current.children[0];
current.priority += 1;
Expand Down Expand Up @@ -173,11 +180,14 @@ impl<T> Node<T> {
current = current.children.last_mut().unwrap();
current.priority += 1;

let segment = remaining
.iter()
.position(|b| *b == b'/')
.unwrap_or(remaining.len());

// Make sure the route parameter matches.
if let Some(wildcard) = remaining.get(..current.prefix.len()) {
if *wildcard != *current.prefix {
return Err(InsertError::conflict(&route, remaining, current));
}
if remaining[..segment] != *current.prefix {
return Err(InsertError::conflict(&route, remaining, current));
}

// Catch-all routes cannot have children.
Expand Down Expand Up @@ -402,10 +412,39 @@ impl<T> Node<T> {
prefix = prefix.slice_off(wildcard.start);
}

let (node_type, wildcard) = match prefix.get(wildcard.len()) {
// The entire route segment consists of the wildcard.
None | Some(&b'/') => {
let wildcard_prefix = prefix.slice_until(wildcard.len());
prefix = prefix.slice_off(wildcard_prefix.len());
(NodeType::Param, wildcard_prefix)
}
// The route parameter is followed a static suffix within the current segment.
_ => {
let end = prefix
.iter()
.position(|&b| b == b'/')
.unwrap_or(prefix.len());

let wildcard_prefix = prefix.slice_until(end);
let suffix = wildcard_prefix.slice_off(wildcard.len());

// Multiple parameters within the same segment, e.g. `/{foo}{bar}`.
if matches!(find_wildcard(suffix), Ok(Some(_))) {
return Err(InsertError::InvalidParamSegment);
}

prefix = prefix.slice_off(end);

let suffix_start = wildcard.len();
(NodeType::ParamSuffix { suffix_start }, wildcard_prefix)
}
};

// Add the parameter as a child node.
let child = Self {
node_type: NodeType::Param,
prefix: prefix.slice_until(wildcard.len()).to_owned(),
node_type,
prefix: wildcard.to_owned(),
..Self::default()
};

Expand All @@ -415,8 +454,7 @@ impl<T> Node<T> {
current.priority += 1;

// If the route doesn't end in the wildcard, we have to insert the suffix as a child.
if wildcard.len() < prefix.len() {
prefix = prefix.slice_off(wildcard.len());
if !prefix.is_empty() {
let child = Self {
priority: 1,
..Self::default()
Expand Down Expand Up @@ -616,6 +654,75 @@ impl<T> Node<T> {
// Otherwise, there are no matching routes in the tree.
return Err(MatchError::NotFound);
}
NodeType::ParamSuffix { suffix_start } => {
let suffix = &current.prefix[suffix_start..];

// Check for more path segments.
let end = match path.iter().position(|&c| c == b'/') {
// Double `//` implying an empty parameter, no match.
Some(0) => {
try_backtrack!();
return Err(MatchError::NotFound);
}
// Found another segment.
Some(i) => i,
// This is the last path segment.
None => path.len(),
};

// The path cannot contain a non-empty parameter and the suffix.
if suffix.len() >= end {
try_backtrack!();
return Err(MatchError::NotFound);
}

// Ensure the suffix matches.
for (a, b) in path[..end].iter().rev().zip(suffix.iter().rev()) {
if a != b {
try_backtrack!();
return Err(MatchError::NotFound);
}
}

let param = &path[..end - suffix.len()];
let rest = &path[end..];

if rest.is_empty() {
let value = match current.value {
// Found the matching value.
Some(ref value) => value,
// Otherwise, this route does not match.
None => {
try_backtrack!();
return Err(MatchError::NotFound);
}
};

// Store the parameter value.
params.push(b"", param);

// Remap the keys of any route parameters we accumulated during the search.
params.for_each_key_mut(|(i, key)| *key = &current.remapping[i]);

return Ok((value, params));
}

// If there is a static child, continue the search.
if let [child] = current.children.as_slice() {
// Store the parameter value.
params.push(b"", param);

// Continue searching.
path = rest;
current = child;
backtracking = false;
continue 'walk;
}

// Otherwise, this route does not match.
try_backtrack!();
return Err(MatchError::NotFound);
}
NodeType::CatchAll => {
// Catch-all segments are only allowed at the end of the route, meaning
// this node must contain the value.
Expand Down Expand Up @@ -785,13 +892,6 @@ fn find_wildcard(path: UnescapedRef<'_>) -> Result<Option<Range<usize>>, InsertE
return Err(InsertError::InvalidParam);
}

if let Some(&c) = path.get(i + 1) {
// Prefixes after route parameters are not supported.
if c != b'/' {
return Err(InsertError::InvalidParamSegment);
}
}

return Ok(Some(start..i + 1));
}
// `*` and `/` are invalid in parameter names.
Expand Down
14 changes: 13 additions & 1 deletion tests/insert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ fn wildcard_conflict() {
("/user_{bar}", Err(conflict("/user_{name}"))),
("/id{id}", Ok(())),
("/id/{id}", Ok(())),
("/x/{id}", Ok(())),
("/x/{id}/", Ok(())),
("/x/{id}y", Err(conflict("/x/{id}/"))),
("/x/{id}y/", Err(conflict("/x/{id}/"))),
("/x/x{id}", Ok(())),
("/x/x{id}y", Err(conflict("/x/x{id}"))),
("/qux/id", Ok(())),
("/qux/{id}y", Ok(())),
("/qux/{id}", Err(conflict("/qux/{id}y"))),
("/qux/{id}/", Err(conflict("/qux/{id}y"))),
("/qux/{id}x", Err(conflict("/qux/{id}y"))),
("/qux/x{id}y", Ok(())),
("/qux/x{id}", Err(conflict("/qux/x{id}y"))),
])
.run()
}
Expand Down Expand Up @@ -210,7 +223,6 @@ fn invalid_param() {
("}", Err(InsertError::InvalidParam)),
("x{y", Err(InsertError::InvalidParam)),
("x}", Err(InsertError::InvalidParam)),
("/{foo}s", Err(InsertError::InvalidParamSegment)),
])
.run();
}
Expand Down
41 changes: 40 additions & 1 deletion tests/match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,50 @@ fn escaped() {
#[test]
fn empty_param() {
MatchTest {
routes: vec!["/y/{foo}", "/x/{foo}/z", "/z/{*xx}"],
routes: vec![
"/y/{foo}",
"/x/{foo}/z",
"/z/{*foo}",
"/a/x{foo}",
"/b/{foo}x",
],
matches: vec![
("/y/", "", Err(())),
("/x//z", "", Err(())),
("/z/", "", Err(())),
("/a/x", "", Err(())),
("/b/x", "", Err(())),
],
}
.run();
}

#[test]
fn wildcard_suffix() {
MatchTest {
routes: vec![
"/",
"/{foo}x",
"/foox",
"/{foo}x/bar",
"/{foo}x/bar/baz",
"/x{foo}",
"/x{foo}/bar",
],
matches: vec![
("/", "/", p! {}),
("/foox", "/foox", p! {}),
("/barx", "/{foo}x", p! { "foo" => "bar" }),
("/mx", "/{foo}x", p! { "foo" => "m" }),
("/mx/", "", Err(())),
("/mxm", "", Err(())),
("/mx/bar", "/{foo}x/bar", p! { "foo" => "m" }),
("/mxm/bar", "", Err(())),
("/x", "", Err(())),
("/xfoo", "/x{foo}", p! { "foo" => "foo" }),
("/xfoox", "/x{foo}", p! { "foo" => "foox" }),
("/xfoox/bar", "/x{foo}/bar", p! { "foo" => "foox" }),
("/xfoox/bar/baz", "/{foo}x/bar/baz", p! { "foo" => "xfoo" }),
],
}
.run();
Expand Down
Loading