🐛 [Bug]: Naming of routes works wrong after mount

[Antonov-guap]

Bug Description

Naming of routes after mounting sub-apps into root app works wrong.
All routes turn into flat map of routes, but expected that they all separately work without any changing in original sub-apps.
Also there is no way to set names of sub-apps (but it could be great, as groups have this feature).

How to Reproduce

Steps to reproduce the behavior:

1. Create few sub-apps
2. Setup some routes with names
3. Add sub-apps to root app
4. Try to use names in handlers or in main.go to find any Routes

Expected Behavior

Expected, that I can use naming in some sub-apps and it works. And after that if I mount sub-app to root-app, naming also works, but with correct prefixes (of paths and of names, like it works with groups).

I added some test, that could be placed into the end of app_test.go. Check if the proposal behaviour is correct.

Fiber Version

v2.50.0

Code Snippet (optional)

func Test_Mount_Route_Names(t *testing.T) {
	// create sub-app with 2 handlers:
	subApp1 := New()
	subApp1.Name("app1.") // add some name to the app
	subApp1.Get("/users", func(c *Ctx) error {
		url, err := c.GetRouteURL("add-user", Map{})
		utils.AssertEqual(t, nil, err)
		utils.AssertEqual(t, "/app1/users", url, "handler: app1.add-user") // the prefix is /app1 because of the mount
		// if subApp1 is not mounted, expected url just /users
		return nil
	}).Name("get-users")
	subApp1.Post("/users", func(c *Ctx) error {
		route := c.App().GetRoute("get-users")
		utils.AssertEqual(t, MethodGet, route.Method, "handler: app1.get-users method")
		utils.AssertEqual(t, "/app1/users", route.Path, "handler: app1.get-users path")
		return nil
	}).Name("add-user")

	// create sub-app with 2 handlers inside a group:
	subApp2 := New()
	subApp2.Name("app2.") // add some name to the app
	app2Grp := subApp2.Group("/users").Name("users.")
	app2Grp.Get("", nil).Name("get")
	app2Grp.Post("", nil).Name("add")

	// put both sub-apps into root app
	rootApp := New()
	rootApp.Mount("/app1", subApp1)
	rootApp.Mount("/app2", subApp2)
	rootApp.startupProcess()

	// take route directly from sub-app
	route := subApp1.GetRoute("get-users")
	utils.AssertEqual(t, MethodGet, route.Method)
	utils.AssertEqual(t, "/users", route.Path)
	route = subApp1.GetRoute("add-user")
	utils.AssertEqual(t, MethodPost, route.Method)
	utils.AssertEqual(t, "/users", route.Path)

	// take route directly from sub-app with group
	route = subApp2.GetRoute("users.get")
	utils.AssertEqual(t, MethodGet, route.Method)
	utils.AssertEqual(t, "/users", route.Path)
	route = subApp2.GetRoute("users.add")
	utils.AssertEqual(t, MethodPost, route.Method)
	utils.AssertEqual(t, "/users", route.Path)

	// take route from root app (using names of sub-apps)
	route = rootApp.GetRoute("app1.add-user")
	utils.AssertEqual(t, MethodPost, route.Method)
	utils.AssertEqual(t, "/app1/users", route.Path)
	route = rootApp.GetRoute("app2.users.add")
	utils.AssertEqual(t, MethodPost, route.Method)
	utils.AssertEqual(t, "/app2/users", route.Path)

	// GetRouteURL inside handler
	req := httptest.NewRequest(MethodGet, "/app1/users", nil)
	_, _ = rootApp.Test(req)

	// ctx.App().GetRoute() inside handler
	req = httptest.NewRequest(MethodPost, "/app1/users", nil)
	_, _ = rootApp.Test(req)
}

Checklist:

• I agree to follow Fiber's Code of Conduct.
• I have checked for existing issues that describe my problem prior to opening this one.
• I understand that improperly formatted bug reports may be closed without explanation.

[ReneWerner87]

related to #2685 #2686
@Antonov-guap have you tried the fix yet?
#2686 (comment)

does it work after that?

[Antonov-guap]

related to #2685 #2686 @Antonov-guap have you tried the fix yet? #2686 (comment)

does it work after that?

Yes, I tried. The fix of Method+Path works well with flat application (single). But with multi-aps (with mount) is much more difficult to fix. I checked on master the testcase. It fails.

[Antonov-guap]

be careful, I fixed some problems in testcase, use fresh one

[ReneWerner87]

@efectn can you have a look at this

[Antonov-guap]

@ReneWerner87 please, read the test-case. Check, if I expect behaviour well. If some mistakes or questions, let me know.

[ReneWerner87]

okay now i understand

naming apps is currently not yet possible

only the last route, because an app is not a route this does not work either

furthermore, currently, in order to merge the stacks of the apps, the startup process must be executed, otherwise the routes of the subapps cannot be found in the main app

rootApp.startupProcess()

or

rootApp.mountStartupProcess()

but this does not happen

what do you think? is it a bug or feature? should we add this? @gaby @efectn

[Antonov-guap]

Adding names to Apps looks like a new feature. Actually, I don't really need it. But all other is a problem of how mounting works. It look more like a bug. I tried also to add rootApp.startupProcess() after mount. It is not solving the problem. Still not getting routes by name from handler or from rootApp.
I added to test rootApp.startupProcess() and fixed utils.AssertEqual (i used actual/expected vice versa).

[ReneWerner87]

correct, because the copy method is used when merging (the subapps routes) and the other properties/infos are missing there

[Antonov-guap]

correct, because the copy method is used when merging (the subapps routes) and the other properties/infos are missing there

yes, there is no Naming info in the copy :(

[ReneWerner87]

will add it, sec

[Antonov-guap]

The easy way: to add naming info when merging. But the problem is: when you add 2 sub-apps having the same names, they will conflict on names (if you have the same names in the tree). And when you make redirect inside handler with ctx.RedirectToLocation, it could take wrong route to make path. That is why it is not that easy to fix. Mounting is a great feature, but using it without handy Naming of routes is very difficult.

[ReneWerner87]

they will conflict on names (if you have the same names in the tree)

this can also happen in the same app

[Antonov-guap]

n the same app

When you work on the same app, expected that you control unique route names and any conflicts are on your focus.
But when you mount some app into your app, it is better when you don't check all route names, but just trust the framework to mount without bugs. I agree that now it is not a bug, but a feature :)

[ReneWerner87]

after the merge of the pull request, i will close the bug, you can then create a feature request for our backlog

feature is already useful

[Antonov-guap]

@ReneWerner87 thank you very much! Very fast solving of problems. Fixing of both problems will make working with Fiber much more convenient.

[Antonov-guap]

Now I just have to wait until tag-release ;) When it is usually expected? Few days or weeks?

[Antonov-guap]

Now I just have to wait until tag-release ;) When it is usually expected? Few days or weeks?

[ReneWerner87]

some days