Skip to content

Commit

Permalink
fix views when both app and sub-app have view engine, better behavior…
Browse files Browse the repository at this point in the history
… for mount path
  • Loading branch information
efectn committed Oct 21, 2022
1 parent b84cd20 commit b1b6ce8
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 36 deletions.
51 changes: 29 additions & 22 deletions app.go
Expand Up @@ -108,7 +108,7 @@ type App struct {
// Converts byte slice to a string
getString func(b []byte) string
// Mounted and main apps
appList map[string]*App
appList []*App
// Hooks
hooks *Hooks
// Latest route & group
Expand Down Expand Up @@ -487,7 +487,7 @@ func New(config ...Config) *App {
config: Config{},
getBytes: utils.UnsafeBytes,
getString: utils.UnsafeString,
appList: make(map[string]*App),
appList: make([]*App, 0),
latestRoute: &Route{},
latestGroup: &Group{},
}
Expand Down Expand Up @@ -552,7 +552,7 @@ func New(config ...Config) *App {
app.config.ColorScheme = defaultColors(app.config.ColorScheme)

// Init appList
app.appList[""] = app
app.appList = append(app.appList, app)

// Init app
app.init()
Expand Down Expand Up @@ -596,12 +596,12 @@ func (app *App) Mount(prefix string, fiber *App) Router {
}

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

// Fill some fields of sub-app
fiber.mountPath = prefix
fiber.parentApp = app

// Execute onMount hooks
Expand Down Expand Up @@ -983,9 +983,9 @@ func (app *App) ErrorHandler(ctx *Ctx, err error) error {
mountedPrefixParts int
)

for prefix, subApp := range app.appList {
if prefix != "" && strings.HasPrefix(ctx.path, prefix) {
parts := len(strings.Split(prefix, "/"))
for _, subApp := range app.appList {
if subApp.mountPath != "" && strings.HasPrefix(ctx.path, subApp.mountPath) {
parts := len(strings.Split(subApp.mountPath, "/"))
if mountedPrefixParts <= parts {
mountedErrHandler = subApp.config.ErrorHandler
mountedPrefixParts = parts
Expand Down Expand Up @@ -1048,47 +1048,54 @@ func (app *App) startupProcess() *App {
}

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

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

if _, ok := app.appList[prefix]; !ok {
app.appList[prefix] = subApp
// Check same app added before
var isAdded bool
for _, oldSubApp := range app.appList {
if oldSubApp.mountPath == subApp.mountPath {
isAdded = true
}
}

if !isAdded {
app.appList = append(app.appList, subApp)
}

// The first element of appList is always the app itself. If there are no other sub apps, we should skip appending nested apps.
if len(subApp.appList) > 1 {
app.appendSubAppLists(subApp.appList, prefix)
app.appendSubAppLists(subApp.appList, subApp.mountPath)
}

}
}

// 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 {
func (app *App) addSubAppsRoutes(appList []*App, parent ...string) {
for _, subApp := range appList {
// skip real app
if prefix == "" {
if subApp.mountPath == "" {
continue
}

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

// 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), true)
app.addRoute(route.Method, app.addPrefixToRoute(subApp.mountPath, route), true)
}
}

Expand Down
18 changes: 10 additions & 8 deletions app_test.go
Expand Up @@ -316,7 +316,6 @@ func Test_App_Mount(t *testing.T) {

app := New()
app.Mount("/john", micro)

resp, err := app.Test(httptest.NewRequest(MethodGet, "/john/doe", nil))
utils.AssertEqual(t, nil, err, "app.Test(req)")
utils.AssertEqual(t, 200, resp.StatusCode, "Status code")
Expand Down Expand Up @@ -363,15 +362,18 @@ func Test_App_Mount_Nested(t *testing.T) {

// go test -run Test_App_MountPath
func Test_App_MountPath(t *testing.T) {
micro := New()
micro.Get("/doe", func(c *Ctx) error {
return c.SendStatus(StatusOK)
})

app := New()
app.Mount("/john", micro)
one := New()
two := New()
three := New()

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

utils.AssertEqual(t, "/john", micro.MountPath())
utils.AssertEqual(t, "/one", one.MountPath())
utils.AssertEqual(t, "/one/two", two.MountPath())
utils.AssertEqual(t, "/one/two/three", three.MountPath())
utils.AssertEqual(t, "", app.MountPath())
}

Expand Down
7 changes: 4 additions & 3 deletions ctx.go
Expand Up @@ -1336,9 +1336,10 @@ func (c *Ctx) Render(name string, bind interface{}, layouts ...string) error {
// Pass-locals-to-views & bind
c.renderExtensions(bind)

rendered := false
for prefix, app := range c.app.appList {
if prefix == "" || strings.Contains(c.OriginalURL(), prefix) {
var rendered bool
for i := len(c.app.appList) - 1; i >= 0; i-- {
app := c.app.appList[i]
if (app.mountPath != "" && strings.Contains(c.OriginalURL(), app.mountPath)) || (app.mountPath == "" && app.parentApp == nil) {
if len(layouts) == 0 && app.config.ViewsLayout != "" {
layouts = []string{
app.config.ViewsLayout,
Expand Down
46 changes: 46 additions & 0 deletions ctx_test.go
Expand Up @@ -2761,6 +2761,52 @@ func Test_Ctx_Render_Mount(t *testing.T) {
utils.AssertEqual(t, "<h1>Hello a!</h1>", string(body))
}

// go test -run Test_Ctx_Render_Mount_ParentHasViews
func Test_Ctx_Render_Mount_ParentHasViews(t *testing.T) {
t.Parallel()

engine := &testTemplateEngine{}
err := engine.Load()
utils.AssertEqual(t, nil, err)

sub := New(Config{
Views: html.New("./.github/testdata/template", ".gohtml"),
})

app := New(Config{
Views: engine,
})

app.Get("/test", func(c *Ctx) error {
return c.Render("index.tmpl", Map{
"Title": "Hello, World!",
})
})

sub.Get("/:name", func(c *Ctx) error {
return c.Render("hello_world", Map{
"Name": c.Params("name"),
})
})
app.Mount("/hello", sub)

resp, err := app.Test(httptest.NewRequest(MethodGet, "/hello/a", nil))
utils.AssertEqual(t, StatusOK, resp.StatusCode, "Status code")
utils.AssertEqual(t, nil, err, "app.Test(req)")

body, err := ioutil.ReadAll(resp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, "<h1>Hello a!</h1>", string(body))

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

body, err = ioutil.ReadAll(resp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, "<h1>Hello, World!</h1>", string(body))
}

func Test_Ctx_Render_MountGroup(t *testing.T) {
t.Parallel()

Expand Down
6 changes: 3 additions & 3 deletions group.go
Expand Up @@ -29,12 +29,12 @@ func (grp *Group) Mount(prefix string, fiber *App) Router {
}

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

// Fill some fields of sub-app
fiber.mountPath = prefix
fiber.parentApp = grp.app

// Execute onMount hooks
Expand Down

1 comment on commit b1b6ce8

@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: b1b6ce8 Previous: c187c6a Ratio
Benchmark_AcquireCtx 1495 ns/op 1568 B/op 5 allocs/op 706.6 ns/op 1568 B/op 5 allocs/op 2.12
Benchmark_Ctx_IPs_With_IP_Validation 528.9 ns/op 112 B/op 4 allocs/op 220.7 ns/op 48 B/op 1 allocs/op 2.40
Benchmark_Ctx_IP_With_ProxyHeader_and_IP_Validation 225.3 ns/op 32 B/op 2 allocs/op 89.51 ns/op 0 B/op 0 allocs/op 2.52

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

Please sign in to comment.