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

wildcard route error #388

Closed
libnat opened this issue Jul 26, 2015 · 29 comments · Fixed by #2663
Closed

wildcard route error #388

libnat opened this issue Jul 26, 2015 · 29 comments · Fixed by #2663

Comments

@libnat
Copy link

libnat commented Jul 26, 2015

I think this router is valid, but it cause an error.

r.GET("/teachers/list", func (c *gin.Context){})
r.GET("/teachers/:id/profile", func (c *gin.Context){})

Error:
[GIN-debug] GET /teachers/list --> main.func·001 (3 handlers)
[GIN-debug] GET /teachers/:id/profile --> main.func·002 (3 handlers)
panic: wildcard route ':id' conflicts with existing children in path '/teachers/:id/profile'

@ralph327
Copy link

This issue is related to https://github.com/julienschmidt/httprouter/.

/teachers/list and teachers/:id conflict.

Look at Issue 73 from the project for more info: julienschmidt/httprouter#73

@javierprovecho
Copy link
Member

I suggest to use 

/teachers/list

And

/teacher/:id/profile

It is a better routing.

NOTE: The information in this e-mail is confidential, being solely for the addressee named above. If you read this message and not the intended recipient, please note that it is completely forbidden any use, disclosure, distribution and / or duplication of this communication without the consent of the sender. If you have received this message in error, please notify us immediately by electronic mail and its elimination.

On Sun, Jul 26, 2015 at 8:45 PM, Ralph [email protected] wrote:

This issue is related to https://github.com/julienschmidt/httprouter/.
/teachers/list and teachers/:id conflict.

Look at Issue 73 from the project for more info: julienschmidt/httprouter#73

Reply to this email directly or view it on GitHub:
#388 (comment)

@libnat
Copy link
Author

libnat commented Jul 27, 2015

@javierprovecho Thanks for your suggestion. It is an option. The naming style I chose is similar to twitter REST api. I think teachers means a resource collection. /teachers/:id is used to get one object from the collection.
It is an option to choose another naming style to avoid this error. However I think the issue is still there.

@justindag
Copy link

@javierprovecho - Thank you for creating this framework, overall it's really well done. However, this is a SERIOUS limitation. To @JustinTan 's point.. Having to coerce your URLs into something not very RESTy or adding additional string parsing in your handlers is not really a solution IMHO. Please prioritize upgrading the routing capability. (gorilla/mux is really nice). Thank you.

@javierprovecho
Copy link
Member

@justindag we will take note of your suggestion, however gin is a framework focused in speed, feature we achieved as #1. thank you 😉

NOTE: The information in this e-mail is confidential, being solely for the addressee named above. If you read this message and not the intended recipient, please note that it is completely forbidden any use, disclosure, distribution and / or duplication of this communication without the consent of the sender. If you have received this message in error, please notify us immediately by electronic mail and its elimination.

On Tue, Aug 18, 2015 at 3:46 AM, justindag [email protected]
wrote:

@javierprovecho - Thank you for creating this framework, overall it's really well done. However, this is a SERIOUS limitation. To @JustinTan 's point.. Having to coerce your URLs into something not very RESTy or adding additional string parsing in your handlers is not really a solution IMHO. Please prioritize upgrading the routing capability. (gorilla/mux is really nice). Thank you.

Reply to this email directly or view it on GitHub:
#388 (comment)

@bradrydzewski
Copy link

I hit the same issue. I think this is a pretty common pattern, and something I would love to see gin accommodate. You can see similar patterns in GitHub, Twitter, Facebook, etc:

github.com/
github.com/issues
github.com/:user
github.com/:user/:repo

twitter.com/mentions
twitter.com/i/notifications
twitter.com/:username

www.facebook.com/events/upcoming
www.facebook.com/messages/
www.facebook.com/:username

I definitely appreciate gin's performance and simple api, and am huge fans of what you guys have done. I do, however, find the above restrictions a bit limiting for real-world applications.

@ralph327
Copy link

Would it be possible to create a fork of julienschmidt/httprouter/ to achieve these kinds of patterns? Maybe a subpackage could be created to drop-in the altered router in place of the router that Gin normally uses.

This way a user has the choice between pure speed and more flexible routes.

@bradrydzewski
Copy link

@ralph327 it looks like v2 of the httprouter will resolve this issue (see julienschmidt/httprouter#73) however, I'm not sure gin uses httprouter any more, does it? I didn't see it listed in Godeps and somewhere I though I read it was removed.

@ralph327
Copy link

@bradrydzewski they use a custom version of it. It's baked right into gin so there are no dependencies.

@libnat
Copy link
Author

libnat commented Sep 28, 2015

I am developing APIs for my mobile client. I have completed a lightweight RESTFul server in order to support the routing. https://github.com/justintan/wine

@MrSagan
Copy link

MrSagan commented Nov 20, 2015

So... Any progress on this?

@johnnncodes
Copy link

+1. I'm experiencing the same issue.

@jyoon17
Copy link

jyoon17 commented Jan 22, 2016

+1

@oharlem
Copy link

oharlem commented Feb 3, 2016

I'm working on a complex API, more than 100 endpoints, a lot of resources requiring much more actions than just the holy 4 get/post/put/delete. For example, for liking comments of stories, a correct, let's say more canonical approach would be to use /stories/:storyID/comments/:commentID/like.
With dozens of endpoints like this, I had to come up with a bunch of workarounds and that honestly freaks me out totally. Especially if you try to work "by the book" and build/follow RAML specs.

@javierprovecho , your work is truly great, but your comment above "we will take note of your suggestion, however gin is a framework focused in speed, feature we achieved as" is very arrogant.

Maybe I missed the point and following normal API design would compromise performance of Gin? If so, it would be great to know the details for all of us to understand to what extent we can rely on Gin.

Cheers
D.

@CaptainCodeman
Copy link

I can see both arguments. On the one hand, the proper RESTful routes tend to be clear and unambiguous with less chance for conflicts:

GET /teachers for the list
GET /teachers/:id for an item in the list

With 'proper' URL design it isn't a problem. Hint: make your URLs more like the git, gplus examples inthe router test package.

But, if you have other URLs to support it isn't possible. They cause a conflict even though, they can clearly be distinguished:

/:topic/comments
/:user/profile

When the difference is the parameter, it can be worked around by making the parameter segment the same name, i.e. changing :topic and :user above to :id works. But if one of the segments needs to be static then you are hooped.

The way echo handles this is to give priority in the order of segments: static routes -> parameters -> wildcards. That seems to me to be the most 'correct' way to approach it.

Hopefully that is possible to so without slowing down the router (too much) although some of the speed currently obtained is by ordering router matching to prioritize those with the most sub-routes (which may artificially boost benchmarks but doesn't necessarily mean that those routes are the busiest in a real app).

So, benchmarks may suffer a little to support this but it doesn't mean the router really becomes any slower (maybe the benchmarks need to be updated).

@veegee
Copy link

veegee commented Mar 29, 2016

This is highly disappointing. The author arrogantly refuses to acknowledge this limitation as a serious design flaw. This router encourages poor API design; it's not "proper", it's just a cop out. Luckily, better alternatives like gorilla/mux exist that are much more flexible and handle cases like this correctly.

@CaptainCodeman
Copy link

It's a design decision, I think attributing motives to things is a little unfair. The best solution is to simply use the router that best fits your needs - there's no need to insult anyone.

@javierprovecho
Copy link
Member

@mpmlj sorry if I was "arrogant", definitely not my intention. Gin was born because of httprouter, possibly the fastest router in that moment, and so was the objetive to keep the speed of it.

@veegee please dont criticize only, and propose a solution if you have any

@CaptainCodeman about your suggestion, I think /:topic/comments and /:user/profile is wrong. If you really need to declare those routes, I would implement a single handler, which redirects to an apropiate endpoint such as /users/:user/profile.

@veegee
Copy link

veegee commented Mar 31, 2016

@javierprovecho The solution is to change the router implementation to correctly handle obviously non-conflicting routes instead of worrying about one or two CPU clock cycles.

@CaptainCodeman
Copy link

I would implement a single handler, which redirects to an apropiate endpoint such as /users/:user/profile.

Yes, in an ideal world. Sometimes though you may have URLs that can't be changed for legacy reasons. I found the route limitation is the one thing that prevents me using gin more often.

I'd vote for making it more flexible like Echo and it sounds like the next version of httpRouter will go that way but if not, it's a design choice I understand.

@javierprovecho
Copy link
Member

@veegee ginv1 is not going to change and break compatibility. However I'll try to make the router as much flexible as possible in ginv2 (which it will work with pluggable http packages). thank you!

@javierprovecho
Copy link
Member

@CaptainCodeman definitely some other frameworks such as echo are a good place to search for inspiration in new features or enhancements. 😉

@nazwa
Copy link

nazwa commented Mar 31, 2016

I must admit the route parameter conflict was a bit annoying at times, but actually in a way, it always forced me to think of better (more structured) approaches to my endpoints.

So yes - I totally support the current structure and all the benefits it provides. There is simply no need for complex regex matching on something as crucial as routing.

@veegee
Copy link

veegee commented Mar 31, 2016

@javierprovecho at the end of the day, the design is up to you, of course. I accidentally stumbled across the Goji library and I'd like to share what the author has to say because I believe it's very important:

Is it fast?

Yeah, it is. Goji is among the fastest HTTP routers out there, and is very gentle on the garbage collector.

But that's sort of missing the point. Almost all Go routers are fast enough for almost all purposes. In my opinion, what matters more is how simple and flexible the routing semantics are.

[...]

It's easy (and quite a bit of fun!) to get carried away by microbenchmarks, but at the end of the day you're not going to miss those extra hundred nanoseconds on a request. What matters is that you aren't compromising on the API for a handful of CPU cycles.

He's hit the nail on the head. The time/memory/allocations consumed by the router absolutely eclipse the resources consumed by the request handler itself. And by crippling the router to the point where it forbids certain (totally otherwise valid) routes, there's very little to gain and a lot to lose.

@nazwa
Copy link

nazwa commented Mar 31, 2016

Being flexible is important but also you can never have one tool that fits all scenarios.
If you need complex routing - use a framework that allows it.
If you need to handle thousands of transactions a day on a tiny server - use one that 's been designed for such use. As they say - "every little helps".

@veegee
Copy link

veegee commented Mar 31, 2016

@nazwa Even the slowest Go router to exist today can very easily handle thousands of requests per second on a Raspberry Pi without getting in the way. It's very hard to fathom how short a nanosecond is, but the time difference between a "slow" router and a "fast" router is measured in the order nanoseconds. It's really a negligible amount unless your entire application consists of nothing but routing. If your request handler isn't taking at least 10x the resources as the router, you're doing something wrong.

@bradrydzewski
Copy link

@nazwa are you speaking for the project maintainers here? Is the official stance of the project to invest in alternatives if you care about this feature?

I suggest we get an official reply here so that this stops spamming my inbox. Is there interest in resolving this issue? Is there interest in exploring other algorithms that are equally as fast and efficient but don't have this limitation? Is there interest in implement httprouter v2 if, as advertised, it removes this restriction? If yes let's stop the back and forth on whether or not there is merit (I certainly think there is) and use this thread for status updates or suggested implementations for improvements. If there is no interest in making this improvement let's lock and close the issue.

@nazwa
Copy link

nazwa commented Mar 31, 2016

@bradrydzewski I'm here of my own account, sharing my own two cents - that's it. The whole conversation felt like a massive rant about "this needs to change", so thought I'd defend current gin a little 👍

@javierprovecho
Copy link
Member

@bradrydzewski (@nazwa) is a contributor from the beginnings of this project. I'm locking this issue to avoid more spam reach your inbox, but also bringing here @manucorporat so he can try to answer this question if he has anything to say about it.

@gin-gonic gin-gonic locked and limited conversation to collaborators Mar 31, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet