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

Support middleware hierarchy for wildcards #23

Open
DerekDuchesne opened this issue Jul 19, 2016 · 5 comments
Open

Support middleware hierarchy for wildcards #23

DerekDuchesne opened this issue Jul 19, 2016 · 5 comments

Comments

@DerekDuchesne
Copy link

Hello,

Currently, when registering middleware on paths with wildcards, you have to use the entire path, not just a prefix.

For example:
kami.Use("/users/", CheckAdminPermission) would affect /users/:id/likes/count because it contains the "/users/" prefix.

However:
kami.Use("/users/:id/", CheckAdminPermission) would not affect /users/:id/likes/count because of the wildcard ":id"

Would it be possible to support registering prefixes with wildcards in the future? Could you also update the documentation to say that the full path has to be used for wildcards?

I've also noticed that you can only register one middleware function per path with a wildcard. Is it feasible to add support for registering multiple middleware functions on a single path?

Thanks, and excellent project!

@guregu
Copy link
Owner

guregu commented Jul 28, 2016

Hello, sorry for the late response.

Wildcard middleware was kind of hacked together. It could definitely use some love.

About registering prefixes: it is definitely possible, but a nicer API would require us to dig into the httptreemux library and add some custom modifications for kami. I haven't got around to doing this yet.

Currently you could do something like this to get similar results:

kami.Use("/users/:id/", CheckAdminPermission)
kami.Use("/users/:id/*", CheckAdminPermission)

It would definitely be possible to allow multiple middleware functions to be registered for one wildcard path. This would be easier to do than supporting prefixes. I will work on this soon.

Thank you for your kind words and let me know if you have any other questions.

@guregu
Copy link
Owner

guregu commented Sep 8, 2016

I've also noticed that you can only register one middleware function per path with a wildcard. Is it feasible to add support for registering multiple middleware functions on a single path?

I've taken care of this in #27! @DerekDuchesne

@DerekDuchesne
Copy link
Author

Perfect. Thanks, @guregu!

@asgaines
Copy link
Contributor

I've been enjoying this project thus far, but stumbled against the same concerns mentioned by @DerekDuchesne for a couple hours. I think it's worthy to mention in the documentation the temporary fix of adding * to provide the same behavior with non-parameterized url strings.

I would, however, love to have the prefix matching behave identically to non-parameterized strings since it causes confusion about the differences.

@guregu
Copy link
Owner

guregu commented Nov 17, 2017

@asgaines Sorry for the confusion. I agree that it would be nice for both to behave the same. It's definitely something I'd want to add to "kami 2.0"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants