From 0cab312f3f87d03350e5553b5fe9b7ecdca92a83 Mon Sep 17 00:00:00 2001 From: Cory Koch Date: Sun, 10 Nov 2024 23:08:05 -0500 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=94=A5=20feat:=20Add=20Support=20for?= =?UTF-8?q?=20Removing=20Routes=20(#3230)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add new methods named RemoveRoute and RemoveRouteByName. * Update register method to prevent duplicate routes. * Clean up tests * Update docs --- docs/api/app.md | 53 +++++++++ docs/whats_new.md | 8 ++ router.go | 76 ++++++++++++- router_test.go | 273 +++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 394 insertions(+), 16 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index 6582159c..3447e9d4 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -761,3 +761,56 @@ func main() { ``` In this example, a new route is defined and then `RebuildTree()` is called to ensure the new route is registered and available. + + +## RemoveRoute + +This method removes a route by path. You must call the `RebuildTree()` method after the remove in to ensure the route is removed. + +```go title="Signature" +func (app *App) RemoveRoute(path string, methods ...string) +``` + +This method removes a route by name +```go title="Signature" +func (app *App) RemoveRouteByName(name string, methods ...string) +``` + +```go title="Example" +package main + +import ( + "log" + + "github.com/gofiber/fiber/v3" +) + +func main() { + app := fiber.New() + + app.Get("/api/feature-a", func(c *fiber.Ctx) error { + app.RemoveRoute("/api/feature", fiber.MethodGet) + app.RebuildTree() + // Redefine route + app.Get("/api/feature", func(c *fiber.Ctx) error { + return c.SendString("Testing feature-a") + }) + + app.RebuildTree() + return c.SendStatus(fiber.StatusOK) + }) + app.Get("/api/feature-b", func(c *fiber.Ctx) error { + app.RemoveRoute("/api/feature", fiber.MethodGet) + app.RebuildTree() + // Redefine route + app.Get("/api/feature", func(c *fiber.Ctx) error { + return c.SendString("Testing feature-b") + }) + + app.RebuildTree() + return c.SendStatus(fiber.StatusOK) + }) + + log.Fatal(app.Listen(":3000")) +} +``` diff --git a/docs/whats_new.md b/docs/whats_new.md index bfc6f25c..e0575dd8 100644 --- a/docs/whats_new.md +++ b/docs/whats_new.md @@ -941,6 +941,14 @@ In this example, a new route is defined, and `RebuildTree()` is called to ensure Note: Use this method with caution. It is **not** thread-safe and can be very performance-intensive. Therefore, it should be used sparingly and primarily in development mode. It should not be invoke concurrently. +## RemoveRoute + +- **RemoveRoute**: Removes route by path + +- **RemoveRouteByName**: Removes route by name + +For more details, refer to the [app documentation](./api/app.md#removeroute): + ### 🧠 Context Fiber v3 introduces several new features and changes to the Ctx interface, enhancing its functionality and flexibility. diff --git a/router.go b/router.go index 2091cfc6..cfe83adf 100644 --- a/router.go +++ b/router.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "html" + "slices" "sort" "strings" "sync/atomic" @@ -302,6 +303,13 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler if method != methodUse && app.methodInt(method) == -1 { panic(fmt.Sprintf("add: invalid http method %s\n", method)) } + + // Duplicate Route Handling + if app.routeExists(method, pathRaw) { + matchPathFunc := func(r *Route) bool { return r.Path == pathRaw } + app.deleteRoute([]string{method}, matchPathFunc) + } + // is mounted app isMount := group != nil && group.app != app // A route requires atleast one ctx handler @@ -375,6 +383,72 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler } } +func (app *App) routeExists(method, pathRaw string) bool { + pathToCheck := pathRaw + if !app.config.CaseSensitive { + pathToCheck = utils.ToLower(pathToCheck) + } + + return slices.ContainsFunc(app.stack[app.methodInt(method)], func(r *Route) bool { + routePath := r.path + if !app.config.CaseSensitive { + routePath = utils.ToLower(routePath) + } + + return routePath == pathToCheck + }) +} + +// RemoveRoute is used to remove a route from the stack by path. +// This only needs to be called to remove a route, route registration prevents duplicate routes. +// You should call RebuildTree after using this to ensure consistency of the tree. +func (app *App) RemoveRoute(path string, methods ...string) { + pathMatchFunc := func(r *Route) bool { return r.Path == path } + app.deleteRoute(methods, pathMatchFunc) +} + +// RemoveRouteByName is used to remove a route from the stack by name. +// This only needs to be called to remove a route, route registration prevents duplicate routes. +// You should call RebuildTree after using this to ensure consistency of the tree. +func (app *App) RemoveRouteByName(name string, methods ...string) { + matchFunc := func(r *Route) bool { return r.Name == name } + app.deleteRoute(methods, matchFunc) +} + +func (app *App) deleteRoute(methods []string, matchFunc func(r *Route) bool) { + app.mutex.Lock() + defer app.mutex.Unlock() + + for _, method := range methods { + // Uppercase HTTP methods + method = utils.ToUpper(method) + + // Get unique HTTP method identifier + m := app.methodInt(method) + if m == -1 { + continue // Skip invalid HTTP methods + } + + // Find the index of the route to remove + index := slices.IndexFunc(app.stack[m], matchFunc) + if index == -1 { + continue // Route not found + } + + route := app.stack[m][index] + + // Decrement global handler count + atomic.AddUint32(&app.handlersCount, ^uint32(len(route.Handlers)-1)) //nolint:gosec // Not a concern + // Decrement global route position + atomic.AddUint32(&app.routesCount, ^uint32(0)) + + // Remove route from tree stack + app.stack[m] = slices.Delete(app.stack[m], index, index+1) + } + + app.routesRefreshed = true +} + func (app *App) addRoute(method string, route *Route, isMounted ...bool) { app.mutex.Lock() defer app.mutex.Unlock() @@ -415,7 +489,7 @@ func (app *App) addRoute(method string, route *Route, isMounted ...bool) { // This method is useful when you want to register routes dynamically after the app has started. // It is not recommended to use this method on production environments because rebuilding // the tree is performance-intensive and not thread-safe in runtime. Since building the tree -// is only done in the startupProcess of the app, this method does not makes sure that the +// is only done in the startupProcess of the app, this method does not make sure that the // routeTree is being safely changed, as it would add a great deal of overhead in the request. // Latest benchmark results showed a degradation from 82.79 ns/op to 94.48 ns/op and can be found in: // https://github.com/gofiber/fiber/issues/2769#issuecomment-2227385283 diff --git a/router_test.go b/router_test.go index 5509039c..a1d9c4da 100644 --- a/router_test.go +++ b/router_test.go @@ -12,6 +12,10 @@ import ( "net/http" "net/http/httptest" "os" + "reflect" + "runtime" + "strings" + "sync" "testing" "github.com/gofiber/utils/v2" @@ -369,31 +373,270 @@ func Test_Router_NotFound_HTML_Inject(t *testing.T) { require.Equal(t, "Cannot DELETE /does/not/exist<script>alert('foo');</script>", string(c.Response.Body())) } -func Test_App_Rebuild_Tree(t *testing.T) { - t.Parallel() - app := New() - +func registerTreeManipulationRoutes(app *App, middleware ...func(Ctx) error) { app.Get("/test", func(c Ctx) error { app.Get("/dynamically-defined", func(c Ctx) error { - return c.SendStatus(http.StatusOK) + return c.SendStatus(StatusOK) }) app.RebuildTree() - return c.SendStatus(http.StatusOK) + return c.SendStatus(StatusOK) + }, middleware...) +} + +func verifyRequest(tb testing.TB, app *App, path string, expectedStatus int) *http.Response { + tb.Helper() + + resp, err := app.Test(httptest.NewRequest(MethodGet, path, nil)) + require.NoError(tb, err, "app.Test(req)") + require.Equal(tb, expectedStatus, resp.StatusCode, "Status code") + + return resp +} + +func verifyRouteHandlerCounts(tb testing.TB, app *App, expectedRoutesCount int) { + tb.Helper() + + // this is taken from listen.go's printRoutesMessage app method + var routes []RouteMessage + for _, routeStack := range app.stack { + for _, route := range routeStack { + routeMsg := RouteMessage{ + name: route.Name, + method: route.Method, + path: route.Path, + } + + for _, handler := range route.Handlers { + routeMsg.handlers += runtime.FuncForPC(reflect.ValueOf(handler).Pointer()).Name() + " " + } + + routes = append(routes, routeMsg) + } + } + + for _, route := range routes { + require.Equal(tb, expectedRoutesCount, strings.Count(route.handlers, " ")) + } +} + +func verifyThereAreNoRoutes(tb testing.TB, app *App) { + tb.Helper() + + require.Equal(tb, uint32(0), app.handlersCount) + require.Equal(tb, uint32(0), app.routesCount) + verifyRouteHandlerCounts(tb, app, 0) +} + +func Test_App_Rebuild_Tree(t *testing.T) { + t.Parallel() + app := New() + + registerTreeManipulationRoutes(app) + + verifyRequest(t, app, "/dynamically-defined", StatusNotFound) + verifyRequest(t, app, "/test", StatusOK) + verifyRequest(t, app, "/dynamically-defined", StatusOK) +} + +func Test_App_Remove_Route_A_B_Feature_Testing(t *testing.T) { + t.Parallel() + app := New() + + app.Get("/api/feature-a", func(c Ctx) error { + app.RemoveRoute("/api/feature", MethodGet) + app.RebuildTree() + // Redefine route + app.Get("/api/feature", func(c Ctx) error { + return c.SendString("Testing feature-a") + }) + + app.RebuildTree() + return c.SendStatus(StatusOK) + }) + app.Get("/api/feature-b", func(c Ctx) error { + app.RemoveRoute("/api/feature", MethodGet) + app.RebuildTree() + // Redefine route + app.Get("/api/feature", func(c Ctx) error { + return c.SendString("Testing feature-b") + }) + + app.RebuildTree() + return c.SendStatus(StatusOK) }) - resp, err := app.Test(httptest.NewRequest(MethodGet, "/dynamically-defined", nil)) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, http.StatusNotFound, resp.StatusCode, "Status code") + verifyRequest(t, app, "/api/feature-a", StatusOK) - resp, err = app.Test(httptest.NewRequest(MethodGet, "/test", nil)) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, http.StatusOK, resp.StatusCode, "Status code") + resp := verifyRequest(t, app, "/api/feature", StatusOK) + require.Equal(t, "Testing feature-a", resp, "Response Message") - resp, err = app.Test(httptest.NewRequest(MethodGet, "/dynamically-defined", nil)) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, http.StatusOK, resp.StatusCode, "Status code") + resp = verifyRequest(t, app, "/api/feature-b", StatusOK) + require.Equal(t, "Testing feature-b", resp, "Response Message") +} + +func Test_App_Remove_Route_By_Name(t *testing.T) { + t.Parallel() + app := New() + + app.Get("/api/test", func(c Ctx) error { + return c.SendStatus(StatusOK) + }).Name("test") + + app.RemoveRouteByName("test", MethodGet) + app.RebuildTree() + + verifyRequest(t, app, "/test", StatusNotFound) + verifyThereAreNoRoutes(t, app) +} + +func Test_App_Remove_Route_By_Name_Non_Existing_Route(t *testing.T) { + t.Parallel() + app := New() + + app.RemoveRouteByName("test", MethodGet) + app.RebuildTree() + + verifyThereAreNoRoutes(t, app) +} + +func Test_App_Remove_Route_Nested(t *testing.T) { + t.Parallel() + app := New() + + api := app.Group("/api") + + v1 := api.Group("/v1") + v1.Get("/test", func(c Ctx) error { + return c.SendStatus(StatusOK) + }) + + verifyRequest(t, app, "/api/v1/test", StatusOK) + app.RemoveRoute("/api/v1/test", MethodGet) + + verifyThereAreNoRoutes(t, app) +} + +func Test_App_Remove_Route_Parameterized(t *testing.T) { + t.Parallel() + app := New() + + app.Get("/test/:id", func(c Ctx) error { + return c.SendStatus(StatusOK) + }) + verifyRequest(t, app, "/test/:id", StatusOK) + app.RemoveRoute("/test/:id", MethodGet) + + verifyThereAreNoRoutes(t, app) +} + +func Test_App_Remove_Route(t *testing.T) { + t.Parallel() + app := New() + + app.Get("/test", func(c Ctx) error { + return c.SendStatus(StatusOK) + }) + + app.RemoveRoute("/test", MethodGet) + app.RebuildTree() + + verifyRequest(t, app, "/test", StatusNotFound) +} + +func Test_App_Remove_Route_Non_Existing_Route(t *testing.T) { + t.Parallel() + app := New() + + app.RemoveRoute("/test", MethodGet, MethodHead) + app.RebuildTree() + + verifyThereAreNoRoutes(t, app) +} + +func Test_App_Remove_Route_Concurrent(t *testing.T) { + t.Parallel() + app := New() + + // Add test route + app.Get("/test", func(c Ctx) error { + return c.SendStatus(StatusOK) + }) + + // Concurrently remove and add routes + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + app.RemoveRoute("/test", MethodGet) + app.Get("/test", func(c Ctx) error { + return c.SendStatus(StatusOK) + }) + }() + } + wg.Wait() + + // Verify final state + app.RebuildTree() + verifyRequest(t, app, "/test", StatusOK) +} + +func Test_App_Route_Registration_Prevent_Duplicate(t *testing.T) { + t.Parallel() + app := New() + + registerTreeManipulationRoutes(app) + registerTreeManipulationRoutes(app) + + verifyRequest(t, app, "/dynamically-defined", StatusNotFound) + require.Equal(t, uint32(1), app.handlersCount) + + verifyRequest(t, app, "/test", StatusOK) + require.Equal(t, uint32(2), app.handlersCount) + + verifyRequest(t, app, "/dynamically-defined", StatusOK) + require.Equal(t, uint32(2), app.handlersCount) + + verifyRequest(t, app, "/test", StatusOK) + require.Equal(t, uint32(2), app.handlersCount) + + verifyRequest(t, app, "/dynamically-defined", StatusOK) + require.Equal(t, uint32(2), app.handlersCount) + require.Equal(t, uint32(2), app.routesCount) + + verifyRouteHandlerCounts(t, app, 1) +} + +func Test_Route_Registration_Prevent_Duplicate_With_Middleware(t *testing.T) { + t.Parallel() + app := New() + + middleware := func(c Ctx) error { + return c.Next() + } + + registerTreeManipulationRoutes(app, middleware) + registerTreeManipulationRoutes(app) + + verifyRequest(t, app, "/dynamically-defined", StatusNotFound) + require.Equal(t, uint32(2), app.handlersCount) + + verifyRequest(t, app, "/test", StatusOK) + require.Equal(t, uint32(3), app.handlersCount) + + verifyRequest(t, app, "/dynamically-defined", StatusOK) + require.Equal(t, uint32(3), app.handlersCount) + + verifyRequest(t, app, "/test", StatusOK) + require.Equal(t, uint32(3), app.handlersCount) + + verifyRequest(t, app, "/dynamically-defined", StatusOK) + require.Equal(t, uint32(3), app.handlersCount) + require.Equal(t, uint32(2), app.routesCount) + + verifyRouteHandlerCounts(t, app, 1) } ////////////////////////////////////////////// From 006bd188e56b190caf8b10bd9939560c83c8d3b7 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Mon, 10 Mar 2025 21:57:09 -0400 Subject: [PATCH 2/5] Update router.go --- router.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/router.go b/router.go index 0ad94a00..cc777279 100644 --- a/router.go +++ b/router.go @@ -338,6 +338,7 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler if pathRaw[0] != '/' { pathRaw = "/" + pathRaw } + pathPretty := pathRaw if !app.config.CaseSensitive { pathPretty = utils.ToLower(pathPretty) @@ -345,11 +346,10 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler if !app.config.StrictRouting && len(pathPretty) > 1 { pathPretty = utils.TrimRight(pathPretty, '/') } - pathClean := RemoveEscapeChar(pathPretty) + pathClean := RemoveEscapeChar(pathPretty) parsedRaw := parseRoute(pathRaw, app.customConstraints...) parsedPretty := parseRoute(pathPretty, app.customConstraints...) - isMount := group != nil && group.app != app for _, method := range methods { @@ -358,6 +358,12 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler panic(fmt.Sprintf("add: invalid http method %s\n", method)) } + // Duplicate Route Handling + if app.routeExists(method, pathRaw) { + matchPathFunc := func(r *Route) bool { return r.Path == pathRaw } + app.deleteRoute([]string{method}, matchPathFunc) + } + isUse := method == methodUse isStar := pathClean == "/*" isRoot := pathClean == "/" From 918d9f9afc212b06b10a8875c57df08265992b73 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Mon, 10 Mar 2025 22:00:21 -0400 Subject: [PATCH 3/5] Fix markdown --- docs/api/app.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/app.md b/docs/api/app.md index b4e6bc1a..f37ef246 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -762,7 +762,6 @@ func main() { In this example, a new route is defined and then `RebuildTree()` is called to ensure the new route is registered and available. - ## RemoveRoute This method removes a route by path. You must call the `RebuildTree()` method after the remove in to ensure the route is removed. @@ -772,6 +771,7 @@ func (app *App) RemoveRoute(path string, methods ...string) ``` This method removes a route by name + ```go title="Signature" func (app *App) RemoveRouteByName(name string, methods ...string) ``` From f2817497aa1000e3b45c4cc6bdcc2336545756b1 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Tue, 11 Mar 2025 08:34:55 -0400 Subject: [PATCH 4/5] Some fixes --- router.go | 85 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/router.go b/router.go index cc777279..c94491df 100644 --- a/router.go +++ b/router.go @@ -358,12 +358,6 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler panic(fmt.Sprintf("add: invalid http method %s\n", method)) } - // Duplicate Route Handling - if app.routeExists(method, pathRaw) { - matchPathFunc := func(r *Route) bool { return r.Path == pathRaw } - app.deleteRoute([]string{method}, matchPathFunc) - } - isUse := method == methodUse isStar := pathClean == "/*" isRoot := pathClean == "/" @@ -402,27 +396,32 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler } } -func (app *App) routeExists(method, pathRaw string) bool { - pathToCheck := pathRaw - if !app.config.CaseSensitive { - pathToCheck = utils.ToLower(pathToCheck) +func (app *App) normalizePath(path string) string { + if path == "" { + path = "/" } - - return slices.ContainsFunc(app.stack[app.methodInt(method)], func(r *Route) bool { - routePath := r.path - if !app.config.CaseSensitive { - routePath = utils.ToLower(routePath) - } - - return routePath == pathToCheck - }) + if path[0] != '/' { + path = "/" + path + } + if !app.config.CaseSensitive { + path = utils.ToLower(path) + } + if !app.config.StrictRouting && len(path) > 1 { + path = utils.TrimRight(path, '/') + } + return RemoveEscapeChar(path) } // RemoveRoute is used to remove a route from the stack by path. // This only needs to be called to remove a route, route registration prevents duplicate routes. // You should call RebuildTree after using this to ensure consistency of the tree. func (app *App) RemoveRoute(path string, methods ...string) { - pathMatchFunc := func(r *Route) bool { return r.Path == path } + // Normalize same as register uses + norm := app.normalizePath(path) + + pathMatchFunc := func(r *Route) bool { + return r.path == norm // compare private normalized path + } app.deleteRoute(methods, pathMatchFunc) } @@ -472,7 +471,6 @@ func (app *App) addRoute(method string, route *Route, isMounted ...bool) { app.mutex.Lock() defer app.mutex.Unlock() - // Check mounted routes var mounted bool if len(isMounted) > 0 { mounted = isMounted[0] @@ -481,20 +479,41 @@ func (app *App) addRoute(method string, route *Route, isMounted ...bool) { // Get unique HTTP method identifier m := app.methodInt(method) - // prevent identically route registration - l := len(app.stack[m]) - if l > 0 && app.stack[m][l-1].Path == route.Path && route.use == app.stack[m][l-1].use && !route.mount && !app.stack[m][l-1].mount { - preRoute := app.stack[m][l-1] - preRoute.Handlers = append(preRoute.Handlers, route.Handlers...) - } else { - // Increment global route position - route.pos = atomic.AddUint32(&app.routesCount, 1) - route.Method = method - // Add route to the stack - app.stack[m] = append(app.stack[m], route) - app.routesRefreshed = true + // Check for an existing route with the same normalized path, + // same "use" flag, mount flag, and method. + // If found, replace the old route with the new one. + for i, existing := range app.stack[m] { + if existing.path == route.path && + existing.use == route.use && + existing.mount == route.mount && + existing.Method == route.Method { + if route.use { // middleware: merge handlers instead of replacing + app.stack[m][i].Handlers = append(existing.Handlers, route.Handlers...) //nolint:gocritic // Not a concern + } else { + // For non-middleware routes, replace as before + atomic.AddUint32(&app.handlersCount, ^uint32(len(existing.Handlers)-1)) //nolint:gosec // Not a concern + route.pos = existing.pos + app.stack[m][i] = route + } + app.routesRefreshed = true + if !mounted { + app.latestRoute = route + if err := app.hooks.executeOnRouteHooks(*route); err != nil { + panic(err) + } + } + return + } } + // No duplicate route exists; add the new route normally. + route.pos = atomic.AddUint32(&app.routesCount, 1) + route.Method = method + + // Add route to the stack + app.stack[m] = append(app.stack[m], route) + app.routesRefreshed = true + // Execute onRoute hooks & change latestRoute if not adding mounted route if !mounted { app.latestRoute = route From 961996050bced214402ce29de96c9721ea9eca95 Mon Sep 17 00:00:00 2001 From: Muhammed Efe Cetin Date: Wed, 19 Mar 2025 23:48:34 +0300 Subject: [PATCH 5/5] update --- router.go | 46 +++++++----- router_test.go | 200 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 217 insertions(+), 29 deletions(-) diff --git a/router.go b/router.go index c94491df..28f934f2 100644 --- a/router.go +++ b/router.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "html" - "slices" "sort" "sync/atomic" @@ -415,25 +414,25 @@ func (app *App) normalizePath(path string) string { // RemoveRoute is used to remove a route from the stack by path. // This only needs to be called to remove a route, route registration prevents duplicate routes. // You should call RebuildTree after using this to ensure consistency of the tree. -func (app *App) RemoveRoute(path string, methods ...string) { +func (app *App) RemoveRoute(path string, removeMiddlewares bool, methods ...string) { // Normalize same as register uses norm := app.normalizePath(path) pathMatchFunc := func(r *Route) bool { return r.path == norm // compare private normalized path } - app.deleteRoute(methods, pathMatchFunc) + app.deleteRoute(methods, removeMiddlewares, pathMatchFunc) } // RemoveRouteByName is used to remove a route from the stack by name. // This only needs to be called to remove a route, route registration prevents duplicate routes. // You should call RebuildTree after using this to ensure consistency of the tree. -func (app *App) RemoveRouteByName(name string, methods ...string) { +func (app *App) RemoveRouteByName(name string, removeMiddlewares bool, methods ...string) { matchFunc := func(r *Route) bool { return r.Name == name } - app.deleteRoute(methods, matchFunc) + app.deleteRoute(methods, removeMiddlewares, matchFunc) } -func (app *App) deleteRoute(methods []string, matchFunc func(r *Route) bool) { +func (app *App) deleteRoute(methods []string, removeMiddlewares bool, matchFunc func(r *Route) bool) { app.mutex.Lock() defer app.mutex.Unlock() @@ -447,21 +446,28 @@ func (app *App) deleteRoute(methods []string, matchFunc func(r *Route) bool) { continue // Skip invalid HTTP methods } - // Find the index of the route to remove - index := slices.IndexFunc(app.stack[m], matchFunc) - if index == -1 { - continue // Route not found + for i, route := range app.stack[m] { + var removedUseHandler bool + // only remove middlewares when use is true and method is use, if not middleware just check path + if (removeMiddlewares && route.use && matchFunc(route)) || (!route.use && matchFunc(route)) { + // Remove route from stack + if i+1 < len(app.stack[m]) { + app.stack[m] = append(app.stack[m][:i], app.stack[m][i+1:]...) + } else { + app.stack[m] = app.stack[m][:i] + } + app.routesRefreshed = true + + // Decrement global handler count. In middleware routes, only decrement once + if (route.use && !removedUseHandler) || !route.use { + removedUseHandler = true + atomic.AddUint32(&app.handlersCount, ^uint32(len(route.Handlers)-1)) //nolint:gosec // Not a concern + } + + // Decrement global route count + atomic.AddUint32(&app.routesCount, ^uint32(0)) //nolint:gosec // Not a concern + } } - - route := app.stack[m][index] - - // Decrement global handler count - atomic.AddUint32(&app.handlersCount, ^uint32(len(route.Handlers)-1)) //nolint:gosec // Not a concern - // Decrement global route position - atomic.AddUint32(&app.routesCount, ^uint32(0)) - - // Remove route from tree stack - app.stack[m] = slices.Delete(app.stack[m], index, index+1) } app.routesRefreshed = true diff --git a/router_test.go b/router_test.go index 45463a08..d0d378c7 100644 --- a/router_test.go +++ b/router_test.go @@ -487,7 +487,7 @@ func Test_App_Remove_Route_A_B_Feature_Testing(t *testing.T) { app := New() app.Get("/api/feature-a", func(c Ctx) error { - app.RemoveRoute("/api/feature", MethodGet) + app.RemoveRoute("/api/feature", false, MethodGet) app.RebuildTree() // Redefine route app.Get("/api/feature", func(c Ctx) error { @@ -498,7 +498,7 @@ func Test_App_Remove_Route_A_B_Feature_Testing(t *testing.T) { return c.SendStatus(StatusOK) }) app.Get("/api/feature-b", func(c Ctx) error { - app.RemoveRoute("/api/feature", MethodGet) + app.RemoveRoute("/api/feature", false, MethodGet) app.RebuildTree() // Redefine route app.Get("/api/feature", func(c Ctx) error { @@ -526,7 +526,7 @@ func Test_App_Remove_Route_By_Name(t *testing.T) { return c.SendStatus(StatusOK) }).Name("test") - app.RemoveRouteByName("test", MethodGet) + app.RemoveRouteByName("test", false, MethodGet) app.RebuildTree() verifyRequest(t, app, "/test", StatusNotFound) @@ -537,7 +537,7 @@ func Test_App_Remove_Route_By_Name_Non_Existing_Route(t *testing.T) { t.Parallel() app := New() - app.RemoveRouteByName("test", MethodGet) + app.RemoveRouteByName("test", false, MethodGet) app.RebuildTree() verifyThereAreNoRoutes(t, app) @@ -555,7 +555,7 @@ func Test_App_Remove_Route_Nested(t *testing.T) { }) verifyRequest(t, app, "/api/v1/test", StatusOK) - app.RemoveRoute("/api/v1/test", MethodGet) + app.RemoveRoute("/api/v1/test", false, MethodGet) verifyThereAreNoRoutes(t, app) } @@ -568,7 +568,7 @@ func Test_App_Remove_Route_Parameterized(t *testing.T) { return c.SendStatus(StatusOK) }) verifyRequest(t, app, "/test/:id", StatusOK) - app.RemoveRoute("/test/:id", MethodGet) + app.RemoveRoute("/test/:id", false, MethodGet) verifyThereAreNoRoutes(t, app) } @@ -581,7 +581,7 @@ func Test_App_Remove_Route(t *testing.T) { return c.SendStatus(StatusOK) }) - app.RemoveRoute("/test", MethodGet) + app.RemoveRoute("/test", false, MethodGet) app.RebuildTree() verifyRequest(t, app, "/test", StatusNotFound) @@ -591,7 +591,7 @@ func Test_App_Remove_Route_Non_Existing_Route(t *testing.T) { t.Parallel() app := New() - app.RemoveRoute("/test", MethodGet, MethodHead) + app.RemoveRoute("/test", false, MethodGet, MethodHead) app.RebuildTree() verifyThereAreNoRoutes(t, app) @@ -612,7 +612,7 @@ func Test_App_Remove_Route_Concurrent(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - app.RemoveRoute("/test", MethodGet) + app.RemoveRoute("/test", false, MethodGet) app.Get("/test", func(c Ctx) error { return c.SendStatus(StatusOK) }) @@ -681,6 +681,188 @@ func Test_Route_Registration_Prevent_Duplicate_With_Middleware(t *testing.T) { verifyRouteHandlerCounts(t, app, 1) } +func TestNormalizePath(t *testing.T) { + tests := []struct { + name string + path string + caseSensitive bool + strictRouting bool + expected string + }{ + { + name: "Empty path", + path: "", + caseSensitive: true, + strictRouting: true, + expected: "/", + }, + { + name: "No leading slash", + path: "users", + caseSensitive: true, + strictRouting: true, + expected: "/users", + }, + { + name: "With trailing slash and strict routing", + path: "/users/", + caseSensitive: true, + strictRouting: true, + expected: "/users/", + }, + { + name: "With trailing slash and non-strict routing", + path: "/users/", + caseSensitive: true, + strictRouting: false, + expected: "/users", + }, + { + name: "Case sensitive", + path: "/Users", + caseSensitive: true, + strictRouting: true, + expected: "/Users", + }, + { + name: "Case insensitive", + path: "/Users", + caseSensitive: false, + strictRouting: true, + expected: "/users", + }, + { + name: "With escape characters", + path: "/users\\/profile", + caseSensitive: true, + strictRouting: true, + expected: "/users/profile", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + app := &App{ + config: Config{ + CaseSensitive: tt.caseSensitive, + StrictRouting: tt.strictRouting, + }, + } + result := app.normalizePath(tt.path) + require.Equal(t, tt.expected, result) + }) + } +} + +func TestRemoveRoute(t *testing.T) { + app := New() + + var buf strings.Builder + + app.Use(func(c Ctx) error { + buf.WriteString("1") + return c.Next() + }) + + app.Post("/", func(c Ctx) error { + buf.WriteString("2") + return c.SendStatus(StatusOK) + }) + + app.Use("/test", func(c Ctx) error { + buf.WriteString("3") + return c.Next() + }) + + app.Get("/test", func(c Ctx) error { + buf.WriteString("4") + return c.SendStatus(StatusOK) + }) + + app.Post("/test", func(c Ctx) error { + buf.WriteString("5") + return c.SendStatus(StatusOK) + }) + + require.Equal(t, uint32(5), app.handlersCount) + require.Equal(t, uint32(21), app.routesCount) + + req, err := http.NewRequest(MethodPost, "/", nil) + require.NoError(t, err) + + resp, err := app.Test(req) + require.NoError(t, err) + + require.Equal(t, 200, resp.StatusCode) + require.Equal(t, "12", buf.String()) + + buf.Reset() + + req, err = http.NewRequest(MethodGet, "/test", nil) + require.NoError(t, err) + + resp, err = app.Test(req) + require.NoError(t, err) + + require.Equal(t, 200, resp.StatusCode) + require.Equal(t, "134", buf.String()) + + buf.Reset() + + app.RemoveRoute("/test", false, MethodGet) + app.RebuildTree() + + require.Equal(t, uint32(4), app.handlersCount) + require.Equal(t, uint32(20), app.routesCount) + + app.RemoveRoute("/test", false, MethodPost) + app.RebuildTree() + + require.Equal(t, uint32(3), app.handlersCount) + require.Equal(t, uint32(19), app.routesCount) + + req, err = http.NewRequest(MethodPost, "/test", nil) + require.NoError(t, err) + + resp, err = app.Test(req) + require.NoError(t, err) + + require.Equal(t, 404, resp.StatusCode) + require.Equal(t, "13", buf.String()) + + buf.Reset() + + req, err = http.NewRequest(MethodGet, "/test", nil) + require.NoError(t, err) + + resp, err = app.Test(req) + require.NoError(t, err) + + require.Equal(t, 404, resp.StatusCode) + require.Equal(t, "13", buf.String()) + + buf.Reset() + + app.RemoveRoute("/", false, MethodGet, MethodPost) + + require.Equal(t, uint32(2), app.handlersCount) + require.Equal(t, uint32(18), app.routesCount) + + req, err = http.NewRequest(MethodGet, "/", nil) + require.NoError(t, err) + + resp, err = app.Test(req) + require.NoError(t, err) + + require.Equal(t, 404, resp.StatusCode) + require.Equal(t, "1", buf.String()) + + app.RemoveRoute("/test", true, MethodGet, MethodPost) + + require.Equal(t, uint32(2), app.handlersCount) + require.Equal(t, uint32(16), app.routesCount) +} + ////////////////////////////////////////////// ///////////////// BENCHMARKS ///////////////// //////////////////////////////////////////////