Path params with conflicting names

[rickb777]

Issue Description

I had created routes using GET and PUT with path parameters. Using the same names in both works fine, but if the names are different it doesn't work.

Checklist

• [x] Dependencies installed
• [x] No typos
• [x] Searched existing issues and docs

Expected behaviour

I would expect each path to own its own path parameters.

Actual behaviour

Not all path parameters expected are found. The order or declaration affects which ones are missing.

Steps to reproduce

In router_test.go, change TestRouterTwoParam to have both GET and PUT paths and for these to have distinct parameter names.

func TestRouterTwoParam(t *testing.T) {
	e := New()
	r := e.router
	r.Add(http.MethodPut, "/users/:vid/files/:gid", func(Context) error {
		return nil
	})
	r.Add(http.MethodGet, "/users/:uid/files/:fid", func(Context) error {
		return nil
	})
	c := e.NewContext(nil, nil).(*context)

	r.Find(http.MethodGet, "/users/1/files/2", c)
	assert.Equal(t, "1", c.Param("uid"))
	assert.Equal(t, "2", c.Param("fid"))

	r.Find(http.MethodPut, "/users/3/files/4", c)
	assert.Equal(t, "3", c.Param("vid"))
	assert.Equal(t, "4", c.Param("gid"))
}

This test fails. If the two Add statements are swapped, it still fails but in a different place.

Version/commit

v4.1.17-99-g936c48a

[iambenkay]

Hmm this is surely from the router resolution algorithm. using the same names work but it fails when different names are used... Maybe I'll try looking into it. Do you have any leads?

[pafuent]

This is because the nodes on the Router only have an internal structure to store the path parameters (pnames), and the same node is used for all the HTTP methods. That's the reason why the path parameters that ends up in the node for that url are the ones of the PUT (vid and gid) I guess I have and idea of how to fix it, but I need to think a little bit more about it to find out if it is the simplest and more performant solution.

[aldas]

After thinking and reasoning about why currently router has that behavour. I would say - it is correct.

Why would parameter at same place be conceptually different for GET vs PUT methods?

in REST sense having different parameter identifier for same field "/users/:vid/files/:gid" and /users/:uid/files/:fid -> :vid vs :uid it not conceptually correct. In case when one route parameter at same place is uid and in another gid - It would say - both should be the same for all router with same prefix. This is because by REST idea (route describes path to entity). So in our case both (vid and uid) should refer to same uniquely linkable path i.e. having same user entity id by having same parameter name.

Probably in forcing user to use same parameter names for parameter at same place is more consistent for their API and code readability.

[pafuent]

@aldas I agree with you.

I have a little PR that panics when the same route with different route params is added (regarding HTTP method). If everyone agrees I can submit it. If someone upgrades Echo to that code, I think it will be easy to fix it, but besides of that maybe the PR should be tagged as v5. Any thoughts?