Skip to content

Commit

Permalink
🐛 bug: fix mounting doesn't work if when to declare it before routes
Browse files Browse the repository at this point in the history
  • Loading branch information
efectn committed Sep 25, 2022
1 parent 14a5f61 commit bb8f003
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 24 deletions.
74 changes: 63 additions & 11 deletions app.go
Expand Up @@ -116,6 +116,8 @@ type App struct {
latestGroup *Group
// TLS handler
tlsHandler *TLSHandler
// check added routes of sub-apps
subAppsRoutesAdded bool
}

// Config is a struct holding the server settings.
Expand Down Expand Up @@ -584,27 +586,17 @@ func (app *App) SetTLSHandler(tlsHandler *TLSHandler) {
// any of the fiber's sub apps are added to the application's error handlers
// to be invoked on errors that happen within the prefix route.
func (app *App) Mount(prefix string, fiber *App) Router {
stack := fiber.Stack()
prefix = strings.TrimRight(prefix, "/")
if prefix == "" {
prefix = "/"
}

for m := range stack {
for r := range stack[m] {
route := app.copyRoute(stack[m][r])
app.addRoute(route.Method, app.addPrefixToRoute(prefix, route))
}
}

// Support for configs of mounted-apps and sub-mounted-apps
for mountedPrefixes, subApp := range fiber.appList {
app.appList[prefix+mountedPrefixes] = subApp
subApp.init()
}

atomic.AddUint32(&app.handlersCount, fiber.handlersCount)

return app
}

Expand Down Expand Up @@ -975,6 +967,7 @@ func (app *App) ErrorHandler(ctx *Ctx, err error) error {
)

for prefix, subApp := range app.appList {
fmt.Println(prefix)
if prefix != "" && strings.HasPrefix(ctx.path, prefix) {
parts := len(strings.Split(prefix, "/"))
if mountedPrefixParts <= parts {
Expand Down Expand Up @@ -1024,7 +1017,66 @@ func (app *App) startupProcess() *App {
}

app.mutex.Lock()
defer app.mutex.Unlock()

// add routes of sub-apps
if !app.subAppsRoutesAdded {
app.appendSubAppLists(app.appList)
app.addSubAppsRoutes(app.appList)

app.subAppsRoutesAdded = true
}

// build route tree stack
app.buildTree()
app.mutex.Unlock()

return app
}

// appendSubAppLists supports nested for sub apps
func (app *App) appendSubAppLists(appList map[string]*App, parent ...string) {
for prefix, subApp := range appList {
// skip real app
if prefix == "" {
continue
}

if len(parent) > 0 {
prefix = parent[0] + prefix
}

if _, ok := app.appList[prefix]; !ok {
app.appList[prefix] = subApp
}

if len(subApp.appList) > 1 {
app.appendSubAppLists(subApp.appList, prefix)
}

}
}

// addSubAppsRoutes adds routes of sub apps nestedly when to start the server
func (app *App) addSubAppsRoutes(appList map[string]*App, parent ...string) {
for prefix, subApp := range appList {
// skip real app
if prefix == "" {
continue
}

if len(parent) > 0 {
prefix = parent[0] + prefix
}

// add routes
stack := subApp.stack
for m := range stack {
for r := range stack[m] {
route := app.copyRoute(stack[m][r])
app.addRoute(route.Method, app.addPrefixToRoute(prefix, route))
}
}

atomic.AddUint32(&app.handlersCount, subApp.handlersCount)
}
}
38 changes: 38 additions & 0 deletions app_test.go
Expand Up @@ -323,6 +323,44 @@ func Test_App_Mount(t *testing.T) {
utils.AssertEqual(t, uint32(2), app.handlersCount)
}

// go test -run Test_App_Mount_Nested
func Test_App_Mount_Nested(t *testing.T) {
app := New()
one := New()
two := New()
three := New()

two.Mount("/three", three)
app.Mount("/one", one)
one.Mount("/two", two)

one.Get("/doe", func(c *Ctx) error {
return c.SendStatus(StatusOK)
})

two.Get("/nested", func(c *Ctx) error {
return c.SendStatus(StatusOK)
})

three.Get("/test", func(c *Ctx) error {
return c.SendStatus(StatusOK)
})

resp, err := app.Test(httptest.NewRequest(MethodGet, "/one/doe", nil))
utils.AssertEqual(t, nil, err, "app.Test(req)")
utils.AssertEqual(t, 200, resp.StatusCode, "Status code")

resp, err = app.Test(httptest.NewRequest(MethodGet, "/one/two/nested", nil))
utils.AssertEqual(t, nil, err, "app.Test(req)")
utils.AssertEqual(t, 200, resp.StatusCode, "Status code")

resp, err = app.Test(httptest.NewRequest(MethodGet, "/one/two/three/test", nil))
utils.AssertEqual(t, nil, err, "app.Test(req)")
utils.AssertEqual(t, 200, resp.StatusCode, "Status code")

utils.AssertEqual(t, uint32(6), app.handlersCount)
}

func Test_App_Use_Params(t *testing.T) {
app := New()

Expand Down
11 changes: 0 additions & 11 deletions group.go
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"reflect"
"strings"
"sync/atomic"
)

// Group struct
Expand All @@ -23,28 +22,18 @@ type Group struct {
// It's very useful to split up a large API as many independent routers and
// compose them as a single service using Mount.
func (grp *Group) Mount(prefix string, fiber *App) Router {
stack := fiber.Stack()
groupPath := getGroupPath(grp.Prefix, prefix)
groupPath = strings.TrimRight(groupPath, "/")
if groupPath == "" {
groupPath = "/"
}

for m := range stack {
for r := range stack[m] {
route := grp.app.copyRoute(stack[m][r])
grp.app.addRoute(route.Method, grp.app.addPrefixToRoute(groupPath, route))
}
}

// Support for configs of mounted-apps and sub-mounted-apps
for mountedPrefixes, subApp := range fiber.appList {
grp.app.appList[groupPath+mountedPrefixes] = subApp
subApp.init()
}

atomic.AddUint32(&grp.app.handlersCount, fiber.handlersCount)

return grp
}

Expand Down
6 changes: 4 additions & 2 deletions router.go
Expand Up @@ -436,19 +436,20 @@ func (app *App) addRoute(method string, route *Route) {
app.routesRefreshed = true
}

app.mutex.Lock()
//app.mutex.Lock()
app.latestRoute = route
if err := app.hooks.executeOnRouteHooks(*route); err != nil {
panic(err)
}
app.mutex.Unlock()
//app.mutex.Unlock()
}

// buildTree build the prefix tree from the previously registered routes
func (app *App) buildTree() *App {
if !app.routesRefreshed {
return app
}

// loop all the methods and stacks and create the prefix tree
for m := range intMethod {
tsMap := make(map[string][]*Route)
Expand All @@ -462,6 +463,7 @@ func (app *App) buildTree() *App {
}
app.treeStack[m] = tsMap
}

// loop the methods and tree stacks and add global stack and sort everything
for m := range intMethod {
tsMap := app.treeStack[m]
Expand Down

1 comment on commit bb8f003

@ReneWerner87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: bb8f003 Previous: 14a5f61 Ratio
Benchmark_AcquireCtx 1582 ns/op 1568 B/op 5 allocs/op 637.4 ns/op 1568 B/op 5 allocs/op 2.48
Benchmark_App_ETag 7814 ns/op 1044 B/op 3 allocs/op 3270 ns/op 1044 B/op 3 allocs/op 2.39
Benchmark_App_ETag_Weak 7665 ns/op 1068 B/op 4 allocs/op 3665 ns/op 1068 B/op 4 allocs/op 2.09
Benchmark_Utils_ETag 7348 ns/op 1044 B/op 3 allocs/op 3312 ns/op 1044 B/op 3 allocs/op 2.22
Benchmark_Utils_ETag_Weak 7651 ns/op 1068 B/op 4 allocs/op 3365 ns/op 1068 B/op 4 allocs/op 2.27

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.