Fix URL prefix

[qdongxu]

This PR addresses Issue #2356.

What is being done in this PR?

Type in here a description of the changes in this PR.
The web console cannot be opened after setting a URL prefix in actions/app.go :

	appOnce.Do(func() {
		app = buffalo.New(buffalo.Options{
			Env:         ENV,
			SessionName: "_prefix_test_session",
			Prefix:      "/prefix/",
		})

The error is :

home/index.plush.html: line 5: "rootPath": unknown identifier

What are the main choices made to get to this solution?

Type in here a description of the choices made to get to this solution.

routenamer.go considers "/" as the root path only, ignoring a URL prefix as a root path.

List the manual test cases you've covered before sending this PR:

1. Add a prefix as above then open http://127.0.0.1:3000/prefix/ in the browser.

[sio4]

This PR is not acceptable:

1. This changes the behavior and will cause unwanted results if existing users use the current result of names. (e.g. prefixSomethingPath instead somethingPath for the prefixed /something) This is a breaking change.
2. The fix just fixes the root path only. If the goal of this PR is to keep the route name the same regardless of the prefix, the fix should address the other paths too. (one possible way is trimming the prefix from the given path before the evaluation)

Basically, I agree that the path name should be preserved when we use the prefix to make the app can be ported (mounted) in a different prefix easily without code level change. However, we cannot change this behavior in v1. Also, I have the plan (#2240) to rewrite related things including App, Home, Group, and route information then this behavior could be redefined at that moment.

[qdongxu]

This PR is not acceptable:

1. This changes the behavior and will cause unwanted results if existing users use the current result of names. (e.g. prefixSomethingPath instead somethingPath for the prefixed /something) This is a breaking change.
2. The fix just fixes the root path only. If the goal of this PR is to keep the route name the same regardless of the prefix, the fix should address the other paths too. (one possible way is trimming the prefix from the given path before the evaluation)

Basically, I agree that the path name should be preserved when we use the prefix to make the app can be ported (mounted) in a different prefix easily without code level change. However, we cannot change this behavior in v1. Also, I have the plan (#2240) to rewrite related things including App, Home, Group, and route information then this behavior could be redefined at that moment.

Hi @sio4, trimming the prefix from the route name, resolved both concerns. This is a fully backward-compatible solution.

[sio4]

This PR is not acceptable:

1. This changes the behavior and will cause unwanted results if existing users use the current result of names. (e.g. prefixSomethingPath instead somethingPath for the prefixed /something) This is a breaking change.
2. The fix just fixes the root path only. If the goal of this PR is to keep the route name the same regardless of the prefix, the fix should address the other paths too. (one possible way is trimming the prefix from the given path before the evaluation)

Basically, I agree that the path name should be preserved when we use the prefix to make the app can be ported (mounted) in a different prefix easily without code level change. However, we cannot change this behavior in v1. Also, I have the plan (#2240) to rewrite related things including App, Home, Group, and route information then this behavior could be redefined at that moment.

Hi @sio4, trimming the prefix from the route name, resolved both concerns. This is a fully backward-compatible solution.

While the current code generates a route name as prefixPath and prefixHomePath but the patched version will generate rootPath and homePath. What do you mean by fully backward-compatible?

[qdongxu]

While the current code generates a route name as prefixPath and prefixHomePath but the patched version will generate rootPath and homePath. What do you mean by fully backward-compatible?

No. the code has been updated as generating a route name as rootPath and homePath without the prefix.
fully backward-compatible just means both rootPath and homePath keep unchanged despite a prefix is configured or not.

Input with a prefix:

app = buffalo.New(buffalo.Options{
	Env:         ENV,
	SessionName: "_prefix_test_session",
	Prefix:      "/prefix",
})

app.GET("/", HomeHandler)

app.GET("/about/", About)
app.GET("/prefix/about/", About)

h1 := app.VirtualHost("h1.com")
h1.GET("/func1", VirtualHost1About)

h2 := app.VirtualHost("h2.com")
h2.GET("/func1", VirtualHost2About)

g1 := app.Group("/group/")
g1.GET("func2", About)

subg1 := g1.Group("subg")
subg1.GET("func3", About)

app.Resource("/base-res", &buffalo.BaseResource{})

Route names are identical as no prefix configured:

METHOD	PATH	NAME	HANDLER
GET	/prefix/	rootPath	prefix_test/actions.HomeHandler
GET	/prefix/about/	aboutPath	prefix_test/actions.About
GET	/prefix/prefix/about/	prefixAboutPath	prefix_test/actions.About
GET	/prefix/func1/	func1Path	prefix_test/actions.VirtualHost1About
GET	/prefix/func1/	func1Path	prefix_test/actions.VirtualHost2About
GET	/prefix/group/func2/	groupFunc2Path	prefix_test/actions.About
GET	/prefix/group/subg/func3/	groupSubgFunc3Path	prefix_test/actions.About
GET	/prefix/base-res/	baseResPath	github.com/gobuffalo/buffalo.BaseResource.List
POST	/prefix/base-res/	baseResPath	github.com/gobuffalo/buffalo.BaseResource.Create
GET	/prefix/base-res/{base_id}/	baseReBaseIDPath	github.com/gobuffalo/buffalo.BaseResource.Show
PUT	/prefix/base-res/{base_id}/	baseReBaseIDPath	github.com/gobuffalo/buffalo.BaseResource.Update
DELETE	/prefix/base-res/{base_id}/	baseReBaseIDPath	github.com/gobuffalo/buffalo.BaseResource.Destroy

[sio4]

While the current code generates a route name as prefixPath and prefixHomePath but the patched version will generate rootPath and homePath. What do you mean by fully backward-compatible?

No. the code has been updated as generating a route name as rootPath and homePath without the prefix. fully backward-compatible just means both rootPath and homePath keep unchanged despite a prefix is configured or not.

No, what I mean is that the result should be preserved as the same as today (which is prefixPath instead of rootPath) for the current major version. If we release a new version with this change without bumping the major version, the existing application that already uses the current route name as prefixPath will be broken when they update the module. This is breaking change even though the current behavior is not good.

Also, your PR is targeted to the main branch but a PR against the branch is not acceptable for now. We are planning a new major version on the branch, but not today.

I would like to keep this PR as the status of blocked and will revisit it once we started working on the new major version soon.

[sio4]

Also, in the next major version, the App/Home/Group structure could be completely changed as I already linked the open issue #2240. So I am not fully sure if we can merge this change directly or just need to fix the issue in a different way.

[qdongxu]

Currently rootPath(), and javascriptTag() in the render package do not work with Prefix. the web console cannot open at all when a prefix is configured. The only to configure a reverse proxy is by routing by port.

It will be great if this issue is scheduled in #2240. thanks for your patient clarification, @sio4

[qdongxu]

This PR closed as it will be addressed in #2240