Skip to content

Commit

Permalink
🐛 bug: fix middleware naming and returned values of group methods (#2477
Browse files Browse the repository at this point in the history
)

* 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 <efectn@protonmail.com>
  • Loading branch information
ytsruh and efectn authored Jun 5, 2023
1 parent 4c12938 commit c955d76
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 21 deletions.
25 changes: 17 additions & 8 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
59 changes: 59 additions & 0 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
39 changes: 31 additions & 8 deletions group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
13 changes: 12 additions & 1 deletion hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,29 @@ 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)

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) {
Expand Down
6 changes: 2 additions & 4 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = "."
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit c955d76

Please sign in to comment.