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
Making Echo#Reverse() deterministic #1893
base: master
Are you sure you want to change the base?
Making Echo#Reverse() deterministic #1893
Conversation
When the same handler/name is assigned to more than one route, Echo now returns the first route every time.
Codecov Report
@@ Coverage Diff @@
## master #1893 +/- ##
=======================================
Coverage 90.22% 90.22%
=======================================
Files 31 31
Lines 2773 2773
=======================================
Hits 2502 2502
Misses 173 173
Partials 98 98
Continue to review full report at Codecov.
|
@pafuent I do not know if making it slice helps. Router Node can have only single handler per method type. So if you add route with same path+method multiple times last one is set and previous is forgotten but if you keep adding it to slice you would have routes in that slice that are no longer in actually router. Line 297 in fdacff0
|
p.s. I think router *Router
routers map[string]*Router // <--- those are not checked |
You are right, and as you mention, that behavior is a property of the current implementation, but it shouldn't be something that Echo users should/need to do. If I got correctly your point, the app should set the same route and method more than one time, and that doesn't seem to be a valid use case. Besides of that I agree that a slice is not suited to handle this use case, so I'll try to came up with something else.
Well, when I read your comment, I though the same, but after writing a test that in my mind should fail (but passed), I discovered that because Echo#Host is implemented through Groups, and that Groups at the end calls Echo#add, the routes of all the routers in Echo#routers are on routes of Echo#routes. This of course sounds pretty awkward to me. I'll try to see if I can also change this to make more sense. |
Some historical info why
One thing worth investigating is node.ppath has original route value and why p.s. fields |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed within a month if no further activity occurs. Thank you for your contributions. |
This is addressed in |
When the same handler/name is assigned to more than one route, Echo now returns the first route every time.
Fixes #1237