Skip to content

Allow different param names in different methods with same path scheme #2209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 11, 2022
Merged

Allow different param names in different methods with same path scheme #2209

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3e45d2a
Change methodHandler element type to methodContext
ortyomka Jun 29, 2022
d9b49f8
Allow different param names in the smae path with different methods
ortyomka Jun 29, 2022
5ec1f7e
Rename methodContext to routeMethod
ortyomka Jul 6, 2022
83a5acc
Add backtracking to nearest path
ortyomka Jul 8, 2022
627588a
Remove params from NotAllowed
ortyomka Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 63 additions & 47 deletions router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ type (
routes map[string]*Route
echo *Echo
}
methodContext struct {
Copy link
Contributor

@aldas aldas Jul 5, 2022

Choose a reason for hiding this comment

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

I do not think that context describes this struct correctly. In world of http servers and Echo context is something more temporary. I have used routeMethod in v5 for similar struct but in v5 methodHandler struct is also renamed to routeMethods.

Copy link
Contributor Author

@ortyomka ortyomka Jul 6, 2022

Choose a reason for hiding this comment

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

Renamed to routeMethod

ppath string
pnames []string
handler HandlerFunc
}
node struct {
kind kind
label byte
prefix string
parent *node
staticChildren children
ppath string
pnames []string
methodHandler *methodHandler
paramChild *node
anyChild *node
Expand All @@ -32,17 +35,17 @@ type (
kind uint8
children []*node
methodHandler struct {
connect HandlerFunc
delete HandlerFunc
get HandlerFunc
head HandlerFunc
options HandlerFunc
patch HandlerFunc
post HandlerFunc
propfind HandlerFunc
put HandlerFunc
trace HandlerFunc
report HandlerFunc
connect *methodContext
delete *methodContext
get *methodContext
head *methodContext
options *methodContext
patch *methodContext
post *methodContext
propfind *methodContext
put *methodContext
trace *methodContext
report *methodContext
allowHeader string
}
)
Expand Down Expand Up @@ -209,9 +212,11 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string
currentNode.prefix = search
if h != nil {
currentNode.kind = t
currentNode.addHandler(method, h)
currentNode.ppath = ppath
currentNode.pnames = pnames
currentNode.addHandler(method, &methodContext{
ppath: ppath,
pnames: pnames,
handler: h,
})
}
currentNode.isLeaf = currentNode.staticChildren == nil && currentNode.paramChild == nil && currentNode.anyChild == nil
} else if lcpLen < prefixLen {
Expand All @@ -222,8 +227,6 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string
currentNode,
currentNode.staticChildren,
currentNode.methodHandler,
currentNode.ppath,
currentNode.pnames,
currentNode.paramChild,
currentNode.anyChild,
)
Expand All @@ -244,8 +247,6 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string
currentNode.prefix = currentNode.prefix[:lcpLen]
currentNode.staticChildren = nil
currentNode.methodHandler = new(methodHandler)
currentNode.ppath = ""
currentNode.pnames = nil
currentNode.paramChild = nil
currentNode.anyChild = nil
currentNode.isLeaf = false
Expand All @@ -257,13 +258,19 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string
if lcpLen == searchLen {
// At parent node
currentNode.kind = t
currentNode.addHandler(method, h)
currentNode.ppath = ppath
currentNode.pnames = pnames
currentNode.addHandler(method, &methodContext{
ppath: ppath,
pnames: pnames,
handler: h,
})
} else {
// Create child node
n = newNode(t, search[lcpLen:], currentNode, nil, new(methodHandler), ppath, pnames, nil, nil)
n.addHandler(method, h)
n = newNode(t, search[lcpLen:], currentNode, nil, new(methodHandler), nil, nil)
n.addHandler(method, &methodContext{
ppath: ppath,
pnames: pnames,
handler: h,
})
// Only Static children could reach here
currentNode.addStaticChild(n)
}
Expand All @@ -277,8 +284,12 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string
continue
}
// Create child node
n := newNode(t, search, currentNode, nil, new(methodHandler), ppath, pnames, nil, nil)
n.addHandler(method, h)
n := newNode(t, search, currentNode, nil, new(methodHandler), nil, nil)
n.addHandler(method, &methodContext{
ppath: ppath,
pnames: pnames,
handler: h,
})
switch t {
case staticKind:
currentNode.addStaticChild(n)
Expand All @@ -291,26 +302,24 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string
} else {
// Node already exists
if h != nil {
currentNode.addHandler(method, h)
currentNode.ppath = ppath
if len(currentNode.pnames) == 0 { // Issue #729
currentNode.pnames = pnames
}
currentNode.addHandler(method, &methodContext{
ppath: ppath,
pnames: pnames,
handler: h,
})
}
}
return
}
}

func newNode(t kind, pre string, p *node, sc children, mh *methodHandler, ppath string, pnames []string, paramChildren, anyChildren *node) *node {
func newNode(t kind, pre string, p *node, sc children, mh *methodHandler, paramChildren, anyChildren *node) *node {
return &node{
kind: t,
label: pre[0],
prefix: pre,
parent: p,
staticChildren: sc,
ppath: ppath,
pnames: pnames,
methodHandler: mh,
paramChild: paramChildren,
anyChild: anyChildren,
Expand Down Expand Up @@ -345,7 +354,12 @@ func (n *node) findChildWithLabel(l byte) *node {
return nil
}

func (n *node) addHandler(method string, h HandlerFunc) {
func (n *node) addHandler(method string, h *methodContext) {
if h.handler == nil {
n.isHandler = n.methodHandler.isHandler()
return
}

switch method {
case http.MethodConnect:
n.methodHandler.connect = h
Expand All @@ -372,14 +386,10 @@ func (n *node) addHandler(method string, h HandlerFunc) {
}

n.methodHandler.updateAllowHeader()
if h != nil {
n.isHandler = true
} else {
n.isHandler = n.methodHandler.isHandler()
}
n.isHandler = true
}

func (n *node) findHandler(method string) HandlerFunc {
func (n *node) findHandler(method string) *methodContext {
switch method {
case http.MethodConnect:
return n.methodHandler.connect
Expand Down Expand Up @@ -530,7 +540,7 @@ func (r *Router) Find(method, path string, c Context) {
previousBestMatchNode = currentNode
}
if h := currentNode.findHandler(method); h != nil {
matchedHandler = h
matchedHandler = h.handler
break
}
}
Expand Down Expand Up @@ -569,7 +579,11 @@ func (r *Router) Find(method, path string, c Context) {
if child := currentNode.anyChild; child != nil {
// If any node is found, use remaining path for paramValues
currentNode = child
paramValues[len(currentNode.pnames)-1] = search
if m := currentNode.findHandler(method); m != nil {
paramValues[len(m.pnames)-1] = search
} else {
break
}
// update indexes/search in case we need to backtrack when no handler match is found
paramIndex++
searchIndex += +len(search)
Expand All @@ -581,7 +595,7 @@ func (r *Router) Find(method, path string, c Context) {
previousBestMatchNode = currentNode
}
if h := currentNode.findHandler(method); h != nil {
matchedHandler = h
matchedHandler = h.handler
break
}
}
Expand Down Expand Up @@ -620,6 +634,8 @@ func (r *Router) Find(method, path string, c Context) {
}
}
}
ctx.path = currentNode.ppath
ctx.pnames = currentNode.pnames
if m := currentNode.findHandler(method); m != nil {
ctx.path = m.ppath
ctx.pnames = m.pnames
}
}
23 changes: 22 additions & 1 deletion router_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2318,6 +2318,27 @@ func TestRouterPanicWhenParamNoRootOnlyChildsFailsFind(t *testing.T) {
}
}

// Issue #1726
func TestRouterDifferentParamsInPath(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"))
}

func TestRouterHandleMethodOptions(t *testing.T) {
e := New()
r := e.router
Expand Down Expand Up @@ -2380,7 +2401,7 @@ func TestRouterHandleMethodOptions(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, tc.expectStatus, rec.Code)
}
assert.Equal(t, tc.expectAllowHeader, c.Response().Header().Get("Allow"))
assert.Equal(t, tc.expectAllowHeader, c.Response().Header().Get(HeaderAllow))
})
}
}
Expand Down