From c955d76f5d45a251fde2197deef59a6ec07e6aa1 Mon Sep 17 00:00:00 2001 From: Chris Hurst <46472228+ytsruh@users.noreply.github.com> Date: Mon, 5 Jun 2023 12:00:51 +0100 Subject: [PATCH] :bug: bug: fix middleware naming and returned values of group methods (#2477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Bug fix: route names not updating * fixed lint error * updated tests with renaming edge case * fix group naming partially * add todo * fix todo * fix todo --------- Co-authored-by: Muhammed Efe Çetin --- app.go | 25 +++++++++++++++------- app_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++ group.go | 39 +++++++++++++++++++++++++++------- hooks_test.go | 13 +++++++++++- router.go | 6 ++---- 5 files changed, 121 insertions(+), 21 deletions(-) diff --git a/app.go b/app.go index d5f07003..69b7d089 100644 --- a/app.go +++ b/app.go @@ -609,18 +609,23 @@ func (app *App) SetTLSHandler(tlsHandler *TLSHandler) { // Name Assign name to specific route. func (app *App) Name(name string) Router { app.mutex.Lock() + defer app.mutex.Unlock() - latestGroup := app.latestRoute.group - if latestGroup != nil { - app.latestRoute.Name = latestGroup.name + name - } else { - app.latestRoute.Name = name + for _, routes := range app.stack { + for _, route := range routes { + if route.Path == app.latestRoute.path { + route.Name = name + + if route.group != nil { + route.Name = route.group.name + route.Name + } + } + } } if err := app.hooks.executeOnNameHooks(*app.latestRoute); err != nil { panic(err) } - app.mutex.Unlock() return app } @@ -754,12 +759,16 @@ func (app *App) Patch(path string, handlers ...Handler) Router { // Add allows you to specify a HTTP method to register a route func (app *App) Add(method, path string, handlers ...Handler) Router { - return app.register(method, path, nil, handlers...) + app.register(method, path, nil, handlers...) + + return app } // Static will create a file server serving static files func (app *App) Static(prefix, root string, config ...Static) Router { - return app.registerStatic(prefix, root, config...) + app.registerStatic(prefix, root, config...) + + return app } // All will register the handler on all HTTP methods diff --git a/app_test.go b/app_test.go index 8ab6c76f..09600b72 100644 --- a/app_test.go +++ b/app_test.go @@ -1803,3 +1803,62 @@ func TestApp_GetRoutes(t *testing.T) { utils.AssertEqual(t, name, route.Name) } } + +func Test_Middleware_Route_Naming_With_Use(t *testing.T) { + named := "named" + app := New() + + app.Get("/unnamed", func(c *Ctx) error { + return c.Next() + }) + + app.Post("/named", func(c *Ctx) error { + return c.Next() + }).Name(named) + + app.Use(func(c *Ctx) error { + return c.Next() + }) // no name - logging MW + + app.Use(func(c *Ctx) error { + return c.Next() + }).Name("corsMW") + + app.Use(func(c *Ctx) error { + return c.Next() + }).Name("compressMW") + + app.Use(func(c *Ctx) error { + return c.Next() + }) // no name - cache MW + + grp := app.Group("/pages").Name("pages.") + grp.Use(func(c *Ctx) error { + return c.Next() + }).Name("csrfMW") + + grp.Get("/home", func(c *Ctx) error { + return c.Next() + }).Name("home") + + grp.Get("/unnamed", func(c *Ctx) error { + return c.Next() + }) + + for _, route := range app.GetRoutes() { + switch route.Path { + case "/": + utils.AssertEqual(t, "compressMW", route.Name) + case "/unnamed": + utils.AssertEqual(t, "", route.Name) + case "named": + utils.AssertEqual(t, named, route.Name) + case "/pages": + utils.AssertEqual(t, "pages.csrfMW", route.Name) + case "/pages/home": + utils.AssertEqual(t, "pages.home", route.Name) + case "/pages/unnamed": + utils.AssertEqual(t, "", route.Name) + } + } +} diff --git a/group.go b/group.go index 91c28062..0e546a3f 100644 --- a/group.go +++ b/group.go @@ -11,17 +11,26 @@ import ( // Group struct type Group struct { - app *App - parentGroup *Group - name string + app *App + parentGroup *Group + name string + anyRouteDefined bool Prefix string } -// Name Assign name to specific route. +// Name Assign name to specific route or group itself. +// +// If this method is used before any route added to group, it'll set group name and OnGroupNameHook will be used. +// Otherwise, it'll set route name and OnName hook will be used. func (grp *Group) Name(name string) Router { - grp.app.mutex.Lock() + if grp.anyRouteDefined { + grp.app.Name(name) + return grp + } + + grp.app.mutex.Lock() if grp.parentGroup != nil { grp.name = grp.parentGroup.name + name } else { @@ -76,6 +85,10 @@ func (grp *Group) Use(args ...interface{}) Router { grp.app.register(methodUse, getGroupPath(grp.Prefix, prefix), grp, handlers...) } + if !grp.anyRouteDefined { + grp.anyRouteDefined = true + } + return grp } @@ -135,12 +148,22 @@ func (grp *Group) Patch(path string, handlers ...Handler) Router { // Add allows you to specify a HTTP method to register a route func (grp *Group) Add(method, path string, handlers ...Handler) Router { - return grp.app.register(method, getGroupPath(grp.Prefix, path), grp, handlers...) + grp.app.register(method, getGroupPath(grp.Prefix, path), grp, handlers...) + if !grp.anyRouteDefined { + grp.anyRouteDefined = true + } + + return grp } // Static will create a file server serving static files func (grp *Group) Static(prefix, root string, config ...Static) Router { - return grp.app.registerStatic(getGroupPath(grp.Prefix, prefix), root, config...) + grp.app.registerStatic(getGroupPath(grp.Prefix, prefix), root, config...) + if !grp.anyRouteDefined { + grp.anyRouteDefined = true + } + + return grp } // All will register the handler on all HTTP methods @@ -158,7 +181,7 @@ func (grp *Group) All(path string, handlers ...Handler) Router { func (grp *Group) Group(prefix string, handlers ...Handler) Router { prefix = getGroupPath(grp.Prefix, prefix) if len(handlers) > 0 { - _ = grp.app.register(methodUse, prefix, grp, handlers...) + grp.app.register(methodUse, prefix, grp, handlers...) } // Create new group diff --git a/hooks_test.go b/hooks_test.go index 6a2fc3bf..83bd8a91 100644 --- a/hooks_test.go +++ b/hooks_test.go @@ -139,6 +139,9 @@ func Test_Hook_OnGroupName(t *testing.T) { buf := bytebufferpool.Get() defer bytebufferpool.Put(buf) + buf2 := bytebufferpool.Get() + defer bytebufferpool.Put(buf2) + app.Hooks().OnGroupName(func(g Group) error { _, err := buf.WriteString(g.name) utils.AssertEqual(t, nil, err) @@ -146,11 +149,19 @@ func Test_Hook_OnGroupName(t *testing.T) { return nil }) + app.Hooks().OnName(func(r Route) error { + _, err := buf2.WriteString(r.Name) + utils.AssertEqual(t, nil, err) + + return nil + }) + grp := app.Group("/x").Name("x.") - grp.Get("/test", testSimpleHandler) + grp.Get("/test", testSimpleHandler).Name("test") grp.Get("/test2", testSimpleHandler) utils.AssertEqual(t, "x.", buf.String()) + utils.AssertEqual(t, "x.test", buf2.String()) } func Test_Hook_OnGroupName_Error(t *testing.T) { diff --git a/router.go b/router.go index a5aac7f6..312e161e 100644 --- a/router.go +++ b/router.go @@ -225,7 +225,7 @@ func (*App) copyRoute(route *Route) *Route { } } -func (app *App) register(method, pathRaw string, group *Group, handlers ...Handler) Router { +func (app *App) register(method, pathRaw string, group *Group, handlers ...Handler) { // Uppercase HTTP methods method = utils.ToUpper(method) // Check if the HTTP method is valid unless it's USE @@ -302,10 +302,9 @@ func (app *App) register(method, pathRaw string, group *Group, handlers ...Handl // Add route to stack app.addRoute(method, &route, isMount) } - return app } -func (app *App) registerStatic(prefix, root string, config ...Static) Router { +func (app *App) registerStatic(prefix, root string, config ...Static) { // For security we want to restrict to the current work directory. if root == "" { root = "." @@ -441,7 +440,6 @@ func (app *App) registerStatic(prefix, root string, config ...Static) Router { app.addRoute(MethodGet, &route) // Add HEAD route app.addRoute(MethodHead, &route) - return app } func (app *App) addRoute(method string, route *Route, isMounted ...bool) {