Add mixed param and non-param paths (port of httprouter#329) (#2663)

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
This commit is contained in:
Ross Wolf 2021-04-05 20:49:08 -06:00 committed by GitHub
parent a331dc6a31
commit f3de8132c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 112 additions and 68 deletions

View File

@ -190,6 +190,8 @@ People and companies, who have contributed, in alphabetical order.
**@rogierlommers (Rogier Lommers)**
- Add updated static serve example
**@rw-access (Ross Wolf)**
- Added support to mix exact and param routes
**@se77en (Damon Zhao)**
- Improve color logging

View File

@ -1,5 +1,11 @@
# Gin ChangeLog
## Gin v1.7.0
### ENHANCEMENTS
* Support params and exact routes without creating conflicts [#2663](https://github.com/gin-gonic/gin/pull/2663)
## Gin v1.6.3
### ENHANCEMENTS

View File

@ -243,6 +243,13 @@ func main() {
c.FullPath() == "/user/:name/*action" // true
})
// This handler will add a new router for /user/groups.
// Exact routes are resolved before param routes, regardless of the order they were defined.
// Routes starting with /user/groups are never interpreted as /user/:name/... routes
router.GET("/user/groups", func(c *gin.Context) {
c.String(http.StatusOK, "The available groups are [...]", name)
})
router.Run(":8080")
}
```

101
tree.go
View File

@ -80,6 +80,16 @@ func longestCommonPrefix(a, b string) int {
return i
}
// addChild will add a child node, keeping wildcards at the end
func (n *node) addChild(child *node) {
if n.wildChild && len(n.children) > 0 {
wildcardChild := n.children[len(n.children)-1]
n.children = append(n.children[:len(n.children)-1], child, wildcardChild)
} else {
n.children = append(n.children, child)
}
}
func countParams(path string) uint16 {
var n uint16
s := bytesconv.StringToBytes(path)
@ -103,7 +113,7 @@ type node struct {
wildChild bool
nType nodeType
priority uint32
children []*node
children []*node // child nodes, at most 1 :param style node at the end of the array
handlers HandlersChain
fullPath string
}
@ -177,36 +187,9 @@ walk:
// Make new node a child of this node
if i < len(path) {
path = path[i:]
if n.wildChild {
parentFullPathIndex += len(n.path)
n = n.children[0]
n.priority++
// Check if the wildcard matches
if len(path) >= len(n.path) && n.path == path[:len(n.path)] &&
// Adding a child to a catchAll is not possible
n.nType != catchAll &&
// Check for longer wildcard, e.g. :name and :names
(len(n.path) >= len(path) || path[len(n.path)] == '/') {
continue walk
}
pathSeg := path
if n.nType != catchAll {
pathSeg = strings.SplitN(path, "/", 2)[0]
}
prefix := fullPath[:strings.Index(fullPath, pathSeg)] + n.path
panic("'" + pathSeg +
"' in new path '" + fullPath +
"' conflicts with existing wildcard '" + n.path +
"' in existing prefix '" + prefix +
"'")
}
c := path[0]
// slash after param
// '/' after param
if n.nType == param && c == '/' && len(n.children) == 1 {
parentFullPathIndex += len(n.path)
n = n.children[0]
@ -225,21 +208,47 @@ walk:
}
// Otherwise insert it
if c != ':' && c != '*' {
if c != ':' && c != '*' && n.nType != catchAll {
// []byte for proper unicode char conversion, see #65
n.indices += bytesconv.BytesToString([]byte{c})
child := &node{
fullPath: fullPath,
}
n.children = append(n.children, child)
n.addChild(child)
n.incrementChildPrio(len(n.indices) - 1)
n = child
} else if n.wildChild {
// inserting a wildcard node, need to check if it conflicts with the existing wildcard
n = n.children[len(n.children)-1]
n.priority++
// Check if the wildcard matches
if len(path) >= len(n.path) && n.path == path[:len(n.path)] &&
// Adding a child to a catchAll is not possible
n.nType != catchAll &&
// Check for longer wildcard, e.g. :name and :names
(len(n.path) >= len(path) || path[len(n.path)] == '/') {
continue walk
}
// Wildcard conflict
pathSeg := path
if n.nType != catchAll {
pathSeg = strings.SplitN(pathSeg, "/", 2)[0]
}
prefix := fullPath[:strings.Index(fullPath, pathSeg)] + n.path
panic("'" + pathSeg +
"' in new path '" + fullPath +
"' conflicts with existing wildcard '" + n.path +
"' in existing prefix '" + prefix +
"'")
}
n.insertChild(path, fullPath, handlers)
return
}
// Otherwise and handle to current node
// Otherwise add handle to current node
if n.handlers != nil {
panic("handlers are already registered for path '" + fullPath + "'")
}
@ -293,13 +302,6 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain)
panic("wildcards must be named with a non-empty name in path '" + fullPath + "'")
}
// Check if this node has existing children which would be
// unreachable if we insert the wildcard here
if len(n.children) > 0 {
panic("wildcard segment '" + wildcard +
"' conflicts with existing children in path '" + fullPath + "'")
}
if wildcard[0] == ':' { // param
if i > 0 {
// Insert prefix before the current wildcard
@ -307,13 +309,13 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain)
path = path[i:]
}
n.wildChild = true
child := &node{
nType: param,
path: wildcard,
fullPath: fullPath,
}
n.children = []*node{child}
n.addChild(child)
n.wildChild = true
n = child
n.priority++
@ -326,7 +328,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain)
priority: 1,
fullPath: fullPath,
}
n.children = []*node{child}
n.addChild(child)
n = child
continue
}
@ -360,7 +362,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain)
fullPath: fullPath,
}
n.children = []*node{child}
n.addChild(child)
n.indices = string('/')
n = child
n.priority++
@ -404,10 +406,8 @@ walk: // Outer loop for walking the tree
if len(path) > len(prefix) {
if path[:len(prefix)] == prefix {
path = path[len(prefix):]
// If this node does not have a wildcard (param or catchAll)
// child, we can just look up the next child node and continue
// to walk down the tree
if !n.wildChild {
// Try all the non-wildcard children first by matching the indices
idxc := path[0]
for i, c := range []byte(n.indices) {
if c == idxc {
@ -416,6 +416,8 @@ walk: // Outer loop for walking the tree
}
}
// If there is no wildcard pattern, recommend a redirection
if !n.wildChild {
// Nothing found.
// We can recommend to redirect to the same URL without a
// trailing slash if a leaf exists for that path.
@ -423,8 +425,9 @@ walk: // Outer loop for walking the tree
return
}
// Handle wildcard child
n = n.children[0]
// Handle wildcard child, which is always at the end of the array
n = n.children[len(n.children)-1]
switch n.nType {
case param:
// Find param end (either '/' or path end)

View File

@ -137,6 +137,8 @@ func TestTreeWildcard(t *testing.T) {
"/",
"/cmd/:tool/:sub",
"/cmd/:tool/",
"/cmd/whoami",
"/cmd/whoami/root/",
"/src/*filepath",
"/search/",
"/search/:query",
@ -155,8 +157,12 @@ func TestTreeWildcard(t *testing.T) {
checkRequests(t, tree, testRequests{
{"/", false, "/", nil},
{"/cmd/test/", false, "/cmd/:tool/", Params{Param{Key: "tool", Value: "test"}}},
{"/cmd/test", true, "", Params{Param{Key: "tool", Value: "test"}}},
{"/cmd/test", true, "/cmd/:tool/", Params{Param{"tool", "test"}}},
{"/cmd/test/", false, "/cmd/:tool/", Params{Param{"tool", "test"}}},
{"/cmd/whoami", false, "/cmd/whoami", nil},
{"/cmd/whoami/", true, "/cmd/whoami", nil},
{"/cmd/whoami/root/", false, "/cmd/whoami/root/", nil},
{"/cmd/whoami/root", true, "/cmd/whoami/root/", nil},
{"/cmd/test/3", false, "/cmd/:tool/:sub", Params{Param{Key: "tool", Value: "test"}, Param{Key: "sub", Value: "3"}}},
{"/src/", false, "/src/*filepath", Params{Param{Key: "filepath", Value: "/"}}},
{"/src/some/file.png", false, "/src/*filepath", Params{Param{Key: "filepath", Value: "/some/file.png"}}},
@ -245,20 +251,38 @@ func testRoutes(t *testing.T, routes []testRoute) {
func TestTreeWildcardConflict(t *testing.T) {
routes := []testRoute{
{"/cmd/:tool/:sub", false},
{"/cmd/vet", true},
{"/cmd/vet", false},
{"/foo/bar", false},
{"/foo/:name", false},
{"/foo/:names", true},
{"/cmd/*path", true},
{"/cmd/:badvar", true},
{"/cmd/:tool/names", false},
{"/cmd/:tool/:badsub/details", true},
{"/src/*filepath", false},
{"/src/:file", true},
{"/src/static.json", true},
{"/src/*filepathx", true},
{"/src/", true},
{"/src/foo/bar", true},
{"/src1/", false},
{"/src1/*filepath", true},
{"/src2*filepath", true},
{"/src2/*filepath", false},
{"/search/:query", false},
{"/search/invalid", true},
{"/search/valid", false},
{"/user_:name", false},
{"/user_x", true},
{"/user_x", false},
{"/user_:name", false},
{"/id:id", false},
{"/id/:id", true},
{"/id/:id", false},
}
testRoutes(t, routes)
}
func TestCatchAllAfterSlash(t *testing.T) {
routes := []testRoute{
{"/non-leading-*catchall", true},
}
testRoutes(t, routes)
}
@ -266,14 +290,17 @@ func TestTreeWildcardConflict(t *testing.T) {
func TestTreeChildConflict(t *testing.T) {
routes := []testRoute{
{"/cmd/vet", false},
{"/cmd/:tool/:sub", true},
{"/cmd/:tool", false},
{"/cmd/:tool/:sub", false},
{"/cmd/:tool/misc", false},
{"/cmd/:tool/:othersub", true},
{"/src/AUTHORS", false},
{"/src/*filepath", true},
{"/user_x", false},
{"/user_:name", true},
{"/user_:name", false},
{"/id/:id", false},
{"/id:id", true},
{"/:id", true},
{"/id:id", false},
{"/:id", false},
{"/*filepath", true},
}
testRoutes(t, routes)
@ -688,8 +715,7 @@ func TestTreeWildcardConflictEx(t *testing.T) {
{"/who/are/foo", "/foo", `/who/are/\*you`, `/\*you`},
{"/who/are/foo/", "/foo/", `/who/are/\*you`, `/\*you`},
{"/who/are/foo/bar", "/foo/bar", `/who/are/\*you`, `/\*you`},
{"/conxxx", "xxx", `/con:tact`, `:tact`},
{"/conooo/xxx", "ooo", `/con:tact`, `:tact`},
{"/con:nection", ":nection", `/con:tact`, `:tact`},
}
for _, conflict := range conflicts {