From cf59d6dbfa9da2b59b46fe52f984bcf45dec93f1 Mon Sep 17 00:00:00 2001
From: Cory Koch <yunusabd786@gmail.com>
Date: Sun, 10 Nov 2024 23:08:05 -0500
Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5=20feat:=20Add=20Support=20for=20Re?=
 =?UTF-8?q?moving=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

* Add Dockerfile
---
 Dockerfile        |  53 ++++++++++
 docs/api/app.md   |  53 ++++++++++
 docs/whats_new.md |   8 ++
 router.go         |  76 +++++++++++++-
 router_test.go    | 260 ++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 439 insertions(+), 11 deletions(-)
 create mode 100644 Dockerfile

diff --git a/Dockerfile b/Dockerfile
new file mode 100644
index 0000000000..8a57e2e85c
--- /dev/null
+++ b/Dockerfile
@@ -0,0 +1,53 @@
+# Usage:
+#
+## Linux:
+# ```
+### To utilize the BuildKit cache use:
+# DOCKER_BUILDKIT=1 docker build -t fiber .
+#
+# docker run -it --rm fiber make lint && make test
+# ```
+#
+## Windows: 
+# ```
+### To utilize the BuildKit cache use:
+# docker build -t fiber .
+# 
+# docker run -it --rm fiber make lint && make test
+# ```
+#
+# Note: BuildKit is the default builder for users on Docker Desktop.
+
+
+# Use the official Golang image to create a build artifact.
+FROM golang:latest AS builder
+
+RUN <<EOF
+apt-get update && \
+apt-get install -y bsdmainutils && \
+rm -rf /var/lib/apt/lists/*
+EOF
+
+ARG TARGETOS
+ARG TARGETARCH
+
+ENV CGO_ENABLED 0
+ENV GOPATH /go
+ENV GOCACHE /go-build
+
+# Create and change to the app directory.
+WORKDIR /app
+
+# Create and change to the app directory.
+# Cache go modules
+# TODO use this?
+#go mod tidy && \
+RUN --mount=type=cache,target=/go/pkg/mod/cache
+    go mod download -x
+
+COPY . .
+
+# Build the binary.
+RUN --mount=type=cache,target=/go/pkg/mod/cache \
+    --mount=type=cache,target=/go/go-build \
+    GO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -v -o bin/server
diff --git a/docs/api/app.md b/docs/api/app.md
index 6582159c0f..7399adb05f 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()
+```
+
+This method removes a route by name
+```go title="Signature"
+func (app *App) RemoveRouteByName()
+```
+
+```go title="Example"
+package main
+
+import (
+    "log"
+
+    "github.com/gofiber/fiber/v3"
+)
+
+func main() {
+    app := fiber.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(http.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(http.StatusOK)
+    })
+
+    log.Fatal(app.Listen(":3000"))
+}
+```
diff --git a/docs/whats_new.md b/docs/whats_new.md
index bfc6f25c29..e0575dd8c7 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 2091cfc6cb..c5243c257a 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 string, 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 5509039c66..07a4610e86 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,267 @@ func Test_Router_NotFound_HTML_Inject(t *testing.T) {
 	require.Equal(t, "Cannot DELETE /does/not/exist&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;", string(c.Response.Body()))
 }
 
+func sendStatusOK(c Ctx) error {
+	return c.SendStatus(http.StatusOK)
+}
+
+func registerTreeManipulationRoutes(app *App, middleware ...func(Ctx) error) {
+
+	app.Get("/test", func(c Ctx) error {
+		app.Get("/dynamically-defined", sendStatusOK)
+
+		app.RebuildTree()
+
+		return c.SendStatus(http.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()
 
-	app.Get("/test", func(c Ctx) error {
-		app.Get("/dynamically-defined", func(c Ctx) error {
-			return c.SendStatus(http.StatusOK)
+	registerTreeManipulationRoutes(app)
+
+	verifyRequest(t, app, "/dynamically-defined", http.StatusNotFound)
+	verifyRequest(t, app, "/test", http.StatusOK)
+	verifyRequest(t, app, "/dynamically-defined", http.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(http.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(http.StatusOK)
 	})
 
-	resp, err := app.Test(httptest.NewRequest(MethodGet, "/dynamically-defined", nil))
+	resp, err := app.Test(httptest.NewRequest(MethodGet, "/api/feature-a", nil))
 	require.NoError(t, err, "app.Test(req)")
-	require.Equal(t, http.StatusNotFound, resp.StatusCode, "Status code")
+	require.Equal(t, http.StatusOK, resp, "Status Code")
 
-	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, err = app.Test(httptest.NewRequest(MethodGet, "/api/feature", nil))
+	require.Equal(t, "Testing feature-a", resp, "Response Message")
 
-	resp, err = app.Test(httptest.NewRequest(MethodGet, "/dynamically-defined", nil))
+	resp, err = app.Test(httptest.NewRequest(MethodGet, "api/feature-b", nil))
 	require.NoError(t, err, "app.Test(req)")
-	require.Equal(t, http.StatusOK, resp.StatusCode, "Status code")
+	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", sendStatusOK).Name("test")
+
+	app.RemoveRouteByName("test", MethodGet)
+	app.RebuildTree()
+
+	verifyRequest(t, app, "/test", http.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", sendStatusOK)
+
+	verifyRequest(t, app, "/api/v1/test", http.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", sendStatusOK)
+	verifyRequest(t, app, "/test/:id", http.StatusOK)
+	app.RemoveRoute("/test/:id", MethodGet)
+
+	verifyThereAreNoRoutes(t, app)
+
+}
+
+func Test_App_Remove_Route(t *testing.T) {
+	t.Parallel()
+	app := New()
+
+	app.Get("/test", sendStatusOK)
+
+	app.RemoveRoute("/test", MethodGet)
+	app.RebuildTree()
+
+	verifyRequest(t, app, "/test", http.StatusNotFound)
+}
+
+func Test_App_Remove_Route_Non_Existing_Route(t *testing.T) {
+	t.Parallel()
+	app := New()
+
+	app.RemoveRoute("/test", MethodGet, http.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", sendStatusOK)
+
+	// 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", sendStatusOK)
+		}()
+	}
+	wg.Wait()
+
+	// Verify final state
+	app.RebuildTree()
+	verifyRequest(t, app, "/test", http.StatusOK)
+}
+
+func Test_App_Route_Registration_Prevent_Duplicate(t *testing.T) {
+	t.Parallel()
+	app := New()
+
+	registerTreeManipulationRoutes(app)
+	registerTreeManipulationRoutes(app)
+
+	verifyRequest(t, app, "/dynamically-defined", http.StatusNotFound)
+	require.Equal(t, uint32(1), app.handlersCount)
+
+	verifyRequest(t, app, "/test", http.StatusOK)
+	require.Equal(t, uint32(2), app.handlersCount)
+
+	verifyRequest(t, app, "/dynamically-defined", http.StatusOK)
+	require.Equal(t, uint32(2), app.handlersCount)
+
+	verifyRequest(t, app, "/test", http.StatusOK)
+	require.Equal(t, uint32(2), app.handlersCount)
+
+	verifyRequest(t, app, "/dynamically-defined", http.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", http.StatusNotFound)
+	require.Equal(t, uint32(2), app.handlersCount)
+
+	verifyRequest(t, app, "/test", http.StatusOK)
+	require.Equal(t, uint32(3), app.handlersCount)
+
+	verifyRequest(t, app, "/dynamically-defined", http.StatusOK)
+	require.Equal(t, uint32(3), app.handlersCount)
+
+	verifyRequest(t, app, "/test", http.StatusOK)
+	require.Equal(t, uint32(3), app.handlersCount)
+
+	verifyRequest(t, app, "/dynamically-defined", http.StatusOK)
+	require.Equal(t, uint32(3), app.handlersCount)
+	require.Equal(t, uint32(2), app.routesCount)
+
+	verifyRouteHandlerCounts(t, app, 1)
 }
 
 //////////////////////////////////////////////