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

Better support for overlapping parameters #678

Closed
ibraheemdev opened this issue Dec 30, 2021 · 1 comment
Closed

Better support for overlapping parameters #678

ibraheemdev opened this issue Dec 30, 2021 · 1 comment
Labels
A-axum C-bug Category: This is a bug.
Milestone

Comments

@ibraheemdev
Copy link
Contributor

ibraheemdev commented Dec 30, 2021

Currently routes like /:a/:b and /:a are supported, but only if they have a common prefix (/:a). This means a route like /:a/:b conflicts with /:c. Having the user forced to change the parameter named doesn't feel very nice.

cc ibraheemdev/matchit#13

@davidpdrsn davidpdrsn added A-axum C-bug Category: This is a bug. labels Dec 30, 2021
@davidpdrsn davidpdrsn added this to the 0.5 milestone Feb 22, 2022
@davidpdrsn
Copy link
Member

I've spent a bunch of time working on this and while its possible it also adds a bunch of extra complexity.

The solution is to normalize the params so routes like /:a/foo/:b would become /:axum_param_0/foo/:axum_param/2. So split the path on / and rename params based on their depth. You then also need to store the mapping back to the original param name so Path can access it.

Opaque nested services is also fun since you need to carry the param mapping forward via a request extension so inner routers can access it. You also need to be careful that inner routers don't use the same normalized param names as outer routers which, since inner routers are created first.

All these things are do-able but I don't feel they're worth the additional complexity. I haven't heard anyone else ask about this so I'd consider this an edge case. Ideally it should be fixed in matchit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-axum C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants