feat: add wildcard path route#586
Conversation
github-actions
Bot
added
A-docs
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
ElijahAhianyo
left a comment
ElijahAhianyo
left a comment
There was a problem hiding this comment.
@dharshan-0 Thanks a lot for your contribution. This is a great start! Let's address the comments, and it's good to merge.
|
@ElijahAhianyo Thanks for your comments, I will look into it. |
…nto wildcard-feature pulled remote changes
Co-authored-by: EBADF <elijahahianyo@gmail.com>
ElijahAhianyo
left a comment
ElijahAhianyo
left a comment
There was a problem hiding this comment.
@dharshan-0 Thanks for your contribution once again!
seqre
left a comment
seqre
left a comment
There was a problem hiding this comment.
Overall, it's a great job, thank you for your contribution!
I see one possible clash with overlapping routes, eg. /foo/{*path} and /foo/bar. To be honest, I'm not sure which route the framework would use atm. I think that it should first use the qualified path if it exists, and if it doesn't, then match the wildcard one.
I'd like for you to add a test(s) checking that functionality (also with different ordering of route definition in Router::with_urls) and if it does not work like that, to adapt the code. Also, mention that in the documentation, please.
Once that's done, I'll be happy to approve!
|
Thats a valid point, I will look into that |
| #[cot::test] | ||
| async fn prefer_literal_over_placeholder_wildcard() { | ||
| let router = Router::with_urls(vec![ | ||
| Route::with_handler("/foo/{prarm}", MockHandler), |
There was a problem hiding this comment.
| Route::with_handler("/foo/{prarm}", MockHandler), | |
| Route::with_handler("/foo/{param}", MockHandler), |
| pub(crate) fn compare_weights(a: &[u8], b: &[u8]) -> std::cmp::Ordering { | ||
| let max_len = std::cmp::max(a.len(), b.len()); | ||
| let mut score: isize = 0; | ||
| let mut lexical_order_a = String::with_capacity(a.len()); | ||
| let mut lexical_order_b = String::with_capacity(b.len()); | ||
|
|
||
| for i in 0..max_len { | ||
| let (wa, wb) = match (a.get(i), b.get(i)) { | ||
| (None, _) => return std::cmp::Ordering::Less, | ||
| (_, None) => return std::cmp::Ordering::Greater, | ||
| (Some(&w), Some(&v)) => (w, v), | ||
| }; | ||
| lexical_order_a.push(char::from_digit(u32::from(wa), 10).unwrap()); | ||
| lexical_order_b.push(char::from_digit(u32::from(wb), 10).unwrap()); | ||
|
|
||
| if wa == 2 && wb == 2 { | ||
| return lexical_order_a.cmp(&lexical_order_b); | ||
| } | ||
| if wa == 2 { | ||
| return std::cmp::Ordering::Greater; | ||
| } | ||
| if wb == 2 { | ||
| return std::cmp::Ordering::Less; | ||
| } | ||
| if wa != wb { | ||
| if wa < wb { | ||
| score += 1; | ||
| } else { | ||
| score -= 1; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if score != 0 { | ||
| return if score > 0 { | ||
| std::cmp::Ordering::Less | ||
| } else { | ||
| std::cmp::Ordering::Greater | ||
| }; | ||
| } | ||
|
|
||
| lexical_order_a.cmp(&lexical_order_b) |
There was a problem hiding this comment.
Why this whole logic is here? Why can't you just compare the weights vectors?
3 participants
Related issue or discussion
Description
It closes #545 by adding wildcard routing feature.
It uses this
{*param_name}to define wildcard.Type of change
Checklist
just test-all)just clippy)cargo fmt)