diff --git a/docs/doc.md b/docs/doc.md index d1c33b87..2fee0b90 100644 --- a/docs/doc.md +++ b/docs/doc.md @@ -171,6 +171,16 @@ func main() { c.String(http.StatusOK, "The available groups are [...]") }) + // Gin also supports literal colon suffixes used by custom verbs, such as Google AIP-136. + router.POST("/users:batchGet", func(c *gin.Context) { + c.String(http.StatusOK, "batch get users") + }) + + router.POST("/customers/:customerID:mutate", func(c *gin.Context) { + customerID := c.Param("customerID") + c.String(http.StatusOK, "mutate customer %s", customerID) + }) + router.Run(":8080") } ``` diff --git a/routes_test.go b/routes_test.go index 1cae3fce..2cc39446 100644 --- a/routes_test.go +++ b/routes_test.go @@ -789,3 +789,107 @@ func TestEngineHandleMethodNotAllowedCornerCase(t *testing.T) { w := PerformRequest(r, http.MethodGet, "/base/v1/user/groups") assert.Equal(t, http.StatusNotFound, w.Code) } + +func TestRouterGoogleAIPCustomVerbRoutes(t *testing.T) { + router := New() + + router.POST("/users:batchGet", func(c *Context) { + assert.Equal(t, "/users:batchGet", c.FullPath()) + c.String(http.StatusOK, "batch-get") + }) + router.POST("/users:go", func(c *Context) { + assert.Equal(t, "/users:go", c.FullPath()) + assert.Empty(t, c.Param("go")) + c.String(http.StatusOK, "go") + }) + router.POST("/users:batchCreate", func(c *Context) { + assert.Equal(t, "/users:batchCreate", c.FullPath()) + c.String(http.StatusOK, "batch-create") + }) + + w := PerformRequest(router, http.MethodPost, "/users:batchGet") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "batch-get", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/users:batchCreate") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "batch-create", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/users:go") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "go", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/users:anything") + assert.Equal(t, http.StatusNotFound, w.Code) +} + +func TestRouterGoogleAIPCustomVerbAfterParam(t *testing.T) { + router := New() + + router.POST("/customers/:customer_id", func(c *Context) { + assert.Equal(t, "/customers/:customer_id", c.FullPath()) + assert.Equal(t, "123", c.Param("customer_id")) + c.String(http.StatusOK, "get") + }) + router.POST("/customers/:customer_id:mutate", func(c *Context) { + assert.Equal(t, "/customers/:customer_id:mutate", c.FullPath()) + assert.Equal(t, "123", c.Param("customer_id")) + c.String(http.StatusOK, "mutate") + }) + router.POST("/customers/:customer_id:mutate/static", func(c *Context) { + assert.Equal(t, "/customers/:customer_id:mutate/static", c.FullPath()) + assert.Equal(t, "123", c.Param("customer_id")) + c.String(http.StatusOK, "static") + }) + router.POST("/customers/:customer_id:mutate/:name", func(c *Context) { + assert.Equal(t, "/customers/:customer_id:mutate/:name", c.FullPath()) + assert.Equal(t, "123", c.Param("customer_id")) + c.String(http.StatusOK, c.Param("name")) + }) + router.POST("/customers/:customer_id/devices", func(c *Context) { + assert.Equal(t, "/customers/:customer_id/devices", c.FullPath()) + assert.Equal(t, "123", c.Param("customer_id")) + c.String(http.StatusOK, "devices") + }) + + w := PerformRequest(router, http.MethodPost, "/customers/123") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "get", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/customers/123:mutate") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "mutate", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/customers/123:mutate/static") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "static", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/customers/123:mutate/details") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "details", w.Body.String()) + + w = PerformRequest(router, http.MethodPost, "/customers/123/devices") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "devices", w.Body.String()) +} + +func TestRouterPrefixParamRoutesRemainSupported(t *testing.T) { + router := New() + + router.GET("/id:id", func(c *Context) { + assert.Equal(t, "/id:id", c.FullPath()) + c.String(http.StatusOK, c.Param("id")) + }) + router.GET("/v:version", func(c *Context) { + assert.Equal(t, "/v:version", c.FullPath()) + c.String(http.StatusOK, c.Param("version")) + }) + + w := PerformRequest(router, http.MethodGet, "/id123") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "123", w.Body.String()) + + w = PerformRequest(router, http.MethodGet, "/v1") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "1", w.Body.String()) +} diff --git a/tree.go b/tree.go index 580abbaf..11d6edcd 100644 --- a/tree.go +++ b/tree.go @@ -78,15 +78,71 @@ func (n *node) addChild(child *node) { } func countParams(path string) uint16 { - colons := strings.Count(path, ":") - stars := strings.Count(path, "*") - return safeUint16(colons + stars) + var count uint16 + skipLeadingColon := false + for { + wildcard, i, _ := findWildcard(path, skipLeadingColon) + if i < 0 { + return count + } + count++ + + path = path[i+len(wildcard):] + skipLeadingColon = len(path) > 0 && wildcard[0] == ':' && path[0] == ':' + } } func countSections(path string) uint16 { return safeUint16(strings.Count(path, "/")) } +func isParamStart(path string, i int) bool { + if i == 0 || path[i-1] == '/' { + return true + } + if i == len(path)-1 || path[i+1] == '/' { + return false + } + + switch path[i-1] { + case '_', '-', '.': + return true + } + + segmentStart := strings.LastIndexByte(path[:i], '/') + 1 + prefix := path[segmentStart:i] + return len(prefix) == 0 || prefix[len(prefix)-1] != 's' +} + +func (n *node) childIndex(c byte) int { + for i, idx := range []byte(n.indices) { + if idx == c { + return i + } + } + return -1 +} + +func (n *node) findStaticParamChild(path string, end int, match func(*node, string) bool) (int, int) { + for j := end - 1; j >= 0; j-- { + for i, c := range []byte(n.indices) { + if c == '/' || path[j] != c || !strings.HasPrefix(path[j:], n.children[i].path) { + continue + } + if match == nil || match(n.children[i], path[j:]) { + return i, j + } + } + } + return -1, end +} + +func staticParamChildCanMatch(child *node, path string) bool { + skippedNodes := make([]skippedNode, 0, int(countSections(path))+1) + value := child.getValue(path, nil, &skippedNodes, false) + return value.handlers != nil || value.tsr +} + type nodeType uint8 const ( @@ -180,9 +236,13 @@ walk: c := path[0] // '/' after param - if n.nType == param && c == '/' && len(n.children) == 1 { + if n.nType == param && c == '/' { + childIndex := n.childIndex('/') + if childIndex < 0 { + goto insert + } parentFullPathIndex += len(n.path) - n = n.children[0] + n = n.children[childIndex] n.priority++ continue walk } @@ -198,7 +258,9 @@ walk: } // Otherwise insert it - if c != ':' && c != '*' && n.nType != catchAll { + insert: + skipLeadingColonForInsert := n.nType == param && c == ':' + if (c != ':' && c != '*' || n.nType == param && c == ':') && n.nType != catchAll { // []byte for proper unicode char conversion, see #65 n.indices += bytesconv.BytesToString([]byte{c}) child := &node{ @@ -217,7 +279,7 @@ walk: // 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)] == '/') { + (len(n.path) >= len(path) || path[len(n.path)] == '/' || path[len(n.path)] == ':') { continue walk } @@ -234,7 +296,7 @@ walk: "'") } - n.insertChild(path, fullPath, handlers) + n.insertChildWithSkip(path, fullPath, handlers, skipLeadingColonForInsert) return } @@ -250,7 +312,7 @@ walk: // Search for a wildcard segment and check the name for invalid characters. // Returns -1 as index, if no wildcard was found. -func findWildcard(path string) (wildcard string, i int, valid bool) { +func findWildcard(path string, skipLeadingColon bool) (wildcard string, i int, valid bool) { // Find start escapeColon := false for start, c := range []byte(path) { @@ -265,7 +327,15 @@ func findWildcard(path string) (wildcard string, i int, valid bool) { escapeColon = true continue } - // A wildcard starts with ':' (param) or '*' (catch-all) + // A wildcard starts with ':' (param) or '*' (catch-all). + // Colons inside literal segments stay static so custom verb routes + // such as /users:batchGet can coexist with regular parameters. + if skipLeadingColon && start == 0 && c == ':' { + continue + } + if c == ':' && !isParamStart(path, start) { + continue + } if c != ':' && c != '*' { continue } @@ -276,7 +346,12 @@ func findWildcard(path string) (wildcard string, i int, valid bool) { switch c { case '/': return path[start : start+1+end], start, valid - case ':', '*': + case ':': + if path[start] == ':' { + return path[start : start+1+end], start, valid + } + valid = false + case '*': valid = false } } @@ -286,9 +361,13 @@ func findWildcard(path string) (wildcard string, i int, valid bool) { } func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) { + n.insertChildWithSkip(path, fullPath, handlers, false) +} + +func (n *node) insertChildWithSkip(path string, fullPath string, handlers HandlersChain, skipLeadingColon bool) { for { // Find prefix until first wildcard - wildcard, i, valid := findWildcard(path) + wildcard, i, valid := findWildcard(path, skipLeadingColon) if i < 0 { // No wildcard found break } @@ -325,11 +404,13 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) // will be another subpath starting with '/' if len(wildcard) < len(path) { path = path[len(wildcard):] + skipLeadingColon = path[0] == ':' child := &node{ priority: 1, fullPath: fullPath, } + n.indices += bytesconv.BytesToString([]byte{path[0]}) n.addChild(child) n = child continue @@ -485,14 +566,12 @@ walk: // Outer loop for walking the tree switch n.nType { case param: - // fix truncate the parameter - // tree_test.go line: 204 - // Find param end (either '/' or path end) end := 0 for end < len(path) && path[end] != '/' { end++ } + childIndex, end := n.findStaticParamChild(path, end, staticParamChildCanMatch) // Save param value if params != nil { @@ -521,12 +600,22 @@ walk: // Outer loop for walking the tree } } + if childIndex >= 0 { + path = path[end:] + n = n.children[childIndex] + continue walk + } + // we need to go deeper! if end < len(path) { if len(n.children) > 0 { - path = path[end:] - n = n.children[0] - continue walk + for i, c := range []byte(n.indices) { + if c == '/' { + path = path[end:] + n = n.children[i] + continue walk + } + } } // ... but we can't @@ -538,10 +627,10 @@ walk: // Outer loop for walking the tree value.fullPath = n.fullPath return value } - if len(n.children) == 1 { + if childIndex := n.childIndex('/'); childIndex >= 0 { // No handle found. Check if a handle for this path + a // trailing slash exists for TSR recommendation - n = n.children[0] + n = n.children[childIndex] value.tsr = (n.path == "/" && n.handlers != nil) || (n.path == "" && n.indices == "/") } return value @@ -891,18 +980,30 @@ walk: // Outer loop for walking the tree for end < len(path) && path[end] != '/' { end++ } + childIndex, end := n.findStaticParamChild(path, end, nil) // Add param value to case insensitive path ciPath = append(ciPath, path[:end]...) + if childIndex >= 0 { + n = n.children[childIndex] + npLen = len(n.path) + path = path[end:] + continue + } + // We need to go deeper! if end < len(path) { if len(n.children) > 0 { - // Continue with child node - n = n.children[0] - npLen = len(n.path) - path = path[end:] - continue + for i, c := range []byte(n.indices) { + if c == '/' { + // Continue with child node + n = n.children[i] + npLen = len(n.path) + path = path[end:] + continue walk + } + } } // ... but we can't @@ -916,12 +1017,14 @@ walk: // Outer loop for walking the tree return ciPath } - if fixTrailingSlash && len(n.children) == 1 { + if fixTrailingSlash { // No handle found. Check if a handle for this path + a // trailing slash exists - n = n.children[0] - if n.path == "/" && n.handlers != nil { - return append(ciPath, '/') + if childIndex := n.childIndex('/'); childIndex >= 0 { + n = n.children[childIndex] + if n.path == "/" && n.handlers != nil { + return append(ciPath, '/') + } } } diff --git a/tree_test.go b/tree_test.go index 23339af4..527b982f 100644 --- a/tree_test.go +++ b/tree_test.go @@ -93,6 +93,18 @@ func TestCountParams(t *testing.T) { if countParams("/path/:param1/static/*catch-all") != 2 { t.Fail() } + if countParams("/users:batchGet") != 0 { + t.Fail() + } + if countParams("/users:go") != 0 { + t.Fail() + } + if countParams("/v:version") != 1 { + t.Fail() + } + if countParams("/customers/:customer_id:mutate") != 1 { + t.Fail() + } if countParams(strings.Repeat("/:param", 256)) != 256 { t.Fail() } @@ -496,8 +508,6 @@ func TestEmptyWildcardName(t *testing.T) { tree := &node{} routes := [...]string{ - "/user:", - "/user:/", "/cmd/:/", "/src/*", } @@ -540,9 +550,9 @@ func TestTreeDoubleWildcard(t *testing.T) { const panicMsg = "only one wildcard per path segment is allowed" routes := [...]string{ - "/:foo:bar", - "/:foo:bar/", "/:foo*bar", + "/:foo*bar:baz", + "/*foo:bar", } for _, route := range routes { @@ -557,6 +567,49 @@ func TestTreeDoubleWildcard(t *testing.T) { } } +func TestTreeCustomVerb(t *testing.T) { + tree := &node{} + + routes := [...]string{ + "/user:", + "/user:/", + "/users:batchGet", + "/users:batchCreate", + "/customers/:customer_id", + "/customers/:customer_id:mutate", + } + for _, route := range routes { + tree.addRoute(route, fakeHandler(route)) + } + + checkRequests(t, tree, testRequests{ + {"/user:", false, "/user:", nil}, + {"/user:/", false, "/user:/", nil}, + {"/users:batchGet", false, "/users:batchGet", nil}, + {"/users:batchCreate", false, "/users:batchCreate", nil}, + {"/customers/123", false, "/customers/:customer_id", Params{Param{Key: "customer_id", Value: "123"}}}, + {"/customers/123:mutate", false, "/customers/:customer_id:mutate", Params{Param{Key: "customer_id", Value: "123"}}}, + {"/customers/123:mutatex", false, "/customers/:customer_id", Params{Param{Key: "customer_id", Value: "123:mutatex"}}}, + }) +} + +func TestTreeCustomVerbBeforeBaseParam(t *testing.T) { + tree := &node{} + + routes := [...]string{ + "/customers/:customer_id:mutate", + "/customers/:customer_id", + } + for _, route := range routes { + tree.addRoute(route, fakeHandler(route)) + } + + checkRequests(t, tree, testRequests{ + {"/customers/123", false, "/customers/:customer_id", Params{Param{Key: "customer_id", Value: "123"}}}, + {"/customers/123:mutate", false, "/customers/:customer_id:mutate", Params{Param{Key: "customer_id", Value: "123"}}}, + }) +} + /*func TestTreeDuplicateWildcard(t *testing.T) { tree := &node{} routes := [...]string{ @@ -721,6 +774,8 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) { "/hi", "/b/", "/ABC/", + "/users:batchGet", + "/customers/:customer_id:mutate", "/search/:query", "/cmd/:tool/", "/src/*filepath", @@ -798,6 +853,8 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) { {"/aBc/", "/ABC/", true, false}, {"/abC", "/ABC/", true, true}, {"/abC/", "/ABC/", true, false}, + {"/USERS:BATCHGET", "/users:batchGet", true, false}, + {"/USERS:BATCHGET/", "/users:batchGet", true, true}, {"/SEARCH/QUERY", "/search/QUERY", true, false}, {"/SEARCH/QUERY/", "/search/QUERY", true, true}, {"/CMD/TOOL/", "/cmd/TOOL/", true, false}, @@ -949,7 +1006,6 @@ 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`}, - {"/con:nection", ":nection", `/con:tact`, `:tact`}, } for _, conflict := range conflicts {