Repository: gin-gonic/gin
Pull request #1826 (2019-03-26 07:53:30)

Hold matched route full path in the Context
This is a proposed change to return handler full path from the Context.

### Where it can be useful?

When using lambda functions as a handler names returned from `(*Context).HandlerName` become not lean. With this change it becomes easy to analyse which route was called exactly. This can be useful when trying to find out which routes are not being called in the app.

### Alternatives

It is possible to create a wrapper around route definition:

```go
func logGET(router *gin.Engine, path string, handler func (*gin.Context)) {
    router.GET(path, func (c *gin.Context) {
        // Do something with path
        handler(c)
    })
}

...

logGET(router, "/user/:id", func (c *gin.Context) {
    c.Status(200)
})
```

This approach is doable but considering that Gin already holds full path in its structures I though that it might be better to just expose it.

P.S. README no longer contains any docs so I am not certain where I should document this feature.



thinkerou: @zaynetro thanks! ref: #1390 #1447 
codecov[bot]: # [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=h1) Report
> Merging [#1826](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=desc) into [master](https://codecov.io/gh/gin-gonic/gin/commit/1d462bbe3713bc2fea40ed45c80a06ce856d379f?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/gin-gonic/gin/pull/1826/graphs/tree.svg?width=650&token=Qpqxhwery8&height=150&src=pr)](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1826      +/-   ##
==========================================
+ Coverage   98.64%   98.65%   +<.01%     
==========================================
  Files          41       41              
  Lines        2146     2160      +14     
==========================================
+ Hits         2117     2131      +14     
  Misses         18       18              
  Partials       11       11
```


| [Impacted Files](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [gin.go](https://codecov.io/gh/gin-gonic/gin/pull/1826/diff?src=pr&el=tree#diff-Z2luLmdv) | `99.11% <100%> (ø)` | :arrow_up: |
| [context.go](https://codecov.io/gh/gin-gonic/gin/pull/1826/diff?src=pr&el=tree#diff-Y29udGV4dC5nbw==) | `98.38% <100%> (+0.01%)` | :arrow_up: |
| [tree.go](https://codecov.io/gh/gin-gonic/gin/pull/1826/diff?src=pr&el=tree#diff-dHJlZS5nbw==) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=footer). Last update [1d462bb...7d7c208](https://codecov.io/gh/gin-gonic/gin/pull/1826?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

zaynetro: Oh, I wasn't aware of multiple previous attempts...
thinkerou: The pull request benchmark data:
```
➜  go-http-routing-benchmark git:(master) ✗ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 58480 Bytes

#GPlusAPI Routes: 13
   Gin: 4368 Bytes

#ParseAPI Routes: 26
   Gin: 7744 Bytes

#Static Routes: 157
   Gin: 34064 Bytes

goos: darwin
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
BenchmarkGin_Param        	20000000	        83.5 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5       	10000000	       136 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20      	 5000000	       333 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParamWrite   	10000000	       158 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic 	20000000	       104 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam  	10000000	       163 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll    	   50000	     32729 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic  	20000000	        82.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam   	20000000	       103 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params 	10000000	       131 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll     	 1000000	      1476 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic  	20000000	        88.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam   	20000000	        97.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params 	20000000	       103 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll     	  500000	      2528 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll    	  100000	     22313 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	30.647s
```

The master branch benchmark data:
```
➜  go-http-routing-benchmark git:(master) ✗ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 52064 Bytes

#GPlusAPI Routes: 13
   Gin: 3936 Bytes

#ParseAPI Routes: 26
   Gin: 6912 Bytes

#Static Routes: 157
   Gin: 30480 Bytes

goos: darwin
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
BenchmarkGin_Param        	20000000	        97.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5       	10000000	       180 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20      	 3000000	       544 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParamWrite   	10000000	       178 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic 	10000000	       124 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam  	10000000	       220 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll    	   30000	     48147 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic  	20000000	        89.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam   	10000000	       149 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params 	10000000	       213 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll     	 1000000	      2136 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic  	20000000	        94.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam   	20000000	       111 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params 	10000000	       149 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll     	  500000	      3791 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll    	   50000	     29414 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	32.626s
```
thinkerou: About the proposal, #1390 committed by @4396 on 12 Jun 2018 first, and #1447 committed by @youngbloood on 28 Jul 2018, but they have a bit performance issue, please see https://github.com/gin-gonic/gin/pull/1390#issuecomment-491606065 and https://github.com/gin-gonic/gin/pull/1447#issuecomment-444717435.
For respecting everybody's contribution and positivity, we should handle and merge the first commit pull request, but sorry the first and second have some performance issue, maybe we should merge the pull request that the goal is the same. thanks!

zaynetro: I am fine with anything as long as the changes land on master.
appleboy: @zaynetro Could you also update the documentation?
zaynetro: I wasn't sure where to update the documentation so decided to extend an existing example.

Patch:
diff --git a/README.md b/README.md
index 2737e9a..092e91f 100644
--- a/README.md
+++ b/README.md
@@ -252,6 +252,11 @@ func main() {
 		c.String(http.StatusOK, message)
 	})
 
+	// For each matched request Context will hold the route definition
+	router.POST("/user/:name/*action", func(c *gin.Context) {
+		c.FullPath() == "/user/:name/*action" // true
+	})
+
 	router.Run(":8080")
 }
 ```
diff --git a/context.go b/context.go
index a3a7bc4..425b627 100644
--- a/context.go
+++ b/context.go
@@ -48,6 +48,7 @@ type Context struct {
 	Params   Params
 	handlers HandlersChain
 	index    int8
+	fullPath string
 
 	engine *Engine
 
@@ -70,6 +71,7 @@ func (c *Context) reset() {
 	c.Params = c.Params[0:0]
 	c.handlers = nil
 	c.index = -1
+	c.fullPath = ""
 	c.Keys = nil
 	c.Errors = c.Errors[0:0]
 	c.Accepted = nil
@@ -111,6 +113,15 @@ func (c *Context) Handler() HandlerFunc {
 	return c.handlers.Last()
 }
 
+// FullPath returns a matched route full path. For not found routes
+// returns an empty string.
+//     router.GET("/user/:id", func(c *gin.Context) {
+//         c.FullPath() == "/user/:id" // true
+//     })
+func (c *Context) FullPath() string {
+	return c.fullPath
+}
+
 /************************************/
 /*********** FLOW CONTROL ***********/
 /************************************/
diff --git a/gin.go b/gin.go
index 4dbe983..220f040 100644
--- a/gin.go
+++ b/gin.go
@@ -252,6 +252,7 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) {
 	root := engine.trees.get(method)
 	if root == nil {
 		root = new(node)
+		root.fullPath = "/"
 		engine.trees = append(engine.trees, methodTree{method: method, root: root})
 	}
 	root.addRoute(path, handlers)
@@ -382,16 +383,17 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
 		}
 		root := t[i].root
 		// Find route in tree
-		handlers, params, tsr := root.getValue(rPath, c.Params, unescape)
-		if handlers != nil {
-			c.handlers = handlers
-			c.Params = params
+		value := root.getValue(rPath, c.Params, unescape)
+		if value.handlers != nil {
+			c.handlers = value.handlers
+			c.Params = value.params
+			c.fullPath = value.fullPath
 			c.Next()
 			c.writermem.WriteHeaderNow()
 			return
 		}
 		if httpMethod != "CONNECT" && rPath != "/" {
-			if tsr && engine.RedirectTrailingSlash {
+			if value.tsr && engine.RedirectTrailingSlash {
 				redirectTrailingSlash(c)
 				return
 			}
@@ -407,7 +409,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
 			if tree.method == httpMethod {
 				continue
 			}
-			if handlers, _, _ := tree.root.getValue(rPath, nil, unescape); handlers != nil {
+			if value := tree.root.getValue(rPath, nil, unescape); value.handlers != nil {
 				c.handlers = engine.allNoMethod
 				serveError(c, http.StatusMethodNotAllowed, default405Body)
 				return
diff --git a/tree.go b/tree.go
index 07d6b4b..9a789f2 100644
--- a/tree.go
+++ b/tree.go
@@ -94,6 +94,7 @@ type node struct {
 	nType     nodeType
 	maxParams uint8
 	wildChild bool
+	fullPath  string
 }
 
 // increments priority of the given child and reorders if necessary.
@@ -154,6 +155,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 					children:  n.children,
 					handlers:  n.handlers,
 					priority:  n.priority - 1,
+					fullPath:  fullPath,
 				}
 
 				// Update maxParams (max of all children)
@@ -229,6 +231,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 					n.indices += string([]byte{c})
 					child := &node{
 						maxParams: numParams,
+						fullPath:  fullPath,
 					}
 					n.children = append(n.children, child)
 					n.incrementChildPrio(len(n.indices) - 1)
@@ -296,6 +299,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 			child := &node{
 				nType:     param,
 				maxParams: numParams,
+				fullPath:  fullPath,
 			}
 			n.children = []*node{child}
 			n.wildChild = true
@@ -312,6 +316,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 				child := &node{
 					maxParams: numParams,
 					priority:  1,
+					fullPath:  fullPath,
 				}
 				n.children = []*node{child}
 				n = child
@@ -339,6 +344,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 				wildChild: true,
 				nType:     catchAll,
 				maxParams: 1,
+				fullPath:  fullPath,
 			}
 			n.children = []*node{child}
 			n.indices = string(path[i])
@@ -352,6 +358,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 				maxParams: 1,
 				handlers:  handlers,
 				priority:  1,
+				fullPath:  fullPath,
 			}
 			n.children = []*node{child}
 
@@ -364,13 +371,21 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 	n.handlers = handlers
 }
 
+// nodeValue holds return values of (*Node).getValue method
+type nodeValue struct {
+	handlers HandlersChain
+	params   Params
+	tsr      bool
+	fullPath string
+}
+
 // getValue returns the handle registered with the given path (key). The values of
 // wildcards are saved to a map.
 // If no handle can be found, a TSR (trailing slash redirect) recommendation is
 // made if a handle exists with an extra (without the) trailing slash for the
 // given path.
-func (n *node) getValue(path string, po Params, unescape bool) (handlers HandlersChain, p Params, tsr bool) {
-	p = po
+func (n *node) getValue(path string, po Params, unescape bool) (value nodeValue) {
+	value.params = po
 walk: // Outer loop for walking the tree
 	for {
 		if len(path) > len(n.path) {
@@ -391,7 +406,7 @@ walk: // Outer loop for walking the tree
 					// Nothing found.
 					// We can recommend to redirect to the same URL without a
 					// trailing slash if a leaf exists for that path.
-					tsr = path == "/" && n.handlers != nil
+					value.tsr = path == "/" && n.handlers != nil
 					return
 				}
 
@@ -406,20 +421,20 @@ walk: // Outer loop for walking the tree
 					}
 
 					// save param value
-					if cap(p) < int(n.maxParams) {
-						p = make(Params, 0, n.maxParams)
+					if cap(value.params) < int(n.maxParams) {
+						value.params = make(Params, 0, n.maxParams)
 					}
-					i := len(p)
-					p = p[:i+1] // expand slice within preallocated capacity
-					p[i].Key = n.path[1:]
+					i := len(value.params)
+					value.params = value.params[:i+1] // expand slice within preallocated capacity
+					value.params[i].Key = n.path[1:]
 					val := path[:end]
 					if unescape {
 						var err error
-						if p[i].Value, err = url.QueryUnescape(val); err != nil {
-							p[i].Value = val // fallback, in case of error
+						if value.params[i].Value, err = url.QueryUnescape(val); err != nil {
+							value.params[i].Value = val // fallback, in case of error
 						}
 					} else {
-						p[i].Value = val
+						value.params[i].Value = val
 					}
 
 					// we need to go deeper!
@@ -431,40 +446,42 @@ walk: // Outer loop for walking the tree
 						}
 
 						// ... but we can't
-						tsr = len(path) == end+1
+						value.tsr = len(path) == end+1
 						return
 					}
 
-					if handlers = n.handlers; handlers != nil {
+					if value.handlers = n.handlers; value.handlers != nil {
+						value.fullPath = n.fullPath
 						return
 					}
 					if len(n.children) == 1 {
 						// No handle found. Check if a handle for this path + a
 						// trailing slash exists for TSR recommendation
 						n = n.children[0]
-						tsr = n.path == "/" && n.handlers != nil
+						value.tsr = n.path == "/" && n.handlers != nil
 					}
 
 					return
 
 				case catchAll:
 					// save param value
-					if cap(p) < int(n.maxParams) {
-						p = make(Params, 0, n.maxParams)
+					if cap(value.params) < int(n.maxParams) {
+						value.params = make(Params, 0, n.maxParams)
 					}
-					i := len(p)
-					p = p[:i+1] // expand slice within preallocated capacity
-					p[i].Key = n.path[2:]
+					i := len(value.params)
+					value.params = value.params[:i+1] // expand slice within preallocated capacity
+					value.params[i].Key = n.path[2:]
 					if unescape {
 						var err error
-						if p[i].Value, err = url.QueryUnescape(path); err != nil {
-							p[i].Value = path // fallback, in case of error
+						if value.params[i].Value, err = url.QueryUnescape(path); err != nil {
+							value.params[i].Value = path // fallback, in case of error
 						}
 					} else {
-						p[i].Value = path
+						value.params[i].Value = path
 					}
 
-					handlers = n.handlers
+					value.handlers = n.handlers
+					value.fullPath = n.fullPath
 					return
 
 				default:
@@ -474,12 +491,13 @@ walk: // Outer loop for walking the tree
 		} else if path == n.path {
 			// We should have reached the node containing the handle.
 			// Check if this node has a handle registered.
-			if handlers = n.handlers; handlers != nil {
+			if value.handlers = n.handlers; value.handlers != nil {
+				value.fullPath = n.fullPath
 				return
 			}
 
 			if path == "/" && n.wildChild && n.nType != root {
-				tsr = true
+				value.tsr = true
 				return
 			}
 
@@ -488,7 +506,7 @@ walk: // Outer loop for walking the tree
 			for i := 0; i < len(n.indices); i++ {
 				if n.indices[i] == '/' {
 					n = n.children[i]
-					tsr = (len(n.path) == 1 && n.handlers != nil) ||
+					value.tsr = (len(n.path) == 1 && n.handlers != nil) ||
 						(n.nType == catchAll && n.children[0].handlers != nil)
 					return
 				}
@@ -499,7 +517,7 @@ walk: // Outer loop for walking the tree
 
 		// Nothing found. We can recommend to redirect to the same URL with an
 		// extra trailing slash if a leaf exists for that path
-		tsr = (path == "/") ||
+		value.tsr = (path == "/") ||
 			(len(n.path) == len(path)+1 && n.path[len(path)] == '/' &&
 				path == n.path[:len(n.path)-1] && n.handlers != nil)
 		return


Test patch:
diff --git a/routes_test.go b/routes_test.go
index e16c137..457c923 100644
--- a/routes_test.go
+++ b/routes_test.go
@@ -554,3 +554,38 @@ func TestRouteServeErrorWithWriteHeader(t *testing.T) {
 	assert.Equal(t, 421, w.Code)
 	assert.Equal(t, 0, w.Body.Len())
 }
+
+func TestRouteContextHoldsFullPath(t *testing.T) {
+	router := New()
+
+	// Test routes
+	routes := []string{
+		"/",
+		"/simple",
+		"/project/:name",
+		"/project/:name/build/*params",
+	}
+
+	for _, route := range routes {
+		actualRoute := route
+		router.GET(route, func(c *Context) {
+			// For each defined route context should contain its full path
+			assert.Equal(t, actualRoute, c.FullPath())
+			c.AbortWithStatus(http.StatusOK)
+		})
+	}
+
+	for _, route := range routes {
+		w := performRequest(router, "GET", route)
+		assert.Equal(t, http.StatusOK, w.Code)
+	}
+
+	// Test not found
+	router.Use(func(c *Context) {
+		// For not found routes full path is empty
+		assert.Equal(t, "", c.FullPath())
+	})
+
+	w := performRequest(router, "GET", "/not-found")
+	assert.Equal(t, http.StatusNotFound, w.Code)
+}
diff --git a/tree_test.go b/tree_test.go
index dbb0352..e6e2886 100644
--- a/tree_test.go
+++ b/tree_test.go
@@ -35,22 +35,22 @@ func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ..
 	}
 
 	for _, request := range requests {
-		handler, ps, _ := tree.getValue(request.path, nil, unescape)
+		value := tree.getValue(request.path, nil, unescape)
 
-		if handler == nil {
+		if value.handlers == nil {
 			if !request.nilHandler {
 				t.Errorf("handle mismatch for route '%s': Expected non-nil handle", request.path)
 			}
 		} else if request.nilHandler {
 			t.Errorf("handle mismatch for route '%s': Expected nil handle", request.path)
 		} else {
-			handler[0](nil)
+			value.handlers[0](nil)
 			if fakeHandlerValue != request.route {
 				t.Errorf("handle mismatch for route '%s': Wrong handle (%s != %s)", request.path, fakeHandlerValue, request.route)
 			}
 		}
 
-		if !reflect.DeepEqual(ps, request.ps) {
+		if !reflect.DeepEqual(value.params, request.ps) {
 			t.Errorf("Params mismatch for route '%s'", request.path)
 		}
 	}
@@ -454,10 +454,10 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
 		"/doc/",
 	}
 	for _, route := range tsrRoutes {
-		handler, _, tsr := tree.getValue(route, nil, false)
-		if handler != nil {
+		value := tree.getValue(route, nil, false)
+		if value.handlers != nil {
 			t.Fatalf("non-nil handler for TSR route '%s", route)
-		} else if !tsr {
+		} else if !value.tsr {
 			t.Errorf("expected TSR recommendation for route '%s'", route)
 		}
 	}
@@ -471,10 +471,10 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
 		"/api/world/abc",
 	}
 	for _, route := range noTsrRoutes {
-		handler, _, tsr := tree.getValue(route, nil, false)
-		if handler != nil {
+		value := tree.getValue(route, nil, false)
+		if value.handlers != nil {
 			t.Fatalf("non-nil handler for No-TSR route '%s", route)
-		} else if tsr {
+		} else if value.tsr {
 			t.Errorf("expected no TSR recommendation for route '%s'", route)
 		}
 	}
@@ -490,10 +490,10 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) {
 		t.Fatalf("panic inserting test route: %v", recv)
 	}
 
-	handler, _, tsr := tree.getValue("/", nil, false)
-	if handler != nil {
+	value := tree.getValue("/", nil, false)
+	if value.handlers != nil {
 		t.Fatalf("non-nil handler")
-	} else if tsr {
+	} else if value.tsr {
 		t.Errorf("expected no TSR recommendation")
 	}
 }