Routes and onhandlers

[aldas]

• Fix situation when Echo instance is used to serve multiple hosts. In this case all registered routes are seen from e.Routes() map but problem arises when multiple hosts have routes with same method+path - in this case latest added will only be in e.Routes() output.

  • e.Routes() will only report routes added to default router (hosts = "")
  • Routes for specific hosts are accessed by e.Routers()["domain2.router.com"].Routes()
  • Router has now new method Reverse(name string, params ...interface{}) string. Echos own Reverse() will call default router Reverse now.

• Added handler to echo instance to help keeping track what routes are registered in a centralized way. There is new handler field:

e := echo.New()
e.OnAddRouteHandler = func(host string, route Route, handler HandlerFunc, middleware []MiddlewareFunc) {
  // for example: add this route info to your own registry 
}

[codecov]

Codecov Report

Base: 92.37% // Head: 92.75% // Increases project coverage by +0.37% 🎉

Coverage data is based on head (ae20870) compared to base (0ce7302).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2337      +/-   ##
==========================================
+ Coverage   92.37%   92.75%   +0.37%     
==========================================
  Files          37       37              
  Lines        4446     4471      +25     
==========================================
+ Hits         4107     4147      +40     
+ Misses        247      236      -11     
+ Partials       92       88       -4     

Impacted Files	Coverage Δ
echo.go	95.47% <100.00%> (+0.29%)	⬆️
router.go	97.79% <100.00%> (+0.14%)	⬆️
middleware/logger.go	84.89% <0.00%> (-1.68%)	⬇️
ip.go	100.00% <0.00%> (ø)
middleware/compress.go	81.15% <0.00%> (ø)
middleware/body_dump.go	90.47% <0.00%> (ø)
middleware/extractor.go	99.13% <0.00%> (+0.01%)	⬆️
middleware/request_logger.go	97.56% <0.00%> (+0.08%)	⬆️
context.go	88.88% <0.00%> (+0.19%)	⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[aldas]

@lammel could you review. I think this would be last PR for next release.

[lammel]

Looks good! Only some minor questions arised and some suggestions for comments.

But nothing to hold me back from saying.... Approved!

[lammel]

Could you detail why Add() does not add a route?
If it is just a test for backward compatibility it should not be in the testcase loop here I guess.

[aldas]

because Add signature is func (r *Router) Add(method, path string, h HandlerFunc) {
and add signature is func (r *Router) add(method, path, name string, h HandlerFunc) *Route { and I needed way to return routes from Router.

add was introduced so we could create Route object inside router. As router should be the creator of routes not Echo object. Previously it was done inside echo.add method and cause problems when multiple different hosts are used in application

echo/echo.go

Lines 530 to 545 in 8d4ac4c

	func (e *Echo) add(host, method, path string, handler HandlerFunc, middleware ...MiddlewareFunc) *Route {
	name := handlerName(handler)
	router := e.findRouter(host)
	// FIXME: when handler+middleware are both nil ... make it behave like handler removal
	router.Add(method, path, func(c Context) error {
	h := applyMiddleware(handler, middleware...)
	return h(c)
	})
	r := &Route{
	Method: method,
	Path: path,
	Name: name,
	}
	e.router.routes[method+path] = r
	return r
	}

and this Add call in this test is to document that behavior.

[lammel]

So to summarize: This test ensures the routes added via Add() do not show up in non-global routers.

Maybe you can improve the comment. I had to lookup the code
Suggestion (please verify if it makes sense):

-                       // Add does not add route. because of backwards compatibility we can not change this method signature
+                       // Verify backwards compatibility for documented behavior for router.Add() 
+                       // Routes added via Add() shall not show up for dedicated routers (e.g. used for host based routes)
 			route.Add("LOCK", "/users", handlerFunc)

[aldas]

@lammel please re-review. I changed the suggested comments.

[lammel]

LGTM