From 4aa6310d6ed686f01cbe5cd5ab6871b1cf7922b7 Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Mon, 15 Jul 2024 16:32:33 -0300 Subject: [PATCH 01/11] feat: add rebuild tree method --- docs/api/app.md | 10 ++++++++++ router.go | 15 +++++++++++++++ router_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/docs/api/app.md b/docs/api/app.md index c3ecb552f8..9dc2476d0b 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -573,3 +573,13 @@ Hooks is a method to return [hooks](./hooks.md) property. ```go title="Signature" func (app *App) Hooks() *Hooks ``` + +## RebuildTree + +RebuildTree is a method destined to rebuild the route tree stack and allow dynamically route registers. It returns an app pointer. + +```go title="Signature" +func (app *App) RebuildTree() *App +``` + +**NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. \ No newline at end of file diff --git a/router.go b/router.go index eae9adef70..3bd6c77987 100644 --- a/router.go +++ b/router.go @@ -409,6 +409,21 @@ func (app *App) addRoute(method string, route *Route, isMounted ...bool) { } } +// BuildTree rebuilds the prefix tree from the previously registered routes. +// 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 +// 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 +func (app *App) RebuildTree() *App { + app.mutex.Lock() + defer app.mutex.Unlock() + + return app.buildTree() +} + // buildTree build the prefix tree from the previously registered routes func (app *App) buildTree() *App { if !app.routesRefreshed { diff --git a/router_test.go b/router_test.go index 715fbf7661..5509039c66 100644 --- a/router_test.go +++ b/router_test.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "net/http" "net/http/httptest" "os" "testing" @@ -368,6 +369,33 @@ 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() + + app.Get("/test", func(c Ctx) error { + app.Get("/dynamically-defined", func(c Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + app.RebuildTree() + + return c.SendStatus(http.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") + + 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, "/dynamically-defined", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, http.StatusOK, resp.StatusCode, "Status code") +} + ////////////////////////////////////////////// ///////////////// BENCHMARKS ///////////////// ////////////////////////////////////////////// From 436815c1e2e08c940ac4f7cfc038f2db52a2fec3 Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Mon, 15 Jul 2024 16:54:22 -0300 Subject: [PATCH 02/11] docs: add newline at the end of app.md --- 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 9dc2476d0b..4e23da8909 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -582,4 +582,4 @@ RebuildTree is a method destined to rebuild the route tree stack and allow dynam func (app *App) RebuildTree() *App ``` -**NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. \ No newline at end of file +**NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. From e22a260d466b4938254c6ed567c06a25b82136d2 Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Mon, 15 Jul 2024 16:58:53 -0300 Subject: [PATCH 03/11] docs: add an example of dynamic defined routes --- docs/api/app.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/api/app.md b/docs/api/app.md index 4e23da8909..52b2bd6ff5 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -582,4 +582,16 @@ RebuildTree is a method destined to rebuild the route tree stack and allow dynam func (app *App) RebuildTree() *App ``` -**NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. +**NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. Here's an example of defining routes dynamically: + +```go +app.Get("/define", func(c Ctx) error { + app.Get("/dynamically-defined", func(c Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + app.RebuildTree() + + return c.SendStatus(http.StatusOK) +}) +``` From d2749c982e3dd74f4a014c5a826c31572f6976e7 Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Mon, 15 Jul 2024 17:00:29 -0300 Subject: [PATCH 04/11] docs: remove tabs from example code on app.md --- docs/api/app.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index 52b2bd6ff5..b41dbb336a 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -586,12 +586,12 @@ func (app *App) RebuildTree() *App ```go app.Get("/define", func(c Ctx) error { - app.Get("/dynamically-defined", func(c Ctx) error { - return c.SendStatus(http.StatusOK) - }) + app.Get("/dynamically-defined", func(c Ctx) error { + return c.SendStatus(http.StatusOK) + }) - app.RebuildTree() + app.RebuildTree() - return c.SendStatus(http.StatusOK) + return c.SendStatus(http.StatusOK) }) ``` From b4dd5d32edf88f0cdfdc34db90ca93db9c228800 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Mon, 15 Jul 2024 22:48:45 -0400 Subject: [PATCH 05/11] Update docs/api/app.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- docs/api/app.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index b41dbb336a..ee351f7aa0 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -585,13 +585,13 @@ func (app *App) RebuildTree() *App **NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. Here's an example of defining routes dynamically: ```go -app.Get("/define", func(c Ctx) error { - app.Get("/dynamically-defined", func(c Ctx) error { - return c.SendStatus(http.StatusOK) - }) +app.Get("/define", func(c Ctx) error { // Define a new route dynamically + app.Get("/dynamically-defined", func(c Ctx) error { // Adding a dynamically defined route + return c.SendStatus(http.StatusOK) + }) - app.RebuildTree() + app.RebuildTree() // Rebuild the route tree to register the new route - return c.SendStatus(http.StatusOK) + return c.SendStatus(http.StatusOK) }) ``` From 852b0e3fc65a67701734382d6a72cab0d2d9cfcc Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Mon, 15 Jul 2024 22:52:42 -0400 Subject: [PATCH 06/11] Update app.md --- docs/api/app.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index ee351f7aa0..ef9c2ea08d 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -576,13 +576,17 @@ func (app *App) Hooks() *Hooks ## RebuildTree -RebuildTree is a method destined to rebuild the route tree stack and allow dynamically route registers. It returns an app pointer. +The RebuildTree method is designed to rebuild the route tree and enable dynamic route registration. It returns a pointer to the App instance. ```go title="Signature" func (app *App) RebuildTree() *App ``` -**NOTE**: This method should be used in the most careful way possible, since it's not currently possible to make it thread-safe (it would add a big performance overhead to do so) and calling it is very performance-intensive, so it's recommended to be used only in development mode and never concurrently. Here's an example of defining routes dynamically: +**Note:** Use this method with caution. It is **not** thread-safe and calling it can be very performance-intensive, so it should be used sparingly and only in development mode. Avoid using it concurrently. + +### Example Usage + +Hereโ€™s an example of how to define and register routes dynamically: ```go app.Get("/define", func(c Ctx) error { // Define a new route dynamically @@ -595,3 +599,5 @@ app.Get("/define", func(c Ctx) error { // Define a new route dynamically return c.SendStatus(http.StatusOK) }) ``` + +In this example, a new route is defined and then `RebuildTree()` is called to make sure the new route is registered and available. From eda83aacc35682e600dac4e3a3480338779f2db5 Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Tue, 16 Jul 2024 13:22:09 -0300 Subject: [PATCH 07/11] docs: add RebuildTree to what's new documentation --- docs/whats_new.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/whats_new.md b/docs/whats_new.md index 9155792f6b..b6c7e6ff51 100644 --- a/docs/whats_new.md +++ b/docs/whats_new.md @@ -389,6 +389,31 @@ app.Route("/api").Route("/user/:id?") }); ``` +### ๐Ÿ—บ RebuildTree + +We have added a new method that allows the route tree stack to be rebuilt in runtime, with it you can add a route while your application is running and rebuild the route tree stack to make it registered and available for calls. + +You can find more reference on it in the [app](./api/app.md#rebuildtree): + +#### Example Usage + +```go +app.Get("/define", func(c Ctx) error { // Define a new route dynamically + app.Get("/dynamically-defined", func(c Ctx) error { // Adding a dynamically defined route + return c.SendStatus(http.StatusOK) + }) + + app.RebuildTree() // Rebuild the route tree to register the new route + + return c.SendStatus(http.StatusOK) +}) +``` + +In this example, a new route is defined and then `RebuildTree()` is called to make sure the new route is registered and available. + +**Note:** Use this method with caution. It is **not** thread-safe and calling it can be very performance-intensive, so it should be used sparingly and only in +development mode. Avoid using it concurrently. + ### ๐Ÿง  Context ### ๐Ÿ“Ž Parser From 38efd374fde9cfc073ddd1cbdf6e42b8df7419a2 Mon Sep 17 00:00:00 2001 From: Lucas Lemos Date: Tue, 16 Jul 2024 13:38:13 -0300 Subject: [PATCH 08/11] fix: markdown errors in documentation Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- docs/whats_new.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/whats_new.md b/docs/whats_new.md index b6c7e6ff51..aa1d05a759 100644 --- a/docs/whats_new.md +++ b/docs/whats_new.md @@ -391,7 +391,7 @@ app.Route("/api").Route("/user/:id?") ### ๐Ÿ—บ RebuildTree -We have added a new method that allows the route tree stack to be rebuilt in runtime, with it you can add a route while your application is running and rebuild the route tree stack to make it registered and available for calls. +We have added a new method that allows the route tree stack to be rebuilt in runtime, with it, you can add a route while your application is running and rebuild the route tree stack to make it registered and available for calls. You can find more reference on it in the [app](./api/app.md#rebuildtree): @@ -411,7 +411,7 @@ app.Get("/define", func(c Ctx) error { // Define a new route dynamically In this example, a new route is defined and then `RebuildTree()` is called to make sure the new route is registered and available. -**Note:** Use this method with caution. It is **not** thread-safe and calling it can be very performance-intensive, so it should be used sparingly and only in +**Note:** Use this method with caution. It is **not** thread-safe and calling it can be very performance-intensive, so it should be used sparingly and only in development mode. Avoid using it concurrently. ### ๐Ÿง  Context From f2ef5d4bec57d57037417437940d0a93f5ed2870 Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Tue, 16 Jul 2024 13:40:29 -0300 Subject: [PATCH 09/11] refactor: add mutex lock to the addRoute function --- router.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/router.go b/router.go index 3bd6c77987..a3e9bc5585 100644 --- a/router.go +++ b/router.go @@ -375,6 +375,9 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler } 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 { From 141f858f4a92cb9162bc076310b2e4bc6c9e31ca Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Tue, 16 Jul 2024 14:03:58 -0300 Subject: [PATCH 10/11] refactor: remove mutex lock from addRoute --- router.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/router.go b/router.go index a3e9bc5585..3bd6c77987 100644 --- a/router.go +++ b/router.go @@ -375,9 +375,6 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler } 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 { From 9d736d632c4733c1c7507ee3a7969de26278926c Mon Sep 17 00:00:00 2001 From: luk3skyw4lker Date: Tue, 16 Jul 2024 14:14:36 -0300 Subject: [PATCH 11/11] refactor: fix mutex deadlock in addRoute --- router.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/router.go b/router.go index 3bd6c77987..bc93f67977 100644 --- a/router.go +++ b/router.go @@ -375,6 +375,9 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler } 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 { @@ -400,12 +403,10 @@ func (app *App) addRoute(method string, route *Route, isMounted ...bool) { // Execute onRoute hooks & change latestRoute if not adding mounted route if !mounted { - app.mutex.Lock() app.latestRoute = route if err := app.hooks.executeOnRouteHooks(*route); err != nil { panic(err) } - app.mutex.Unlock() } }